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

Signed-off-by: peefy <[email protected]>
kcl-lang · Aug 5, 2024 · 5417342 · 5417342
1 parent 96b6ae8
commit 5417342
Show file tree

Hide file tree

Showing 29 changed files with 293 additions and 104 deletions.
diff --git a/kclvm/api/src/testdata/exec-program-with-include-schema-type-path.response.json b/kclvm/api/src/testdata/exec-program-with-include-schema-type-path.response.json
@@ -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": ""
+}
diff --git a/kclvm/api/src/testdata/rename/main.k b/kclvm/api/src/testdata/rename/main.k
diff --git a/kclvm/compiler/src/codegen/llvm/context.rs b/kclvm/compiler/src/codegen/llvm/context.rs
@@ -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
@@ -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()
@@ -1173,6 +1174,7 @@ impl<'ctx> ProgramCodeGen for LLVMCodeGenContext<'ctx> {
     }
 
     /// Current filename
+    #[inline]
     fn current_filename(&self) -> String {
         self.filename_stack
             .borrow_mut()
@@ -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() {
@@ -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);
@@ -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() {

diff --git a/kclvm/compiler/src/codegen/llvm/node.rs b/kclvm/compiler/src/codegen/llvm/node.rs
@@ -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 {
@@ -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!(
                     "${}.{}",

diff --git a/kclvm/evaluator/src/context.rs b/kclvm/evaluator/src/context.rs
@@ -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.

diff --git a/kclvm/evaluator/src/ty.rs b/kclvm/evaluator/src/ty.rs
@@ -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;
 
@@ -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
 }
@@ -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;
             }
         }

diff --git a/kclvm/runtime/src/api/kclvm.rs b/kclvm/runtime/src/api/kclvm.rs
@@ -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};

diff --git a/kclvm/runtime/src/stdlib/builtin.rs b/kclvm/runtime/src/stdlib/builtin.rs
@@ -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() {

diff --git a/kclvm/runtime/src/value/mod.rs b/kclvm/runtime/src/value/mod.rs
@@ -58,7 +58,6 @@ pub mod val_unary;
 pub mod val_bin;
 
 pub mod val_plan;
-pub use val_plan::*;
 
 pub mod val_str;
 

diff --git a/kclvm/runtime/src/value/val_is_in.rs b/kclvm/runtime/src/value/val_is_in.rs
@@ -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!(

diff --git a/kclvm/runtime/src/value/val_json.rs b/kclvm/runtime/src/value/val_json.rs
@@ -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 $(,)?) => {

diff --git a/kclvm/runtime/src/value/val_plan.rs b/kclvm/runtime/src/value/val_plan.rs
@@ -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()
 }
 
@@ -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;
 
@@ -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();
@@ -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());
@@ -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);