From 70603bb752870ea4fe34acc2125320e1e8deeec3 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Fri, 22 Oct 2021 17:36:58 +0100 Subject: [PATCH] Add static assertions for unnecessary VN_IS/VN_AS/VN_CAST Fail at compile time if the result of these macros can be statically determined (i.e.: they aways succeed or always fail). Remove unnecessary casts discovered. No functional change. --- src/V3Active.cpp | 3 +-- src/V3ActiveTop.cpp | 3 +-- src/V3Ast.h | 57 +++++++++++++++++++++++++++++++++++---------- src/V3AstNodes.h | 8 ++----- src/V3Broken.cpp | 2 +- src/V3Const.cpp | 8 +++---- src/V3EmitCFunc.cpp | 8 ++----- src/V3EmitCFunc.h | 4 ++-- src/V3EmitV.cpp | 2 +- src/V3Gate.cpp | 7 +++--- src/V3Inline.cpp | 2 +- src/V3LinkDot.cpp | 5 ++-- src/V3Order.cpp | 8 +++---- src/V3Param.cpp | 2 +- src/V3Simulate.h | 4 ++-- src/V3TraceDecl.cpp | 2 +- src/V3Width.cpp | 2 +- src/astgen | 15 +++++++----- 18 files changed, 83 insertions(+), 59 deletions(-) diff --git a/src/V3Active.cpp b/src/V3Active.cpp index f7292f9e6..d7501522f 100644 --- a/src/V3Active.cpp +++ b/src/V3Active.cpp @@ -489,8 +489,7 @@ private: // METHODS void visitAlways(AstNode* nodep, AstSenTree* oldsensesp, VAlwaysKwd kwd) { // Move always to appropriate ACTIVE based on its sense list - if (oldsensesp && oldsensesp->sensesp() && VN_IS(oldsensesp->sensesp(), SenItem) - && VN_AS(oldsensesp->sensesp(), SenItem)->isNever()) { + if (oldsensesp && oldsensesp->sensesp() && oldsensesp->sensesp()->isNever()) { // Never executing. Kill it. UASSERT_OBJ(!oldsensesp->sensesp()->nextp(), nodep, "Never senitem should be alone, else the never should be eliminated."); diff --git a/src/V3ActiveTop.cpp b/src/V3ActiveTop.cpp index d76ebd6db..3fe652d58 100644 --- a/src/V3ActiveTop.cpp +++ b/src/V3ActiveTop.cpp @@ -70,8 +70,7 @@ private: V3Const::constifyExpensiveEdit(nodep); AstSenTree* sensesp = nodep->sensesp(); UASSERT_OBJ(sensesp, nodep, "nullptr"); - if (sensesp->sensesp() && VN_IS(sensesp->sensesp(), SenItem) - && VN_AS(sensesp->sensesp(), SenItem)->isNever()) { + if (sensesp->sensesp() && sensesp->sensesp()->isNever()) { // Never executing. Kill it. UASSERT_OBJ(!sensesp->sensesp()->nextp(), nodep, "Never senitem should be alone, else the never should be eliminated."); diff --git a/src/V3Ast.h b/src/V3Ast.h index dbe75cde4..b5d8262a0 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -62,17 +62,20 @@ using MTaskIdSet = std::set; // Set of mtaskIds for Var sorting // (V)erilator (N)ode is: Returns true iff AstNode is of the given AstNode subtype, and not // nullptr. -#define VN_IS(nodep, nodetypename) (AstNode::privateIs(nodep)) +#define VN_IS(nodep, nodetypename) (AstNode::privateIs(nodep)) // (V)erilator (N)ode cast: More efficient but otherwise same as dynamic_cast, use this instead. // Cast to given type if node is of such type, otherwise returns nullptr. -#define VN_CAST(nodep, nodetypename) (AstNode::privateCast(nodep)) -#define VN_CAST_CONST(nodep, nodetypename) (AstNode::privateCastConst(nodep)) +#define VN_CAST(nodep, nodetypename) \ + (AstNode::privateCast(nodep)) +#define VN_CAST_CONST(nodep, nodetypename) \ + (AstNode::privateCastConst(nodep)) // (V)erilator (N)ode as: Assert node is of given type then cast to that type. Node can be nullptr. // Use this to downcast instead of VN_CAST when you know the true type of the node. -#define VN_AS(nodep, nodetypename) (AstNode::privateAs(nodep)) -#define VN_AS_CONST(nodep, nodetypename) (AstNode::privateAsConst(nodep)) +#define VN_AS(nodep, nodetypename) (AstNode::privateAs(nodep)) +#define VN_AS_CONST(nodep, nodetypename) \ + (AstNode::privateAsConst(nodep)) // (V)erilator (N)ode deleted: Pointer to deleted AstNode (for assertions only) #define VN_DELETED(nodep) VL_UNLIKELY((vluint64_t)(nodep) == 0x1) @@ -1847,28 +1850,58 @@ private: // For internal use only. template inline static bool privateTypeTest(const AstNode* nodep); + template constexpr static bool uselessCast() { + using NonRef = typename std::remove_reference::type; + using NonPtr = typename std::remove_pointer::type; + using NonCV = typename std::remove_cv::type; + return std::is_base_of::value; + } + + template constexpr static bool impossibleCast() { + using NonRef = typename std::remove_reference::type; + using NonPtr = typename std::remove_pointer::type; + using NonCV = typename std::remove_cv::type; + return !std::is_base_of::value; + } + public: // For use via the VN_IS macro only - template inline static bool privateIs(const AstNode* nodep) { + template inline static bool privateIs(const AstNode* nodep) { + static_assert(!uselessCast(), "Unnecessary VN_IS, node known to have target type."); + static_assert(!impossibleCast(), "Unnecessary VN_IS, node cannot be this type."); return nodep && privateTypeTest(nodep); } // For use via the VN_CAST macro only - template inline static T* privateCast(AstNode* nodep) { - return privateIs(nodep) ? reinterpret_cast(nodep) : nullptr; + template inline static T* privateCast(AstNode* nodep) { + static_assert(!uselessCast(), + "Unnecessary VN_CAST, node known to have target type."); + static_assert(!impossibleCast(), "Unnecessary VN_CAST, node cannot be this type."); + return nodep && privateTypeTest(nodep) ? reinterpret_cast(nodep) : nullptr; } // For use via the VN_CAST_CONST macro only - template inline static const T* privateCastConst(const AstNode* nodep) { - return privateIs(nodep) ? reinterpret_cast(nodep) : nullptr; + template + inline static const T* privateCastConst(const AstNode* nodep) { + static_assert(!uselessCast(), + "Unnecessary VN_CAST_CONST, node known to have target type."); + static_assert(!impossibleCast(), + "Unnecessary VN_CAST_CONST, node cannot be this type."); + return nodep && privateTypeTest(nodep) ? reinterpret_cast(nodep) : nullptr; } // For use via the VN_AS macro only - template inline static T* privateAs(AstNode* nodep) { + template inline static T* privateAs(AstNode* nodep) { + static_assert(!uselessCast(), "Unnecessary VN_AS, node known to have target type."); + static_assert(!impossibleCast(), "Unnecessary VN_AS, node cannot be this type."); UASSERT_OBJ(!nodep || privateTypeTest(nodep), nodep, "AstNode is not of expected type, but instead has type '" << nodep->typeName() << "'"); return reinterpret_cast(nodep); } // For use via the VN_AS_CONST macro only - template inline static const T* privateAsConst(const AstNode* nodep) { + template inline static const T* privateAsConst(const AstNode* nodep) { + static_assert(!uselessCast(), + "Unnecessary VN_AS_CONST, node known to have target type."); + static_assert(!impossibleCast(), + "Unnecessary VN_AS_CONST, node cannot be this type."); UASSERT_OBJ(!nodep || privateTypeTest(nodep), nodep, "AstNode is not of expected type, but instead has type '" << nodep->typeName() << "'"); diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index 76d2c1a8f..303bf7894 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -1456,13 +1456,9 @@ public: ASTNODE_NODE_FUNCS(EnumItemRef) virtual void dump(std::ostream& str) const override; virtual string name() const override { return itemp()->name(); } - virtual const char* broken() const override { - BROKEN_RTN(!VN_IS(itemp(), EnumItem)); - return nullptr; - } virtual int instrCount() const override { return 0; } virtual void cloneRelink() override { - if (m_itemp->clonep()) m_itemp = VN_AS(m_itemp->clonep(), EnumItem); + if (m_itemp->clonep()) m_itemp = m_itemp->clonep(); } virtual bool same(const AstNode* samep) const override { const AstEnumItemRef* sp = static_cast(samep); @@ -4976,7 +4972,7 @@ public: virtual void dump(std::ostream& str) const override; virtual const char* broken() const override { for (KeyItemMap::const_iterator it = m_map.begin(); it != m_map.end(); ++it) { - BROKEN_RTN(!VN_IS(it->second, InitItem)); + BROKEN_RTN(!it->second); BROKEN_RTN(!it->second->brokeExists()); } return nullptr; diff --git a/src/V3Broken.cpp b/src/V3Broken.cpp index fb10a1972..66beaeb2a 100644 --- a/src/V3Broken.cpp +++ b/src/V3Broken.cpp @@ -197,7 +197,7 @@ private: if (nodep->dtypep()) { UASSERT_OBJ(nodep->dtypep()->brokeExists(), nodep, "Broken link in node->dtypep() to " << cvtToHex(nodep->dtypep())); - UASSERT_OBJ(VN_IS(nodep->dtypep(), NodeDType), nodep, + UASSERT_OBJ(nodep->dtypep(), nodep, "Non-dtype link in node->dtypep() to " << cvtToHex(nodep->dtypep())); } if (v3Global.assertDTypesResolved()) { diff --git a/src/V3Const.cpp b/src/V3Const.cpp index 6b8187fe6..48b4a2822 100644 --- a/src/V3Const.cpp +++ b/src/V3Const.cpp @@ -2671,7 +2671,7 @@ private: { AstUser4InUse m_inuse4; // Mark x in SENITEM(x) - for (AstSenItem* senp = VN_AS(nodep->sensesp(), SenItem); senp; + for (AstSenItem* senp = nodep->sensesp(); senp; senp = VN_AS(senp->nextp(), SenItem)) { if (senp->varrefp() && senp->varrefp()->varScopep()) { senp->varrefp()->varScopep()->user4(1); @@ -2682,7 +2682,7 @@ private: // Sort the sensitivity names so "posedge a or b" and "posedge b or a" end up together. // Also, remove duplicate assignments, and fold POS&NEGs into ANYEDGEs // Make things a little faster; check first if we need a sort - for (AstSenItem *nextp, *senp = VN_AS(nodep->sensesp(), SenItem); senp; senp = nextp) { + for (AstSenItem *nextp, *senp = nodep->sensesp(); senp; senp = nextp) { nextp = VN_AS(senp->nextp(), SenItem); // cppcheck-suppress unassignedVariable // cppcheck bug SenItemCmp cmp; @@ -2690,7 +2690,7 @@ private: // Something's out of order, sort it senp = nullptr; std::vector vec; - for (AstSenItem* senp = VN_AS(nodep->sensesp(), SenItem); senp; + for (AstSenItem* senp = nodep->sensesp(); senp; senp = VN_AS(senp->nextp(), SenItem)) { vec.push_back(senp); } @@ -2702,7 +2702,7 @@ private: } // Pass2, remove dup edges - for (AstSenItem *nextp, *senp = VN_AS(nodep->sensesp(), SenItem); senp; senp = nextp) { + for (AstSenItem *nextp, *senp = nodep->sensesp(); senp; senp = nextp) { nextp = VN_AS(senp->nextp(), SenItem); AstSenItem* const litemp = senp; AstSenItem* const ritemp = nextp; diff --git a/src/V3EmitCFunc.cpp b/src/V3EmitCFunc.cpp index 429396bc0..0910c6cfb 100644 --- a/src/V3EmitCFunc.cpp +++ b/src/V3EmitCFunc.cpp @@ -533,13 +533,11 @@ void EmitCFunc::emitConstant(AstConst* nodep, AstVarRef* assigntop, const string puts(","); if (!assigntop) { puts(assignString); - } else if (VN_IS(assigntop, VarRef)) { + } else { if (!assigntop->selfPointer().empty()) { emitDereference(assigntop->selfPointerProtect(m_useSelfForThis)); } puts(assigntop->varp()->nameProtect()); - } else { - iterateAndNextNull(assigntop); } for (int word = VL_WORDS_I(upWidth) - 1; word >= 0; word--) { // Only 32 bits - llx + long long here just to appease CPP format warning @@ -558,13 +556,11 @@ void EmitCFunc::emitConstant(AstConst* nodep, AstVarRef* assigntop, const string puts(","); if (!assigntop) { puts(assignString); - } else if (VN_IS(assigntop, VarRef)) { + } else { if (!assigntop->selfPointer().empty()) { emitDereference(assigntop->selfPointerProtect(m_useSelfForThis)); } puts(assigntop->varp()->nameProtect()); - } else { - iterateAndNextNull(assigntop); } for (int word = EMITC_NUM_CONSTW - 1; word >= 0; word--) { // Only 32 bits - llx + long long here just to appease CPP format warning diff --git a/src/V3EmitCFunc.h b/src/V3EmitCFunc.h index 0c0b85060..55d3a8151 100644 --- a/src/V3EmitCFunc.h +++ b/src/V3EmitCFunc.h @@ -898,7 +898,7 @@ public: } } virtual void visit(AstTextBlock* nodep) override { - visit(VN_AS(nodep, NodeSimpleText)); + visit(static_cast(nodep)); for (AstNode* childp = nodep->nodesp(); childp; childp = childp->nextp()) { iterate(childp); if (nodep->commas() && childp->nextp()) puts(", "); @@ -978,7 +978,7 @@ public: } virtual void visit(AstRedXor* nodep) override { if (nodep->lhsp()->isWide()) { - visit(VN_AS(nodep, NodeUniop)); + visit(static_cast(nodep)); } else { AstVarRef* const vrefp = VN_CAST(nodep->lhsp(), VarRef); const int widthPow2 = vrefp ? vrefp->varp()->dtypep()->widthPow2() diff --git a/src/V3EmitV.cpp b/src/V3EmitV.cpp index da749f237..8c21a17b3 100644 --- a/src/V3EmitV.cpp +++ b/src/V3EmitV.cpp @@ -382,7 +382,7 @@ class EmitVBaseVisitor VL_NOT_FINAL : public EmitCBaseVisitor { } } virtual void visit(AstTextBlock* nodep) override { - visit(VN_AS(nodep, NodeSimpleText)); + visit(static_cast(nodep)); { VL_RESTORER(m_suppressSemi); m_suppressVarSemi = nodep->commas(); diff --git a/src/V3Gate.cpp b/src/V3Gate.cpp index 3c7b94755..9c88263bb 100644 --- a/src/V3Gate.cpp +++ b/src/V3Gate.cpp @@ -843,10 +843,9 @@ private: UASSERT_OBJ(nodep->access().isReadOnly(), nodep, "Can't replace lvalue assignments with const var"); AstNode* substp = m_replaceTreep->cloneTree(false); - UASSERT_OBJ( - !(VN_IS(nodep, NodeVarRef) && VN_IS(substp, NodeVarRef) && nodep->same(substp)), - // Prevent an infinite loop... - substp, "Replacing node with itself; perhaps circular logic?"); + UASSERT_OBJ(!(VN_IS(substp, NodeVarRef) && nodep->same(substp)), + // Prevent an infinite loop... + substp, "Replacing node with itself; perhaps circular logic?"); // Which fileline() to use? // If replacing with logic, an error/warning is likely to want to point to the logic // IE what we're replacing with. diff --git a/src/V3Inline.cpp b/src/V3Inline.cpp index f56933dd9..539728e1b 100644 --- a/src/V3Inline.cpp +++ b/src/V3Inline.cpp @@ -359,7 +359,7 @@ private: // Each inlined cell that contain an interface variable need to // copy the IfaceRefDType and point it to the newly cloned // interface cell. - AstIfaceRefDType* newdp = VN_AS(ifacerefp->cloneTree(false), IfaceRefDType); + AstIfaceRefDType* newdp = ifacerefp->cloneTree(false); nodep->dtypep(newdp); ifacerefp->addNextHere(newdp); // Relink to point to newly cloned cell diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index 3a119b0fa..e0948ffb7 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -2093,7 +2093,7 @@ private: nodep->v3error("'super' used on non-extended class (IEEE 1800-2017 8.15)"); m_ds.m_dotErr = true; } else { - const auto cextp = VN_AS(classp->extendsp(), ClassExtends); + const auto cextp = classp->extendsp(); UASSERT_OBJ(cextp, nodep, "Bad super extends link"); const auto sclassp = cextp->classp(); UASSERT_OBJ(sclassp, nodep, "Bad superclass"); @@ -2311,8 +2311,7 @@ private: refp->dotted(dotted.substr(0, pos)); newp = refp; } else { - newp = new AstUnlinkedRef(nodep->fileline(), VN_AS(refp, VarXRef), - refp->name(), + newp = new AstUnlinkedRef(nodep->fileline(), refp, refp->name(), m_ds.m_unlinkedScopep->unlinkFrBack()); m_ds.m_unlinkedScopep = nullptr; m_ds.m_unresolved = false; diff --git a/src/V3Order.cpp b/src/V3Order.cpp index 754a42cb4..0c1994fa3 100644 --- a/src/V3Order.cpp +++ b/src/V3Order.cpp @@ -1262,11 +1262,11 @@ static bool domainsExclusive(const AstSenTree* fromp, const AstSenTree* top) { const bool fromInitial = fromp->hasInitial() || fromp->hasSettle(); if (toInitial != fromInitial) return true; - const AstSenItem* fromSenListp = VN_AS(fromp->sensesp(), SenItem); - const AstSenItem* toSenListp = VN_AS(top->sensesp(), SenItem); + const AstSenItem* fromSenListp = fromp->sensesp(); + const AstSenItem* toSenListp = top->sensesp(); - UASSERT_OBJ(fromSenListp, fromp, "sensitivity list item is not an AstSenItem"); - UASSERT_OBJ(toSenListp, top, "sensitivity list item is not an AstSenItem"); + UASSERT_OBJ(fromSenListp, fromp, "sensitivity list empty"); + UASSERT_OBJ(toSenListp, top, "sensitivity list empty"); if (fromSenListp->nextp()) return false; if (toSenListp->nextp()) return false; diff --git a/src/V3Param.cpp b/src/V3Param.cpp index b0ba43339..d59863661 100644 --- a/src/V3Param.cpp +++ b/src/V3Param.cpp @@ -452,7 +452,7 @@ class ParamProcessor final { << " has hier_block metacomment, hierarchical verilation" << " supports only integer/floating point/string parameters"); } - } else if (VN_IS(pinp->modPTypep(), ParamTypeDType)) { + } else { pinp->v3error(AstNode::prettyNameQ(modp->origName()) << " has hier_block metacomment, but 'parameter type' is not supported"); } diff --git a/src/V3Simulate.h b/src/V3Simulate.h index 3a41dae75..35e601a87 100644 --- a/src/V3Simulate.h +++ b/src/V3Simulate.h @@ -966,11 +966,11 @@ private: badNodeType(nodep); return; } - AstNodeFTask* funcp = VN_AS(nodep->taskp(), NodeFTask); + AstNodeFTask* funcp = nodep->taskp(); UASSERT_OBJ(funcp, nodep, "Not linked"); if (m_params) V3Width::widthParamsEdit(funcp); VL_DANGLING(funcp); // Make sure we've sized the function - funcp = VN_AS(nodep->taskp(), NodeFTask); + funcp = nodep->taskp(); UASSERT_OBJ(funcp, nodep, "Not linked"); // Apply function call values to function V3TaskConnects tconnects = V3Task::taskConnects(nodep, nodep->taskp()->stmtsp()); diff --git a/src/V3TraceDecl.cpp b/src/V3TraceDecl.cpp index 8019b41da..c6c335f57 100644 --- a/src/V3TraceDecl.cpp +++ b/src/V3TraceDecl.cpp @@ -145,7 +145,7 @@ private: iterateChildren(nodep); } virtual void visit(AstScope* nodep) override { - AstCell* const cellp = VN_CAST(nodep->aboveCellp(), Cell); + AstCell* const cellp = nodep->aboveCellp(); if (cellp && VN_IS(cellp->modp(), Iface)) { AstCFunc* const origSubFunc = m_initSubFuncp; int origSubStmts = m_initSubStmts; diff --git a/src/V3Width.cpp b/src/V3Width.cpp index f52145765..5f6647ab6 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -4365,7 +4365,7 @@ private: } virtual void visit(AstFuncRef* nodep) override { - visit(VN_AS(nodep, NodeFTaskRef)); + visit(static_cast(nodep)); nodep->dtypeFrom(nodep->taskp()); // if (debug()) nodep->dumpTree(cout, " FuncOut: "); } diff --git a/src/astgen b/src/astgen index 0c4bb5cab..f04126587 100755 --- a/src/astgen +++ b/src/astgen @@ -333,12 +333,11 @@ class Cpt: " iterateAndNextNull(nodep->lhsp());\n" + "".join(out_for_type_sc)) if out_for_type[0]: - self.print( - " iterateAndNextNull(nodep->rhsp());\n" + - " AstNodeTriop *tnp = VN_CAST(nodep, NodeTriop);\n" - + - " if (tnp && tnp->thsp()) iterateAndNextNull(tnp->thsp());\n" - + "".join(out_for_type) + " }\n") + self.print(" iterateAndNextNull(nodep->rhsp());\n") + if is_subclass_of(typen, "NodeTriop"): + self.print( + " iterateAndNextNull(nodep->thsp());\n") + self.print("".join(out_for_type) + " }\n") elif len(out_for_type) > 0: # Other types with something to print skip = typen in self.tree_skip_visit gen = "Gen" if skip else "" @@ -450,6 +449,10 @@ def children_of(typen): return cllist +def is_subclass_of(typen, what): + return typen == what or (typen in children_of(what)) + + # ---------------------------------------------------------------------