[TableGen] NFC: Switch to range-based for loops in OptParserEmitter

[jansvoboda11]

Summary

This simplifies the code a bit. No functionality change.

[dexonsmith]

Thanks for the cleanup! I have a few nits below, some of which will help to reduce the diff. LGTM once you fix those.

llvm/utils/TableGen/OptParserEmitter.cpp

237

• This name is confusing to me because PrefixIt doesn't seem to be an iterator (but It suggests it is). Can this be Prefix instead? Or maybe P?
• Should this be const auto & to avoid the copy? If not, please spell out the type so it's easy to confirm at the call site that a copy is cheap enough.

244–246

• You might consider dropping the braces since you're touching this code anyway.
• I think the type here could be a by-value StringRef to avoid having to change it if the string storage implementation changes in the future.

255

You can reduce the diff with:

for (const Record &R : llvm::make_pointee_range(Groups)) {

A side benefit is we keep a non-non-null variable from looking like it could be null to the casual reader of later lines.

340–341

StringRef is cheap to copy; just use it by-value. Generally we try to avoid taking it by-reference (unless it's being modified, or if it somehow avoids an #include although it's so pervasive that's probably not worth doing).

391

As above, you can use make_pointee_range.

458

make_pointee_range

[jansvoboda11]

Thanks for the suggestions! llvm::make_pointee_range is handy.

[jansvoboda11]

Incorporate review feedback

llvm/utils/TableGen/OptParserEmitter.cpp

void EmitOptParser(RecordKeeper &Records, raw_ostream &OS) {
 
   array_pod_sort(Opts.begin(), Opts.end(), CompareOptionRecords);
   // Generate prefix groups.
   typedef SmallVector<SmallString<2>, 2> PrefixKeyT;
   typedef std::map<PrefixKeyT, std::string> PrefixesT;
   PrefixesT Prefixes;
   Prefixes.insert(std::make_pair(PrefixKeyT(), "prefix_0"));
   unsigned CurPrefix = 0;
-  for (unsigned i = 0, e = Opts.size(); i != e; ++i) {
-    const Record &R = *Opts[i];
+  for (const Record &R : llvm::make_pointee_range(Opts)) {
     std::vector<StringRef> prf = R.getValueAsListOfStrings("Prefixes");
     PrefixKeyT prfkey(prf.begin(), prf.end());
     unsigned NewPrefix = CurPrefix + 1;
     if (Prefixes.insert(std::make_pair(prfkey, (Twine("prefix_") +
                                               Twine(NewPrefix)).str())).second)
       CurPrefix = NewPrefix;
   }
 
   // Dump prefixes.
 
   OS << "/////////\n";
   OS << "// Prefixes\n\n";
   OS << "#ifdef PREFIX\n";
   OS << "#define COMMA ,\n";
-  for (PrefixesT::const_iterator I = Prefixes.begin(), E = Prefixes.end();
-                                  I != E; ++I) {
+  for (const auto &Prefix : Prefixes) {
     OS << "PREFIX(";
 
     // Prefix name.
-    OS << I->second;
+    OS << Prefix.second;
 
     // Prefix values.
     OS << ", {";
-    for (PrefixKeyT::const_iterator PI = I->first.begin(),
-                                    PE = I->first.end(); PI != PE; ++PI) {
-      OS << "\"" << *PI << "\" COMMA ";
-    }
+    for (StringRef PrefixKey : Prefix.first)
+      OS << "\"" << PrefixKey << "\" COMMA ";
     OS << "nullptr})\n";
   }
   OS << "#undef COMMA\n";
   OS << "#endif // PREFIX\n\n";
 
   OS << "/////////\n";
   OS << "// Groups\n\n";
   OS << "#ifdef OPTION\n";
-  for (unsigned i = 0, e = Groups.size(); i != e; ++i) {
-    const Record &R = *Groups[i];
-
+  for (const Record &R : llvm::make_pointee_range(Groups)) {
     // Start a single option entry.
     OS << "OPTION(";
 
     // The option prefix;
     OS << "nullptr";
 
     // The option string.
     OS << ", \"" << R.getValueAsString("Name") << '"';
auto WriteOptRecordFields = [&](raw_ostream &OS, const Record &R) {
     // would become "foo\0bar\0". Note that the compiler adds an implicit
     // terminating \0 at the end.
     OS << ", ";
     std::vector<StringRef> AliasArgs = R.getValueAsListOfStrings("AliasArgs");
     if (AliasArgs.size() == 0) {
       OS << "nullptr";
     } else {
       OS << "\"";
-      for (size_t i = 0, e = AliasArgs.size(); i != e; ++i)
-        OS << AliasArgs[i] << "\\0";
+      for (StringRef AliasArg : AliasArgs)
+        OS << AliasArg << "\\0";
       OS << "\"";
     }
 
     // The option flags.
     OS << ", ";
     int NumFlags = 0;
     const ListInit *LI = R.getValueAsListInit("Flags");
     for (Init *I : *LI)
void EmitOptParser(RecordKeeper &Records, raw_ostream &OS) {
   };
 
   auto IsMarshallingOption = [](const Record &R) {
     return !isa<UnsetInit>(R.getValueInit("KeyPath")) &&
            !R.getValueAsString("KeyPath").empty();
   };
 
   std::vector<const Record *> OptsWithMarshalling;
-  for (unsigned I = 0, E = Opts.size(); I != E; ++I) {
-    const Record &R = *Opts[I];
-
+  for (const Record &R : llvm::make_pointee_range(Opts)) {
     // Start a single option entry.
     OS << "OPTION(";
     WriteOptRecordFields(OS, R);
     OS << ")\n";
     if (IsMarshallingOption(R))
       OptsWithMarshalling.push_back(&R);
   }
   OS << "#endif // OPTION\n";
void EmitOptParser(RecordKeeper &Records, raw_ostream &OS) {
 
   OS << "#endif // SIMPLE_ENUM_VALUE_TABLE\n";
   OS << "\n";
 
   OS << "\n";
   OS << "#ifdef OPTTABLE_ARG_INIT\n";
   OS << "//////////\n";
   OS << "// Option Values\n\n";
-  for (unsigned I = 0, E = Opts.size(); I != E; ++I) {
-    const Record &R = *Opts[I];
+  for (const Record &R : llvm::make_pointee_range(Opts)) {
     if (isa<UnsetInit>(R.getValueInit("ValuesCode")))
       continue;
     OS << "{\n";
     OS << "bool ValuesWereAdded;\n";
     OS << R.getValueAsString("ValuesCode");
     OS << "\n";
     for (StringRef Prefix : R.getValueAsListOfStrings("Prefixes")) {
       OS << "ValuesWereAdded = Opt.addValues(";
       std::string S(Prefix);