fix: shadowing bug

This commit is contained in:
Shunsuke Shibayama 2023-07-12 14:54:33 +09:00
parent febea7bf9b
commit ecee5b7192
5 changed files with 119 additions and 41 deletions

8
Cargo.lock generated
View File

@ -153,7 +153,7 @@ checksum = "7fcaabb2fef8c910e7f4c7ce9f67a1283a1715879a7c230ca9d6d1ae31f16d91"
[[package]] [[package]]
name = "els" name = "els"
version = "0.1.28-nightly.7" version = "0.1.28-nightly.7"
source = "git+https://github.com/erg-lang/erg?branch=main#281262b7bf7c41cbb0b738e660cea37926016780" source = "git+https://github.com/erg-lang/erg?branch=main#953b5806b599ae5061c1d29bb78b88cd758c9d3c"
dependencies = [ dependencies = [
"erg_common", "erg_common",
"erg_compiler", "erg_compiler",
@ -177,7 +177,7 @@ dependencies = [
[[package]] [[package]]
name = "erg_common" name = "erg_common"
version = "0.6.16-nightly.7" version = "0.6.16-nightly.7"
source = "git+https://github.com/erg-lang/erg?branch=main#281262b7bf7c41cbb0b738e660cea37926016780" source = "git+https://github.com/erg-lang/erg?branch=main#953b5806b599ae5061c1d29bb78b88cd758c9d3c"
dependencies = [ dependencies = [
"backtrace-on-stack-overflow", "backtrace-on-stack-overflow",
"parking_lot", "parking_lot",
@ -187,7 +187,7 @@ dependencies = [
[[package]] [[package]]
name = "erg_compiler" name = "erg_compiler"
version = "0.6.16-nightly.7" version = "0.6.16-nightly.7"
source = "git+https://github.com/erg-lang/erg?branch=main#281262b7bf7c41cbb0b738e660cea37926016780" source = "git+https://github.com/erg-lang/erg?branch=main#953b5806b599ae5061c1d29bb78b88cd758c9d3c"
dependencies = [ dependencies = [
"erg_common", "erg_common",
"erg_parser", "erg_parser",
@ -196,7 +196,7 @@ dependencies = [
[[package]] [[package]]
name = "erg_parser" name = "erg_parser"
version = "0.6.16-nightly.7" version = "0.6.16-nightly.7"
source = "git+https://github.com/erg-lang/erg?branch=main#281262b7bf7c41cbb0b738e660cea37926016780" source = "git+https://github.com/erg-lang/erg?branch=main#953b5806b599ae5061c1d29bb78b88cd758c9d3c"
dependencies = [ dependencies = [
"erg_common", "erg_common",
"unicode-xid", "unicode-xid",

View File

@ -35,6 +35,21 @@ use crate::error::*;
pub const ARROW: Token = Token::dummy(TokenKind::FuncArrow, "->"); pub const ARROW: Token = Token::dummy(TokenKind::FuncArrow, "->");
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub enum CanShadow {
Yes,
No,
}
impl CanShadow {
pub const fn is_yes(&self) -> bool {
matches!(self, Self::Yes)
}
pub const fn is_no(&self) -> bool {
matches!(self, Self::No)
}
}
#[derive(Debug, Clone, PartialEq, Eq, Hash)] #[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub enum NameKind { pub enum NameKind {
Variable, Variable,
@ -233,7 +248,7 @@ pub struct ASTConverter {
block_id_counter: usize, block_id_counter: usize,
block_ids: Vec<usize>, block_ids: Vec<usize>,
/// Erg does not allow variables to be defined multiple times, so rename them using this /// Erg does not allow variables to be defined multiple times, so rename them using this
names: HashMap<String, NameInfo>, names: Vec<HashMap<String, NameInfo>>,
warns: CompileErrors, warns: CompileErrors,
errs: CompileErrors, errs: CompileErrors,
} }
@ -246,18 +261,46 @@ impl ASTConverter {
namespace: vec![String::from("<module>")], namespace: vec![String::from("<module>")],
block_id_counter: 0, block_id_counter: 0,
block_ids: vec![0], block_ids: vec![0],
names: HashMap::new(), names: vec![HashMap::new()],
warns: CompileErrors::empty(), warns: CompileErrors::empty(),
errs: CompileErrors::empty(), errs: CompileErrors::empty(),
} }
} }
fn get_name(&self, name: &str) -> Option<&NameInfo> { fn get_name(&self, name: &str) -> Option<&NameInfo> {
self.names.get(name) for ns in self.names.iter().rev() {
if let Some(ni) = ns.get(name) {
return Some(ni);
}
}
None
} }
fn get_mut_name(&mut self, name: &str) -> Option<&mut NameInfo> { fn get_mut_name(&mut self, name: &str) -> Option<&mut NameInfo> {
self.names.get_mut(name) for ns in self.names.iter_mut().rev() {
if let Some(ni) = ns.get_mut(name) {
return Some(ni);
}
}
None
}
fn define_name(&mut self, name: String, info: NameInfo) {
self.names.last_mut().unwrap().insert(name, info);
}
fn declare_name(&mut self, name: String, info: NameInfo) {
self.names.first_mut().unwrap().insert(name, info);
}
fn grow(&mut self, namespace: String) {
self.namespace.push(namespace);
self.names.push(HashMap::new());
}
fn pop(&mut self) {
self.namespace.pop();
self.names.pop();
} }
fn cur_block_id(&self) -> usize { fn cur_block_id(&self) -> usize {
@ -268,14 +311,21 @@ impl ASTConverter {
self.namespace.join(".") self.namespace.join(".")
} }
fn register_name_info(&mut self, name: &str, kind: NameKind) { fn register_name_info(&mut self, name: &str, kind: NameKind) -> CanShadow {
let cur_namespace = self.cur_namespace(); let cur_namespace = self.cur_namespace();
if let Some(name_info) = self.names.get_mut(name) { let cur_block_id = self.cur_block_id();
if name_info.defined_in == cur_namespace { if let Some(name_info) = self.get_mut_name(name) {
if name_info.defined_in == cur_namespace && name_info.defined_block_id == cur_block_id {
name_info.defined_times += 1; name_info.defined_times += 1;
} else if name_info.defined_in.is_unknown() { }
if name_info.defined_in.is_unknown() {
name_info.defined_in = DefinedPlace::Known(cur_namespace); name_info.defined_in = DefinedPlace::Known(cur_namespace);
name_info.defined_times += 1; name_info.defined_times += 1; // 0 -> 1
}
if name_info.defined_block_id == cur_block_id {
CanShadow::Yes
} else {
CanShadow::No
} }
} else { } else {
// In Erg, classes can only be defined in uppercase // In Erg, classes can only be defined in uppercase
@ -287,7 +337,8 @@ impl ASTConverter {
}; };
let defined_in = DefinedPlace::Known(self.cur_namespace()); let defined_in = DefinedPlace::Known(self.cur_namespace());
let info = NameInfo::new(rename, defined_in, self.cur_block_id(), 1); let info = NameInfo::new(rename, defined_in, self.cur_block_id(), 1);
self.names.insert(String::from(name), info); self.define_name(String::from(name), info);
CanShadow::Yes
} }
} }
@ -328,7 +379,7 @@ impl ASTConverter {
}; };
let mut info = NameInfo::new(None, defined_in, cur_block_id, 0); let mut info = NameInfo::new(None, defined_in, cur_block_id, 0);
info.add_referrer(cur_namespace); info.add_referrer(cur_namespace);
self.names.insert(name.clone(), info); self.declare_name(name.clone(), info);
name name
}; };
let token = Token::new( let token = Token::new(
@ -365,6 +416,7 @@ impl ASTConverter {
Identifier::new(VisModifierSpec::Public(dot), name) Identifier::new(VisModifierSpec::Public(dot), name)
} }
// Duplicate param names will result in an error at the parser. So we don't need to check it here.
fn convert_param_pattern(&mut self, arg: String, loc: PyLocation) -> ParamPattern { fn convert_param_pattern(&mut self, arg: String, loc: PyLocation) -> ParamPattern {
self.register_name_info(&arg, NameKind::Variable); self.register_name_info(&arg, NameKind::Variable);
let ident = self.convert_ident(arg, loc); let ident = self.convert_ident(arg, loc);
@ -1091,10 +1143,10 @@ impl ASTConverter {
Expr::BinOp(BinOp::new(op, lhs, rhs)) Expr::BinOp(BinOp::new(op, lhs, rhs))
} }
py_ast::Expr::Lambda(lambda) => { py_ast::Expr::Lambda(lambda) => {
self.namespace.push("<lambda>".to_string()); self.grow("<lambda>".to_string());
let params = self.convert_params(*lambda.args); let params = self.convert_params(*lambda.args);
let body = vec![self.convert_expr(*lambda.body)]; let body = vec![self.convert_expr(*lambda.body)];
self.namespace.pop(); self.pop();
let sig = LambdaSignature::new(params, None, TypeBoundSpecs::empty()); let sig = LambdaSignature::new(params, None, TypeBoundSpecs::empty());
let op = Token::from_str(TokenKind::FuncArrow, "->"); let op = Token::from_str(TokenKind::FuncArrow, "->");
Expr::Lambda(Lambda::new(sig, op, Block::new(body), DefId(0))) Expr::Lambda(Lambda::new(sig, op, Block::new(body), DefId(0)))
@ -1372,8 +1424,7 @@ impl ASTConverter {
if def if def
.sig .sig
.ident() .ident()
.map(|id| &id.inspect()[..] == "__init__") .is_some_and(|id| &id.inspect()[..] == "__init__")
.unwrap_or(false)
{ {
if let Some(call_def) = self.extract_init(&mut base_type, def) { if let Some(call_def) = self.extract_init(&mut base_type, def) {
attrs.insert(0, ClassAttr::Def(call_def)); attrs.insert(0, ClassAttr::Def(call_def));
@ -1456,15 +1507,11 @@ impl ASTConverter {
range: PySourceRange, range: PySourceRange,
) -> Expr { ) -> Expr {
// if reassigning of a function referenced by other functions is occurred, it is an error // if reassigning of a function referenced by other functions is occurred, it is an error
if self if self.get_name(&name).is_some_and(|info| {
.get_name(&name) info.defined_times > 0
.map(|info| { && info.defined_in == DefinedPlace::Known(self.cur_namespace())
info.defined_times > 0 && !info.referenced.difference(&set! {name.clone()}).is_empty()
&& info.defined_in == DefinedPlace::Known(self.cur_namespace()) }) {
&& !info.referenced.difference(&set! {name.clone()}).is_empty()
})
.unwrap_or(false)
{
let err = reassign_func_error( let err = reassign_func_error(
self.cfg.input.clone(), self.cfg.input.clone(),
pyloc_to_ergloc(range), pyloc_to_ergloc(range),
@ -1485,7 +1532,7 @@ impl ASTConverter {
column: loc.column.saturating_add(4), column: loc.column.saturating_add(4),
}; };
let ident = self.convert_ident(name, func_name_loc); let ident = self.convert_ident(name, func_name_loc);
self.namespace.push(ident.inspect().to_string()); self.grow(ident.inspect().to_string());
let params = self.convert_params(params); let params = self.convert_params(params);
let return_t = returns.map(|ret| { let return_t = returns.map(|ret| {
let t_spec = self.convert_type_spec(ret.clone()); let t_spec = self.convert_type_spec(ret.clone());
@ -1507,7 +1554,7 @@ impl ASTConverter {
let block = self.convert_block(body, BlockKind::Function); let block = self.convert_block(body, BlockKind::Function);
let body = DefBody::new(EQUAL, block, DefId(0)); let body = DefBody::new(EQUAL, block, DefId(0));
let def = Def::new(sig, body); let def = Def::new(sig, body);
self.namespace.pop(); self.pop();
Expr::Def(def) Expr::Def(def)
} }
} }
@ -1550,7 +1597,7 @@ impl ASTConverter {
}; };
let ident = self.convert_ident(name, class_name_loc); let ident = self.convert_ident(name, class_name_loc);
let sig = Signature::Var(VarSignature::new(VarPattern::Ident(ident.clone()), None)); let sig = Signature::Var(VarSignature::new(VarPattern::Ident(ident.clone()), None));
self.namespace.push(ident.inspect().to_string()); self.grow(ident.inspect().to_string());
let (base_type, methods) = self.extract_method_list(ident, body, inherit); let (base_type, methods) = self.extract_method_list(ident, body, inherit);
let classdef = if inherit { let classdef = if inherit {
// TODO: multiple inheritance // TODO: multiple inheritance
@ -1585,7 +1632,7 @@ impl ASTConverter {
let def = Def::new(sig, body); let def = Def::new(sig, body);
ClassDef::new(def, methods) ClassDef::new(def, methods)
}; };
self.namespace.pop(); self.pop();
Expr::ClassDef(classdef) Expr::ClassDef(classdef)
} }
@ -1646,14 +1693,22 @@ impl ASTConverter {
let lhs = assign.targets.remove(0); let lhs = assign.targets.remove(0);
match lhs { match lhs {
py_ast::Expr::Name(name) => { py_ast::Expr::Name(name) => {
let block = Block::new(vec![self.convert_expr(*assign.value)]); let expr = self.convert_expr(*assign.value);
let body = DefBody::new(EQUAL, block, DefId(0)); let can_shadow = self.register_name_info(&name.id, NameKind::Variable);
self.register_name_info(&name.id, NameKind::Variable);
let ident = self.convert_ident(name.id.to_string(), name.location()); let ident = self.convert_ident(name.id.to_string(), name.location());
let sig = if can_shadow.is_yes() {
Signature::Var(VarSignature::new(VarPattern::Ident(ident), None)); let block = Block::new(vec![expr]);
let def = Def::new(sig, body); let body = DefBody::new(EQUAL, block, DefId(0));
Expr::Def(def) let sig = Signature::Var(VarSignature::new(
VarPattern::Ident(ident),
None,
));
let def = Def::new(sig, body);
Expr::Def(def)
} else {
let redef = ReDef::new(Accessor::Ident(ident), expr);
Expr::ReDef(redef)
}
} }
py_ast::Expr::Attribute(attr) => { py_ast::Expr::Attribute(attr) => {
let value = self.convert_expr(*attr.value); let value = self.convert_expr(*attr.value);
@ -1758,8 +1813,7 @@ impl ASTConverter {
let prev_ident = self.convert_ident(name.id.to_string(), name.location()); let prev_ident = self.convert_ident(name.id.to_string(), name.location());
if self if self
.get_name(name.id.as_str()) .get_name(name.id.as_str())
.map(|info| info.defined_block_id == self.cur_block_id()) .is_some_and(|info| info.defined_block_id == self.cur_block_id())
.unwrap_or(false)
{ {
self.register_name_info(&name.id, NameKind::Variable); self.register_name_info(&name.id, NameKind::Variable);
let ident = self.convert_ident(name.id.to_string(), name.location()); let ident = self.convert_ident(name.id.to_string(), name.location());

View File

@ -10,3 +10,8 @@ def g(x: int):
if True: if True:
x = "a" # ERR x = "a" # ERR
return x return x
def h(x: str):
if True:
x = "a" # OK
return x

14
tests/shadowing.py Normal file
View File

@ -0,0 +1,14 @@
i: int = 0
i: str = "a" # OK
if True:
i = 1 # ERR
else:
i = 2 # ERR
while False:
i = 3 # ERR
def f(x: int):
i = 1 # OK
return x + i

View File

@ -118,3 +118,8 @@ fn exec_collections() -> Result<(), String> {
fn exec_call() -> Result<(), String> { fn exec_call() -> Result<(), String> {
expect("tests/call.py", 0, 3) expect("tests/call.py", 0, 3)
} }
#[test]
fn exec_shadowing() -> Result<(), String> {
expect("tests/shadowing.py", 0, 3)
}