[PATCH] Modernize the TableGen code by using C++11 range-based for loops

Justin Bogner mail at justinbogner.com
Mon Oct 6 14:44:08 PDT 2014


This is a pretty big change considering it's supposed to be
non-functional. Changes that just mass convert to use new C++ features
like this are hard to review because of their size, so they can pretty
easily add minor bugs accidentally. I don't really think we should do
this.

It's generally better to just update things you're directly working on
as you go. That is, if you're changing something in a loop and find that
changing the loop to use range-for makes it easier to read and work on,
that great, but changing tens of loops in one go isn't really worth it
in terms of risk and reward.

In any case, I started to review this before deciding that I think it's
too large, so there are some comments below that you might find helpful
when converting loops to use range-based for.

Ehsan Akhgari <ehsan.akhgari at gmail.com> writes:
> Addressed the review comment.
>
> http://reviews.llvm.org/D5488
>
> Files:
>   utils/TableGen/ClangASTNodesEmitter.cpp
>   utils/TableGen/ClangAttrEmitter.cpp
>   utils/TableGen/ClangCommentHTMLNamedCharacterReferenceEmitter.cpp
>   utils/TableGen/ClangDiagnosticsEmitter.cpp
>   utils/TableGen/ClangSACheckersEmitter.cpp
>
> Index: utils/TableGen/ClangASTNodesEmitter.cpp
> ===================================================================
> --- utils/TableGen/ClangASTNodesEmitter.cpp
> +++ utils/TableGen/ClangASTNodesEmitter.cpp
> @@ -157,9 +157,7 @@
>  
>    ChildMap Tree;
>  
> -  for (unsigned i = 0, e = Stmts.size(); i != e; ++i) {
> -    Record *R = Stmts[i];
> -
> +  for (const auto &R : Stmts) {

Shouldn't this be ``const auto *R``? References to pointers are a bit
confusing.

>      if (R->getValue("Base"))
>        Tree.insert(std::make_pair(R->getValueAsDef("Base"), R));
>      else
> @@ -202,10 +200,8 @@
>      = Records.getAllDerivedDefinitions("DeclContext");
>    RecordVector Decls = Records.getAllDerivedDefinitions("Decl");
>    RecordSet DeclContexts (DeclContextsVector.begin(), DeclContextsVector.end());
> -   
> -  for (RecordVector::iterator i = Decls.begin(), e = Decls.end(); i != e; ++i) {
> -    Record *R = *i;
>  
> +  for (const auto *R : Decls) {
>      if (R->getValue("Base")) {
>        Record *B = R->getValueAsDef("Base");
>        if (DeclContexts.find(B) != DeclContexts.end()) {
> @@ -217,11 +213,9 @@
>  
>    // To keep identical order, RecordVector may be used
>    // instead of RecordSet.
> -  for (RecordVector::iterator
> -         i = DeclContextsVector.begin(), e = DeclContextsVector.end();
> -       i != e; ++i)
> -    if (DeclContexts.find(*i) != DeclContexts.end())
> -      OS << "DECL_CONTEXT(" << (*i)->getName() << ")\n";
> +  for (const auto &DC : DeclContextsVector)
> +    if (DeclContexts.find(DC) != DeclContexts.end())
> +      OS << "DECL_CONTEXT(" << DC->getName() << ")\n";
>  
>    OS << "#undef DECL_CONTEXT\n";
>    OS << "#undef DECL_CONTEXT_BASE\n";
> Index: utils/TableGen/ClangAttrEmitter.cpp
> ===================================================================
> --- utils/TableGen/ClangAttrEmitter.cpp
> +++ utils/TableGen/ClangAttrEmitter.cpp
> @@ -726,7 +726,7 @@
>        for (size_t I = 0; I < enums.size(); ++I) {
>          if (Uniques.insert(enums[I]).second)
>            OS << "    case " << getAttrName() << "Attr::" << enums[I]
> -             << ": return \"" << values[I] << "\";\n";       
> +             << ": return \"" << values[I] << "\";\n";
>        }
>        OS << "    }\n"
>           << "    llvm_unreachable(\"No enumerator with that value\");\n"
> @@ -1895,17 +1895,15 @@
>    GenerateHasAttrSpellingStringSwitch(Pragma, OS, "Pragma");
>    OS << "case AttrSyntax::CXX: {\n";
>    // C++11-style attributes are further split out based on the Scope.
> -  for (std::map<std::string, std::vector<Record *>>::iterator I = CXX.begin(),
> -                                                              E = CXX.end();
> -       I != E; ++I) {
> -    if (I != CXX.begin())
> +  for (const auto &I : CXX) {
> +    if (I != *CXX.begin())

This looks wrong. Instead of comparing the iterator you're comparing the
value here. Better to leave this as a traditional for-loop.

If the verbosity of the loop header is bothering you, feel free to use
``auto`` for the iterator type:

    for (auto I = CXX.begin(), E = CXX.end(); I != E; ++I) {

>        OS << " else ";
> -    if (I->first.empty())
> +    if (I.first.empty())
>        OS << "if (!Scope || Scope->getName() == \"\") {\n";
>      else
> -      OS << "if (Scope->getName() == \"" << I->first << "\") {\n";
> +      OS << "if (Scope->getName() == \"" << I.first << "\") {\n";
>      OS << "  return llvm::StringSwitch<bool>(Name)\n";
> -    GenerateHasAttrSpellingStringSwitch(I->second, OS, "CXX11", I->first);
> +    GenerateHasAttrSpellingStringSwitch(I.second, OS, "CXX11", I.first);
>      OS << "}";
>    }
>    OS << "\n}\n";
> Index: utils/TableGen/ClangCommentHTMLNamedCharacterReferenceEmitter.cpp
> ===================================================================
> --- utils/TableGen/ClangCommentHTMLNamedCharacterReferenceEmitter.cpp
> +++ utils/TableGen/ClangCommentHTMLNamedCharacterReferenceEmitter.cpp
> @@ -52,15 +52,13 @@
>    std::vector<Record *> Tags = Records.getAllDerivedDefinitions("NCR");
>    std::vector<StringMatcher::StringPair> NameToUTF8;
>    SmallString<32> CLiteral;
> -  for (std::vector<Record *>::iterator I = Tags.begin(), E = Tags.end();
> -       I != E; ++I) {
> -    Record &Tag = **I;
> -    std::string Spelling = Tag.getValueAsString("Spelling");
> -    uint64_t CodePoint = Tag.getValueAsInt("CodePoint");
> +  for (const auto *Tag : Tags) {
> +    std::string Spelling = Tag->getValueAsString("Spelling");
> +    uint64_t CodePoint = Tag->getValueAsInt("CodePoint");
>      CLiteral.clear();
>      CLiteral.append("return ");
>      if (!translateCodePointToUTF8(CodePoint, CLiteral)) {
> -      SrcMgr.PrintMessage(Tag.getLoc().front(),
> +      SrcMgr.PrintMessage(Tag->getLoc().front(),
>                            SourceMgr::DK_Error,
>                            Twine("invalid code point"));
>        continue;
> Index: utils/TableGen/ClangDiagnosticsEmitter.cpp
> ===================================================================
> --- utils/TableGen/ClangDiagnosticsEmitter.cpp
> +++ utils/TableGen/ClangDiagnosticsEmitter.cpp
> @@ -45,11 +45,11 @@
>    DiagGroupParentMap(RecordKeeper &records) : Records(records) {
>      std::vector<Record*> DiagGroups
>        = Records.getAllDerivedDefinitions("DiagGroup");
> -    for (unsigned i = 0, e = DiagGroups.size(); i != e; ++i) {
> +    for (const auto &DG : DiagGroups) {

Again, this looks like ``const auto *`` would be more appropriate.

>        std::vector<Record*> SubGroups =
> -        DiagGroups[i]->getValueAsListOfDefs("SubGroups");
> -      for (unsigned j = 0, e = SubGroups.size(); j != e; ++j)
> -        Mapping[SubGroups[j]].push_back(DiagGroups[i]);
> +        DG->getValueAsListOfDefs("SubGroups");
> +      for (const auto &SG : SubGroups)
> +        Mapping[SG].push_back(DG);
>      }
>    }
>  
> @@ -69,8 +69,8 @@
>    // The diag group may the subgroup of one or more other diagnostic groups,
>    // check these for a category as well.
>    const std::vector<Record*> &Parents = DiagGroupParents.getParents(Group);
> -  for (unsigned i = 0, e = Parents.size(); i != e; ++i) {
> -    CatName = getCategoryFromDiagGroup(Parents[i], DiagGroupParents);
> +  for (const auto &Parent : Parents) {

This one too. There are likely more cases where the values are pointers
that are using references that could be cleaned up. I'm going to stop
mentioning them now.

> +    CatName = getCategoryFromDiagGroup(Parent, DiagGroupParents);
>      if (!CatName.empty()) return CatName;
>    }
>    return "";
> @@ -107,8 +107,8 @@
>  
>        std::vector<Record*> Diags =
>        Records.getAllDerivedDefinitions("Diagnostic");
> -      for (unsigned i = 0, e = Diags.size(); i != e; ++i) {
> -        std::string Category = getDiagnosticCategory(Diags[i], ParentInfo);
> +      for (const auto &D : Diags) {
> +        std::string Category = getDiagnosticCategory(D, ParentInfo);
>          if (Category.empty()) continue;  // Skip diags with no category.
>  
>          unsigned &ID = CategoryIDs[Category];
> @@ -167,8 +167,7 @@
>                               const std::vector<Record*> &DiagGroups,
>                               std::map<std::string, GroupInfo> &DiagsInGroup) {
>  
> -  for (unsigned i = 0, e = Diags.size(); i != e; ++i) {
> -    const Record *R = Diags[i];
> +  for (const auto &R : Diags) {
>      DefInit *DI = dyn_cast<DefInit>(R->getValueInit("Group"));
>      if (!DI)
>        continue;
> @@ -183,8 +182,7 @@
>  
>    // 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) {
> -    Record *Group = DiagGroups[i];
> +  for (const auto &Group : DiagGroups) {
>      GroupInfo &GI = DiagsInGroup[Group->getValueAsString("GroupName")];
>      if (Group->isAnonymous()) {
>        if (GI.DiagsInGroup.size() > 1)
> @@ -197,48 +195,41 @@
>      }
>  
>      std::vector<Record*> SubGroups = Group->getValueAsListOfDefs("SubGroups");
> -    for (unsigned j = 0, e = SubGroups.size(); j != e; ++j)
> -      GI.SubGroups.push_back(SubGroups[j]->getValueAsString("GroupName"));
> +    for (const auto &SG : SubGroups)
> +      GI.SubGroups.push_back(SG->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 &I : DiagsInGroup)
> +    I.second.IDNo = IDNo++;

Best not to use ``I`` for things that aren't iterators or indices.
Something like ``Diag`` might be better here. There are a few more cases
below where you switch to a range-for but keep using ``I``. Please
update those names to more meaningful ones too.

>  
>    // Sort the implicit groups, so we can warn about them deterministically.
>    SmallVector<GroupInfo *, 16> SortedGroups(ImplicitGroups.begin(),
>                                              ImplicitGroups.end());
> -  for (SmallVectorImpl<GroupInfo *>::iterator I = SortedGroups.begin(),
> -                                              E = SortedGroups.end();
> -       I != E; ++I) {
> -    MutableArrayRef<const Record *> GroupDiags = (*I)->DiagsInGroup;
> +  for (const auto &I : SortedGroups) {
> +    MutableArrayRef<const Record *> GroupDiags = I->DiagsInGroup;
>      std::sort(GroupDiags.begin(), GroupDiags.end(), beforeThanCompare);
>    }
>    std::sort(SortedGroups.begin(), SortedGroups.end(), beforeThanCompareGroups);
>  
>    // Warn about the same group being used anonymously in multiple places.
> -  for (SmallVectorImpl<GroupInfo *>::const_iterator I = SortedGroups.begin(),
> -                                                    E = SortedGroups.end();
> -       I != E; ++I) {
> -    ArrayRef<const Record *> GroupDiags = (*I)->DiagsInGroup;
> -
> -    if ((*I)->ExplicitDef) {
> -      std::string Name = (*I)->ExplicitDef->getValueAsString("GroupName");
> -      for (ArrayRef<const Record *>::const_iterator DI = GroupDiags.begin(),
> -                                                    DE = GroupDiags.end();
> -           DI != DE; ++DI) {
> -        const DefInit *GroupInit = cast<DefInit>((*DI)->getValueInit("Group"));
> +  for (const auto &I : SortedGroups) {
> +    ArrayRef<const Record *> GroupDiags = I->DiagsInGroup;
> +
> +    if (I->ExplicitDef) {
> +      std::string Name = I->ExplicitDef->getValueAsString("GroupName");
> +      for (const auto &DI : GroupDiags) {
> +        const DefInit *GroupInit = cast<DefInit>(DI->getValueInit("Group"));
>          const Record *NextDiagGroup = GroupInit->getDef();
> -        if (NextDiagGroup == (*I)->ExplicitDef)
> +        if (NextDiagGroup == I->ExplicitDef)
>            continue;
>  
> -        SMRange InGroupRange = findSuperClassRange(*DI, "InGroup");
> +        SMRange InGroupRange = findSuperClassRange(DI, "InGroup");
>          SmallString<64> Replacement;
>          if (InGroupRange.isValid()) {
>            Replacement += "InGroup<";
> -          Replacement += (*I)->ExplicitDef->getName();
> +          Replacement += I->ExplicitDef->getName();
>            Replacement += ">";
>          }
>          SMFixIt FixIt(InGroupRange, Replacement.str());
> @@ -250,7 +241,7 @@
>                              None,
>                              InGroupRange.isValid() ? FixIt
>                                                     : ArrayRef<SMFixIt>());
> -        SrcMgr.PrintMessage((*I)->ExplicitDef->getLoc().front(),
> +        SrcMgr.PrintMessage(I->ExplicitDef->getLoc().front(),
>                              SourceMgr::DK_Note, "group defined here");
>        }
>      } else {
> @@ -344,8 +335,8 @@
>      return true;
>  
>    const std::vector<Record*> &Parents = DiagGroupParents.getParents(Group);
> -  for (unsigned i = 0, e = Parents.size(); i != e; ++i)
> -    if (isSubGroupOfGroup(Parents[i], GName))
> +  for (const auto &Parent : Parents)
> +    if (isSubGroupOfGroup(Parent, GName))
>        return true;
>  
>    return false;
> @@ -387,18 +378,17 @@
>    // that group, we can safely add this group to -Wpedantic.
>    if (groupInPedantic(Group, /* increment */ true)) {
>      const std::vector<Record*> &Parents = DiagGroupParents.getParents(Group);
> -    for (unsigned i = 0, e = Parents.size(); i != e; ++i)
> -      markGroup(Parents[i]);
> +    for (const auto &Parent : Parents)
> +      markGroup(Parent);
>    }
>  }
>  
>  void InferPedantic::compute(VecOrSet DiagsInPedantic,
>                              VecOrSet GroupsInPedantic) {
>    // 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) {
> -    Record *R = Diags[i];
> +  for (const auto &R : Diags) {
>      if (isExtension(R) && isOffByDefault(R)) {
>        DiagsSet.insert(R);
>        if (DefInit *Group = dyn_cast<DefInit>(R->getValueInit("Group"))) {
> @@ -413,8 +403,7 @@
>    // 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) {
> -    Record *R = Diags[i];
> +  for (const auto &R : Diags) {
>      if (!DiagsSet.count(R))
>        continue;
>      // Check if the group is implicitly in -Wpedantic.  If so,
> @@ -439,15 +428,14 @@
>    // 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) {
> -    Record *Group = DiagGroups[i];
> +  for (const auto &Group : DiagGroups) {
>      if (!groupInPedantic(Group))
>        continue;
>  
>      unsigned ParentsInPedantic = 0;
>      const std::vector<Record*> &Parents = DiagGroupParents.getParents(Group);
> -    for (unsigned j = 0, ej = Parents.size(); j != ej; ++j) {
> -      if (groupInPedantic(Parents[j]))
> +    for (const auto &Parent : Parents) {
> +      if (groupInPedantic(Parent))
>          ++ParentsInPedantic;
>      }
>      // If all the parents are in -Wpedantic, this means that this diagnostic
> @@ -511,49 +499,47 @@
>    InferPedantic inferPedantic(DGParentMap, Diags, DiagGroups, DiagsInGroup);
>    inferPedantic.compute(&DiagsInPedantic, (RecordVec*)nullptr);
>  
> -  for (unsigned i = 0, e = Diags.size(); i != e; ++i) {
> -    const Record &R = *Diags[i];
> -
> +  for (const auto &R : Diags) {
>      // Check if this is an error that is accidentally in a warning
>      // group.
> -    if (isError(R)) {
> -      if (DefInit *Group = dyn_cast<DefInit>(R.getValueInit("Group"))) {
> +    if (isError(*R)) {
> +      if (DefInit *Group = dyn_cast<DefInit>(R->getValueInit("Group"))) {
>          const Record *GroupRec = Group->getDef();
>          const std::string &GroupName = GroupRec->getValueAsString("GroupName");
> -        PrintFatalError(R.getLoc(), "Error " + R.getName() +
> +        PrintFatalError(R->getLoc(), "Error " + R->getName() +
>                        " cannot be in a warning group [" + GroupName + "]");
>        }
>      }
>  
>      // Check that all remarks have an associated diagnostic group.
> -    if (isRemark(R)) {
> -      if (!isa<DefInit>(R.getValueInit("Group"))) {
> -        PrintFatalError(R.getLoc(), "Error " + R.getName() +
> +    if (isRemark(*R)) {
> +      if (!isa<DefInit>(R->getValueInit("Group"))) {
> +        PrintFatalError(R->getLoc(), "Error " + R->getName() +
>                                          " not in any diagnostic group");
>        }
>      }
>  
>      // Filter by component.
> -    if (!Component.empty() && Component != R.getValueAsString("Component"))
> +    if (!Component.empty() && Component != R->getValueAsString("Component"))
>        continue;
>  
> -    OS << "DIAG(" << R.getName() << ", ";
> -    OS << R.getValueAsDef("Class")->getName();
> +    OS << "DIAG(" << R->getName() << ", ";
> +    OS << R->getValueAsDef("Class")->getName();
>      OS << ", (unsigned)diag::Severity::"
> -       << R.getValueAsDef("DefaultSeverity")->getValueAsString("Name");
> +       << R->getValueAsDef("DefaultSeverity")->getValueAsString("Name");
>  
>      // Description string.
>      OS << ", \"";
> -    OS.write_escaped(R.getValueAsString("Text")) << '"';
> +    OS.write_escaped(R->getValueAsString("Text")) << '"';
>  
>      // Warning associated with the diagnostic. This is stored as an index into
>      // the alphabetically sorted warning table.
> -    if (DefInit *DI = dyn_cast<DefInit>(R.getValueInit("Group"))) {
> +    if (DefInit *DI = dyn_cast<DefInit>(R->getValueInit("Group"))) {
>        std::map<std::string, GroupInfo>::iterator I =
>            DiagsInGroup.find(DI->getDef()->getValueAsString("GroupName"));
>        assert(I != DiagsInGroup.end());
>        OS << ", " << I->second.IDNo;
> -    } else if (DiagsInPedantic.count(&R)) {
> +    } else if (DiagsInPedantic.count(R)) {
>        std::map<std::string, GroupInfo>::iterator I =
>          DiagsInGroup.find("pedantic");
>        assert(I != DiagsInGroup.end() && "pedantic group not defined");
> @@ -563,21 +549,21 @@
>      }
>  
>      // SFINAE response.
> -    OS << ", " << R.getValueAsDef("SFINAE")->getName();
> +    OS << ", " << R->getValueAsDef("SFINAE")->getName();
>  
>      // Default warning has no Werror bit.
> -    if (R.getValueAsBit("WarningNoWerror"))
> +    if (R->getValueAsBit("WarningNoWerror"))
>        OS << ", true";
>      else
>        OS << ", false";
>  
> -    if (R.getValueAsBit("ShowInSystemHeader"))
> +    if (R->getValueAsBit("ShowInSystemHeader"))
>        OS << ", true";
>      else
>        OS << ", false";
>  
>      // Category number.
> -    OS << ", " << CategoryIDs.getID(getDiagnosticCategory(&R, DGParentMap));
> +    OS << ", " << CategoryIDs.getID(getDiagnosticCategory(R, DGParentMap));
>      OS << ")\n";
>    }
>  }
> @@ -591,8 +577,8 @@
>    if (name.empty())
>      return "DiagCat_None";
>    SmallString<256> enumName = llvm::StringRef("DiagCat_");
> -  for (llvm::StringRef::iterator I = name.begin(), E = name.end(); I != E; ++I)
> -    enumName += isalnum(*I) ? *I : '_';
> +  for (const auto &I : name)
> +    enumName += isalnum(I) ? I : '_';
>    return enumName.str();
>  }
>  
> @@ -843,12 +829,9 @@
>    inferPedantic.compute(&DiagsInPedantic, &GroupsInPedantic);
>  
>    StringToOffsetTable GroupNames;
> -  for (std::map<std::string, GroupInfo>::const_iterator
> -           I = DiagsInGroup.begin(),
> -           E = DiagsInGroup.end();
> -       I != E; ++I) {
> +  for (const auto &I : DiagsInGroup) {
>      // Store a pascal-style length byte at the beginning of the string.
> -    std::string Name = char(I->first.size()) + I->first;
> +    std::string Name = char(I.first.size()) + I.first;
>      GroupNames.GetOrAddStringOffset(Name, false);
>    }
>  
> @@ -882,18 +865,15 @@
>  
>    std::vector<RecordIndexElement> Index;
>    Index.reserve(Diags.size());
> -  for (unsigned i = 0, e = Diags.size(); i != e; ++i) {
> -    const Record &R = *(Diags[i]);
> -    Index.push_back(RecordIndexElement(R));
> +  for (const auto &R : Diags) {
> +    Index.push_back(RecordIndexElement(*R));
>    }
>  
>    std::sort(Index.begin(), Index.end(),
>              [](const RecordIndexElement &Lhs,
>                 const RecordIndexElement &Rhs) { return Lhs.Name < Rhs.Name; });
>  
> -  for (unsigned i = 0, e = Index.size(); i != e; ++i) {
> -    const RecordIndexElement &R = Index[i];
> -
> +  for (const auto &R : Index) {
>      OS << "DIAG_NAME_INDEX(" << R.Name << ")\n";
>    }
>  }
> Index: utils/TableGen/ClangSACheckersEmitter.cpp
> ===================================================================
> --- utils/TableGen/ClangSACheckersEmitter.cpp
> +++ utils/TableGen/ClangSACheckersEmitter.cpp
> @@ -83,14 +83,12 @@
>  static void addPackageToCheckerGroup(const Record *package, const Record *group,
>                    llvm::DenseMap<const Record *, GroupInfo *> &recordGroupMap) {
>    llvm::DenseSet<const Record *> &checkers = recordGroupMap[package]->Checkers;
> -  for (llvm::DenseSet<const Record *>::iterator
> -         I = checkers.begin(), E = checkers.end(); I != E; ++I)
> -    recordGroupMap[group]->Checkers.insert(*I);
> +  for (const auto &I : checkers)
> +    recordGroupMap[group]->Checkers.insert(I);
>  
>    llvm::DenseSet<const Record *> &subGroups = recordGroupMap[package]->SubGroups;
> -  for (llvm::DenseSet<const Record *>::iterator
> -         I = subGroups.begin(), E = subGroups.end(); I != E; ++I)
> -    addPackageToCheckerGroup(*I, group, recordGroupMap);
> +  for (const auto &I : subGroups)
> +    addPackageToCheckerGroup(I, group, recordGroupMap);
>  }
>  
>  namespace clang {
> @@ -106,8 +104,7 @@
>    llvm::DenseMap<const Record *, GroupInfo *> recordGroupMap;
>  
>    std::vector<Record*> packages = Records.getAllDerivedDefinitions("Package");
> -  for (unsigned i = 0, e = packages.size(); i != e; ++i) {
> -    Record *R = packages[i];
> +  for (const auto &R : packages) {
>      std::string fullName = getPackageFullName(R);
>      if (!fullName.empty()) {
>        GroupInfo &info = groupInfoByName[fullName];
> @@ -118,17 +115,15 @@
>  
>    std::vector<Record*>
>        checkerGroups = Records.getAllDerivedDefinitions("CheckerGroup");
> -  for (unsigned i = 0, e = checkerGroups.size(); i != e; ++i) {
> -    Record *R = checkerGroups[i];
> +  for (const auto &R : checkerGroups) {
>      std::string name = R->getValueAsString("GroupName");
>      if (!name.empty()) {
>        GroupInfo &info = groupInfoByName[name];
>        recordGroupMap[R] = &info;
>      }
>    }
>  
> -  for (unsigned i = 0, e = checkers.size(); i != e; ++i) {
> -    Record *R = checkers[i];
> +  for (const auto &R : checkers) {
>      Record *package = nullptr;
>      if (DefInit *
>            DI = dyn_cast<DefInit>(R->getValueInit("ParentPackage")))
> @@ -164,25 +159,24 @@
>  
>    // If a package is in group, add all its checkers and its sub-packages
>    // checkers into the group.
> -  for (unsigned i = 0, e = packages.size(); i != e; ++i)
> -    if (DefInit *DI = dyn_cast<DefInit>(packages[i]->getValueInit("Group")))
> -      addPackageToCheckerGroup(packages[i], DI->getDef(), recordGroupMap);
> +  for (const auto &package : packages)
> +    if (DefInit *DI = dyn_cast<DefInit>(package->getValueInit("Group")))
> +      addPackageToCheckerGroup(package, DI->getDef(), recordGroupMap);
>  
>    typedef std::map<std::string, const Record *> SortedRecords;
>    typedef llvm::DenseMap<const Record *, unsigned> RecToSortIndex;
>  
>    SortedRecords sortedGroups;
>    RecToSortIndex groupToSortIndex;
>    OS << "\n#ifdef GET_GROUPS\n";
>    {
> -    for (unsigned i = 0, e = checkerGroups.size(); i != e; ++i)
> -      sortedGroups[checkerGroups[i]->getValueAsString("GroupName")]
> -                   = checkerGroups[i];
> +    for (const auto &checkerGroup : checkerGroups)
> +      sortedGroups[checkerGroup->getValueAsString("GroupName")]
> +                   = checkerGroup;
>  
>      unsigned sortIndex = 0;
> -    for (SortedRecords::iterator
> -           I = sortedGroups.begin(), E = sortedGroups.end(); I != E; ++I) {
> -      const Record *R = I->second;
> +    for (const auto &I : sortedGroups) {
> +      const Record *R = I.second;
>    
>        OS << "GROUP(" << "\"";
>        OS.write_escaped(R->getValueAsString("GroupName")) << "\"";
> @@ -196,12 +190,11 @@
>    OS << "\n#ifdef GET_PACKAGES\n";
>    {
>      SortedRecords sortedPackages;
> -    for (unsigned i = 0, e = packages.size(); i != e; ++i)
> -      sortedPackages[getPackageFullName(packages[i])] = packages[i];
> +    for (const auto &package : packages)
> +      sortedPackages[getPackageFullName(package)] = package;
>    
> -    for (SortedRecords::iterator
> -           I = sortedPackages.begin(), E = sortedPackages.end(); I != E; ++I) {
> -      const Record &R = *I->second;
> +    for (const auto &I : sortedPackages) {
> +      const Record &R = *I.second;
>    
>        OS << "PACKAGE(" << "\"";
>        OS.write_escaped(getPackageFullName(&R)) << "\", ";
> @@ -221,99 +214,90 @@
>    OS << "#endif // GET_PACKAGES\n\n";
>    
>    OS << "\n#ifdef GET_CHECKERS\n";
> -  for (unsigned i = 0, e = checkers.size(); i != e; ++i) {
> -    const Record &R = *checkers[i];
> -
> +  for (const auto &R : checkers) {
>      OS << "CHECKER(" << "\"";
>      std::string name;
> -    if (isCheckerNamed(&R))
> -      name = getCheckerFullName(&R);
> +    if (isCheckerNamed(R))
> +      name = getCheckerFullName(R);
>      OS.write_escaped(name) << "\", ";
> -    OS << R.getName() << ", ";
> -    OS << getStringValue(R, "DescFile") << ", ";
> +    OS << R->getName() << ", ";
> +    OS << getStringValue(*R, "DescFile") << ", ";
>      OS << "\"";
> -    OS.write_escaped(getStringValue(R, "HelpText")) << "\", ";
> +    OS.write_escaped(getStringValue(*R, "HelpText")) << "\", ";
>      // Group index
> -    if (DefInit *DI = dyn_cast<DefInit>(R.getValueInit("Group")))
> +    if (DefInit *DI = dyn_cast<DefInit>(R->getValueInit("Group")))
>        OS << groupToSortIndex[DI->getDef()] << ", ";
>      else
>        OS << "-1, ";
>      // Hidden bit
> -    if (isHidden(R))
> +    if (isHidden(*R))
>        OS << "true";
>      else
>        OS << "false";
>      OS << ")\n";
>    }
>    OS << "#endif // GET_CHECKERS\n\n";
>  
>    unsigned index = 0;
> -  for (std::map<std::string, GroupInfo>::iterator
> -         I = groupInfoByName.begin(), E = groupInfoByName.end(); I != E; ++I)
> -    I->second.Index = index++;
> +  for (auto &I : groupInfoByName)
> +    I.second.Index = index++;
>  
>    // Walk through the packages/groups/checkers emitting an array for each
>    // set of checkers and an array for each set of subpackages.
>  
>    OS << "\n#ifdef GET_MEMBER_ARRAYS\n";
>    unsigned maxLen = 0;
> -  for (std::map<std::string, GroupInfo>::iterator
> -         I = groupInfoByName.begin(), E = groupInfoByName.end(); I != E; ++I) {
> -    maxLen = std::max(maxLen, (unsigned)I->first.size());
> +  for (auto &I : groupInfoByName) {
> +    maxLen = std::max(maxLen, (unsigned)I.first.size());
>  
> -    llvm::DenseSet<const Record *> &checkers = I->second.Checkers;
> +    llvm::DenseSet<const Record *> &checkers = I.second.Checkers;
>      if (!checkers.empty()) {
> -      OS << "static const short CheckerArray" << I->second.Index << "[] = { ";
> +      OS << "static const short CheckerArray" << I.second.Index << "[] = { ";
>        // Make the output order deterministic.
>        std::map<int, const Record *> sorted;
> -      for (llvm::DenseSet<const Record *>::iterator
> -             I = checkers.begin(), E = checkers.end(); I != E; ++I)
> -        sorted[(*I)->getID()] = *I;
> +      for (const auto &I : checkers)
> +        sorted[I->getID()] = I;
>  
> -      for (std::map<int, const Record *>::iterator
> -             I = sorted.begin(), E = sorted.end(); I != E; ++I)
> -        OS << checkerRecIndexMap[I->second] << ", ";
> +      for (const auto &I : sorted)
> +        OS << checkerRecIndexMap[I.second] << ", ";
>        OS << "-1 };\n";
>      }
>      
> -    llvm::DenseSet<const Record *> &subGroups = I->second.SubGroups;
> +    llvm::DenseSet<const Record *> &subGroups = I.second.SubGroups;
>      if (!subGroups.empty()) {
> -      OS << "static const short SubPackageArray" << I->second.Index << "[] = { ";
> +      OS << "static const short SubPackageArray" << I.second.Index << "[] = { ";
>        // Make the output order deterministic.
>        std::map<int, const Record *> sorted;
> -      for (llvm::DenseSet<const Record *>::iterator
> -             I = subGroups.begin(), E = subGroups.end(); I != E; ++I)
> -        sorted[(*I)->getID()] = *I;
> +      for (const auto &I : subGroups)
> +        sorted[I->getID()] = I;
>  
> -      for (std::map<int, const Record *>::iterator
> -             I = sorted.begin(), E = sorted.end(); I != E; ++I) {
> -        OS << recordGroupMap[I->second]->Index << ", ";
> +      for (const auto &I : sorted) {
> +        OS << recordGroupMap[I.second]->Index << ", ";
>        }
>        OS << "-1 };\n";
>      }
>    }
>    OS << "#endif // GET_MEMBER_ARRAYS\n\n";
>  
>    OS << "\n#ifdef GET_CHECKNAME_TABLE\n";
> -  for (std::map<std::string, GroupInfo>::iterator
> -         I = groupInfoByName.begin(), E = groupInfoByName.end(); I != E; ++I) {
> +  for (const auto &I : groupInfoByName) {
>      // Group option string.
>      OS << "  { \"";
> -    OS.write_escaped(I->first) << "\","
> -                               << std::string(maxLen-I->first.size()+1, ' ');
> +    OS.write_escaped(I.first) << "\","
> +                              << std::string(maxLen-I.first.size()+1, ' ');
>      
> -    if (I->second.Checkers.empty())
> +    if (I.second.Checkers.empty())
>        OS << "0, ";
>      else
> -      OS << "CheckerArray" << I->second.Index << ", ";
> +      OS << "CheckerArray" << I.second.Index << ", ";
>      
>      // Subgroups.
> -    if (I->second.SubGroups.empty())
> +    if (I.second.SubGroups.empty())
>        OS << "0, ";
>      else
> -      OS << "SubPackageArray" << I->second.Index << ", ";
> +      OS << "SubPackageArray" << I.second.Index << ", ";
>  
> -    OS << (I->second.Hidden ? "true" : "false");
> +    OS << (I.second.Hidden ? "true" : "false");
>  
>      OS << " },\n";
>    }
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits



More information about the cfe-commits mailing list