Compilation: Add CheckUninstantiated flag

This compilation flag enables more checks to be done in untaken generate branches, or gen loops with invalid loop params.

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

Author: AndrewNolte

bindings/python/CompBindings.cpp

@@ -71,6 +71,7 @@ void registerCompilation(py::module_& m, py::module_& ast, py::module_& driver)
         .value("AllowArrayConcatAssignPattern", CompilationFlags::AllowArrayConcatAssignPattern)
         .value("AllowCrossAutoBinMax", CompilationFlags::AllowCrossAutoBinMax)
         .value("AllowInvalidTop", CompilationFlags::AllowInvalidTop)
+        .value("CheckUninstantiated", CompilationFlags::CheckUninstantiated)
         .finalize();
 
     py::classh<CompilationOptions>(ast, "CompilationOptions")

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 {

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,56 @@ void Compilation::addDiagnostics(const Diagnostics& diagnostics) {
         addDiag(diag);
 }
 
+bool shouldReportUninstantiatedDiag(const DiagCode& code) {
+    static const flat_hash_set<DiagCode> BadLookupDiags = {
+        diag::ScopeIndexOutOfRange,
+        diag::InvalidScopeIndexExpression,
+        diag::CouldNotResolveHierarchicalPath,
+        diag::DotIntoInstArray,
+    };
+
+    switch (code.getSubsystem()) {
+        case DiagSubsystem::Declarations:
+            return true;
+        case DiagSubsystem::Lookup:
+            return !BadLookupDiags.contains(code);
+        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) {
+    if (diagsDisabled)
+        return suppressDiag();
+
+    auto isInstantiated = [](const Symbol* symbol) {
         while (symbol) {
             if (symbol->kind == SymbolKind::GenerateBlock)
-                return symbol->as<GenerateBlockSymbol>().isUninstantiated;
+                return !symbol->as<GenerateBlockSymbol>().isUninstantiated;
 
             auto scope = symbol->getParentScope();
             symbol = scope ? &scope->asSymbol() : nullptr;
         }
-        return false;
+        return true;
     };
 
     // 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 (!isInstantiated(diag.symbol)) {
+        if (!hasFlag(CompilationFlags::CheckUninstantiated) ||
+            !shouldReportUninstantiatedDiag(diag.code))
+            return suppressDiag();
     }
 
     const bool isError = diag.isError();
@@ -2449,7 +2475,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/symbols/BlockSymbols.cpp

@@ -913,7 +913,12 @@ GenerateBlockArraySymbol& GenerateBlockArraySymbol::fromSyntax(Compilation& comp
 
     result->entries = entries.copy(comp);
     if (entries.empty()) {
+        // Create block and add it so visitors can access the uninstantiated block; by
+        // default diags from uninstantiated blocks are suppressed, so this is safe. This is used
+        // for things like unused checking and with CompilationFlags::CheckUninstantiated.
+        // It is still inacessible from gen_arr[0] due to how lookups work.
         createBlock(SVInt(32, 0, true), true);
+        result->addMember(*entries[0]);
     }
     else {
         for (auto entry : entries)

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;
     }

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);