Changeset b9fe89b

Timestamp:

Jun 5, 2023, 1:29:55 PM (4 months ago)

Author:

Michael Brooks <mlbrooks@…>

Branches:

ast-experimental, master

Children:

134e6d9

Parents:

84334d0

Message:

Make the symbol table's error-checking times explicit.

Previously, error checking happened on all WithSymbolTable? uses. Error checking means having a symbol-table add operation potentially cause a user-visible error report. Now, this only happens on the resolver pass's symbol table, while other passes' run in an "assert no errors can happen" mode.

An "ignore errors for now" mode is implemented too, which will be used in upcoming commits, for pre-resolver passes that use the symbol table.

Location:

src

Files:

• AST/Pass.hpp (modified) (1 diff)
• AST/SymbolTable.cpp (modified) (7 diffs)
• AST/SymbolTable.hpp (modified) (5 diffs)
• ResolvExpr/Resolver.cc (modified) (1 diff)

src/AST/Pass.hpp

@@ -414 +414 @@
 };
 
-/// Use when the templated visitor should update the symbol table
+/// Use when the templated visitor should update the symbol table,
+/// that is, when your pass core needs to query the symbol table.
+/// Expected setups:
+/// - For master passes that kick off at the compilation unit
+///   - before resolver: extend WithSymbolTableX<IgnoreErrors>
+///   - after resolver: extend WithSymbolTable and use defaults
+///   - (FYI, for completeness, the resolver's main pass uses ValidateOnAdd when it kicks off)
+/// - For helper passes that kick off at arbitrary points in the AST:
+///   - take an existing symbol table as a parameter, extend WithSymbolTable,
+///     and construct with WithSymbolTable(const SymbolTable &)
 struct WithSymbolTable {
-        SymbolTable symtab;
+        WithSymbolTable(const ast::SymbolTable & from) : symtab(from) {}
+        WithSymbolTable(ast::SymbolTable::ErrorDetection errorMode = ast::SymbolTable::ErrorDetection::AssertClean) : symtab(errorMode) {}
+        ast::SymbolTable symtab;
+};
+template <ast::SymbolTable::ErrorDetection errorMode>
+struct WithSymbolTableX : WithSymbolTable {
+        WithSymbolTableX() : WithSymbolTable(errorMode) {}
 };
 

src/AST/SymbolTable.cpp

@@ -88 +88 @@
 }
 
-SymbolTable::SymbolTable()
+SymbolTable::SymbolTable( ErrorDetection errorMode )
 : idTable(), typeTable(), structTable(), enumTable(), unionTable(), traitTable(),
-  prevScope(), scope( 0 ), repScope( 0 ) { ++*stats().count; }
+  prevScope(), scope( 0 ), repScope( 0 ), errorMode(errorMode) { ++*stats().count; }
 
 SymbolTable::~SymbolTable() { stats().size->push( idTable ? idTable->size() : 0 ); }
+
+void SymbolTable::OnFindError( CodeLocation location, std::string error ) const {
+        assertf( errorMode != AssertClean, "Name collision/redefinition, found during a compilation phase where none should be possible.  Detail: %s", error.c_str() );
+        if (errorMode == ValidateOnAdd) {
+                SemanticError(location, error);
+        }
+        assertf( errorMode == IgnoreErrors, "Unrecognized symbol-table error mode %d", errorMode );
+}
 
 void SymbolTable::enterScope() {
@@ -269 +277 @@
 }
 
