From 8a6ac64f03fcb9bd49ff65b018685ea3c6779645 Mon Sep 17 00:00:00 2001 From: lynskylate Date: Fri, 31 Jul 2020 02:11:00 +0800 Subject: [PATCH 1/6] Introduce symtable test and add is_annotation for symbol --- Lib/test/test_symtable.py | 251 ++++++++++++++++++++++++++++++++++++ compiler/src/symboltable.rs | 42 +++++- vm/src/stdlib/symtable.rs | 81 ++++++++++-- 3 files changed, 361 insertions(+), 13 deletions(-) create mode 100644 Lib/test/test_symtable.py diff --git a/Lib/test/test_symtable.py b/Lib/test/test_symtable.py new file mode 100644 index 000000000..8e83bc72f --- /dev/null +++ b/Lib/test/test_symtable.py @@ -0,0 +1,251 @@ +""" +Test the API of the symtable module. +""" +import symtable +import unittest + + + +TEST_CODE = """ +import sys + +glob = 42 +some_var = 12 + +class Mine: + instance_var = 24 + def a_method(p1, p2): + pass + +def spam(a, b, *var, **kw): + global bar + bar = 47 + some_var = 10 + x = 23 + glob + def internal(): + return x + def other_internal(): + nonlocal some_var + some_var = 3 + return some_var + return internal + +def foo(): + pass + +def namespace_test(): pass +def namespace_test(): pass +""" + + +def find_block(block, name): + for ch in block.get_children(): + if ch.get_name() == name: + return ch + + +class SymtableTest(unittest.TestCase): + + top = symtable.symtable(TEST_CODE, "?", "exec") + # These correspond to scopes in TEST_CODE + Mine = find_block(top, "Mine") + a_method = find_block(Mine, "a_method") + spam = find_block(top, "spam") + internal = find_block(spam, "internal") + other_internal = find_block(spam, "other_internal") + foo = find_block(top, "foo") + + def test_type(self): + self.assertEqual(self.top.get_type(), "module") + self.assertEqual(self.Mine.get_type(), "class") + self.assertEqual(self.a_method.get_type(), "function") + self.assertEqual(self.spam.get_type(), "function") + self.assertEqual(self.internal.get_type(), "function") + + @unittest.skip("TODO: RUSTPYTHON") + def test_id(self): + self.assertGreater(self.top.get_id(), 0) + self.assertGreater(self.Mine.get_id(), 0) + self.assertGreater(self.a_method.get_id(), 0) + self.assertGreater(self.spam.get_id(), 0) + self.assertGreater(self.internal.get_id(), 0) + + @unittest.skip("TODO: RUSTPYTHON") + def test_optimized(self): + self.assertFalse(self.top.is_optimized()) + + self.assertTrue(self.spam.is_optimized()) + + @unittest.skip("TODO: RUSTPYTHON") + def test_nested(self): + self.assertFalse(self.top.is_nested()) + self.assertFalse(self.Mine.is_nested()) + self.assertFalse(self.spam.is_nested()) + self.assertTrue(self.internal.is_nested()) + + def test_children(self): + self.assertTrue(self.top.has_children()) + self.assertTrue(self.Mine.has_children()) + self.assertFalse(self.foo.has_children()) + + def test_lineno(self): + self.assertEqual(self.top.get_lineno(), 0) + self.assertEqual(self.spam.get_lineno(), 12) + + @unittest.skip("TODO: RUSTPYTHON") + def test_function_info(self): + func = self.spam + self.assertEqual(sorted(func.get_parameters()), ["a", "b", "kw", "var"]) + expected = ['a', 'b', 'internal', 'kw', 'other_internal', 'some_var', 'var', 'x'] + self.assertEqual(sorted(func.get_locals()), expected) + self.assertEqual(sorted(func.get_globals()), ["bar", "glob"]) + self.assertEqual(self.internal.get_frees(), ("x",)) + + @unittest.skip("TODO: RUSTPYTHON") + def test_globals(self): + self.assertTrue(self.spam.lookup("glob").is_global()) + self.assertFalse(self.spam.lookup("glob").is_declared_global()) + self.assertTrue(self.spam.lookup("bar").is_global()) + self.assertTrue(self.spam.lookup("bar").is_declared_global()) + self.assertFalse(self.internal.lookup("x").is_global()) + self.assertFalse(self.Mine.lookup("instance_var").is_global()) + self.assertTrue(self.spam.lookup("bar").is_global()) + + @unittest.skip("TODO: RUSTPYTHON") + def test_nonlocal(self): + self.assertFalse(self.spam.lookup("some_var").is_nonlocal()) + self.assertTrue(self.other_internal.lookup("some_var").is_nonlocal()) + expected = ("some_var",) + self.assertEqual(self.other_internal.get_nonlocals(), expected) + + def test_local(self): + self.assertTrue(self.spam.lookup("x").is_local()) + self.assertFalse(self.spam.lookup("bar").is_local()) + + def test_free(self): + self.assertTrue(self.internal.lookup("x").is_free()) + + def test_referenced(self): + self.assertTrue(self.internal.lookup("x").is_referenced()) + self.assertTrue(self.spam.lookup("internal").is_referenced()) + self.assertFalse(self.spam.lookup("x").is_referenced()) + + def test_parameters(self): + for sym in ("a", "var", "kw"): + self.assertTrue(self.spam.lookup(sym).is_parameter()) + self.assertFalse(self.spam.lookup("x").is_parameter()) + + def test_symbol_lookup(self): + self.assertEqual(len(self.top.get_identifiers()), + len(self.top.get_symbols())) + + self.assertRaises(KeyError, self.top.lookup, "not_here") + + def test_namespaces(self): + self.assertTrue(self.top.lookup("Mine").is_namespace()) + self.assertTrue(self.Mine.lookup("a_method").is_namespace()) + self.assertTrue(self.top.lookup("spam").is_namespace()) + self.assertTrue(self.spam.lookup("internal").is_namespace()) + self.assertTrue(self.top.lookup("namespace_test").is_namespace()) + self.assertFalse(self.spam.lookup("x").is_namespace()) + + # TODO(RUSTPYTHON): lookup should return same pythonref + # self.assertTrue(self.top.lookup("spam").get_namespace() is self.spam) + ns_test = self.top.lookup("namespace_test") + self.assertEqual(len(ns_test.get_namespaces()), 2) + self.assertRaises(ValueError, ns_test.get_namespace) + + def test_assigned(self): + self.assertTrue(self.spam.lookup("x").is_assigned()) + self.assertTrue(self.spam.lookup("bar").is_assigned()) + self.assertTrue(self.top.lookup("spam").is_assigned()) + self.assertTrue(self.Mine.lookup("a_method").is_assigned()) + self.assertFalse(self.internal.lookup("x").is_assigned()) + + def test_annotated(self): + st1 = symtable.symtable('def f():\n x: int\n', 'test', 'exec') + st2 = st1.get_children()[0] + self.assertTrue(st2.lookup('x').is_local()) + self.assertTrue(st2.lookup('x').is_annotated()) + self.assertFalse(st2.lookup('x').is_global()) + st3 = symtable.symtable('def f():\n x = 1\n', 'test', 'exec') + st4 = st3.get_children()[0] + self.assertTrue(st4.lookup('x').is_local()) + self.assertFalse(st4.lookup('x').is_annotated()) + + # Test that annotations in the global scope are valid after the + # variable is declared as nonlocal. + st5 = symtable.symtable('global x\nx: int', 'test', 'exec') + self.assertTrue(st5.lookup("x").is_global()) + + # Test that annotations for nonlocals are valid after the + # variable is declared as nonlocal. + st6 = symtable.symtable('def g():\n' + ' x = 2\n' + ' def f():\n' + ' nonlocal x\n' + ' x: int', + 'test', 'exec') + + def test_imported(self): + self.assertTrue(self.top.lookup("sys").is_imported()) + + def test_name(self): + self.assertEqual(self.top.get_name(), "top") + self.assertEqual(self.spam.get_name(), "spam") + self.assertEqual(self.spam.lookup("x").get_name(), "x") + self.assertEqual(self.Mine.get_name(), "Mine") + + @unittest.skip("TODO: RUSTPYTHON") + def test_class_info(self): + self.assertEqual(self.Mine.get_methods(), ('a_method',)) + + @unittest.skip("TODO: RUSTPYTHON") + def test_filename_correct(self): + ### Bug tickler: SyntaxError file name correct whether error raised + ### while parsing or building symbol table. + def checkfilename(brokencode, offset): + try: + symtable.symtable(brokencode, "spam", "exec") + except SyntaxError as e: + self.assertEqual(e.filename, "spam") + self.assertEqual(e.lineno, 1) + self.assertEqual(e.offset, offset) + else: + self.fail("no SyntaxError for %r" % (brokencode,)) + # TODO: RUSTPYTHON, now offset get 15 + checkfilename("def f(x): foo)(", 14) # parse-time + checkfilename("def f(x): global x", 11) # symtable-build-time + symtable.symtable("pass", b"spam", "exec") + with self.assertWarns(DeprecationWarning), \ + self.assertRaises(TypeError): + symtable.symtable("pass", bytearray(b"spam"), "exec") + with self.assertWarns(DeprecationWarning): + symtable.symtable("pass", memoryview(b"spam"), "exec") + with self.assertRaises(TypeError): + symtable.symtable("pass", list(b"spam"), "exec") + + def test_eval(self): + symbols = symtable.symtable("42", "?", "eval") + + @unittest.skip("TODO: RUSTPYTHON") + def test_single(self): + symbols = symtable.symtable("42", "?", "single") + + def test_exec(self): + symbols = symtable.symtable("def f(x): return x", "?", "exec") + + @unittest.skip("TODO: RUSTPYTHON") + def test_bytes(self): + top = symtable.symtable(TEST_CODE.encode('utf8'), "?", "exec") + self.assertIsNotNone(find_block(top, "Mine")) + + code = b'# -*- coding: iso8859-15 -*-\nclass \xb4: pass\n' + + top = symtable.symtable(code, "?", "exec") + self.assertIsNotNone(find_block(top, "\u017d")) + + +if __name__ == '__main__': + unittest.main() \ No newline at end of file diff --git a/compiler/src/symboltable.rs b/compiler/src/symboltable.rs index 5714367bb..f2f0aabd1 100644 --- a/compiler/src/symboltable.rs +++ b/compiler/src/symboltable.rs @@ -97,10 +97,13 @@ pub struct Symbol { pub name: String, // pub table: SymbolTableRef, pub scope: SymbolScope, + // TODO(lynskylate@gmail.com): Use bitflags replace pub is_referenced: bool, pub is_assigned: bool, pub is_parameter: bool, pub is_free: bool, + pub is_annotated: bool, + pub is_imported: bool, // indicates if the symbol gets a value assigned by a named expression in a comprehension // this is required to correct the scope in the analysis. @@ -121,6 +124,8 @@ impl Symbol { is_assigned: false, is_parameter: false, is_free: false, + is_annotated: false, + is_imported: false, is_assign_namedexpr_in_comprehension: false, is_iter: false, } @@ -276,6 +281,11 @@ impl<'a> SymbolTableAnalyzer<'a> { fn analyze_unknown_symbol(&self, symbol: &mut Symbol) { if symbol.is_assigned || symbol.is_parameter { symbol.scope = SymbolScope::Local; + } else if !symbol.is_assigned && symbol.is_referenced { + // if symbol is referenced in its block, but not assigned to. + // https://docs.python.org/3/library/symtable.html?highlight=symtable#symtable.Symbol.is_free + symbol.scope = SymbolScope::Unknown; + symbol.is_free = true; } else { // Interesting stuff about the __class__ variable: // https://docs.python.org/3/reference/datamodel.html?highlight=__class__#creating-the-class-object @@ -394,7 +404,10 @@ enum SymbolUsage { Nonlocal, Used, Assigned, + Imported, + AnnotationAssigned, Parameter, + AnnotationParameter, AssignedNamedExprInCompr, Iter, } @@ -460,7 +473,11 @@ impl SymbolTableBuilder { } fn scan_parameter(&mut self, parameter: &ast::Parameter) -> SymbolTableResult { - self.register_name(¶meter.arg, SymbolUsage::Parameter) + if let Some(_annotation) = ¶meter.annotation { + self.register_name(¶meter.arg, SymbolUsage::AnnotationParameter) + } else { + self.register_name(¶meter.arg, SymbolUsage::Parameter) + } } fn scan_parameters_annotations(&mut self, parameters: &[ast::Parameter]) -> SymbolTableResult { @@ -564,12 +581,12 @@ impl SymbolTableBuilder { for name in names { if let Some(alias) = &name.alias { // `import mymodule as myalias` - self.register_name(alias, SymbolUsage::Assigned)?; + self.register_name(alias, SymbolUsage::Imported)?; } else { // `import module` self.register_name( name.symbol.split('.').next().unwrap(), - SymbolUsage::Assigned, + SymbolUsage::Imported, )?; } } @@ -601,7 +618,12 @@ impl SymbolTableBuilder { annotation, value, } => { - self.scan_expression(target, &ExpressionContext::Store)?; + // https://github.com/python/cpython/blob/master/Python/symtable.c#L1233 + if let ast::ExpressionType::Identifier { ref name } = target.node { + self.register_name(name, SymbolUsage::AnnotationAssigned)?; + } else { + self.scan_expression(target, &ExpressionContext::Store)?; + } self.scan_expression(annotation, &ExpressionContext::Load)?; if let Some(value) = value { self.scan_expression(value, &ExpressionContext::Load)?; @@ -984,9 +1006,21 @@ impl SymbolTableBuilder { }); } } + SymbolUsage::Imported => { + symbol.is_assigned = true; + symbol.is_imported = true; + } SymbolUsage::Parameter => { symbol.is_parameter = true; } + SymbolUsage::AnnotationParameter => { + symbol.is_parameter = true; + symbol.is_annotated = true; + } + SymbolUsage::AnnotationAssigned => { + symbol.is_assigned = true; + symbol.is_annotated = true; + } SymbolUsage::Assigned => { symbol.is_assigned = true; } diff --git a/vm/src/stdlib/symtable.rs b/vm/src/stdlib/symtable.rs index 0d90b0bdb..f52a3888d 100644 --- a/vm/src/stdlib/symtable.rs +++ b/vm/src/stdlib/symtable.rs @@ -109,10 +109,17 @@ impl PySymbolTable { if let Some(symbol) = self.symtable.symbols.get(name) { Ok(PySymbol { symbol: symbol.clone(), + namespaces: self + .symtable + .sub_tables + .iter() + .filter(|table| table.name == name) + .cloned() + .collect(), } .into_ref(vm)) } else { - Err(vm.new_lookup_error(name.to_owned())) + Err(vm.new_key_error(vm.new_str(format!("lookup {} failed", name)))) } } @@ -129,12 +136,23 @@ impl PySymbolTable { #[pymethod(name = "get_symbols")] fn get_symbols(&self, vm: &VirtualMachine) -> PyResult { - let symbols = self - .symtable - .symbols - .values() - .map(|s| (PySymbol { symbol: s.clone() }).into_ref(vm).into_object()) - .collect(); + let mut symbols: Vec = vec![]; + for symbol in self.symtable.symbols.values() { + symbols.push( + PySymbol { + symbol: symbol.clone(), + namespaces: self + .symtable + .sub_tables + .iter() + .filter(|&table| table.name == symbol.name) + .cloned() + .collect(), + } + .into_ref(vm) + .into_object(), + ) + } Ok(vm.ctx.new_list(symbols)) } @@ -158,6 +176,7 @@ impl PySymbolTable { #[pyclass(name = "Symbol")] struct PySymbol { symbol: symboltable::Symbol, + namespaces: Vec, } impl fmt::Debug for PySymbol { @@ -189,6 +208,25 @@ impl PySymbol { self.symbol.is_local() } + #[pymethod(name = "is_imported")] + fn is_imported(&self) -> bool { + self.symbol.is_imported + } + + #[pymethod(name = "is_nested")] + fn is_nested(&self) -> bool { + // TODO + false + } + + #[pymethod(name = "is_nonlocal")] + fn is_nonlocal(&self) -> bool { + match self.symbol.scope { + symboltable::SymbolScope::Nonlocal => true, + _ => false, + } + } + #[pymethod(name = "is_referenced")] fn is_referenced(&self) -> bool { self.symbol.is_referenced @@ -211,7 +249,32 @@ impl PySymbol { #[pymethod(name = "is_namespace")] fn is_namespace(&self) -> bool { - // TODO - false + self.namespaces.len() > 0 + } + + #[pymethod(name = "is_annotated")] + fn is_annotated(&self) -> bool { + self.symbol.is_annotated + } + + #[pymethod(name = "get_namespaces")] + fn get_namespaces(&self, vm: &VirtualMachine) -> PyResult { + let namespaces = self + .namespaces + .iter() + .map(|table| to_py_symbol_table(table.clone()).into_ref(vm).into_object()) + .collect(); + Ok(vm.ctx.new_list(namespaces)) + } + + #[pymethod(name = "get_namespace")] + fn get_namespace(&self, vm: &VirtualMachine) -> PyResult { + if self.namespaces.len() != 1 { + Err(vm.new_value_error("namespace is bound to multiple namespaces".to_owned())) + } else { + Ok(to_py_symbol_table(self.namespaces.first().unwrap().clone()) + .into_ref(vm) + .into_object()) + } } } From 12df480cc9bef173b0e269cd63006e3c530cfd81 Mon Sep 17 00:00:00 2001 From: Yiqun Ling Date: Sat, 1 Aug 2020 01:29:11 +0800 Subject: [PATCH 2/6] Apply suggestions from code review Co-authored-by: Jeong YunWon --- compiler/src/symboltable.rs | 9 +++++---- vm/src/stdlib/symtable.rs | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/compiler/src/symboltable.rs b/compiler/src/symboltable.rs index f2f0aabd1..811fd48b6 100644 --- a/compiler/src/symboltable.rs +++ b/compiler/src/symboltable.rs @@ -473,11 +473,12 @@ impl SymbolTableBuilder { } fn scan_parameter(&mut self, parameter: &ast::Parameter) -> SymbolTableResult { - if let Some(_annotation) = ¶meter.annotation { - self.register_name(¶meter.arg, SymbolUsage::AnnotationParameter) + let usage = if parameter.annotation.is_some() { + SymbolUsage::AnnotationParamater } else { - self.register_name(¶meter.arg, SymbolUsage::Parameter) - } + SymbolUsage::Parameter + }; + self.regsiter_name(¶meter.arg, usage) } fn scan_parameters_annotations(&mut self, parameters: &[ast::Parameter]) -> SymbolTableResult { diff --git a/vm/src/stdlib/symtable.rs b/vm/src/stdlib/symtable.rs index f52a3888d..47359fa23 100644 --- a/vm/src/stdlib/symtable.rs +++ b/vm/src/stdlib/symtable.rs @@ -249,7 +249,7 @@ impl PySymbol { #[pymethod(name = "is_namespace")] fn is_namespace(&self) -> bool { - self.namespaces.len() > 0 + self.namespaces.is_empty() } #[pymethod(name = "is_annotated")] From 0d83cab37a237d0680263942fe151e6abfd1b01f Mon Sep 17 00:00:00 2001 From: lynskylate Date: Sat, 1 Aug 2020 02:05:44 +0800 Subject: [PATCH 3/6] Optimize code --- compiler/src/symboltable.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/src/symboltable.rs b/compiler/src/symboltable.rs index 811fd48b6..79e0b2bed 100644 --- a/compiler/src/symboltable.rs +++ b/compiler/src/symboltable.rs @@ -97,7 +97,7 @@ pub struct Symbol { pub name: String, // pub table: SymbolTableRef, pub scope: SymbolScope, - // TODO(lynskylate@gmail.com): Use bitflags replace + // TODO: Use bitflags replace pub is_referenced: bool, pub is_assigned: bool, pub is_parameter: bool, @@ -474,11 +474,11 @@ impl SymbolTableBuilder { fn scan_parameter(&mut self, parameter: &ast::Parameter) -> SymbolTableResult { let usage = if parameter.annotation.is_some() { - SymbolUsage::AnnotationParamater + SymbolUsage::AnnotationParameter } else { SymbolUsage::Parameter }; - self.regsiter_name(¶meter.arg, usage) + self.register_name(¶meter.arg, usage) } fn scan_parameters_annotations(&mut self, parameters: &[ast::Parameter]) -> SymbolTableResult { From 07476a24fc1d57a9b1985c51b1a98545d53897e3 Mon Sep 17 00:00:00 2001 From: lynskylate Date: Sun, 2 Aug 2020 02:52:07 +0800 Subject: [PATCH 4/6] Fix is namespace --- vm/src/stdlib/symtable.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vm/src/stdlib/symtable.rs b/vm/src/stdlib/symtable.rs index 47359fa23..bab151f00 100644 --- a/vm/src/stdlib/symtable.rs +++ b/vm/src/stdlib/symtable.rs @@ -249,7 +249,7 @@ impl PySymbol { #[pymethod(name = "is_namespace")] fn is_namespace(&self) -> bool { - self.namespaces.is_empty() + !self.namespaces.is_empty() } #[pymethod(name = "is_annotated")] From 7e69e5ef2c092f3b1d3a708023c6e14b5bfebd7b Mon Sep 17 00:00:00 2001 From: lynskylate Date: Sun, 2 Aug 2020 03:43:19 +0800 Subject: [PATCH 5/6] Fix scope change --- compiler/src/symboltable.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/compiler/src/symboltable.rs b/compiler/src/symboltable.rs index 79e0b2bed..e8b80f09e 100644 --- a/compiler/src/symboltable.rs +++ b/compiler/src/symboltable.rs @@ -281,11 +281,6 @@ impl<'a> SymbolTableAnalyzer<'a> { fn analyze_unknown_symbol(&self, symbol: &mut Symbol) { if symbol.is_assigned || symbol.is_parameter { symbol.scope = SymbolScope::Local; - } else if !symbol.is_assigned && symbol.is_referenced { - // if symbol is referenced in its block, but not assigned to. - // https://docs.python.org/3/library/symtable.html?highlight=symtable#symtable.Symbol.is_free - symbol.scope = SymbolScope::Unknown; - symbol.is_free = true; } else { // Interesting stuff about the __class__ variable: // https://docs.python.org/3/reference/datamodel.html?highlight=__class__#creating-the-class-object From 31061b0464c5358ab53cc47a5124ea3befcbd9d2 Mon Sep 17 00:00:00 2001 From: lynskylate Date: Sun, 2 Aug 2020 15:11:08 +0800 Subject: [PATCH 6/6] Optimize code --- vm/src/stdlib/symtable.rs | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/vm/src/stdlib/symtable.rs b/vm/src/stdlib/symtable.rs index bab151f00..2172f974c 100644 --- a/vm/src/stdlib/symtable.rs +++ b/vm/src/stdlib/symtable.rs @@ -136,23 +136,25 @@ impl PySymbolTable { #[pymethod(name = "get_symbols")] fn get_symbols(&self, vm: &VirtualMachine) -> PyResult { - let mut symbols: Vec = vec![]; - for symbol in self.symtable.symbols.values() { - symbols.push( - PySymbol { - symbol: symbol.clone(), + let symbols = self + .symtable + .symbols + .values() + .map(|s| { + (PySymbol { + symbol: s.clone(), namespaces: self .symtable .sub_tables .iter() - .filter(|&table| table.name == symbol.name) + .filter(|&table| table.name == s.name) .cloned() .collect(), - } + }) .into_ref(vm) - .into_object(), - ) - } + .into_object() + }) + .collect(); Ok(vm.ctx.new_list(symbols)) }