Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: runtime type check can ignore type package name #1537

Merged
merged 1 commit into from
Aug 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
{
"json_result": "{\"alice\": {\"age\": 18, \"_type\": \"__main__.Person\"}}",
"yaml_result": "alice:\n age: 18\n _type: __main__.Person"
}
"json_result": "{\"alice\": {\"age\": 18, \"_type\": \"Person\"}}",
"yaml_result": "alice:\n age: 18\n _type: Person",
"log_message": "",
"err_message": ""
}
Empty file.
23 changes: 19 additions & 4 deletions kclvm/compiler/src/codegen/llvm/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,15 @@ use kclvm_sema::pkgpath_without_prefix;
use kclvm_sema::plugin;

use crate::codegen::abi::Align;
use crate::codegen::llvm::utils;
use crate::codegen::OBJECT_FILE_SUFFIX;
use crate::codegen::{error as kcl_error, EmitOptions};
use crate::codegen::{
traits::*, ENTRY_NAME, GLOBAL_VAL_ALIGNMENT, MODULE_NAME, PKG_INIT_FUNCTION_SUFFIX,
};
use crate::codegen::{CodeGenContext, GLOBAL_LEVEL};
use crate::value;

use crate::codegen::OBJECT_FILE_SUFFIX;

/// SCALAR_KEY denotes the temp scalar key for the global variable json plan process.
const SCALAR_KEY: &str = "";
/// Float type string width mapping
Expand Down Expand Up @@ -1164,6 +1164,7 @@ impl<'ctx> TypeCodeGen for LLVMCodeGenContext<'ctx> {}

