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: config list subscript type unsoundness #1591

Merged
merged 1 commit into from
Aug 22, 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
27 changes: 27 additions & 0 deletions kclvm/ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1175,6 +1175,33 @@ pub struct Subscript {
pub has_question: bool,
}

impl Subscript {
/// Whether the subscript is the a[1] or a[-1] form.
pub fn has_name_and_constant_index(&self) -> bool {
if let Expr::Identifier(_) = &self.value.node {
if let Some(index_node) = &self.index {
// Positive index constant
if let Expr::NumberLit(number) = &index_node.node {
matches!(number.value, NumberLitValue::Int(_))
} else if let Expr::Unary(unary_expr) = &index_node.node {
// Negative index constant
if let Expr::NumberLit(number) = &unary_expr.operand.node {
matches!(number.value, NumberLitValue::Int(_))
} else {
false
}
} else {
false
}
} else {
false
}
} else {
false
}
}
}

/// Keyword, e.g.
/// ```kcl
/// arg=value
Expand Down
2 changes: 1 addition & 1 deletion kclvm/driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use kclvm_config::{
},
path::ModRelativePath,
settings::{build_settings_pathbuf, DEFAULT_SETTING_FILE},
workfile::{load_work_file, WorkFile},
workfile::load_work_file,
};
use kclvm_parser::LoadProgramOptions;
use kclvm_utils::path::PathPrefix;
Expand Down
22 changes: 13 additions & 9 deletions kclvm/sema/src/resolver/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,15 +332,20 @@ impl<'ctx> Resolver<'ctx> {
) -> Option<TypeRef> {
if let Some(key) = key {
if let Some(Some(_)) = self.ctx.config_expr_context.last() {
let mut has_index = false;
let names: Vec<String> = match &key.node {
ast::Expr::Identifier(identifier) => identifier.get_names(),
ast::Expr::Subscript(subscript) => {
if let ast::Expr::Identifier(identifier) = &subscript.value.node {
if let Some(index) = &subscript.index {
if matches!(index.node, ast::Expr::NumberLit(_)) {
has_index = true;
identifier.get_names()
} else if let ast::Expr::Unary(unary_expr) = &index.node {
// Negative index constant
if matches!(unary_expr.operand.node, ast::Expr::NumberLit(_)) {
identifier.get_names()
} else {
return None;
}
} else {
return None;
}
Expand All @@ -366,9 +371,6 @@ impl<'ctx> Resolver<'ctx> {
for _ in 0..names.len() - 1 {
val_ty = Type::dict_ref(self.str_ty(), val_ty);
}
if has_index {
val_ty = Type::list_ref(val_ty);
}
if let Some(Some(obj_last)) = self.ctx.config_expr_context.last() {
let ty = obj_last.ty.clone();
self.must_assignable_to(
Expand Down Expand Up @@ -597,12 +599,14 @@ impl<'ctx> Resolver<'ctx> {
val_types.push(val_ty.clone());
val_ty
}
ast::Expr::Subscript(subscript)
if matches!(subscript.value.node, ast::Expr::Identifier(_)) =>
{
ast::Expr::Subscript(subscript) if subscript.has_name_and_constant_index() => {
let val_ty = value_ty.unwrap_or_else(|| self.expr(value));
key_types.push(self.str_ty());
val_types.push(Type::list_ref(val_ty.clone()));
if matches!(op, ast::ConfigEntryOperation::Insert) {
val_types.push(val_ty.clone());
} else {
val_types.push(Type::list_ref(val_ty.clone()));
}
val_ty
}
_ => {
Expand Down
4 changes: 1 addition & 3 deletions kclvm/tools/src/LSP/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@ use crate::word_index::build_word_index;
use anyhow::Result;
use crossbeam_channel::{select, unbounded, Receiver, Sender};
use kclvm_driver::toolchain::{self, Toolchain};
use kclvm_driver::{
lookup_compile_workspaces, lookup_workspace, CompileUnitOptions, WorkSpaceKind,
};
use kclvm_driver::{lookup_compile_workspaces, CompileUnitOptions, WorkSpaceKind};
use kclvm_parser::KCLModuleCache;
use kclvm_sema::core::global_state::GlobalState;
use kclvm_sema::resolver::scope::KCLScopeCache;
Expand Down
5 changes: 5 additions & 0 deletions test/grammar/attr_operator/config_inside/insert/dict_6/main.k
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
config: {str:[int]} = {
a += [0, 1, 2]
a[0] += [4, 5, 6]
a[-1] += [7, 8, 9]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
config:
a:
- 4
- 5
- 6
- 0
- 1
- 7
- 8
- 9
- 2
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@

config: {str:[int]} = {
a[b] += [1, 2, 3]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
error[E2L23]: CompileError
--> ${CWD}/main.k:3:5
|
3 | a[b] += [1, 2, 3]
| ^ name 'a' is not defined
|
Loading