Merge pull request #351 from stfc/350_disable_checks

(Closes #350) disable validation checks by default

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
@@ -59,13 +59,26 @@ 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"
                   "========================\n".format(
                       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,18 +256,25 @@ 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
     # and visibility). We may need a distinct Symbol class so as to provide
     # 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()

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,15 @@ 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")
+    ''' 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")
     assert sym.primitive_type == "integer"
@@ -90,6 +96,21 @@ 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")
+    assert table.lookup("var3").primitive_type == "real"
+
+
 def test_add_use_symbols():
     ''' Test that the add_use_symbols() method behaves as expected. '''
     table = SymbolTable("basic")

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