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.

Sign up for GitHub

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

Jump to bottom

fix: fix duplicate variable name and schema attr name in right value #1523

Merged
merged 2 commits into from
Jul 26, 2024
Merged

fix: fix duplicate variable name and schema attr name in right value #1523

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0ed4076
fix: fix duplicate variable name in right value goto_def, and schema …
He1pa Jul 26, 2024
e2ee1e7
fix ut
He1pa Jul 26, 2024
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
15 changes: 14 additions & 1 deletion kclvm/loader/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,18 @@ fn collect_scope_info(
} else {
kind
};
let get_def_from_owner = match scope_ref.get_kind() {
kclvm_sema::core::scope::ScopeKind::Local => {
match scope_data.try_get_local_scope(&scope_ref) {
Some(local) => match local.get_kind() {
LocalSymbolScopeKind::SchemaConfig | LocalSymbolScopeKind::Check => true,
_ => false,
},
None => false,
}
}
kclvm_sema::core::scope::ScopeKind::Root => false,
};
scopes.insert(
*scope_ref,
ScopeInfo {
Expand All @@ -282,12 +294,13 @@ fn collect_scope_info(
owner: scope.get_owner(),
children: scope.get_children(),
defs: scope
.get_all_defs(scope_data, symbol_data, None, false)
.get_all_defs(scope_data, symbol_data, None, false, get_def_from_owner)
.values()
.copied()
.collect::<Vec<_>>(),
},
);

for s in scope.get_children() {
collect_scope_info(scopes, &s, scope_data, symbol_data, ScopeKind::Module);
}
Expand Down
3 changes: 3 additions & 0 deletions kclvm/sema/src/advanced_resolver/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ pub struct Context<'ctx> {
// which means advanced resolver will will create the corresponding
// ValueSymbol instead of an UnresolvedSymbol
maybe_def: bool,
// whether lookup def in scope owner, default true, only in schema attr value is false
look_up_in_owner: bool,
}

