From cca0b4b0be501bba323a7de79f3a16a6f0f58559 Mon Sep 17 00:00:00 2001 From: Andrew Porter Date: Wed, 15 Jun 2022 15:16:47 +0100 Subject: [PATCH 1/2] #350 disable validation checks by default and test --- src/fparser/two/symbol_table.py | 59 +++++++++++++++------ src/fparser/two/tests/test_symbol_table.py | 23 +++++++- src/fparser/two/tests/test_symbol_tables.py | 9 +++- 3 files changed, 73 insertions(+), 18 deletions(-) diff --git a/src/fparser/two/symbol_table.py b/src/fparser/two/symbol_table.py index a0d68760..58004081 100644 --- a/src/fparser/two/symbol_table.py +++ b/src/fparser/two/symbol_table.py @@ -59,6 +59,9 @@ def __init__(self): self._scoping_unit_classes = [] # The symbol table of the current scope self._current_scope = None + # Whether or not we enable consistency checks in the symbol tables + # that are created. + self._enable_checks = False def __str__(self): result = ("SymbolTables: {0} tables\n" @@ -66,6 +69,16 @@ def __str__(self): len(self._symbol_tables))) return result + "\n".join(sorted(self._symbol_tables.keys())) + def enable_checks(self, value): + ''' + Sets whether or not to enable consistency checks in every symbol + table that is created during a parse. + + :param bool value: whether or not checks are enabled. + + ''' + self._enable_checks = value + def clear(self): ''' Deletes any stored SymbolTables but retains the stored list of @@ -93,7 +106,7 @@ def add(self, name): raise SymbolTableError( "The table of top-level (un-nested) symbol tables already " "contains an entry for '{0}'".format(lower_name)) - table = SymbolTable(lower_name) + table = SymbolTable(lower_name, checking_enabled=self._enable_checks) self._symbol_tables[lower_name] = table return table @@ -135,7 +148,7 @@ def scoping_unit_classes(self, value): if not isinstance(value, list): raise TypeError("Supplied value must be a list but got '{0}'". format(type(value).__name__)) - if not all([isinstance(item, type) for item in value]): + if not all(isinstance(item, type) for item in value): raise TypeError("Supplied list must contain only classes but " "got: {0}".format(value)) self._scoping_unit_classes = value @@ -173,7 +186,8 @@ def enter_scope(self, name): else: # We are already inside a scoping region so create a new table # and setup its parent/child connections. - table = SymbolTable(lname, parent=self._current_scope) + table = SymbolTable(lname, parent=self._current_scope, + checking_enabled=self._enable_checks) self._current_scope.add_child(table) # Finally, make this new table the current scope @@ -242,10 +256,17 @@ class SymbolTable(): ''' Class implementing a single symbol table. + Since this functionality is not yet fully mature, checks that new symbols + don't clash with existing symbols are disabled by default. + Once #201 is complete it is planned to switch this so that the checks + are instead enabled by default. + :param str name: the name of this scope. Will be the name of the \ associated module or routine. :param parent: the symbol table within which this one is nested (if any). :type parent: :py:class:`fparser.two.symbol_table.SymbolTable.Symbol` + :param bool checking_enabled: whether or not validity checks are \ + performed for symbols added to the table. ''' # TODO #201 add support for other symbol properties (kind, shape @@ -253,7 +274,7 @@ class SymbolTable(): # type checking for the various properties. Symbol = namedtuple("Symbol", "name primitive_type") - def __init__(self, name, parent=None): + def __init__(self, name, parent=None, checking_enabled=False): self._name = name.lower() # Symbols defined in this scope that represent data. self._data_symbols = {} @@ -263,6 +284,8 @@ def __init__(self, name, parent=None): # value (if any) is set via setter method. self._parent = None self.parent = parent + # Whether or not to perform validity checks when symbols are added. + self._checking_enabled = checking_enabled # Symbol tables nested within this one. self._children = [] @@ -301,18 +324,23 @@ def add_data_symbol(self, name, primitive_type): raise TypeError( "The primitive type of the symbol must be specified as a str " "but got '{0}'".format(type(primitive_type).__name__)) + lname = name.lower() - if lname in self._data_symbols: - raise SymbolTableError("Symbol table already contains a symbol for" - " a variable with name '{0}'".format(name)) - if lname in self._modules: - raise SymbolTableError("Symbol table already contains a use of a " - "module with name '{0}'".format(name)) - for mod_name in self._modules: - if self._modules[mod_name] and lname in self._modules[mod_name]: + + if self._checking_enabled: + if lname in self._data_symbols: + raise SymbolTableError( + f"Symbol table already contains a symbol for" + f" a variable with name '{name}'") + if lname in self._modules: raise SymbolTableError( - "Symbol table already contains a use of a symbol named " - "'{0}' from module '{1}'".format(name, mod_name)) + f"Symbol table already contains a use of a " + f"module with name '{name}'") + for mod_name, var_list in self._modules.items(): + if var_list and lname in var_list: + raise SymbolTableError( + f"Symbol table already contains a use of a symbol " + f"named '{name}' from module '{mod_name}'") self._data_symbols[lname] = SymbolTable.Symbol(lname, primitive_type.lower()) @@ -342,7 +370,7 @@ def add_use_symbols(self, name, only_list=None): raise TypeError("If present, the only_list must be a list but got " "'{0}'".format(type(only_list).__name__)) if only_list and not all( - [isinstance(item, str) for item in only_list]): + isinstance(item, str) for item in only_list): raise TypeError("If present, the only_list must be a list of str " "but got: {0}".format( [type(item).__name__ for item in only_list])) @@ -481,6 +509,7 @@ def root(self): current = current.parent return current + #: The single, global container for all symbol tables constructed while #: parsing. SYMBOL_TABLES = SymbolTables() diff --git a/src/fparser/two/tests/test_symbol_table.py b/src/fparser/two/tests/test_symbol_table.py index 879c6c0b..e0263d07 100644 --- a/src/fparser/two/tests/test_symbol_table.py +++ b/src/fparser/two/tests/test_symbol_table.py @@ -1,5 +1,5 @@ # ----------------------------------------------------------------------------- -# Copyright (c) 2021 Science and Technology Facilities Council +# Copyright (c) 2021-2022 Science and Technology Facilities Council. # All rights reserved. # # Modifications made as part of the fparser project are distributed @@ -49,6 +49,8 @@ def test_basic_table(): assert table.name == "basic" assert table.parent is None assert table.children == [] + # Consistency checking is disabled by default + assert table._checking_enabled is False with pytest.raises(KeyError) as err: table.lookup("missing") assert "Failed to find symbol named 'missing'" in str(err.value) @@ -57,11 +59,14 @@ def test_basic_table(): sym = table.lookup("var") assert sym.name == "var" assert table.lookup("VAR") is sym + # Check that we can enable consistency checking + table2 = SymbolTable("table2", checking_enabled=True) + assert table2._checking_enabled is True def test_add_data_symbol(): ''' Test that the add_data_symbol() method behaves as expected. ''' - table = SymbolTable("basic") + table = SymbolTable("basic", checking_enabled=True) table.add_data_symbol("var", "integer") sym = table.lookup("var") assert sym.primitive_type == "integer" @@ -90,6 +95,20 @@ def test_add_data_symbol(): "module 'mod1'" in str(err.value)) +def test_add_data_symbols_no_checks(): + ''' Check that we can disable the checks in the + add_data_symbol() method. ''' + table = SymbolTable("basic", checking_enabled=False) + table.add_data_symbol("var", "integer") + table.add_data_symbol("var", "real") + sym = table.lookup("var") + assert sym.primitive_type == "real" + table.add_use_symbols("mod1", ["var3"]) + table.add_data_symbol("mod1", "real") + table.add_use_symbols("mod2", ["var3"]) + table.add_data_symbol("var3", "real") + + def test_add_use_symbols(): ''' Test that the add_use_symbols() method behaves as expected. ''' table = SymbolTable("basic") diff --git a/src/fparser/two/tests/test_symbol_tables.py b/src/fparser/two/tests/test_symbol_tables.py index c209ab4d..e18f0795 100644 --- a/src/fparser/two/tests/test_symbol_tables.py +++ b/src/fparser/two/tests/test_symbol_tables.py @@ -1,5 +1,5 @@ # ----------------------------------------------------------------------------- -# Copyright (c) 2021 Science and Technology Facilities Council +# Copyright (c) 2021-2022 Science and Technology Facilities Council. # All rights reserved. # # Modifications made as part of the fparser project are distributed @@ -48,6 +48,7 @@ def test_construction_addition_removal(): tables = SymbolTables() assert tables._current_scope is None assert tables._symbol_tables == {} + assert tables._enable_checks is False with pytest.raises(KeyError) as err: tables.lookup("missing") assert "missing" in str(err.value) @@ -62,10 +63,16 @@ def test_construction_addition_removal(): "an entry for 'table1'" in str(err.value)) # Add a second table and then remove it table2 = tables.add("taBLe2") + # Check that validation checks are disabled by default + assert table2._checking_enabled is False assert tables.lookup("table2") is table2 tables.remove("table2") with pytest.raises(KeyError) as err: tables.lookup("table2") + # Turn on validation checking + tables.enable_checks(True) + table3 = tables.add("table3") + assert table3._checking_enabled is True # Clear the stored symbol tables tables.clear() assert tables._current_scope is None From fd359076a5eb2815ca92ccd99f70b5a93f4f272a Mon Sep 17 00:00:00 2001 From: Andrew Porter Date: Wed, 15 Jun 2022 16:50:46 +0100 Subject: [PATCH 2/2] #351 final tidying --- src/fparser/two/symbol_table.py | 2 +- src/fparser/two/tests/test_symbol_table.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/fparser/two/symbol_table.py b/src/fparser/two/symbol_table.py index 58004081..c7de8600 100644 --- a/src/fparser/two/symbol_table.py +++ b/src/fparser/two/symbol_table.py @@ -1,7 +1,7 @@ # ----------------------------------------------------------------------------- # BSD 3-Clause License # -# Copyright (c) 2021, Science and Technology Facilities Council. +# Copyright (c) 2021-2022, Science and Technology Facilities Council. # All rights reserved. # # Redistribution and use in source and binary forms, with or without diff --git a/src/fparser/two/tests/test_symbol_table.py b/src/fparser/two/tests/test_symbol_table.py index e0263d07..06ef1d2a 100644 --- a/src/fparser/two/tests/test_symbol_table.py +++ b/src/fparser/two/tests/test_symbol_table.py @@ -65,7 +65,8 @@ def test_basic_table(): def test_add_data_symbol(): - ''' Test that the add_data_symbol() method behaves as expected. ''' + ''' Test that the add_data_symbol() method behaves as expected when + validation is enabled. ''' table = SymbolTable("basic", checking_enabled=True) table.add_data_symbol("var", "integer") sym = table.lookup("var") @@ -107,6 +108,7 @@ def test_add_data_symbols_no_checks(): table.add_data_symbol("mod1", "real") table.add_use_symbols("mod2", ["var3"]) table.add_data_symbol("var3", "real") + assert table.lookup("var3").primitive_type == "real" def test_add_use_symbols():