Skip to content

Commit

Permalink
Fix string to be more standard (verilator#5082) (verilator#5083).
Browse files Browse the repository at this point in the history
  • Loading branch information
@wsnyder
wsnyder committed Jun 1, 2024
1 parent dbf68a9 commit 7c9fa86
Show file tree
Hide file tree
Showing 19 changed files with 125 additions and 57 deletions.
1 change: 1 addition & 0 deletions Changes
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ Verilator 5.025 devel
* Fix CMake builds to export VERILATOR_ROOT (#5063). [Michael Bikovitsky]
* Fix false ASSIGNIN on functions with explicit port map (#5069).
* Fix DFG assertion with SystemC (#5076). [Geza Lore]
* Fix `$typename` string to be more standard (#5082) (#5083). [Andrew Nolte]
* Fix missed optimization in V3Delayed (#5089). [Geza Lore]
* Fix macro expansion in strings per 1800-2023 (#5094). [Geza Lore]
* Fix width extension of unpacked array select (#5095). [Varun Koyyalagunta]
Expand Down
25 changes: 14 additions & 11 deletions src/V3AstNodeDType.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ class AstNodeDType VL_NOT_FINAL : public AstNode {
virtual AstNodeDType* subDTypep() const VL_MT_SAFE { return nullptr; }
virtual bool isFourstate() const;
// Ideally an IEEE $typename
virtual string prettyDTypeName() const { return prettyTypeName(); }
string prettyDTypeNameQ() const { return "'" + prettyDTypeName() + "'"; }
virtual string prettyDTypeName(bool) const { return prettyTypeName(); }
string prettyDTypeNameQ() const { return "'" + prettyDTypeName(false) + "'"; }
//
// Changing the width may confuse the data type resolution, so must clear
// TypeTable cache after use.
Expand Down Expand Up @@ -215,6 +215,7 @@ class AstNodeUOrStructDType VL_NOT_FINAL : public AstNodeDType {
int uniqueNum() const { return m_uniqueNum; }
void dump(std::ostream& str) const override;
void dumpJson(std::ostream& str) const override;
string prettyDTypeName(bool) const override;
bool isCompound() const override { return !packed(); }
// For basicp() we reuse the size to indicate a "fake" basic type of same size
AstBasicDType* basicp() const override {
Expand Down Expand Up @@ -313,7 +314,7 @@ class AstAssocArrayDType final : public AstNodeDType {
return asamep->subDTypep()
&& subDTypep()->skipRefp()->similarDType(asamep->subDTypep()->skipRefp());
}
string prettyDTypeName() const override;
string prettyDTypeName(bool full) const override;
void dumpSmall(std::ostream& str) const override;
AstNodeDType* getChildDTypep() const override { return childDTypep(); }
AstNodeDType* getChild2DTypep() const override { return keyChildDTypep(); }
Expand Down Expand Up @@ -387,7 +388,7 @@ class AstBasicDType final : public AstNodeDType {
return type() == samep->type() && same(samep);
}
string name() const override VL_MT_STABLE { return m.m_keyword.ascii(); }
string prettyDTypeName() const override;
string prettyDTypeName(bool full) const override;
const char* broken() const override {
BROKEN_RTN(dtypep() != this);
return nullptr;
Expand Down Expand Up @@ -504,7 +505,7 @@ class AstCDType final : public AstNodeDType {
}
bool similarDType(const AstNodeDType* samep) const override { return same(samep); }
string name() const override VL_MT_STABLE { return m_name; }
string prettyDTypeName() const override { return m_name; }
string prettyDTypeName(bool) const override { return m_name; }
// METHODS
AstBasicDType* basicp() const override VL_MT_STABLE { return nullptr; }
AstNodeDType* skipRefp() const override VL_MT_STABLE { return (AstNodeDType*)this; }
Expand Down Expand Up @@ -551,6 +552,7 @@ class AstClassRefDType final : public AstNodeDType {
void dump(std::ostream& str = std::cout) const override;
void dumpJson(std::ostream& str = std::cout) const override;
void dumpSmall(std::ostream& str) const override;
string prettyDTypeName(bool full) const override;
string name() const override VL_MT_STABLE;
AstBasicDType* basicp() const override VL_MT_STABLE { return nullptr; }
AstNodeDType* skipRefp() const override VL_MT_STABLE { return (AstNodeDType*)this; }
Expand Down Expand Up @@ -719,7 +721,7 @@ class AstDynArrayDType final : public AstNodeDType {
return asamep->subDTypep()
&& subDTypep()->skipRefp()->similarDType(asamep->subDTypep()->skipRefp());
}
string prettyDTypeName() const override;
string prettyDTypeName(bool full) const override;
void dumpSmall(std::ostream& str) const override;
AstNodeDType* getChildDTypep() const override { return childDTypep(); }
AstNodeDType* subDTypep() const override VL_MT_STABLE {
Expand Down Expand Up @@ -809,6 +811,7 @@ class AstEnumDType final : public AstNodeDType {
void dump(std::ostream& str = std::cout) const override;
void dumpJson(std::ostream& str = std::cout) const override;
void dumpSmall(std::ostream& str) const override;
string prettyDTypeName(bool full) const override;
// METHODS
AstBasicDType* basicp() const override VL_MT_STABLE { return subDTypep()->basicp(); }
AstNodeDType* skipRefp() const override VL_MT_STABLE { return subDTypep()->skipRefp(); }
Expand Down Expand Up @@ -1093,7 +1096,7 @@ class AstQueueDType final : public AstNodeDType {
&& subDTypep()->skipRefp()->similarDType(asamep->subDTypep()->skipRefp());
}
void dumpSmall(std::ostream& str) const override;
string prettyDTypeName() const override;
string prettyDTypeName(bool full) const override;
AstNodeDType* getChildDTypep() const override { return childDTypep(); }
AstNodeDType* subDTypep() const override VL_MT_STABLE {
return m_refDTypep ? m_refDTypep : childDTypep();
Expand Down Expand Up @@ -1156,8 +1159,8 @@ class AstRefDType final : public AstNodeDType {
void dumpJson(std::ostream& str = std::cout) const override;
void dumpSmall(std::ostream& str) const override;
string name() const override VL_MT_STABLE { return m_name; }
string prettyDTypeName() const override {
return subDTypep() ? prettyName(subDTypep()->name()) : prettyName();
string prettyDTypeName(bool full) const override {
return subDTypep() ? prettyName(subDTypep()->prettyDTypeName(full)) : prettyName();
}
AstBasicDType* basicp() const override VL_MT_STABLE {
return subDTypep() ? subDTypep()->basicp() : nullptr;
Expand Down Expand Up @@ -1384,7 +1387,7 @@ class AstPackArrayDType final : public AstNodeArrayDType {
inline AstPackArrayDType(FileLine* fl, VFlagChildDType, AstNodeDType* dtp, AstRange* rangep);
inline AstPackArrayDType(FileLine* fl, AstNodeDType* dtp, AstRange* rangep);
ASTGEN_MEMBERS_AstPackArrayDType;
string prettyDTypeName() const override;
string prettyDTypeName(bool full) const override;
bool isCompound() const override { return false; }
};
class AstUnpackArrayDType final : public AstNodeArrayDType {
Expand All @@ -1411,7 +1414,7 @@ class AstUnpackArrayDType final : public AstNodeArrayDType {
widthFromSub(subDTypep());
}
ASTGEN_MEMBERS_AstUnpackArrayDType;
string prettyDTypeName() const override;
string prettyDTypeName(bool full) const override;
bool same(const AstNode* samep) const override {
const AstUnpackArrayDType* const sp = VN_DBG_AS(samep, UnpackArrayDType);
return m_isCompound == sp->m_isCompound;
Expand Down
56 changes: 45 additions & 11 deletions src/V3AstNodes.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1506,7 +1506,7 @@ void AstBasicDType::dumpJson(std::ostream& str) const {
}
dumpJsonGen(str);
}
string AstBasicDType::prettyDTypeName() const {
string AstBasicDType::prettyDTypeName(bool) const {
std::ostringstream os;
os << keyword().ascii();
if (isRanged() && !rangep() && keyword().width() <= 1) {
Expand Down Expand Up @@ -1631,6 +1631,10 @@ void AstClassRefDType::dumpSmall(std::ostream& str) const {
this->AstNodeDType::dumpSmall(str);
str << "class:" << name();
}
string AstClassRefDType::prettyDTypeName(bool) const {
return "class{}"s + prettyName();
return prettyTypeName();
}
string AstClassRefDType::name() const { return classp() ? classp()->name() : "<unlinked>"; }
void AstNodeCoverOrAssert::dump(std::ostream& str) const {
this->AstNodeStmt::dump(str);
Expand Down Expand Up @@ -1667,6 +1671,22 @@ void AstEnumDType::dumpSmall(std::ostream& str) const {
this->AstNodeDType::dumpSmall(str);
str << "enum";
}
string AstEnumDType::prettyDTypeName(bool full) const {
string result = "enum{";
if (full) { // else shorten for error messages
for (AstEnumItem* itemp = itemsp(); itemp; itemp = VN_AS(itemp->nextp(), EnumItem)) {
result += itemp->prettyName() + "=";
if (AstConst* constp = VN_CAST(itemp->valuep(), Const)) {
result += constp->num().ascii(true, true);
} else {
result += "?";
}
result += ";";
}
}
result += "}" + prettyName();
return result;
}
void AstEnumItemRef::dump(std::ostream& str) const {
this->AstNodeExpr::dump(str);
str << " -> ";
Expand Down Expand Up @@ -1981,6 +2001,18 @@ void AstNodeUOrStructDType::dumpJson(std::ostream& str) const {
dumpJsonBoolFunc(str, isFourstate);
dumpJsonGen(str);
}
string AstNodeUOrStructDType::prettyDTypeName(bool full) const {
string result = verilogKwd() + "{";
if (full) { // else shorten for errors
for (AstMemberDType* itemp = membersp(); itemp;
itemp = VN_AS(itemp->nextp(), MemberDType)) {
result += itemp->subDTypep()->prettyDTypeName(full);
result += " " + itemp->prettyName() + ";";
}
}
result += "}" + prettyName();
return result;
}
void AstNodeDType::dump(std::ostream& str) const {
this->AstNode::dump(str);
if (generic()) str << " [GENERIC]";
Expand Down Expand Up @@ -2020,13 +2052,13 @@ void AstNodeArrayDType::dumpJson(std::ostream& str) const {
dumpJsonStr(str, "declRange", cvtToStr(declRange()));
dumpJsonGen(str);
}
string AstPackArrayDType::prettyDTypeName() const {
string AstPackArrayDType::prettyDTypeName(bool full) const {
std::ostringstream os;
if (const auto subp = subDTypep()) os << subp->prettyDTypeName();
if (const auto subp = subDTypep()) os << subp->prettyDTypeName(full);
os << declRange();
return os.str();
}
string AstUnpackArrayDType::prettyDTypeName() const {
string AstUnpackArrayDType::prettyDTypeName(bool full) const {
std::ostringstream os;
string ranges = cvtToStr(declRange());
// Unfortunately we need a single $ for the first unpacked, and all
Expand All @@ -2036,7 +2068,7 @@ string AstUnpackArrayDType::prettyDTypeName() const {
ranges += cvtToStr(adtypep->declRange());
subp = adtypep->subDTypep()->skipRefp();
}
os << subp->prettyDTypeName() << "$" << ranges;
os << subp->prettyDTypeName(full) << "$" << ranges;
return os.str();
}
std::vector<AstUnpackArrayDType*> AstUnpackArrayDType::unpackDimensions() {
Expand Down Expand Up @@ -2180,20 +2212,22 @@ void AstAssocArrayDType::dumpSmall(std::ostream& str) const {
this->AstNodeDType::dumpSmall(str);
str << "[assoc-" << nodeAddr(keyDTypep()) << "]";
}
string AstAssocArrayDType::prettyDTypeName() const {
return subDTypep()->prettyDTypeName() + "[" + keyDTypep()->prettyDTypeName() + "]";
string AstAssocArrayDType::prettyDTypeName(bool full) const {
return subDTypep()->prettyDTypeName(full) + "$[" + keyDTypep()->prettyDTypeName(full) + "]";
}
void AstDynArrayDType::dumpSmall(std::ostream& str) const {
this->AstNodeDType::dumpSmall(str);
str << "[]";
str << "$[]";
}
string AstDynArrayDType::prettyDTypeName(bool full) const {
return subDTypep()->prettyDTypeName(full) + "$[]";
}
string AstDynArrayDType::prettyDTypeName() const { return subDTypep()->prettyDTypeName() + "[]"; }
void AstQueueDType::dumpSmall(std::ostream& str) const {
this->AstNodeDType::dumpSmall(str);
str << "[queue]";
}
string AstQueueDType::prettyDTypeName() const {
string str = subDTypep()->prettyDTypeName() + "[$";
string AstQueueDType::prettyDTypeName(bool full) const {
string str = subDTypep()->prettyDTypeName(full) + "$[$";
if (boundConst()) str += ":" + cvtToStr(boundConst());
return str + "]";
}
Expand Down
2 changes: 1 addition & 1 deletion src/V3Task.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,7 @@ class TaskVisitor final : public VNVisitor {
// differ we may get C compilation problems later
const std::string dpiType = portp->dpiArgType(false, false);
dpiproto += dpiType;
const std::string vType = portp->dtypep()->prettyDTypeName();
const std::string vType = portp->dtypep()->prettyDTypeName(false);
if (!portp->isDpiOpenArray() && dpiType != vType) {
dpiproto += " /* " + vType + " */ ";
}
Expand Down
13 changes: 7 additions & 6 deletions src/V3Width.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1615,7 +1615,8 @@ class WidthVisitor final : public VNVisitor {
}
case VAttrType::TYPENAME: {
UASSERT_OBJ(nodep->fromp(), nodep, "Unprovided expression");
const string result = nodep->fromp()->dtypep()->prettyDTypeName();
const string result = nodep->fromp()->dtypep()->prettyDTypeName(true);
UINFO(9, "typename '" << result << "' from " << nodep->fromp()->dtypep() << "\n");
AstNode* const newp = new AstConst{nodep->fileline(), AstConst::String{}, result};
nodep->replaceWith(newp);
VL_DO_DANGLING(nodep->deleteTree(), nodep);
Expand Down Expand Up @@ -4091,7 +4092,7 @@ class WidthVisitor final : public VNVisitor {
} else if (const AstNodeDType* nodedtypep
= VN_CAST(patp->keyp(), NodeDType)) {
// data_type: default_value
const string dtype = nodedtypep->dtypep()->prettyDTypeName();
const string dtype = nodedtypep->dtypep()->prettyDTypeName(true);
const auto pair = dtypemap.emplace(dtype, patp);
if (!pair.second) {
// Override stored default_value
Expand Down Expand Up @@ -4195,7 +4196,7 @@ class WidthVisitor final : public VNVisitor {
AstPatMember* patp,
AstNodeUOrStructDType* memp_vdtypep,
AstPatMember* defaultp, const DTypeMap& dtypemap) {
const string memp_DType = memp->virtRefDTypep()->prettyDTypeName();
const string memp_DType = memp->virtRefDTypep()->prettyDTypeName(true);
const auto it = dtypemap.find(memp_DType);
if (it != dtypemap.end()) {
// default_value for data_type
Expand Down Expand Up @@ -5787,8 +5788,8 @@ class WidthVisitor final : public VNVisitor {
&& !similarDTypeRecurse(portDTypep, pinDTypep)) {
pinp->v3error("Ref argument requires matching types;"
<< " port " << portp->prettyNameQ() << " requires "
<< portDTypep->prettyDTypeName() << " but connection is "
<< pinDTypep->prettyDTypeName() << ".");
<< portDTypep->prettyDTypeNameQ() << " but connection is "
<< pinDTypep->prettyDTypeNameQ() << ".");
} else if (portp->isWritable() && pinp->width() != portp->width()) {
pinp->v3widthWarn(portp->width(), pinp->width(),
"Function output argument "
Expand Down Expand Up @@ -7035,7 +7036,7 @@ class WidthVisitor final : public VNVisitor {

AstNodeExpr* checkCvtUS(AstNodeExpr* nodep) {
if (nodep && nodep->isDouble()) {
nodep->v3error("Expected integral (non-" << nodep->dtypep()->prettyDTypeName()
nodep->v3error("Expected integral (non-" << nodep->dtypep()->prettyDTypeNameQ()
<< ") input to "
<< nodep->backp()->prettyTypeName());
nodep = spliceCvtS(nodep, true, 32);
Expand Down
4 changes: 2 additions & 2 deletions test_regress/t/t_cast_class_incompat_bad.out
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
%Error: t/t_cast_class_incompat_bad.v:26:16: Dynamic, not static cast, required to cast 'CLASSREFDTYPE 'BaseExtended'' from 'CLASSREFDTYPE 'Base''
%Error: t/t_cast_class_incompat_bad.v:26:16: Dynamic, not static cast, required to cast 'class{}BaseExtended' from 'class{}Base'
: ... note: In instance 't'
: ... Suggest dynamic $cast
26 | cls_ab = BaseExtended'(cls_a);
| ^~~~~~~~~~~~
%Error: t/t_cast_class_incompat_bad.v:27:15: Incompatible types to static cast to 'CLASSREFDTYPE 'Other'' from 'CLASSREFDTYPE 'BaseExtended''
%Error: t/t_cast_class_incompat_bad.v:27:15: Incompatible types to static cast to 'class{}Other' from 'class{}BaseExtended'
: ... note: In instance 't'
27 | other = Other'(cls_ab);
| ^~~~~
Expand Down
4 changes: 2 additions & 2 deletions test_regress/t/t_castdyn_castconst_bad.out
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
| ^~~~~
... For warning description see https://verilator.org/warn/CASTCONST?v=latest
... Use "/* verilator lint_off CASTCONST */" and lint_on around source to disable this message.
%Warning-CASTCONST: t/t_castdyn_castconst_bad.v:21:11: $cast will always return one as 'CLASSREFDTYPE 'Base'' is always castable from 'CLASSREFDTYPE 'Base''
%Warning-CASTCONST: t/t_castdyn_castconst_bad.v:21:11: $cast will always return one as 'class{}Base' is always castable from 'class{}Base'
: ... note: In instance 't'
: ... Suggest static cast
21 | i = $cast(b, b);
| ^~~~~
%Warning-CASTCONST: t/t_castdyn_castconst_bad.v:22:11: $cast will always return zero as 'CLASSREFDTYPE 'Base'' is not castable from 'CLASSREFDTYPE 'Other''
%Warning-CASTCONST: t/t_castdyn_castconst_bad.v:22:11: $cast will always return zero as 'class{}Base' is not castable from 'class{}Other'
: ... note: In instance 't'
22 | i = $cast(b, o);
| ^~~~~
Expand Down
2 changes: 1 addition & 1 deletion test_regress/t/t_castdyn_unsup_bad.out
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
%Error-UNSUPPORTED: t/t_castdyn_unsup_bad.v:13:7: Unsupported: $cast to 'string[$]' from 'int[string]'
%Error-UNSUPPORTED: t/t_castdyn_unsup_bad.v:13:7: Unsupported: $cast to 'string$[$]' from 'int$[string]'
: ... note: In instance 't'
: ... Suggest try static cast
13 | $cast(q, aarray);
Expand Down
2 changes: 1 addition & 1 deletion test_regress/t/t_class_param_enum_bad.out
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
: ... note: In instance 't'
20 | Converter#(bit) conv2 = conv1;
| ^~~~~
%Error-ENUMVALUE: t/t_class_param_enum_bad.v:21:19: Implicit conversion to enum 'ENUMDTYPE '$unit::enum_t'' from 'logic[31:0]' (IEEE 1800-2023 6.19.3)
%Error-ENUMVALUE: t/t_class_param_enum_bad.v:21:19: Implicit conversion to enum 'enum{}$unit::enum_t' from 'logic[31:0]' (IEEE 1800-2023 6.19.3)
: ... note: In instance 't'
: ... Suggest use enum's mnemonic, or static cast
21 | conv1.toInt(0);
Expand Down
6 changes: 3 additions & 3 deletions test_regress/t/t_enum_type_bad.out
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
%Error-ENUMVALUE: t/t_enum_type_bad.v:28:11: Implicit conversion to enum 't.e_t' from 'logic[31:0]' (IEEE 1800-2023 6.19.3)
%Error-ENUMVALUE: t/t_enum_type_bad.v:28:11: Implicit conversion to enum 'enum{}t.e_t' from 'logic[31:0]' (IEEE 1800-2023 6.19.3)
: ... note: In instance 't'
: ... Suggest use enum's mnemonic, or static cast
28 | e = 1;
| ^
... For error description see https://verilator.org/warn/ENUMVALUE?v=latest
%Error-ENUMVALUE: t/t_enum_type_bad.v:29:11: Implicit conversion to enum 't.o_t' from 't.e_t' (IEEE 1800-2023 6.19.3)
%Error-ENUMVALUE: t/t_enum_type_bad.v:29:11: Implicit conversion to enum 'enum{}t.o_t' from 'enum{}t.e_t' (IEEE 1800-2023 6.19.3)
: ... note: In instance 't'
: ... Suggest use enum's mnemonic, or static cast
29 | o = e;
| ^
%Error-ENUMVALUE: t/t_enum_type_bad.v:35:15: Implicit conversion to enum 't.o_t' from 'ENUMDTYPE 't.e_t'' (IEEE 1800-2023 6.19.3)
%Error-ENUMVALUE: t/t_enum_type_bad.v:35:15: Implicit conversion to enum 'enum{}t.o_t' from 'enum{}t.e_t' (IEEE 1800-2023 6.19.3)
: ... note: In instance 't'
: ... Suggest use enum's mnemonic, or static cast
35 | o = str.m_e;
Expand Down
Loading

0 comments on commit 7c9fa86

Please sign in to comment.