From 03012da11c54f614154400b01a12ad30ceeb8a30 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Mon, 30 Sep 2024 22:25:28 -0400 Subject: [PATCH] Internals: astgen: Detect bad node types after edits. Also add checks for nodes that can be multiple types with syntax `AstNode` --- src/V3AstNodeDType.h | 2 +- src/V3AstNodeExpr.h | 2 +- src/V3AstNodeOther.h | 7 ++--- src/astgen | 64 ++++++++++++++++++++++++++++++++++---------- 4 files changed, 56 insertions(+), 19 deletions(-) diff --git a/src/V3AstNodeDType.h b/src/V3AstNodeDType.h index 5c1e24dc9..df32e9455 100644 --- a/src/V3AstNodeDType.h +++ b/src/V3AstNodeDType.h @@ -1121,7 +1121,7 @@ public: bool isCompound() const override { return true; } }; class AstRefDType final : public AstNodeDType { - // @astgen op1 := typeofp : Optional[AstNode] + // @astgen op1 := typeofp : Optional[AstNode] // @astgen op2 := classOrPackageOpp : Optional[AstNodeExpr] // @astgen op3 := paramsp : List[AstPin] // diff --git a/src/V3AstNodeExpr.h b/src/V3AstNodeExpr.h index a1ca9fe73..adcfb3405 100644 --- a/src/V3AstNodeExpr.h +++ b/src/V3AstNodeExpr.h @@ -589,7 +589,7 @@ public: }; class AstAttrOf final : public AstNodeExpr { // Return a value of a attribute, for example a LSB or array LSB of a signal - // @astgen op1 := fromp : Optional[AstNode] // Expr or DType + // @astgen op1 := fromp : Optional[AstNode] // @astgen op2 := dimp : Optional[AstNodeExpr] VAttrType m_attrType; // What sort of extraction public: diff --git a/src/V3AstNodeOther.h b/src/V3AstNodeOther.h index cb50aaa1f..e2d890e43 100644 --- a/src/V3AstNodeOther.h +++ b/src/V3AstNodeOther.h @@ -1407,7 +1407,7 @@ private: }; class AstPin final : public AstNode { // A port or parameter assignment on an instantiation - // @astgen op1 := exprp : Optional[AstNode] // NodeExpr or NodeDType (nullptr if unconnected) + // @astgen op1 := exprp : Optional[AstNode] // nullptr=unconnected // // @astgen ptr := m_modVarp : Optional[AstVar] // Input/output connects to on submodule // @astgen ptr := m_modPTypep : Optional[AstParamTypeDType] // Param type connects to on sub @@ -1781,7 +1781,8 @@ class AstVar final : public AstNode { // @astgen op2 := delayp : Optional[AstDelay] // Net delay // Initial value that never changes (static const), or constructor argument for // MTASKSTATE variables - // @astgen op3 := valuep : Optional[AstNode] // May be a DType for type parameter defaults + // @astgen op3 := valuep : Optional[AstNode] + // Value is a DType for type parameter defaults // @astgen op4 := attrsp : List[AstNode] // Attributes during early parse // @astgen ptr := m_sensIfacep : Optional[AstIface] // Interface type to which reads from this // var are sensitive @@ -2559,7 +2560,7 @@ public: class AstBracketRange final : public AstNodeRange { // Parser only concept "[lhsp]", an AstUnknownRange, QueueRange or Range, // unknown until lhsp type is determined - // @astgen op1 := elementsp : AstNode // Expr or DType + // @astgen op1 := elementsp : AstNode public: AstBracketRange(FileLine* fl, AstNode* elementsp) : ASTGEN_SUPER_BracketRange(fl) { diff --git a/src/astgen b/src/astgen index 2f1355e10..238216720 100755 --- a/src/astgen +++ b/src/astgen @@ -65,9 +65,9 @@ class Node: assert not self.isCompleted self._subClasses.append(subClass) - def addOp(self, n, name, monad, kind): + def addOp(self, n, name, monad, kind, legals): assert 1 <= n <= 4 - self._ops[n] = (name, monad, kind) + self._ops[n] = (name, monad, kind, legals) self._arity = max(self._arity, n) def getOp(self, n): @@ -79,9 +79,9 @@ class Node: return self.superClass.getOp(n) return None - def addPtr(self, name, monad, kind): + def addPtr(self, name, monad, kind, legals): name = re.sub(r'^m_', '', name) - self._ptrs.append({'name': name, 'monad': monad, 'kind': kind}) + self._ptrs.append({'name': name, 'monad': monad, 'kind': kind, 'legals': legals}) # Computes derived properties over entire class hierarchy. # No more changes to the hierarchy are allowed once this was called @@ -519,7 +519,8 @@ def partitionAndStrip(string, separator): def parseOpType(string): - match = re.match(r'^(\w+)\[(\w+)\]$', string) + # Return [Optional/List, AstNodeKind, LegalAstKinds] + match = re.match(r'^(\w+)\[(.*?)\]$', string) if match: monad, kind = match.groups() if monad not in ("Optional", "List"): @@ -527,9 +528,16 @@ def parseOpType(string): kind = parseOpType(kind) if not kind or kind[0]: return None - return monad, kind[1] - if re.match(r'^Ast(\w+)$', string): - return "", string[3:] + return monad, kind[1], kind[2] + match = re.match(r'^Ast(\w+)<(.*?)>$', string) + if match: + kind = match.group(1) + legals = match.group(2) + legals = re.sub(r'\bAst', '', legals) + return "", kind, legals + match = re.match(r'^Ast(\w+)$', string) + if match: + return "", match.group(1), match.group(1) return None @@ -889,7 +897,7 @@ def write_ast_type_info(filename): opTypeList.append('OP_UNUSED') opNameList.append('op{0}p'.format(n)) else: - name, monad, _ = op + name, monad, _, _ = op if not monad: opTypeList.append('OP_USED') elif monad == "Optional": @@ -930,6 +938,34 @@ def write_ast_impl(filename): emitBlock(" BROKEN_RTN(!m_{name});\n" + " BROKEN_RTN(!m_{name}->brokeExists());\n", name=ptr['name']) + if ptr['legals'] != '': + emitBlock(" BROKEN_RTN(m_{name} && !(", name=ptr['name']) + eor = "" + for legal in ptr['legals'].split('|'): + # We use privateTypeTest, as VN_IS would assert that we know + # the type is correct, but we want to check regardless, + # to find errors after raw node edits/replacements + emitBlock("{eor}privateTypeTest(m_{name})", + eor=eor, + name=ptr['name'], + legal=legal) + eor = " || " + emitBlock("));\n") + for i in range(1, 5): + op = node.getOp(i) + if op is None: + continue + name, _, _, legals = op + if legals != '': + emitBlock(" BROKEN_RTN({name}() && !(", name=name) + eor = "" + for legal in legals.split('|'): + emitBlock("{eor}privateTypeTest({name}())", + eor=eor, + name=name, + legal=legal) + eor = " || " + emitBlock("));\n") # Node's broken rules can be specialized by declaring broken() emitBlock(" return Ast{t}::broken();\n", t=node.name) emitBlock("}}\n", t=node.name) @@ -959,7 +995,7 @@ def write_ast_impl(filename): op = node.getOp(i) if op is None: continue - name, _, _ = op + name, _, _, _ = op emitBlock(" dumpNodeListJson(str, {name}(), \"{name}\", indent);\n", name=name) emitBlock("}}\n") @@ -1025,7 +1061,7 @@ def write_ast_macros(filename): op = node.getOp(n) if not op: continue - name, monad, kind = op + name, monad, kind, _ = op retrieve = ("VN_DBG_AS(op{n}p(), {kind})" if kind != "Node" else "op{n}p()").format(n=n, kind=kind) superOp = node.superClass.getOp(n) @@ -1113,7 +1149,7 @@ def write_dfg_macros(filename): s=node.superClass.name) for n in range(1, node.arity + 1): - name, _, _ = node.getOp(n) + name, _, _, _ = node.getOp(n) emitBlock('''\ DfgVertex* {name}() const {{ return source<{n}>(); }} void {name}(DfgVertex* vtxp) {{ relinkSource<{n}>(vtxp); }} @@ -1300,9 +1336,9 @@ for node in AstNodeList: for n in range(1, node.arity + 1): op = node.getOp(n) if op is not None: - name, monad, kind = op + name, monad, kind, legals = op assert monad == "", "Cannot represent AstNode as DfgVertex" - vertex.addOp(n, name, "", "") + vertex.addOp(n, name, "", "", "") # Compute derived properties over the whole DfgVertex hierarchy DfgVertices["Vertex"].complete()