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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