impl<'ctx> Context<'ctx> {
Expand Down Expand Up @@ -111,6 +113,7 @@ impl<'ctx> AdvancedResolver<'ctx> {
end_pos: Position::dummy_pos(),
cur_node: AstIndex::default(),
maybe_def: false,
look_up_in_owner: true,
},
};
// Scan all scehma symbol
Expand Down
7 changes: 6 additions & 1 deletion kclvm/sema/src/advanced_resolver/node.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1043,6 +1043,7 @@ impl<'ctx> AdvancedResolver<'ctx> {
cur_scope,
self.get_current_module_info(),
maybe_def,
self.ctx.look_up_in_owner,
);
if first_symbol.is_none() {
// Maybe import package symbol
Expand Down Expand Up @@ -1250,6 +1251,7 @@ impl<'ctx> AdvancedResolver<'ctx> {
cur_scope,
self.get_current_module_info(),
true,
self.ctx.look_up_in_owner,
);
match first_symbol {
Some(symbol_ref) => {
Expand Down Expand Up @@ -1785,7 +1787,9 @@ impl<'ctx> AdvancedResolver<'ctx> {
for entry in entries.iter() {
if let Some(key) = &entry.node.key {
self.ctx.maybe_def = true;
self.ctx.look_up_in_owner = true;
self.expr(key)?;
self.ctx.look_up_in_owner = false;
self.ctx.maybe_def = false;
}

Expand All @@ -1797,8 +1801,9 @@ impl<'ctx> AdvancedResolver<'ctx> {
end,
LocalSymbolScopeKind::Value,
);

self.ctx.look_up_in_owner = false;
self.expr(&entry.node.value)?;
self.ctx.look_up_in_owner = true;
self.leave_scope();
}
self.leave_scope();
Expand Down
36 changes: 32 additions & 4 deletions kclvm/sema/src/core/global_state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,15 @@ impl GlobalState {
scope_ref: ScopeRef,
module_info: Option<&ModuleInfo>,
local: bool,
get_def_from_owner: bool,
) -> Option<SymbolRef> {
match self.scopes.get_scope(&scope_ref)?.look_up_def(
name,
&self.scopes,
&self.symbols,
module_info,
local,
get_def_from_owner,
) {
None => self
.symbols
Expand Down Expand Up @@ -179,15 +181,28 @@ impl GlobalState {
///
/// result: [Option<Vec<SymbolRef>>]
/// all definition symbols in the scope
pub fn get_all_defs_in_scope(&self, scope: ScopeRef) -> Option<Vec<SymbolRef>> {
pub fn get_all_defs_in_scope(&self, scope_ref: ScopeRef) -> Option<Vec<SymbolRef>> {
let scopes = &self.scopes;
let scope = scopes.get_scope(&scope)?;
let scope = scopes.get_scope(&scope_ref)?;
let get_def_from_owner = match scope_ref.kind {
ScopeKind::Local => match scopes.try_get_local_scope(&scope_ref) {
Some(local) => match local.kind {
super::scope::LocalSymbolScopeKind::SchemaConfig
| super::scope::LocalSymbolScopeKind::Check => true,
_ => false,
},
None => true,
},
ScopeKind::Root => true,
};

let all_defs: Vec<SymbolRef> = scope
.get_all_defs(
scopes,
&self.symbols,
self.packages.get_module_info(scope.get_filename()),
false,
get_def_from_owner,
)
.values()
.into_iter()
Expand All @@ -208,15 +223,28 @@ impl GlobalState {
///
/// result: [Option<Vec<SymbolRef>>]
/// all definition symbols in the scope
pub fn get_defs_within_scope(&self, scope: ScopeRef) -> Option<Vec<SymbolRef>> {
pub fn get_defs_within_scope(&self, scope_ref: ScopeRef) -> Option<Vec<SymbolRef>> {
let scopes = &self.scopes;
let scope = scopes.get_scope(&scope)?;
let get_def_from_owner = match scope_ref.kind {
ScopeKind::Local => match scopes.try_get_local_scope(&scope_ref) {
Some(local) => match local.kind {
super::scope::LocalSymbolScopeKind::SchemaConfig
| super::scope::LocalSymbolScopeKind::Check => true,
_ => false,
},
None => false,
},
ScopeKind::Root => false,
};

let scope = scopes.get_scope(&scope_ref)?;
let all_defs: Vec<SymbolRef> = scope
.get_defs_within_scope(
scopes,
&self.symbols,
self.packages.get_module_info(scope.get_filename()),
false,
get_def_from_owner,
)
.values()
.into_iter()
Expand Down
115 changes: 86 additions & 29 deletions kclvm/sema/src/core/scope.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ pub trait Scope {
scope_data: &ScopeData,
symbol_data: &Self::SymbolData,
module_info: Option<&ModuleInfo>,
// lookup in local scope
local: bool,
// lookup in scope owner
get_def_from_owner: bool,
) -> Option<SymbolRef>;

/// Get all defs within current scope and parent scope
Expand All @@ -34,6 +37,7 @@ pub trait Scope {
symbol_data: &Self::SymbolData,
module_info: Option<&ModuleInfo>,
recursive: bool,
get_def_from_owner: bool,
) -> HashMap<String, SymbolRef>;

/// Get all defs within current scope
Expand All @@ -43,6 +47,7 @@ pub trait Scope {
symbol_data: &Self::SymbolData,
module_info: Option<&ModuleInfo>,
recursive: bool,
get_def_from_owner: bool,
) -> HashMap<String, SymbolRef>;

fn dump(&self, scope_data: &ScopeData, symbol_data: &Self::SymbolData) -> Option<String>;
Expand Down Expand Up @@ -274,6 +279,7 @@ impl Scope for RootSymbolScope {
symbol_data: &Self::SymbolData,
module_info: Option<&ModuleInfo>,
_local: bool,
_owner: bool,
) -> Option<SymbolRef> {
let package_symbol = symbol_data.get_symbol(self.owner)?;

Expand All @@ -286,6 +292,7 @@ impl Scope for RootSymbolScope {
symbol_data: &Self::SymbolData,
module_info: Option<&ModuleInfo>,
_recursive: bool,
_owner: bool,
) -> HashMap<String, SymbolRef> {
let mut all_defs_map = HashMap::new();
if let Some(owner) = symbol_data.get_symbol(self.owner) {
Expand Down Expand Up @@ -349,9 +356,10 @@ impl Scope for RootSymbolScope {
symbol_data: &Self::SymbolData,
module_info: Option<&ModuleInfo>,
recursive: bool,
owner: bool,
) -> HashMap<String, SymbolRef> {
// get defs within root scope equal to get all defs
self.get_all_defs(scope_data, symbol_data, module_info, recursive)
self.get_all_defs(scope_data, symbol_data, module_info, recursive, owner)
}
}

Expand Down Expand Up @@ -439,26 +447,69 @@ impl Scope for LocalSymbolScope {
symbol_data: &Self::SymbolData,
module_info: Option<&ModuleInfo>,
local: bool,
get_def_from_owner: bool,
) -> Option<SymbolRef> {
match self.defs.get(name) {
Some(symbol_ref) => return Some(*symbol_ref),
None => {
if let Some(owner) = self.owner.as_ref() {
let owner_symbol = symbol_data.get_symbol(*owner)?;
if let Some(symbol_ref) =
owner_symbol.get_attribute(name, symbol_data, module_info)
{
return Some(symbol_ref);
None => match (local, get_def_from_owner) {
// Search in the current scope and owner
(true, true) => {
if let Some(owner) = self.owner.as_ref() {
let owner_symbol = symbol_data.get_symbol(*owner)?;
if let Some(symbol_ref) =
owner_symbol.get_attribute(name, symbol_data, module_info)
{
return Some(symbol_ref);
}
}
};

if local {
None
} else {
}
// Search only in the current scope
(true, false) => {
let parent = scope_data.get_scope(&self.parent)?;
parent.look_up_def(name, scope_data, symbol_data, module_info, false)
return parent.look_up_def(
name,
scope_data,
symbol_data,
module_info,
local,
get_def_from_owner,
);
}
}
// Search in the current scope, parent scope and owner
(false, true) => {
if let Some(owner) = self.owner.as_ref() {
let owner_symbol = symbol_data.get_symbol(*owner)?;
if let Some(symbol_ref) =
owner_symbol.get_attribute(name, symbol_data, module_info)
{
return Some(symbol_ref);
}
};

let parent = scope_data.get_scope(&self.parent)?;
return parent.look_up_def(
name,
scope_data,
symbol_data,
module_info,
local,
get_def_from_owner,
);
}
// Search in the current and parent scope
(false, false) => {
let parent = scope_data.get_scope(&self.parent)?;
return parent.look_up_def(
name,
scope_data,
symbol_data,
module_info,
local,
get_def_from_owner,
);
}
},
}
}

Expand All @@ -468,15 +519,18 @@ impl Scope for LocalSymbolScope {
symbol_data: &Self::SymbolData,
module_info: Option<&ModuleInfo>,
recursive: bool,
owner: bool,
) -> HashMap<String, SymbolRef> {
let mut all_defs_map = HashMap::new();
if let Some(owner) = self.owner {
if let Some(owner) = symbol_data.get_symbol(owner) {
for def_ref in owner.get_all_attributes(symbol_data, module_info) {
if let Some(def) = symbol_data.get_symbol(def_ref) {
let name = def.get_name();
if !all_defs_map.contains_key(&name) {
all_defs_map.insert(name, def_ref);
if owner {
if let Some(owner) = self.owner {
if let Some(owner) = symbol_data.get_symbol(owner) {
for def_ref in owner.get_all_attributes(symbol_data, module_info) {
if let Some(def) = symbol_data.get_symbol(def_ref) {
let name = def.get_name();
if !all_defs_map.contains_key(&name) {
all_defs_map.insert(name, def_ref);
}
}
}
}
Expand Down Expand Up @@ -512,7 +566,7 @@ impl Scope for LocalSymbolScope {

if let Some(parent) = scope_data.get_scope(&self.parent) {
for (name, def_ref) in
parent.get_all_defs(scope_data, symbol_data, module_info, true)
parent.get_all_defs(scope_data, symbol_data, module_info, true, owner)
{
if !all_defs_map.contains_key(&name) {
all_defs_map.insert(name, def_ref);
Expand Down Expand Up @@ -586,15 +640,18 @@ impl Scope for LocalSymbolScope {
symbol_data: &Self::SymbolData,
module_info: Option<&ModuleInfo>,
_recursive: bool,
owner: bool,
) -> HashMap<String, SymbolRef> {
let mut all_defs_map = HashMap::new();
if let Some(owner) = self.owner {
if let Some(owner) = symbol_data.get_symbol(owner) {
for def_ref in owner.get_all_attributes(symbol_data, module_info) {
if let Some(def) = symbol_data.get_symbol(def_ref) {
let name = def.get_name();
if !all_defs_map.contains_key(&name) {
all_defs_map.insert(name, def_ref);
if owner {
if let Some(owner) = self.owner {
if let Some(owner) = symbol_data.get_symbol(owner) {
for def_ref in owner.get_all_attributes(symbol_data, module_info) {
if let Some(def) = symbol_data.get_symbol(def_ref) {
let name = def.get_name();
if !all_defs_map.contains_key(&name) {
all_defs_map.insert(name, def_ref);
}
}
}
}
Expand Down
8 changes: 8 additions & 0 deletions kclvm/tools/src/LSP/src/completion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2056,4 +2056,12 @@ mod tests {
14,
Some('.')
);

completion_label_test_snapshot!(
schema_attr_in_right,
"src/test_data/completion_test/schema/schema.k",
23,
11,
None
);
}
Loading
Loading