From 220daa5f33b97ca3daaf1fc7eb0d879abf8ef1f5 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Tue, 21 Jan 2020 19:54:14 -0500 Subject: [PATCH] Internals: Restore AstNode naming property. #2133. The intention was that all subclasses of AstNode which are intermediate must be abstract as well and called AstNode*. This was violated recently by 28b9db1903e106e03aa8b2fbd25d6c1b9df8c155. This patch restores that property by: - Renaming AstFile to AstNodeFile - Introducing AstNodeSimpleText as the common base of AstText and AstTextBlock, rather than AstTextBlock deriving from AstText. --- docs/CONTRIBUTORS | 1 + docs/internals.adoc | 8 ++++++-- src/V3AstNodes.h | 35 +++++++++++++++++++++-------------- src/V3EmitC.cpp | 8 ++++---- src/V3EmitCMake.cpp | 4 ++-- src/V3EmitMk.cpp | 4 ++-- src/V3EmitV.cpp | 8 ++++---- src/astgen | 9 +++++++++ 8 files changed, 49 insertions(+), 28 deletions(-) diff --git a/docs/CONTRIBUTORS b/docs/CONTRIBUTORS index 0f7156136..34ac03ccf 100644 --- a/docs/CONTRIBUTORS +++ b/docs/CONTRIBUTORS @@ -11,6 +11,7 @@ Chris Randall Driss Hafdi Eric Rippey Garrett Smith +Geza Lore Gianfranco Costamagna Howard Su Iztok Jeras diff --git a/docs/internals.adoc b/docs/internals.adoc index 3a8550200..d9c4b0893 100644 --- a/docs/internals.adoc +++ b/docs/internals.adoc @@ -70,7 +70,10 @@ The AST is represented at the top level by the class `AstNode`. This abstract class has derived classes for the individual components (e.g. `AstGenerate` for a generate block) or groups of components (e.g. `AstNodeFTask` for functions and tasks, which in turn has `AstFunc` -and `AstTask` as derived classes). +and `AstTask` as derived classes). An important property of the `AstNode` +type hierarchy is that all non-final subclasses of `AstNode` (i.e.: those +which themselves have subclasses) must be abstract as well, and be named +with the prefix `AstNode*`. The `astgen` (see below) script relies on this. Each `AstNode` has pointers to up to four children, accessed by the `op1p` through `op4p` methods. These methods are then abstracted in a specific @@ -959,7 +962,8 @@ src/VParseGrammar.y, as this grammar supports the full SystemVerilog language and has a lot of back-and-forth with Verilator's grammar. Copy the appropriate rules to src/verilog.y and modify the productions. -. If a new Ast type is needed, add it to V3AstNodes.h. +. If a new Ast type is needed, add it to V3AstNodes.h. Follow the convention +described above about the AstNode type hierarchy. . Now you can run "test_regress/t/t_{new testcase}.pl --debug" and it'll probably fail but you'll see a test_regress/obj_dir/t_{newtestcase}/*.tree diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index f1f333772..66235aa1a 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -6355,24 +6355,31 @@ public: //====================================================================== // Text based nodes -class AstText : public AstNodeText { +class AstNodeSimpleText : public AstNodeText { private: bool m_tracking; // When emit, it's ok to parse the string to do indentation public: - AstText(FileLine* fl, const string& textp, bool tracking=false) + AstNodeSimpleText(FileLine* fl, const string& textp, bool tracking=false) : AstNodeText(fl, textp), m_tracking(tracking) {} - ASTNODE_NODE_FUNCS(Text) + ASTNODE_BASE_FUNCS(NodeSimpleText) void tracking(bool flag) { m_tracking = flag; } bool tracking() const { return m_tracking; } }; -class AstTextBlock : public AstText { +class AstText : public AstNodeSimpleText { +public: + AstText(FileLine* fl, const string& textp, bool tracking=false) + : AstNodeSimpleText(fl, textp, tracking) {} + ASTNODE_NODE_FUNCS(Text) +}; + +class AstTextBlock : public AstNodeSimpleText { private: bool m_commas; // Comma separate emitted children public: AstTextBlock(FileLine* fl, const string& textp="", bool tracking=false, bool commas=false) - : AstText(fl, textp, tracking), m_commas(commas) {} + : AstNodeSimpleText(fl, textp, tracking), m_commas(commas) {} ASTNODE_NODE_FUNCS(TextBlock) void commas(bool flag) { m_commas = flag; } bool commas() const { return m_commas; } @@ -6457,18 +6464,18 @@ public: //====================================================================== // Emitted file nodes -class AstFile : public AstNode { +class AstNodeFile : public AstNode { // Emitted Otput file // Parents: NETLIST // Children: AstTextBlock private: string m_name; ///< Filename public: - AstFile(FileLine* fl, const string& name) + AstNodeFile(FileLine* fl, const string& name) : AstNode(fl) { m_name = name; } - ASTNODE_BASE_FUNCS(File) + ASTNODE_BASE_FUNCS(NodeFile) virtual string name() const { return m_name; } virtual V3Hash sameHash() const { return V3Hash(); } virtual bool same(const AstNode* samep) const { return true; } @@ -6479,12 +6486,12 @@ public: //====================================================================== // Emit V nodes -class AstVFile : public AstFile { +class AstVFile : public AstNodeFile { // Verilog output file // Parents: NETLIST public: AstVFile(FileLine* fl, const string& name) - : AstFile(fl, name) { } + : AstNodeFile(fl, name) { } ASTNODE_NODE_FUNCS(VFile) virtual void dump(std::ostream& str=std::cout) const; }; @@ -6492,7 +6499,7 @@ public: //====================================================================== // Emit C nodes -class AstCFile : public AstFile { +class AstCFile : public AstNodeFile { // C++ output file // Parents: NETLIST private: @@ -6501,7 +6508,7 @@ private: bool m_support:1; ///< Support file (non systemc) public: AstCFile(FileLine* fl, const string& name) - : AstFile(fl, name) { + : AstNodeFile(fl, name) { m_slow = false; m_source = false; m_support = false; @@ -6877,8 +6884,8 @@ public: AstNodeModule* topModulep() const { // * = Top module in hierarchy (first one added, for now) return VN_CAST(op1p(), NodeModule); } void addModulep(AstNodeModule* modulep) { addOp1p(modulep); } - AstFile* filesp() const { return VN_CAST(op2p(), File);} // op2 = List of files - void addFilesp(AstFile* filep) { addOp2p(filep); } + AstNodeFile* filesp() const { return VN_CAST(op2p(), NodeFile); } // op2 = List of files + void addFilesp(AstNodeFile* filep) { addOp2p(filep); } AstNode* miscsp() const { return op3p(); } // op3 = List of dtypes etc void addMiscsp(AstNode* nodep) { addOp3p(nodep); } AstTypeTable* typeTablep() { return m_typeTablep; } diff --git a/src/V3EmitC.cpp b/src/V3EmitC.cpp index 79adc98da..bf083b7ba 100644 --- a/src/V3EmitC.cpp +++ b/src/V3EmitC.cpp @@ -642,7 +642,7 @@ public: puts(cvtToStr(nodep->fileline()->lineno())); puts(", \"\");\n"); } - virtual void visit(AstText* nodep) VL_OVERRIDE { + virtual void visit(AstNodeSimpleText* nodep) VL_OVERRIDE { if (nodep->tracking() || m_trackText) { puts(nodep->text()); } else { @@ -650,7 +650,7 @@ public: } } virtual void visit(AstTextBlock* nodep) VL_OVERRIDE { - visit(VN_CAST(nodep, Text)); + visit(VN_CAST(nodep, NodeSimpleText)); for (AstNode* childp = nodep->nodesp(); childp; childp = childp->nextp()) { iterate(childp); if (nodep->commas() && childp->nextp()) puts(", "); @@ -3301,8 +3301,8 @@ void V3EmitC::emitcTrace() { void V3EmitC::emitcFiles() { UINFO(2,__FUNCTION__<<": "<filesp(); filep; - filep = VN_CAST(filep->nextp(), File)) { + for (AstNodeFile* filep = v3Global.rootp()->filesp(); filep; + filep = VN_CAST(filep->nextp(), NodeFile)) { AstCFile* cfilep = VN_CAST(filep, CFile); if (cfilep && cfilep->tblockp()) { V3OutCFile of(cfilep->name()); diff --git a/src/V3EmitCMake.cpp b/src/V3EmitCMake.cpp index bed3591e1..4d2a2a79d 100644 --- a/src/V3EmitCMake.cpp +++ b/src/V3EmitCMake.cpp @@ -125,8 +125,8 @@ class CMakeEmitter { *of << "\n### Sources...\n"; std::vector classes_fast, classes_slow, support_fast, support_slow, global; - for (AstFile* nodep = v3Global.rootp()->filesp(); nodep; - nodep = VN_CAST(nodep->nextp(), File)) { + for (AstNodeFile* nodep = v3Global.rootp()->filesp(); nodep; + nodep = VN_CAST(nodep->nextp(), NodeFile)) { AstCFile* cfilep = VN_CAST(nodep, CFile); if (cfilep && cfilep->source()) { if (cfilep->support()) { diff --git a/src/V3EmitMk.cpp b/src/V3EmitMk.cpp index 1831a305c..fd0868f94 100644 --- a/src/V3EmitMk.cpp +++ b/src/V3EmitMk.cpp @@ -101,8 +101,8 @@ public: else if (support==2 && slow) { } else { - for (AstFile* nodep = v3Global.rootp()->filesp(); - nodep; nodep = VN_CAST(nodep->nextp(), File)) { + for (AstNodeFile* nodep = v3Global.rootp()->filesp(); + nodep; nodep = VN_CAST(nodep->nextp(), NodeFile)) { AstCFile* cfilep = VN_CAST(nodep, CFile); if (cfilep && cfilep->source() && cfilep->slow()==(slow!=0) diff --git a/src/V3EmitV.cpp b/src/V3EmitV.cpp index c6d80ee45..47a3ac5e8 100644 --- a/src/V3EmitV.cpp +++ b/src/V3EmitV.cpp @@ -355,7 +355,7 @@ class EmitVBaseVisitor : public EmitCBaseVisitor { virtual void visit(AstFinish* nodep) VL_OVERRIDE { putfs(nodep, "$finish;\n"); } - virtual void visit(AstText* nodep) VL_OVERRIDE { + virtual void visit(AstNodeSimpleText* nodep) VL_OVERRIDE { if (nodep->tracking() || m_trackText) { puts(nodep->text()); } else { @@ -363,7 +363,7 @@ class EmitVBaseVisitor : public EmitCBaseVisitor { } } virtual void visit(AstTextBlock* nodep) VL_OVERRIDE { - visit(VN_CAST(nodep, Text)); + visit(VN_CAST(nodep, NodeSimpleText)); for (AstNode* childp = nodep->nodesp(); childp; childp = childp->nextp()) { iterate(childp); if (nodep->commas() && childp->nextp()) puts(", "); @@ -779,8 +779,8 @@ void V3EmitV::verilogPrefixedTree(AstNode* nodep, std::ostream& os, void V3EmitV::emitvFiles() { UINFO(2,__FUNCTION__<<": "<filesp(); filep; - filep = VN_CAST(filep->nextp(), File)) { + for (AstNodeFile* filep = v3Global.rootp()->filesp(); filep; + filep = VN_CAST(filep->nextp(), NodeFile)) { AstVFile* vfilep = VN_CAST(filep, VFile); if (vfilep && vfilep->tblockp()) { V3OutVFile of(vfilep->name()); diff --git a/src/astgen b/src/astgen index 184f329f4..fcf7f718f 100644 --- a/src/astgen +++ b/src/astgen @@ -30,6 +30,15 @@ if (! GetOptions( read_types("$Opt_I[0]/V3Ast.h"); read_types("$Opt_I[0]/V3AstNodes.h"); +foreach my $type (sort (keys %Classes)) { + # Chekc all leafs are not AstNode* and non-leave are AstNode* + my @children = children_of($type); + if ($type =~ /^Node/) { + @children || die "Error: Final AstNode subclasses must not be named AstNode*: Ast$type" + } else { + !@children || die "Error: Non-final AstNode subclasses must be named AstNode*: Ast$type" + } +} read_stages("$Opt_I[0]/Verilator.cpp"); read_refs(glob("$Opt_I[0]/*.y"), glob("$Opt_I[0]/*.h"), glob("$Opt_I[0]/*.cpp")); if ($opt_report) {