impl<'ctx> ProgramCodeGen for LLVMCodeGenContext<'ctx> {
/// Current package path
#[inline]
fn current_pkgpath(&self) -> String {
self.pkgpath_stack
.borrow_mut()
Expand All @@ -1173,6 +1174,7 @@ impl<'ctx> ProgramCodeGen for LLVMCodeGenContext<'ctx> {
}

/// Current filename
#[inline]
fn current_filename(&self) -> String {
self.filename_stack
.borrow_mut()
Expand Down Expand Up @@ -1385,7 +1387,7 @@ impl<'ctx> LLVMCodeGenContext<'ctx> {
if self.no_link && !has_main_pkg {
for pkgpath in self.program.pkgs.keys() {
let pkgpath = format!("{}{}", kclvm_runtime::PKG_PATH_PREFIX, pkgpath);
self.pkgpath_stack.borrow_mut().push(pkgpath.clone());
self.push_pkgpath(&pkgpath);
}
}
if !self.import_names.is_empty() {
Expand Down Expand Up @@ -1413,7 +1415,7 @@ impl<'ctx> LLVMCodeGenContext<'ctx> {
// pkgs may not contains main pkg in no link mode
for (pkgpath, modules) in &self.program.pkgs {
let pkgpath = format!("{}{}", kclvm_runtime::PKG_PATH_PREFIX, pkgpath);
self.pkgpath_stack.borrow_mut().push(pkgpath.clone());
self.push_pkgpath(&pkgpath);
// Init all builtin functions.
self.init_scope(pkgpath.as_str());
self.compile_ast_modules(modules);
Expand Down Expand Up @@ -2206,6 +2208,19 @@ impl<'ctx> LLVMCodeGenContext<'ctx> {
var_map
}

#[inline]
pub(crate) fn push_pkgpath(&self, pkgpath: &str) {
self.pkgpath_stack.borrow_mut().push(pkgpath.to_string());
utils::update_ctx_pkgpath(self, pkgpath);
}

#[inline]
pub(crate) fn pop_pkgpath(&self) {
if let Some(pkgpath) = self.pkgpath_stack.borrow_mut().pop() {
utils::update_ctx_pkgpath(self, &pkgpath);
}
}

/// Load value from name.
pub fn load_value(&self, pkgpath: &str, names: &[&str]) -> CompileResult<'ctx> {
if names.is_empty() {
Expand Down
4 changes: 2 additions & 2 deletions kclvm/compiler/src/codegen/llvm/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ impl<'ctx> TypedResultWalker<'ctx> for LLVMCodeGenContext<'ctx> {
return self.ok_result();
} else {
let pkgpath = format!("{}{}", PKG_PATH_PREFIX, import_stmt.path.node);
self.pkgpath_stack.borrow_mut().push(pkgpath.clone());
self.push_pkgpath(&pkgpath);
let has_pkgpath = self.program.pkgs.contains_key(&import_stmt.path.node);
let func_before_block = if self.no_link {
if has_pkgpath {
Expand Down Expand Up @@ -364,7 +364,7 @@ impl<'ctx> TypedResultWalker<'ctx> for LLVMCodeGenContext<'ctx> {
.expect(kcl_error::INTERNAL_ERROR_MSG),
);
}
self.pkgpath_stack.borrow_mut().pop();
self.pop_pkgpath();
if self.no_link {
let name = format!(
"${}.{}",
Expand Down
5 changes: 4 additions & 1 deletion kclvm/evaluator/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,11 +193,14 @@ impl<'ctx> Evaluator<'ctx> {
#[inline]
pub(crate) fn push_pkgpath(&self, pkgpath: &str) {
self.pkgpath_stack.borrow_mut().push(pkgpath.to_string());
self.runtime_ctx.borrow_mut().set_kcl_pkgpath(pkgpath);
}

#[inline]
pub(crate) fn pop_pkgpath(&self) {
self.pkgpath_stack.borrow_mut().pop();
if let Some(pkgpath) = self.pkgpath_stack.borrow_mut().pop() {
self.runtime_ctx.borrow_mut().set_kcl_pkgpath(&pkgpath);
}
}

/// Append a global body into the scope.
Expand Down
23 changes: 18 additions & 5 deletions kclvm/evaluator/src/ty.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use kclvm_runtime::{
check_type, dereference_type, is_dict_type, is_list_type, is_type_union, schema_config_meta,
schema_runtime_type, separate_kv, split_type_union, ConfigEntryOperationKind, ValueRef,
BUILTIN_TYPES, KCL_TYPE_ANY, PKG_PATH_PREFIX,
schema_runtime_type, separate_kv, split_type_union, val_plan, ConfigEntryOperationKind,
ValueRef, BUILTIN_TYPES, KCL_TYPE_ANY, PKG_PATH_PREFIX,
};
use scopeguard::defer;

Expand Down Expand Up @@ -77,13 +77,21 @@ pub fn type_pack_and_check(
converted_value = convert_collection_value(s, value, tpe);
}
// Runtime type check
checked = check_type(&converted_value, tpe, strict);
checked = check_type(
&converted_value,
&s.runtime_ctx.borrow().panic_info.kcl_pkgpath,
tpe,
strict,
);
if checked {
break;
}
}
if !checked {
panic!("expect {expected_type}, got {}", value.type_str());
panic!(
"expect {expected_type}, got {}",
val_plan::type_of(value, true)
);
}
converted_value
}
Expand Down Expand Up @@ -198,7 +206,12 @@ pub fn convert_collection_value_with_union_types(
for tpe in types {
// Try match every type and convert the value, if matched, return the value.
let value = convert_collection_value(s, value, tpe);
if check_type(&value, tpe, false) {
if check_type(
&value,
&s.runtime_ctx.borrow().panic_info.kcl_pkgpath,
tpe,
false,
) {
return value;
}
}
Expand Down
2 changes: 1 addition & 1 deletion kclvm/runtime/src/api/kclvm.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Copyright The KCL Authors. All rights reserved.

use crate::{new_mut_ptr, IndexMap, PlanOptions};
use crate::{new_mut_ptr, val_plan::PlanOptions, IndexMap};
use generational_arena::Index;
use indexmap::IndexSet;
use serde::{Deserialize, Serialize};
Expand Down
2 changes: 1 addition & 1 deletion kclvm/runtime/src/stdlib/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,7 @@ pub fn type_of(x: &ValueRef, full_name: &ValueRef) -> ValueRef {
result += full_type_str;
result += ".";
}
result += &x.type_str();
result += &schema.name;
return ValueRef::str(result.as_str());
}
} else if x.is_none() {
Expand Down
1 change: 0 additions & 1 deletion kclvm/runtime/src/value/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ pub mod val_unary;
pub mod val_bin;

pub mod val_plan;
pub use val_plan::*;

pub mod val_str;

Expand Down
11 changes: 11 additions & 0 deletions kclvm/runtime/src/value/val_is_in.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,17 @@ impl ValueRef {
)
}

#[inline]
pub fn is_builtin(&self) -> bool {
matches!(
&*self.rc.borrow(),
Value::int_value(_)
| Value::float_value(_)
| Value::bool_value(_)
| Value::str_value(_)
)
}

#[inline]
pub fn is_config(&self) -> bool {
matches!(
Expand Down
2 changes: 1 addition & 1 deletion kclvm/runtime/src/value/val_json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use serde::{
Deserialize, Serialize,
};

use crate::{ConfigEntryOperationKind, Context, ValueRef, KCL_PRIVATE_VAR_PREFIX};
use crate::{val_plan::KCL_PRIVATE_VAR_PREFIX, ConfigEntryOperationKind, Context, ValueRef};

macro_rules! tri {
($e:expr $(,)?) => {
Expand Down
89 changes: 56 additions & 33 deletions kclvm/runtime/src/value/val_plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,28 +166,36 @@ fn handle_schema(ctx: &Context, value: &ValueRef) -> Vec<ValueRef> {

/// Returns the type path of the runtime value `v`.
pub(crate) fn value_type_path(v: &ValueRef, full_name: bool) -> String {
match v.get_potential_schema_type() {
Some(ty_str) => {
if full_name {
match ty_str.strip_prefix('@') {
Some(ty_str) => ty_str.to_string(),
None => ty_str.to_string(),
}
} else {
let parts: Vec<&str> = ty_str.rsplit('.').collect();
match parts.first() {
Some(v) => v.to_string(),
None => type_of(v, full_name),
if v.is_schema() {
type_of(v, full_name)
} else {
match v.get_potential_schema_type() {
Some(ty_str) => {
let ty = if full_name {
match ty_str.strip_prefix('@') {
Some(ty_str) => ty_str.to_string(),
None => ty_str.to_string(),
}
} else {
let parts: Vec<&str> = ty_str.rsplit('.').collect();
match parts.first() {
Some(v) => v.to_string(),
None => type_of(v, full_name),
}
};
match ty.strip_prefix(&format!("{MAIN_PKG_PATH}.")) {
Some(ty) => ty.to_string(),
None => ty,
}
}
None => type_of(v, full_name),
}
None => type_of(v, full_name),
}
}

/// Returns the type path of the runtime value `v`.
#[inline]
fn type_of(v: &ValueRef, full_name: bool) -> String {
pub fn type_of(v: &ValueRef, full_name: bool) -> String {
builtin::type_of(v, &ValueRef::bool(full_name)).as_str()
}

Expand Down Expand Up @@ -272,7 +280,7 @@ impl ValueRef {

#[cfg(test)]
mod test_value_plan {
use crate::{schema_runtime_type, Context, PlanOptions, ValueRef, MAIN_PKG_PATH};
use crate::{schema_runtime_type, val_plan::PlanOptions, Context, ValueRef, MAIN_PKG_PATH};

use super::filter_results;

Expand All @@ -292,6 +300,20 @@ mod test_value_plan {
schema
}

fn get_test_schema_value_with_pkg() -> ValueRef {
let mut schema = ValueRef::dict(None).dict_to_schema(
TEST_SCHEMA_NAME,
"pkg",
&[],
&ValueRef::dict(None),
&ValueRef::dict(None),
None,
None,
);
schema.set_potential_schema_type(&schema_runtime_type(TEST_SCHEMA_NAME, MAIN_PKG_PATH));
schema
}

#[test]
fn test_filter_results() {
let ctx = Context::new();
Expand Down Expand Up @@ -346,9 +368,8 @@ mod test_value_plan {
fn test_value_plan_with_options() {
let mut ctx = Context::new();
ctx.plan_opts = PlanOptions::default();
let schema = get_test_schema_value();
let mut config = ValueRef::dict(None);
config.dict_update_key_value("data", schema);
config.dict_update_key_value("data", get_test_schema_value());
config.dict_update_key_value("_hidden", ValueRef::int(1));
config.dict_update_key_value("vec", ValueRef::list(None));
config.dict_update_key_value("empty", ValueRef::none());
Expand All @@ -360,53 +381,55 @@ mod test_value_plan {
let (json_string, yaml_string) = config.plan(&ctx);
assert_eq!(
json_string,
"{\"data\": {\"_type\": \"__main__.Data\"}, \"vec\": [], \"empty\": null}"
);
assert_eq!(
yaml_string,
"data:\n _type: __main__.Data\nvec: []\nempty: null"
"{\"data\": {\"_type\": \"Data\"}, \"vec\": [], \"empty\": null}"
);
assert_eq!(yaml_string, "data:\n _type: Data\nvec: []\nempty: null");

ctx.plan_opts.show_hidden = true;
let (json_string, yaml_string) = config.plan(&ctx);
assert_eq!(
json_string,
"{\"data\": {\"_type\": \"__main__.Data\"}, \"_hidden\": 1, \"vec\": [], \"empty\": null}"
"{\"data\": {\"_type\": \"Data\"}, \"_hidden\": 1, \"vec\": [], \"empty\": null}"
);
assert_eq!(
yaml_string,
"data:\n _type: __main__.Data\n_hidden: 1\nvec: []\nempty: null"
"data:\n _type: Data\n_hidden: 1\nvec: []\nempty: null"
);

ctx.plan_opts.sort_keys = true;
let (json_string, yaml_string) = config.plan(&ctx);
assert_eq!(
json_string,
"{\"_hidden\": 1, \"data\": {\"_type\": \"__main__.Data\"}, \"empty\": null, \"vec\": []}"
"{\"_hidden\": 1, \"data\": {\"_type\": \"Data\"}, \"empty\": null, \"vec\": []}"
);
assert_eq!(
yaml_string,
"_hidden: 1\ndata:\n _type: __main__.Data\nempty: null\nvec: []"
"_hidden: 1\ndata:\n _type: Data\nempty: null\nvec: []"
);

ctx.plan_opts.disable_none = true;
let (json_string, yaml_string) = config.plan(&ctx);
assert_eq!(
json_string,
"{\"_hidden\": 1, \"data\": {\"_type\": \"__main__.Data\"}, \"vec\": []}"
);
assert_eq!(
yaml_string,
"_hidden: 1\ndata:\n _type: __main__.Data\nvec: []"
"{\"_hidden\": 1, \"data\": {\"_type\": \"Data\"}, \"vec\": []}"
);
assert_eq!(yaml_string, "_hidden: 1\ndata:\n _type: Data\nvec: []");

ctx.plan_opts.disable_empty_list = true;
let (json_string, yaml_string) = config.plan(&ctx);
assert_eq!(
json_string,
"{\"_hidden\": 1, \"data\": {\"_type\": \"__main__.Data\"}}"
"{\"_hidden\": 1, \"data\": {\"_type\": \"Data\"}}"
);
assert_eq!(yaml_string, "_hidden: 1\ndata:\n _type: Data");

config.dict_update_key_value("data_with_pkg", get_test_schema_value_with_pkg());
let (json_string, yaml_string) = config.plan(&ctx);
assert_eq!(json_string, "{\"_hidden\": 1, \"data\": {\"_type\": \"Data\"}, \"data_with_pkg\": {\"_type\": \"pkg.Data\"}}");
assert_eq!(
yaml_string,
"_hidden: 1\ndata:\n _type: Data\ndata_with_pkg:\n _type: pkg.Data"
);
assert_eq!(yaml_string, "_hidden: 1\ndata:\n _type: __main__.Data");

ctx.plan_opts.query_paths = vec!["data".to_string()];
let (json_string, yaml_string) = config.plan(&ctx);
Expand Down
Loading
Loading