Ensure NONSTD warning is always emitted for hier-ref cross references ; Pull through sufficient data to ensure verilog.y doesn't change when we support bare hier-refs as implicit coverpoints

Signed-off-by: Matthew Ballance <matt.ballance@gmail.com>

src/V3AstNodeOther.h

@@ -1185,12 +1185,22 @@ public:
 };
 class AstCoverpointRef final : public AstNode {
     // Reference to a coverpoint used in a cross
-    const string m_name;  // coverpoint name
+    // @astgen op1 := exprp : Optional[AstNodeExpr]  // Non-standard: hierarchical/dotted
+    //                                               // reference (implicit coverpoint), e.g.
+    //                                               // 'cross a.b'; nullptr for a plain coverpoint
+    //                                               // name.  Resolved by the time V3Covergroup
+    //                                               // runs (AstDot is eliminated at link).
+    const string m_name;  // coverpoint name (or flattened dotted path, for diagnostics)
 
 public:
     AstCoverpointRef(FileLine* fl, const string& name)
         : ASTGEN_SUPER_CoverpointRef(fl)
         , m_name{name} {}
+    AstCoverpointRef(FileLine* fl, const string& name, AstNodeExpr* exprp)
+        : ASTGEN_SUPER_CoverpointRef(fl)
+        , m_name{name} {
+        this->exprp(exprp);
+    }
     ASTGEN_MEMBERS_AstCoverpointRef;
     void dump(std::ostream& str) const override;
     void dumpJson(std::ostream& str) const override;

src/V3Covergroup.cpp

@@ -1295,6 +1295,17 @@ class FunctionalCoverageVisitor final : public VNVisitor {
         while (itemp) {
             AstNode* const nextp = itemp->nextp();
             AstCoverpointRef* const refp = VN_AS(itemp, CoverpointRef);
+            if (refp->exprp()) {
+                // Non-standard hierarchical/dotted cross item (e.g. 'cross a.b'): an implicit
+                // coverpoint over the referenced expression.  The grammar already warned NONSTD;
+                // implicit coverpoints are not yet implemented, so drop the whole cross.  This is
+                // the place to generate the implicit coverpoint when support is added (the resolved
+                // expression is in refp->exprp()).
+                refp->v3warn(COVERIGN, "Unsupported: cross of hierarchical reference '"
+                                           << refp->prettyName()
+                                           << "' (implicit coverpoint)");
+                return;
+            }
             // Find the referenced coverpoint via name map (O(log n) vs O(n) linear scan)
             const auto it = m_coverpointMap.find(refp->name());
             AstCoverpoint* const foundCpp = (it != m_coverpointMap.end()) ? it->second : nullptr;

src/verilog.y

@@ -66,6 +66,15 @@ static void STRENGTH_LIST(AstNode* listp, AstStrengthSpec* specp) {
         assignp->strengthSpecp(specp->backp() ? specp->cloneTree(false) : specp);
     }
 }
+// Flatten a parse-time idDotted reference tree (id, a.b, a.b.c) to a dotted name
+// string.  Used for diagnostics on non-standard hierarchical cross items.
+static string CROSS_ITEM_NAME(const AstNode* nodep) {
+    if (const AstDot* const dotp = VN_CAST(nodep, Dot)) {
+        return CROSS_ITEM_NAME(dotp->lhsp()) + (dotp->colon() ? "::" : ".")
+               + CROSS_ITEM_NAME(dotp->rhsp());
+    }
+    return nodep->name();  // AstParseRef (incl. "$root")
+}
 //======================================================================
 // Statics (for here only)
 
@@ -7339,23 +7348,26 @@ cross_itemList<nodep>:  // IEEE: part of list_of_cross_items
         ;
 
 cross_item<nodep>:  // ==IEEE: cross_item
-                id/*cover_point_identifier*/
+        //                      // IEEE: cover_point_identifier | variable_identifier - both are a
-                        { $$ = new AstCoverpointRef{$<fl>1, *$1}; }
+        //                      // simple identifier.  We parse idDotted (a plain hierarchical
-        //                      // Verilator extension beyond strict IEEE (cross_item is a simple
+        //                      // reference a.b.c, with no bit/array selects) to also accept the
-        //                      // identifier): some tools accept a hierarchical/dotted reference
+        //                      // non-standard dotted form (e.g. 'cross a.b') that several
-        //                      // here.  A dotted reference can never name a coverpoint (coverpoint
+        //                      // simulators support; the common simple-identifier case is detected
-        //                      // identifiers are flat, local names), so it is necessarily a plain
+        //                      // and handled exactly as before.
-        //                      // data reference, i.e. an implicit coverpoint over a variable.
+                idDotted
-        //                      // Verilator does not support implicit coverpoints, so V3Covergroup
+                        {
-        //                      // drops the whole cross with a COVERIGN warning; accept it here so
+                          if (AstParseRef* const refp = VN_CAST($1, ParseRef)) {
-        //                      // the file parses instead of erroring on the '.'.
+                              // Standard: simple cover_point_identifier / variable_identifier
-        |       id/*variable*/ crossItemHier
+                              $$ = new AstCoverpointRef{refp->fileline(), refp->name()};
-                        { $$ = new AstCoverpointRef{$<fl>1, *$1 + *$2}; }
+                              VL_DO_DANGLING(refp->deleteTree(), refp);
-        ;
+                          } else {
-
+                              // Verilator extension beyond strict IEEE (cross_item is a simple
-crossItemHier<strp>:  // Verilator extension: dotted suffix of a hierarchical cross_item
+                              // identifier): some tools accept a hierarchical/dotted reference
-                '.' idAny                               { $$ = PARSEP->newString("." + *$2); }
+                              $1->v3warn(NONSTD, "Non-standard hierarchical reference as a coverage "
-        |       crossItemHier '.' idAny                 { $$ = PARSEP->newString(*$1 + "." + *$3); }
+                                                 "cross item (an implicit coverpoint)");
+                              $$ = new AstCoverpointRef{$1->fileline(), CROSS_ITEM_NAME($1), $1};
+                          }
+                        }
         ;
 
 cross_body<nodep>:  // ==IEEE: cross_body

test_regress/t/t_covergroup_cross_opt_unsup.out

@@ -1,3 +1,8 @@
+%Warning-NONSTD: t/t_covergroup_cross_opt_unsup.v:19:34: Non-standard hierarchical reference as a coverage cross item (an implicit coverpoint)
+   19 |     cross_hier: cross cp_a, s_cfg.m_p;
+      |                                  ^
+                 ... For warning description see https://verilator.org/warn/NONSTD?v=latest
+                 ... Use "/* verilator lint_off NONSTD */" and lint_on around source to disable this message.
 %Warning-COVERIGN: t/t_covergroup_cross_opt_unsup.v:13:7: Ignoring unsupported coverage cross option: 'per_instance'
    13 |       option.per_instance = 1;   
       |       ^~~~~~
@@ -7,8 +12,8 @@
                                                          : ... note: In instance 't'
    15 |     cross_implicit: cross cp_a, var_x;
       |                                 ^~~~~
-%Warning-COVERIGN: t/t_covergroup_cross_opt_unsup.v:18:29: Unsupported: cross of 's_cfg.m_p' which is not a coverpoint (implicit coverpoint)
+%Warning-COVERIGN: t/t_covergroup_cross_opt_unsup.v:19:34: Unsupported: cross of hierarchical reference 's_cfg.m_p' (implicit coverpoint)
                                                          : ... note: In instance 't'
-   18 |     cross_hier: cross cp_a, s_cfg.m_p;
+   19 |     cross_hier: cross cp_a, s_cfg.m_p;
-      |                             ^~~~~
+      |                                  ^
 %Error: Exiting due to

test_regress/t/t_covergroup_cross_opt_unsup.v

@@ -13,8 +13,9 @@ module t;
       option.per_instance = 1;  // unsupported for cross; triggers COVERIGN
     }
     cross_implicit: cross cp_a, var_x;
-    // Hierarchical/dotted cross item: can only be a data reference (implicit
+    // Non-standard hierarchical/dotted cross item: can only be a data reference
-    // coverpoint), never a coverpoint; treated as unsupported (COVERIGN)
+    // (implicit coverpoint), never a coverpoint.  Accepted with a NONSTD warning;
+    // implicit coverpoints are unsupported so the cross is dropped (COVERIGN).
     cross_hier: cross cp_a, s_cfg.m_p;
   endgroup
   typedef struct packed {logic m_p; logic h_mode;} cfg_t;