[PATCH] D33711: [TableGen] Clang changes to support Record::getValueAsString and getValueAsListOfStrings returning StringRef instead of std::string

[David Blaikie via Phabricator via cfe-commits]

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Few questions, but address them as you see fit.



================
Comment at: utils/TableGen/ClangDiagnosticsEmitter.cpp:1280-1281
 
     writeHeader((IsRemarkGroup ? "-R" : "-W") +
-                    G->getValueAsString("GroupName"),
+                    G->getValueAsString("GroupName").str(),
                 OS);
----------------
Any reason (I'm guessing there is) not to do the str() on the + expression instead of the RHS? (seemed like you'd done that in other changes here, to minimize the string buffering/reallocation/etc by stringifying once at the top level of the expression)


================
Comment at: utils/TableGen/ClangOptionDocEmitter.cpp:86
     // Pretend no-X and Xno-Y options are aliases of X and XY.
-    auto Name = R->getValueAsString("Name");
+    std::string Name = R->getValueAsString("Name");
     if (Name.size() >= 4) {
----------------
Does this need to be a std::string here? I'm not spotting any mutation (but I could well be missing it) of the value. Is it that StringREf doesn't support some of these substr - like operations?

Oh, I guess it's that all of these operations want a std::string (for lookup in OptionsByName, for the result of string concatenation, etc).

It'd still be marginally more efficient to not have to create a std::string up-front, I'd think, but syntactically annoying/verbose for all those uses?


================
Comment at: utils/TableGen/ClangOptionDocEmitter.cpp:272
+        Alias->getValueAsListOfStrings("Prefixes").front(), Alias,
+        std::vector<std::string>(AliasArgs.begin(), AliasArgs.end()), OS);
     OS << ")";
----------------
craig.topper wrote:
> Had to make a copy into vector std::string because emitOptionWithArgs has another caller that passes a std::string vector.
Would it be better/OK if the other caller was the one making the copy (it'd be cheaper to make a std::vector<StringRef> from a std::vector<std::string> than the other way around - but maybe the other caller is much hotter than this one?)?


https://reviews.llvm.org/D33711