llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Rahul Joshi (jurahul) <details> <summary>Changes</summary> Refactor ClangDiagnosticEmitter to use more StringRefs. Also use more range for loops. Verified that .inc files generated by clang build are identical with and without the change. --- Patch is 29.30 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/115212.diff 1 Files Affected: - (modified) clang/utils/TableGen/ClangDiagnosticsEmitter.cpp (+115-180) ``````````diff diff --git a/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp b/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp index 34e2e8f47ae71a..5155ba7fcf21a5 100644 --- a/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp +++ b/clang/utils/TableGen/ClangDiagnosticsEmitter.cpp @@ -18,6 +18,7 @@ #include "llvm/ADT/SmallString.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringMap.h" +#include "llvm/ADT/StringSet.h" #include "llvm/ADT/StringSwitch.h" #include "llvm/ADT/Twine.h" #include "llvm/Support/Casting.h" @@ -44,35 +45,28 @@ class DiagGroupParentMap { public: DiagGroupParentMap(const RecordKeeper &records) : Records(records) { - ArrayRef<const Record *> DiagGroups = - Records.getAllDerivedDefinitions("DiagGroup"); - for (unsigned i = 0, e = DiagGroups.size(); i != e; ++i) { - std::vector<const Record *> SubGroups = - DiagGroups[i]->getValueAsListOfDefs("SubGroups"); - for (unsigned j = 0, e = SubGroups.size(); j != e; ++j) - Mapping[SubGroups[j]].push_back(DiagGroups[i]); - } + for (const Record *Group : Records.getAllDerivedDefinitions("DiagGroup")) + for (const Record *SubGroup : Group->getValueAsListOfDefs("SubGroups")) + Mapping[SubGroup].push_back(Group); } - const std::vector<const Record *> &getParents(const Record *Group) { - return Mapping[Group]; + ArrayRef<const Record *> getParents(const Record *SubGroup) { + return Mapping[SubGroup]; } }; } // end anonymous namespace. -static std::string +static StringRef getCategoryFromDiagGroup(const Record *Group, DiagGroupParentMap &DiagGroupParents) { // If the DiagGroup has a category, return it. - std::string CatName = std::string(Group->getValueAsString("CategoryName")); + StringRef CatName = Group->getValueAsString("CategoryName"); if (!CatName.empty()) return CatName; // The diag group may the subgroup of one or more other diagnostic groups, // check these for a category as well. - const std::vector<const Record *> &Parents = - DiagGroupParents.getParents(Group); - for (unsigned i = 0, e = Parents.size(); i != e; ++i) { - CatName = getCategoryFromDiagGroup(Parents[i], DiagGroupParents); + for (const Record *Parent : DiagGroupParents.getParents(Group)) { + CatName = getCategoryFromDiagGroup(Parent, DiagGroupParents); if (!CatName.empty()) return CatName; } return ""; @@ -80,25 +74,26 @@ getCategoryFromDiagGroup(const Record *Group, /// getDiagnosticCategory - Return the category that the specified diagnostic /// lives in. -static std::string getDiagnosticCategory(const Record *R, - DiagGroupParentMap &DiagGroupParents) { +static StringRef getDiagnosticCategory(const Record *R, + DiagGroupParentMap &DiagGroupParents) { // If the diagnostic is in a group, and that group has a category, use it. if (const auto *Group = dyn_cast<DefInit>(R->getValueInit("Group"))) { // Check the diagnostic's diag group for a category. - std::string CatName = getCategoryFromDiagGroup(Group->getDef(), - DiagGroupParents); + StringRef CatName = + getCategoryFromDiagGroup(Group->getDef(), DiagGroupParents); if (!CatName.empty()) return CatName; } // If the diagnostic itself has a category, get it. - return std::string(R->getValueAsString("CategoryName")); + return R->getValueAsString("CategoryName"); } namespace { class DiagCategoryIDMap { const RecordKeeper &Records; StringMap<unsigned> CategoryIDs; - std::vector<std::string> CategoryStrings; + std::vector<StringRef> CategoryStrings; + public: DiagCategoryIDMap(const RecordKeeper &records) : Records(records) { DiagGroupParentMap ParentInfo(Records); @@ -107,10 +102,9 @@ namespace { CategoryStrings.push_back(""); CategoryIDs[""] = 0; - ArrayRef<const Record *> Diags = - Records.getAllDerivedDefinitions("Diagnostic"); - for (unsigned i = 0, e = Diags.size(); i != e; ++i) { - std::string Category = getDiagnosticCategory(Diags[i], ParentInfo); + for (const Record *Diag : + Records.getAllDerivedDefinitions("Diagnostic")) { + StringRef Category = getDiagnosticCategory(Diag, ParentInfo); if (Category.empty()) continue; // Skip diags with no category. unsigned &ID = CategoryIDs[Category]; @@ -125,7 +119,7 @@ namespace { return CategoryIDs[CategoryString]; } - typedef std::vector<std::string>::const_iterator const_iterator; + typedef std::vector<StringRef>::const_iterator const_iterator; const_iterator begin() const { return CategoryStrings.begin(); } const_iterator end() const { return CategoryStrings.end(); } }; @@ -133,7 +127,7 @@ namespace { struct GroupInfo { StringRef GroupName; std::vector<const Record*> DiagsInGroup; - std::vector<std::string> SubGroups; + std::vector<StringRef> SubGroups; unsigned IDNo = 0; SmallVector<const Record *, 1> Defs; @@ -155,40 +149,34 @@ static bool diagGroupBeforeByName(const Record *LHS, const Record *RHS) { /// Invert the 1-[0/1] mapping of diags to group into a one to many /// mapping of groups to diags in the group. -static void groupDiagnostics(ArrayRef<const Record *> Diags, - ArrayRef<const Record *> DiagGroups, - std::map<std::string, GroupInfo> &DiagsInGroup) { - - for (unsigned i = 0, e = Diags.size(); i != e; ++i) { - const Record *R = Diags[i]; +static std::map<StringRef, GroupInfo> +groupDiagnostics(ArrayRef<const Record *> Diags, + ArrayRef<const Record *> DiagGroups) { + std::map<StringRef, GroupInfo> DiagsInGroup; + for (const Record *R : Diags) { const auto *DI = dyn_cast<DefInit>(R->getValueInit("Group")); if (!DI) continue; assert(R->getValueAsDef("Class")->getName() != "CLASS_NOTE" && "Note can't be in a DiagGroup"); - std::string GroupName = - std::string(DI->getDef()->getValueAsString("GroupName")); + StringRef GroupName = DI->getDef()->getValueAsString("GroupName"); DiagsInGroup[GroupName].DiagsInGroup.push_back(R); } // Add all DiagGroup's to the DiagsInGroup list to make sure we pick up empty // groups (these are warnings that GCC supports that clang never produces). - for (unsigned i = 0, e = DiagGroups.size(); i != e; ++i) { - const Record *Group = DiagGroups[i]; - GroupInfo &GI = - DiagsInGroup[std::string(Group->getValueAsString("GroupName"))]; + for (const Record *Group : DiagGroups) { + GroupInfo &GI = DiagsInGroup[Group->getValueAsString("GroupName")]; GI.GroupName = Group->getName(); GI.Defs.push_back(Group); for (const Record *SubGroup : Group->getValueAsListOfDefs("SubGroups")) - GI.SubGroups.push_back(SubGroup->getValueAsString("GroupName").str()); + GI.SubGroups.push_back(SubGroup->getValueAsString("GroupName")); } // Assign unique ID numbers to the groups. - unsigned IDNo = 0; - for (std::map<std::string, GroupInfo>::iterator - I = DiagsInGroup.begin(), E = DiagsInGroup.end(); I != E; ++I, ++IDNo) - I->second.IDNo = IDNo; + for (auto [IdNo, Iter] : enumerate(DiagsInGroup)) + Iter.second.IDNo = IdNo; // Warn if the same group is defined more than once (including implicitly). for (auto &Group : DiagsInGroup) { @@ -238,6 +226,7 @@ static void groupDiagnostics(ArrayRef<const Record *> Diags, } } } + return DiagsInGroup; } //===----------------------------------------------------------------------===// @@ -256,14 +245,14 @@ class InferPedantic { DiagGroupParentMap &DiagGroupParents; ArrayRef<const Record *> Diags; const std::vector<const Record *> DiagGroups; - std::map<std::string, GroupInfo> &DiagsInGroup; + std::map<StringRef, GroupInfo> &DiagsInGroup; DenseSet<const Record *> DiagsSet; GMap GroupCount; public: InferPedantic(DiagGroupParentMap &DiagGroupParents, ArrayRef<const Record *> Diags, ArrayRef<const Record *> DiagGroups, - std::map<std::string, GroupInfo> &DiagsInGroup) + std::map<StringRef, GroupInfo> &DiagsInGroup) : DiagGroupParents(DiagGroupParents), Diags(Diags), DiagGroups(DiagGroups), DiagsInGroup(DiagsInGroup) {} @@ -292,15 +281,12 @@ class InferPedantic { } // end anonymous namespace bool InferPedantic::isSubGroupOfGroup(const Record *Group, StringRef GName) { - const std::string &GroupName = - std::string(Group->getValueAsString("GroupName")); + StringRef GroupName = Group->getValueAsString("GroupName"); if (GName == GroupName) return true; - const std::vector<const Record *> &Parents = - DiagGroupParents.getParents(Group); - for (unsigned i = 0, e = Parents.size(); i != e; ++i) - if (isSubGroupOfGroup(Parents[i], GName)) + for (const Record *Parent : DiagGroupParents.getParents(Group)) + if (isSubGroupOfGroup(Parent, GName)) return true; return false; @@ -308,14 +294,13 @@ bool InferPedantic::isSubGroupOfGroup(const Record *Group, StringRef GName) { /// Determine if the diagnostic is an extension. bool InferPedantic::isExtension(const Record *Diag) { - const std::string &ClsName = - std::string(Diag->getValueAsDef("Class")->getName()); + StringRef ClsName = Diag->getValueAsDef("Class")->getName(); return ClsName == "CLASS_EXTENSION"; } bool InferPedantic::isOffByDefault(const Record *Diag) { - const std::string &DefSeverity = std::string( - Diag->getValueAsDef("DefaultSeverity")->getValueAsString("Name")); + StringRef DefSeverity = + Diag->getValueAsDef("DefaultSeverity")->getValueAsString("Name"); return DefSeverity == "Ignored"; } @@ -323,8 +308,7 @@ bool InferPedantic::groupInPedantic(const Record *Group, bool increment) { GMap::mapped_type &V = GroupCount[Group]; // Lazily compute the threshold value for the group count. if (!V.second) { - const GroupInfo &GI = - DiagsInGroup[std::string(Group->getValueAsString("GroupName"))]; + const GroupInfo &GI = DiagsInGroup[Group->getValueAsString("GroupName")]; V.second = GI.SubGroups.size() + GI.DiagsInGroup.size(); } @@ -342,12 +326,9 @@ void InferPedantic::markGroup(const Record *Group) { // covered by -Wpedantic, increment the count of parent groups. Once the // group's count is equal to the number of subgroups and diagnostics in // that group, we can safely add this group to -Wpedantic. - if (groupInPedantic(Group, /* increment */ true)) { - const std::vector<const Record *> &Parents = - DiagGroupParents.getParents(Group); - for (unsigned i = 0, e = Parents.size(); i != e; ++i) - markGroup(Parents[i]); - } + if (groupInPedantic(Group, /* increment */ true)) + for (const Record *Parent : DiagGroupParents.getParents(Group)) + markGroup(Parent); } void InferPedantic::compute(VecOrSet DiagsInPedantic, @@ -355,15 +336,13 @@ void InferPedantic::compute(VecOrSet DiagsInPedantic, // All extensions that are not on by default are implicitly in the // "pedantic" group. For those that aren't explicitly included in -Wpedantic, // mark them for consideration to be included in -Wpedantic directly. - for (unsigned i = 0, e = Diags.size(); i != e; ++i) { - const Record *R = Diags[i]; + for (const Record *R : Diags) { if (isExtension(R) && isOffByDefault(R)) { DiagsSet.insert(R); if (const auto *Group = dyn_cast<DefInit>(R->getValueInit("Group"))) { const Record *GroupRec = Group->getDef(); - if (!isSubGroupOfGroup(GroupRec, "pedantic")) { + if (!isSubGroupOfGroup(GroupRec, "pedantic")) markGroup(GroupRec); - } } } } @@ -371,8 +350,7 @@ void InferPedantic::compute(VecOrSet DiagsInPedantic, // Compute the set of diagnostics that are directly in -Wpedantic. We // march through Diags a second time to ensure the results are emitted // in deterministic order. - for (unsigned i = 0, e = Diags.size(); i != e; ++i) { - const Record *R = Diags[i]; + for (const Record *R : Diags) { if (!DiagsSet.count(R)) continue; // Check if the group is implicitly in -Wpedantic. If so, @@ -386,9 +364,8 @@ void InferPedantic::compute(VecOrSet DiagsInPedantic, // -Wpedantic. Include it in -Wpedantic directly. if (auto *V = DiagsInPedantic.dyn_cast<RecordVec *>()) V->push_back(R); - else { - DiagsInPedantic.get<RecordSet*>()->insert(R); - } + else + DiagsInPedantic.get<RecordSet *>()->insert(R); } if (!GroupsInPedantic) @@ -397,8 +374,7 @@ void InferPedantic::compute(VecOrSet DiagsInPedantic, // Compute the set of groups that are directly in -Wpedantic. We // march through the groups to ensure the results are emitted /// in a deterministc order. - for (unsigned i = 0, ei = DiagGroups.size(); i != ei; ++i) { - const Record *Group = DiagGroups[i]; + for (const Record *Group : DiagGroups) { if (!groupInPedantic(Group)) continue; @@ -415,9 +391,8 @@ void InferPedantic::compute(VecOrSet DiagsInPedantic, if (auto *V = GroupsInPedantic.dyn_cast<RecordVec *>()) V->push_back(Group); - else { - GroupsInPedantic.get<RecordSet*>()->insert(Group); - } + else + GroupsInPedantic.get<RecordSet *>()->insert(Group); } } @@ -468,11 +443,9 @@ static StringRef getModifierName(ModifierType MT) { return "objcclass"; case MT_ObjCInstance: return "objcinstance"; - case MT_Unknown: + default: llvm_unreachable("invalid modifier type"); } - // Unhandled case - llvm_unreachable("invalid modifier type"); } struct Piece { @@ -1201,14 +1174,12 @@ std::string DiagnosticTextBuilder::buildForDefinition(const Record *R) { //===----------------------------------------------------------------------===// static bool isError(const Record &Diag) { - const std::string &ClsName = - std::string(Diag.getValueAsDef("Class")->getName()); + StringRef ClsName = Diag.getValueAsDef("Class")->getName(); return ClsName == "CLASS_ERROR"; } static bool isRemark(const Record &Diag) { - const std::string &ClsName = - std::string(Diag.getValueAsDef("Class")->getName()); + StringRef ClsName = Diag.getValueAsDef("Class")->getName(); return ClsName == "CLASS_REMARK"; } @@ -1426,8 +1397,8 @@ void clang::EmitClangDiagsDefs(const RecordKeeper &Records, raw_ostream &OS, ArrayRef<const Record *> DiagGroups = Records.getAllDerivedDefinitions("DiagGroup"); - std::map<std::string, GroupInfo> DiagsInGroup; - groupDiagnostics(Diags, DiagGroups, DiagsInGroup); + std::map<StringRef, GroupInfo> DiagsInGroup = + groupDiagnostics(Diags, DiagGroups); DiagCategoryIDMap CategoryIDs(Records); DiagGroupParentMap DGParentMap(Records); @@ -1445,8 +1416,7 @@ void clang::EmitClangDiagsDefs(const RecordKeeper &Records, raw_ostream &OS, if (isError(R)) { if (const auto *Group = dyn_cast<DefInit>(R.getValueInit("Group"))) { const Record *GroupRec = Group->getDef(); - const std::string &GroupName = - std::string(GroupRec->getValueAsString("GroupName")); + StringRef GroupName = GroupRec->getValueAsString("GroupName"); PrintFatalError(R.getLoc(), "Error " + R.getName() + " cannot be in a warning group [" + GroupName + "]"); } @@ -1479,13 +1449,11 @@ void clang::EmitClangDiagsDefs(const RecordKeeper &Records, raw_ostream &OS, // Warning group associated with the diagnostic. This is stored as an index // into the alphabetically sorted warning group table. if (const auto *DI = dyn_cast<DefInit>(R.getValueInit("Group"))) { - std::map<std::string, GroupInfo>::iterator I = DiagsInGroup.find( - std::string(DI->getDef()->getValueAsString("GroupName"))); + auto I = DiagsInGroup.find(DI->getDef()->getValueAsString("GroupName")); assert(I != DiagsInGroup.end()); OS << ", " << I->second.IDNo; } else if (DiagsInPedantic.count(&R)) { - std::map<std::string, GroupInfo>::iterator I = - DiagsInGroup.find("pedantic"); + auto I = DiagsInGroup.find("pedantic"); assert(I != DiagsInGroup.end() && "pedantic group not defined"); OS << ", " << I->second.IDNo; } else { @@ -1549,29 +1517,26 @@ static std::string getDiagCategoryEnum(StringRef name) { /// } /// \endcode /// -static void emitDiagSubGroups(std::map<std::string, GroupInfo> &DiagsInGroup, +static void emitDiagSubGroups(std::map<StringRef, GroupInfo> &DiagsInGroup, RecordVec &GroupsInPedantic, raw_ostream &OS) { OS << "static const int16_t DiagSubGroups[] = {\n" << " /* Empty */ -1,\n"; - for (auto const &I : DiagsInGroup) { - const bool IsPedantic = I.first == "pedantic"; + for (auto const &[Name, GroupInfo] : DiagsInGroup) { + const bool IsPedantic = Name == "pedantic"; - const std::vector<std::string> &SubGroups = I.second.SubGroups; + const std::vector<StringRef> &SubGroups = GroupInfo.SubGroups; if (!SubGroups.empty() || (IsPedantic && !GroupsInPedantic.empty())) { - OS << " /* DiagSubGroup" << I.second.IDNo << " */ "; + OS << " /* DiagSubGroup" << GroupInfo.IDNo << " */ "; for (auto const &SubGroup : SubGroups) { - std::map<std::string, GroupInfo>::const_iterator RI = - DiagsInGroup.find(SubGroup); + auto RI = DiagsInGroup.find(SubGroup); assert(RI != DiagsInGroup.end() && "Referenced without existing?"); OS << RI->second.IDNo << ", "; } // Emit the groups implicitly in "pedantic". if (IsPedantic) { for (auto const &Group : GroupsInPedantic) { - const std::string &GroupName = - std::string(Group->getValueAsString("GroupName")); - std::map<std::string, GroupInfo>::const_iterator RI = - DiagsInGroup.find(GroupName); + StringRef GroupName = Group->getValueAsString("GroupName"); + auto RI = DiagsInGroup.find(GroupName); assert(RI != DiagsInGroup.end() && "Referenced without existing?"); OS << RI->second.IDNo << ", "; } @@ -1601,7 +1566,7 @@ static void emitDiagSubGroups(std::map<std::string, GroupInfo> &DiagsInGroup, /// }; /// \endcode /// -static void emitDiagArrays(std::map<std::string, GroupInfo> &DiagsInGroup, +static void emitDiagArrays(std::map<StringRef, GroupInfo> &DiagsInGroup, RecordVec &DiagsInPedantic, raw_ostream &OS) { OS << "static const int16_t DiagArrays[] = {\n" << " /* Empty */ -1,\n"; @@ -1653,7 +1618,7 @@ static void emitDiagGroupNames(const StringToOffsetTable &GroupNames, /// static const char DiagGroupNames[]; /// #endif /// \endcode -static void emitAllDiagArrays(std::map<std::string, GroupInfo> &DiagsInGroup, +static void emitAllDiagArrays(std::map<StringRef, GroupInfo> &DiagsInGroup, RecordVec &DiagsInPedantic, RecordVec &GroupsInPedantic, const StringToOffsetTable &GroupNames, @@ -1680,43 +1645,41 @@ static void emitAllDiagArrays(std::map<std::string, GroupInfo> &DiagsInGroup, /// {/* deprecated */ 1981,/* DiagArray1 */ 348, /* DiagSubGroup3 */ 9}, /// #endif /// \endcode -static void emitDiagTable(std::map<std::string, GroupInfo> &DiagsInGroup, +static void emitDiagTable(std::map<StringRef, GroupInfo> &DiagsInGroup, RecordVec &DiagsInPedantic, RecordVec &GroupsInPedantic, const StringToOffsetTable &GroupNames, raw_ostream &OS) { unsigned MaxLen = 0; - for (auto const &I: DiagsInGroup) - MaxLen = std::max(MaxLen, (unsigned)I.first.size()); + for (auto const &[Name, _] : DiagsInGroup) + MaxLen = std::max(MaxLen, (unsigned)Name.size()); OS << "\n#ifdef DIAG_ENTRY\n"; unsigned SubGroupIndex = 1, DiagArrayIndex = 1; - for (auto const &I: DiagsInGroup) { + for (auto const &[Name, GroupInfo] : DiagsInGroup) { // Group option string. OS << "DIAG_ENTRY("; - OS << I.second.GroupName << " /* "; - - if (I.first.find_f... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/115212 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
