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

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL304326: [TableGen] Clang changes to support 
Record::getValueAsString and… (authored by ctopper).

Changed prior to commit:
  https://reviews.llvm.org/D33711?vs=100826&id=100906#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33711

Files:
  cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
  cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp
  cfe/trunk/utils/TableGen/ClangOptionDocEmitter.cpp
  cfe/trunk/utils/TableGen/ClangSACheckersEmitter.cpp

Index: cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
===
--- cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
+++ cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
@@ -718,9 +718,9 @@
   };
 
   // Unique the enums, but maintain the original declaration ordering.
-  std::vector
-  uniqueEnumsInOrder(const std::vector &enums) {
-std::vector uniques;
+  std::vector
+  uniqueEnumsInOrder(const std::vector &enums) {
+std::vector uniques;
 SmallDenseSet unique_set;
 for (const auto &i : enums) {
   if (unique_set.insert(i).second)
@@ -731,7 +731,8 @@
 
   class EnumArgument : public Argument {
 std::string type;
-std::vector values, enums, uniques;
+std::vector values, enums, uniques;
+
   public:
 EnumArgument(const Record &Arg, StringRef Attr)
   : Argument(Arg, Attr), type(Arg.getValueAsString("Type")),
@@ -850,7 +851,7 @@
   
   class VariadicEnumArgument: public VariadicArgument {
 std::string type, QualifiedTypeName;
-std::vector values, enums, uniques;
+std::vector values, enums, uniques;
 
   protected:
 void writeValueImpl(raw_ostream &OS) const override {
@@ -1591,17 +1592,18 @@
   }
 
   std::string getEnumValueName() const {
-std::string Result =
-"SubjectMatchRule_" + MetaSubject->getValueAsString("Name");
+SmallString<128> Result;
+Result += "SubjectMatchRule_";
+Result += MetaSubject->getValueAsString("Name");
 if (isSubRule()) {
   Result += "_";
   if (isNegatedSubRule())
 Result += "not_";
   Result += Constraint->getValueAsString("Name");
 }
 if (isAbstractRule())
   Result += "_abstract";
-return Result;
+return Result.str();
   }
 
   std::string getEnumValue() const { return "attr::" + getEnumValueName(); }
@@ -2603,7 +2605,7 @@
 // append a unique suffix to distinguish this set of target checks from other
 // TargetSpecificAttr records.
 static void GenerateTargetSpecificAttrChecks(const Record *R,
- std::vector &Arches,
+ std::vector &Arches,
  std::string &Test,
  std::string *FnName) {
   // It is assumed that there will be an llvm::Triple object
@@ -2613,8 +2615,9 @@
   Test += "(";
 
   for (auto I = Arches.begin(), E = Arches.end(); I != E; ++I) {
-std::string Part = *I;
-Test += "T.getArch() == llvm::Triple::" + Part;
+StringRef Part = *I;
+Test += "T.getArch() == llvm::Triple::";
+Test += Part;
 if (I + 1 != E)
   Test += " || ";
 if (FnName)
@@ -2627,11 +2630,12 @@
 // We know that there was at least one arch test, so we need to and in the
 // OS tests.
 Test += " && (";
-std::vector OSes = R->getValueAsListOfStrings("OSes");
+std::vector OSes = R->getValueAsListOfStrings("OSes");
 for (auto I = OSes.begin(), E = OSes.end(); I != E; ++I) {
-  std::string Part = *I;
+  StringRef Part = *I;
 
-  Test += "T.getOS() == llvm::Triple::" + Part;
+  Test += "T.getOS() == llvm::Triple::";
+  Test += Part;
   if (I + 1 != E)
 Test += " || ";
   if (FnName)
@@ -2643,10 +2647,11 @@
   // If one or more CXX ABIs are specified, check those as well.
   if (!R->isValueUnset("CXXABIs")) {
 Test += " && (";
-std::vector CXXABIs = R->getValueAsListOfStrings("CXXABIs");
+std::vector CXXABIs = R->getValueAsListOfStrings("CXXABIs");
 for (auto I = CXXABIs.begin(), E = CXXABIs.end(); I != E; ++I) {
-  std::string Part = *I;
-  Test += "Target.getCXXABI().getKind() == TargetCXXABI::" + Part;
+  StringRef Part = *I;
+  Test += "Target.getCXXABI().getKind() == TargetCXXABI::";
+  Test += Part;
   if (I + 1 != E)
 Test += " || ";
   if (FnName)
@@ -2684,7 +2689,7 @@
 std::string Test;
 if (Attr->isSubClassOf("TargetSpecificAttr")) {
   const Record *R = Attr->getValueAsDef("Target");
-  std::vector Arches = R->getValueAsListOfStrings("Arches");
+  std::vector Arches = R->getValueAsListOfStrings("Arches");
   GenerateTargetSpecificAttrChecks(R, Arches, Test, nullptr);
 
   // If this is the C++11 variety, also add in the LangOpts test.
@@ -3323,7 +3328,7 @@
 
   // Get the list of architectures to be tested for.
   const Record *R = Attr.getValueAsD

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

[Matthias Braun via Phabricator via cfe-commits]

MatzeB accepted this revision.
MatzeB added a comment.

I assume you have checked that the tablegen output doesn't change. Apart from 
that I'm all for more StringRef and SmallStrings or even Twines where possible. 
So LGTM.


https://reviews.llvm.org/D33711



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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(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 from a std::vector 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