-namespace {
-        /// true if redeclaration conflict between two types
-        bool addedTypeConflicts( const NamedTypeDecl * existing, const NamedTypeDecl * added ) {
-                if ( existing->base == nullptr ) {
-                        return false;
-                } else if ( added->base == nullptr ) {
-                        return true;
-                } else {
-                        // typedef redeclarations are errors only if types are different
-                        if ( ! ResolvExpr::typesCompatible( existing->base, added->base ) ) {
-                                SemanticError( added->location, "redeclaration of " + added->name );
-                        }
-                }
-                // does not need to be added to the table if both existing and added have a base that are
-                // the same
+bool SymbolTable::addedTypeConflicts(
+                const NamedTypeDecl * existing, const NamedTypeDecl * added ) const {
+        if ( existing->base == nullptr ) {
+                return false;
+        } else if ( added->base == nullptr ) {
                 return true;
-        }
-
-        /// true if redeclaration conflict between two aggregate declarations
-        bool addedDeclConflicts( const AggregateDecl * existing, const AggregateDecl * added ) {
-                if ( ! existing->body ) {
-                        return false;
-                } else if ( added->body ) {
-                        SemanticError( added, "redeclaration of " );
-                }
-                return true;
-        }
+        } else {
+                // typedef redeclarations are errors only if types are different
+                if ( ! ResolvExpr::typesCompatible( existing->base, added->base ) ) {
+                        OnFindError( added->location, "redeclaration of " + added->name );
+                }
+        }
+        // does not need to be added to the table if both existing and added have a base that are
+        // the same
+        return true;
+}
+
+bool SymbolTable::addedDeclConflicts( 
+                const AggregateDecl * existing, const AggregateDecl * added ) const {
+        if ( ! existing->body ) {
+                return false;
+        } else if ( added->body ) {
+                OnFindError( added, "redeclaration of " );
+        }
+        return true;
 }
 
@@ -648 +654 @@
                 if ( deleter && ! existing.deleter ) {
                         if ( handleConflicts.mode == OnConflict::Error ) {
-                                SemanticError( added, "deletion of defined identifier " );
+                                OnFindError( added, "deletion of defined identifier " );
                         }
                         return true;
                 } else if ( ! deleter && existing.deleter ) {
                         if ( handleConflicts.mode == OnConflict::Error ) {
-                                SemanticError( added, "definition of deleted identifier " );
+                                OnFindError( added, "definition of deleted identifier " );
                         }
                         return true;
@@ -661 +667 @@
                 if ( isDefinition( added ) && isDefinition( existing.id ) ) {
                         if ( handleConflicts.mode == OnConflict::Error ) {
-                                SemanticError( added,
+                                OnFindError( added,
                                         isFunction( added ) ?
                                                 "duplicate function definition for " :
@@ -670 +676 @@
         } else {
                 if ( handleConflicts.mode == OnConflict::Error ) {
-                        SemanticError( added, "duplicate definition for " );
+                        OnFindError( added, "duplicate definition for " );
                 }
                 return true;
@@ -722 +728 @@
                 // Check that a Cforall declaration doesn't override any C declaration
                 if ( hasCompatibleCDecl( name, mangleName ) ) {
-                        SemanticError( decl, "Cforall declaration hides C function " );
+                        OnFindError( decl, "Cforall declaration hides C function " );
                 }
         } else {
@@ -728 +734 @@
                 // type-compatibility, which it may not be.
                 if ( hasIncompatibleCDecl( name, mangleName ) ) {
-                        SemanticError( decl, "conflicting overload of C function " );
+                        OnFindError( decl, "conflicting overload of C function " );
                 }
         }

src/AST/SymbolTable.hpp

@@ -93 +93 @@
 
 public:
-        SymbolTable();
+
+        /// Mode to control when (during which pass) user-caused name-declaration errors get reported.
+        /// The default setting `AssertClean` supports, "I expect all user-caused errors to have been
+        /// reported by now," or, "I wouldn't know what to do with an error; are there even any here?"
+        enum ErrorDetection {
+                AssertClean,               ///< invalid user decls => assert fails during addFoo (default)
+                ValidateOnAdd,             ///< invalid user decls => calls SemanticError during addFoo
+                IgnoreErrors               ///< acts as if unspecified decls were removed, forcing validity
+        };
+
+        explicit SymbolTable(
+                ErrorDetection             ///< mode for the lifetime of the symbol table (whole pass)
+        );
+        SymbolTable() : SymbolTable(AssertClean) {}
         ~SymbolTable();
+
+        ErrorDetection getErrorMode() const {
+                return errorMode;
+        }
 
         // when using an indexer manually (e.g., within a mutator traversal), it is necessary to
@@ -158 +175 @@
 
 private:
+        void OnFindError( CodeLocation location, std::string error ) const;
+
+        template< typename T >
+        void OnFindError( const T * obj, const std::string & error ) const {
+                OnFindError( obj->location, toString( error, obj ) );
+        }
+
+        template< typename T >
+        void OnFindError( CodeLocation location, const T * obj, const std::string & error ) const {
+                OnFindError( location, toString( error, obj ) );
+        }
+
         /// Ensures that a proper backtracking scope exists before a mutation
         void lazyInitScope();
@@ -168 +197 @@
         bool removeSpecialOverrides( IdData & decl, MangleTable::Ptr & mangleTable );
 
-        /// Options for handling identifier conflicts
+        /// Error detection mode given at construction (pass-specific).
+        /// Logically const, except that the symbol table's push-pop is achieved by autogenerated
+        /// assignment onto self.  The feield is left motuable to keep this code-gen simple.
+        /// Conceptual constness is preserved by all SymbolTable in a stack sharing the same mode.
+        ErrorDetection errorMode;
+
+        /// Options for handling identifier conflicts.
+        /// Varies according to AST location during traversal: captures semantics of the construct
+        /// being visited as "would shadow" vs "must not collide."
+        /// At a given AST location, is the same for every pass.
         struct OnConflict {
                 enum {
-                        Error,  ///< Throw a semantic error
+                        Error,  ///< Follow the current pass's ErrorDetection mode (may throw a semantic error)
                         Delete  ///< Delete the earlier version with the delete statement
                 } mode;
@@ -191 +229 @@
                 const Decl * deleter );
 
+        /// true if redeclaration conflict between two types
+        bool addedTypeConflicts( const NamedTypeDecl * existing, const NamedTypeDecl * added ) const;
+
+        /// true if redeclaration conflict between two aggregate declarations
+        bool addedDeclConflicts( const AggregateDecl * existing, const AggregateDecl * added ) const;
+
         /// common code for addId, addDeletedId, etc.
         void addIdCommon(
@@ -213 +257 @@
 }
 
+
 // Local Variables: //
 // tab-width: 4 //

src/ResolvExpr/Resolver.cc

@@ -1261 +1261 @@
                 static size_t traceId;
                 Resolver_new( const ast::TranslationGlobal & global ) :
+                        ast::WithSymbolTable(ast::SymbolTable::ErrorDetection::ValidateOnAdd),
                         context{ symtab, global } {}
                 Resolver_new( const ResolveContext & context ) :