Compilation: Add CheckUninstantiated flag

stack-info: PR: MikePopoloski#1901, branch: AndrewNolte/stack/18

Author: AndrewNolte

include/slang/ast/Compilation.h

@@ -145,8 +145,13 @@ enum class SLANG_EXPORT CompilationFlags {
 
     /// Allow top-level modules to be selected even when their parameters have no defaults.
     AllowInvalidTop = 1 << 20,
+
+    /// Elaborate code that would normally be skipped because it is uninstantiated
+    /// (untaken generate branches and uninstantiated module instances) so that
+    /// additional lints, like port and parameter name checks, can run on it.
+    CheckUninstantiated = 1 << 21,
 };
-SLANG_BITMASK(CompilationFlags, AllowInvalidTop)
+SLANG_BITMASK(CompilationFlags, CheckUninstantiated)
 
 /// Contains various options that can control compilation behavior.
 struct SLANG_EXPORT CompilationOptions {

include/slang/ast/Symbol.h

@@ -173,6 +173,9 @@ class SLANG_EXPORT Symbol {
     /// Determines whether this symbol represents a value.
     bool isValue() const;
 
+    /// Determines whether this symbol is instantiated.
+    bool isInstantiated() const;
+
     /// If the symbol has a declared type, returns a pointer to it. Otherwise returns nullptr.
     const DeclaredType* getDeclaredType() const;
 

include/slang/diagnostics/Diagnostics.h

@@ -99,6 +99,17 @@ class SLANG_EXPORT DiagCode {
     /// Explicit boolean conversion operator that defers to @a valid
     constexpr explicit operator bool() const { return valid(); }
 
+    /// Wrapper type for use in switch statements.
+    struct Key {
+        uint32_t value;
+        constexpr operator uint32_t() const { return value; }
+    };
+
+    /// Returns a switchable key for use in switch/case statements.
+    constexpr Key key() const {
+        return {(uint32_t(subsystem) << 16) | code};
+    }
+
     /// Three way comparison.
     constexpr friend auto operator<=>(DiagCode left, DiagCode right) = default;
 

source/ast/Compilation.cpp

@@ -16,6 +16,7 @@
 #include "slang/ast/SystemSubroutine.h"
 #include "slang/ast/types/TypePrinter.h"
 #include "slang/diagnostics/DiagnosticEngine.h"
+#include "slang/diagnostics/Diagnostics.h"
 #include "slang/diagnostics/LookupDiags.h"
 #include "slang/parsing/Parser.h"
 #include "slang/parsing/Preprocessor.h"
@@ -1688,31 +1689,47 @@ void Compilation::addDiagnostics(const Diagnostics& diagnostics) {
         addDiag(diag);
 }
 
+bool shouldReportUninstantiatedDiag(const DiagCode& code) {
+    switch (code.getSubsystem()) {
+        case DiagSubsystem::Declarations:
+            return true;
+        case DiagSubsystem::Lookup:
+            switch (code.key()) {
+                case diag::ScopeIndexOutOfRange.key():
+                case diag::InvalidScopeIndexExpression.key():
+                case diag::CouldNotResolveHierarchicalPath.key():
+                case diag::DotIntoInstArray.key():
+                    return false;
+                default:
+                    break;
+            }
+            return true;
+        default:
+            break;
+    }
+
+    return false;
+}
+
 Diagnostic& Compilation::addDiag(Diagnostic diag) {
     SLANG_ASSERT(!isFrozen());
 
-    if (diagsDisabled) {
+    auto suppressDiag = [&]() -> Diagnostic& {
         tempDiag = std::move(diag);
         return tempDiag;
-    }
-
-    auto isSuppressed = [](const Symbol* symbol) {
-        while (symbol) {
-            if (symbol->kind == SymbolKind::GenerateBlock)
-                return symbol->as<GenerateBlockSymbol>().isUninstantiated;
-
-            auto scope = symbol->getParentScope();
-            symbol = scope ? &scope->asSymbol() : nullptr;
-        }
-        return false;
     };
 
+    if (diagsDisabled)
+        return suppressDiag();
+
     // Filter out diagnostics that came from inside an uninstantiated generate block.
     SLANG_ASSERT(diag.symbol);
     SLANG_ASSERT(diag.location);
-    if (isSuppressed(diag.symbol)) {
-        tempDiag = std::move(diag);
-        return tempDiag;
+
+    if (!diag.symbol->isInstantiated()) {
+        if (!hasFlag(CompilationFlags::CheckUninstantiated) ||
+            !shouldReportUninstantiatedDiag(diag.code))
+            return suppressDiag();
     }
 
     const bool isError = diag.isError();
@@ -2449,7 +2466,8 @@ std::pair<Compilation::DefinitionLookupResult, bool> Compilation::resolveConfigR
 
 Diagnostic* Compilation::errorMissingDef(std::string_view name, const Scope& scope,
                                          SourceRange sourceRange, DiagCode code) const {
-    if (hasFlag(CompilationFlags::IgnoreUnknownModules) || scope.isUninstantiated() || name.empty())
+    if (hasFlag(CompilationFlags::IgnoreUnknownModules) || name.empty() ||
+        (scope.isUninstantiated() && !hasFlag(CompilationFlags::CheckUninstantiated)))
         return nullptr;
 
     if (auto def = getExternDefinition(name, scope)) {

source/ast/Symbol.cpp

@@ -90,6 +90,18 @@ const DeclaredType* Symbol::getDeclaredType() const {
     }
 }
 
+bool Symbol::isInstantiated() const {
+    auto symbol = this;
+    while (symbol) {
+        if (symbol->kind == SymbolKind::GenerateBlock)
+            return !(symbol->as<GenerateBlockSymbol>().isUninstantiated);
+
+        auto scope = symbol->getParentScope();
+        symbol = scope ? &scope->asSymbol() : nullptr;
+    }
+    return true;
+}
+
 static void getHierarchicalPathImpl(const Symbol& symbol, FormatBuffer& buffer,
                                     SmallSet<const Symbol*, 4>& visited) {
     auto scope = symbol.getParentScope();

source/ast/symbols/BlockSymbols.cpp

@@ -913,12 +913,15 @@ GenerateBlockArraySymbol& GenerateBlockArraySymbol::fromSyntax(Compilation& comp
 
     result->entries = entries.copy(comp);
     if (entries.empty()) {
+        // Keep result->entries empty so indexed lookup (g[0], etc.) still reports
+        // the same out-of-range errors for zero-iteration arrays. The synthetic
+        // block is only exposed through generic member traversal; callers that
+        // don't want speculative semantic-checking contents can ignore members
+        // where isInstantiated() is false.
         createBlock(SVInt(32, 0, true), true);
     }
-    else {
-        for (auto entry : entries)
-            result->addMember(*entry);
-    }
+    for (auto entry : entries)
+        result->addMember(*entry);
 
     return *result;
 }

source/ast/symbols/InstanceSymbols.cpp

@@ -505,7 +505,8 @@ void InstanceSymbol::fromSyntax(Compilation& comp, const HierarchyInstantiationS
 
     // If this instance is not instantiated then we'll just fill in a placeholder
     // and move on. This is likely inside an untaken generate branch.
-    if (flags.has(InstanceFlags::Uninstantiated)) {
+    if (flags.has(InstanceFlags::Uninstantiated) &&
+        !comp.hasFlag(CompilationFlags::CheckUninstantiated)) {
         UninstantiatedDefSymbol::fromSyntax(comp, syntax, context, results, implicitNets);
         return;
     }
@@ -967,11 +968,15 @@ InstanceBodySymbol& InstanceBodySymbol::fromDefinition(
 
     ParameterBuilder paramBuilder(*definition.getParentScope(), definition.name,
                                   definition.parameters);
-    paramBuilder.setForceInvalidValues(flags.has(InstanceFlags::Uninstantiated));
-    if (compilation.hasFlag(CompilationFlags::AllowInvalidTop) &&
-        instanceLoc == definition.location) {
+
+    if (flags.has(InstanceFlags::Uninstantiated)) {
+        paramBuilder.setForceInvalidValues(true);
+    }
+    else if (compilation.hasFlag(CompilationFlags::AllowInvalidTop) &&
+             instanceLoc == definition.location) {
         paramBuilder.setUseInvalidForMissing(true);
     }
+
     if (hierarchyOverrideNode)
         paramBuilder.setOverrides(hierarchyOverrideNode);
 

tests/unittests/ast/HierarchyTests.cpp

@@ -1520,6 +1520,77 @@ endmodule
     NO_COMPILATION_ERRORS;
 }
 
+TEST_CASE("Check uninstantiated reports bindable errors") {
+    auto text = R"(
+module child(input logic a);
+endmodule
+
+module top;
+    if (0) begin
+        child child(.missing(1'b1));
+        missing missing();
+        string s;
+        initial s.foobar();
+    end
+endmodule
+)";
+
+    auto hasCode = [](auto& diags, DiagCode code) {
+        return std::ranges::any_of(diags, [&](auto& diag) { return diag.code == code; });
+    };
+
+    // By default the contents of an untaken generate branch are not elaborated, so
+    // none of these problems are reported.
+    {
+        Compilation compilation;
+        compilation.addSyntaxTree(SyntaxTree::fromText(text));
+        NO_COMPILATION_ERRORS;
+    }
+
+    // With the flag the branch is elaborated and structural / lookup errors surface.
+    {
+        CompilationOptions options;
+        options.flags |= CompilationFlags::CheckUninstantiated;
+
+        Compilation compilation(options);
+        compilation.addSyntaxTree(SyntaxTree::fromText(text));
+
+        auto& diags = compilation.getAllDiagnostics();
+        CHECK(hasCode(diags, diag::PortDoesNotExist));
+        CHECK(hasCode(diags, diag::UnknownModule));
+        CHECK(hasCode(diags, diag::UnknownSystemMethod));
+    }
+}
+
+TEST_CASE("Check uninstantiated still suppresses dead-code-only diagnostics") {
+    // Diagnostics that would be false positives in never-taken code (e.g. an out
+    // of bounds index that is only reachable in the untaken branch) stay
+    // suppressed even when CheckUninstantiated is enabled.
+    auto text = R"(
+module top;
+    logic [3:0] arr;
+    if (0) begin
+        wire w = arr[7];
+    end
+endmodule
+)";
+
+    auto hasCode = [](auto& diags, DiagCode code) {
+        return std::ranges::any_of(diags, [&](auto& diag) { return diag.code == code; });
+    };
+
+    {
+        CompilationOptions options;
+        options.flags |= CompilationFlags::CheckUninstantiated;
+
+        Compilation compilation(options);
+        compilation.addSyntaxTree(SyntaxTree::fromText(text));
+
+        auto& diags = compilation.getAllDiagnostics();
+        CHECK_FALSE(hasCode(diags, diag::IndexOOB));
+    }
+}
+
 TEST_CASE("Bind directives") {
     auto tree = SyntaxTree::fromText(R"(
 module baz(input q);