Add interface port usage check

MikePopoloski · Aug 21, 2024 · 65784ef · 65784ef
1 parent 50492ea
commit 65784ef
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 10 deletions.
diff --git a/scripts/diagnostics.txt b/scripts/diagnostics.txt
@@ -490,6 +490,7 @@ error NoChangeEdgeRequired "trigger for first argument to '$nochange' must be po
 error PathPulseInvalidPathName "PATHPULSE$ path '{}' must be of the form 'source$dest'"
 error VirtualInterfaceIfaceMember "virtual interfaces cannot be used as the type for interface items"
 error VirtualInterfaceHierRef "Declaration of virtual interface should not contain hierarchical references to objects outside its body"
+error VirtualInterfaceIfacePort "Declaration of virtual interface should not contain ports that reference other interfaces"
 error DefparamBadHierarchy "defparam in a hierarchy in or under a generate block, array of instances, or bind instantiation cannot change a parameter value outside that hierarchy"
 error BindTargetPrimitive "cannot bind a primitive"
 error BindUnderBind "cannot bind an instance underneath the scope of another bind instance"

diff --git a/source/ast/types/AllTypes.cpp b/source/ast/types/AllTypes.cpp
@@ -1136,12 +1136,22 @@ const Type& VirtualInterfaceType::fromSyntax(const ASTContext& context,
     SmallSet<const DefinitionSymbol*, 4> visited;
     // Find cached instance hierarchical identifiers
     std::function<std::optional<const InstanceSymbol*>(const Scope& scope)> findInst =
-        [&defSym, &findInst, &visited](const Scope& scope) -> std::optional<const InstanceSymbol*> {
+        [&defSym, &findInst, &visited, &context,
+         &iface](const Scope& scope) -> std::optional<const InstanceSymbol*> {
         for (auto& name : scope.getNameMap()) {
             if (auto inst = name.second->as_if<InstanceSymbol>()) {
                 auto instDef = &inst->getDefinition();
-                if (instDef == &defSym && inst->body.hierIdentifiers.has_value())
+                if (instDef == &defSym && inst->body.hierIdentifiers.has_value()) {
+                    for (auto port : inst->body.getPortList()) {
+                        // Check that there are an interface ports if we have elaboration cache
+                        if (port->kind == SymbolKind::InterfacePort) {
+                            auto& diag = context.addDiag(diag::VirtualInterfaceIfacePort,
+                                                         port->location);
+                            diag.addNote(diag::NoteDeclarationHere, iface.location);
+                        }
+                    }
                     return inst;
+                }
 
                 if (visited.contains(instDef))
                     continue;
@@ -1160,6 +1170,14 @@ const Type& VirtualInterfaceType::fromSyntax(const ASTContext& context,
     if (!founded.has_value()) {
         iface.body.collectHierIdents(comp);
         toReport = &iface;
+        // Elaborate ports to check interface type presence if previously elaborated instance was
+        // not found
+        for (auto port : iface.body.getPortList()) {
+            if (port->kind == SymbolKind::InterfacePort) {
+                auto& diag = context.addDiag(diag::VirtualInterfaceIfacePort, port->location);
+                diag.addNote(diag::NoteDeclarationHere, iface.location);
+            }
+        }
     }
     else {
         toReport = founded.value();

diff --git a/tests/unittests/ast/InterfaceTests.cpp b/tests/unittests/ast/InterfaceTests.cpp
@@ -730,11 +730,21 @@ TEST_CASE("Virtual interface declaration errors") {
 localparam type requestType = byte;
 localparam type responseType = int;
 
+interface I;
+
+wire r;
+
+modport ii(input r);
+
+endinterface
+
 module testMod#(N=16);
 
   wire clk, rst;
 
-  allIfc#(N) allInst(clk, rst);
+  I i();
+  allIfc#(N) allInst(clk, rst, i, i.ii);
+  virtual allIfc#(N) allInst1;
   sliceIfc sliceInst();
   virtual sliceIfc sliceInst1;
 
@@ -744,12 +754,12 @@ module testMod1#(N=16);
 
   wire clk, rst;
 
-  allIfc#(N) allInst(clk, rst);
+  virtual allIfc#(N) allInst1;
   virtual sliceIfc1 sliceInst;
 
 endmodule:testMod1
 
-interface automatic allIfc#(N=1)(input clk, rst);
+interface automatic allIfc#(N=1)(input clk, rst, I i, I.ii i1);
 
   var requestType Requests[N];
   var responseType Responses[N];
@@ -778,7 +788,8 @@ interface automatic sliceIfc#(I=0)();
   II ii();  // invalid
   wire reset = ii.reset;  // valid
 
-  allIfc allInst(.clk(), .rst());
+  I i();
+  allIfc allInst(.clk(), .rst(), .i(i), .i1(i.ii));
   var requestType request;
   var responseType response;
 
@@ -809,7 +820,8 @@ interface automatic sliceIfc1#(I=0)();
   II ii();  // invalid
   wire reset = ii.reset;  // valid
 
-  allIfc allInst(.clk(), .rst());
+  I i();
+  allIfc allInst(.clk(), .rst(), .i(i), .i1(i.ii));
   var requestType request;
   var responseType response;
 
@@ -838,13 +850,15 @@ endinterface:sliceIfc1
     compilation.addSyntaxTree(tree);
 
     auto& diags = compilation.getAllDiagnostics();
-    REQUIRE(diags.size() == 8);
-    CHECK(diags[0].code == diag::VirtualInterfaceHierRef);
-    CHECK(diags[1].code == diag::VirtualInterfaceHierRef);
+    REQUIRE(diags.size() == 10);
+    CHECK(diags[0].code == diag::VirtualInterfaceIfacePort);
+    CHECK(diags[1].code == diag::VirtualInterfaceIfacePort);
     CHECK(diags[2].code == diag::VirtualInterfaceHierRef);
     CHECK(diags[3].code == diag::VirtualInterfaceHierRef);
     CHECK(diags[4].code == diag::VirtualInterfaceHierRef);
     CHECK(diags[5].code == diag::VirtualInterfaceHierRef);
     CHECK(diags[6].code == diag::VirtualInterfaceHierRef);
     CHECK(diags[7].code == diag::VirtualInterfaceHierRef);
+    CHECK(diags[8].code == diag::VirtualInterfaceHierRef);
+    CHECK(diags[9].code == diag::VirtualInterfaceHierRef);
 }