[PATCH] D141714: Fix ast print of variables with attributes

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I think we can do better naming the tablegen'ed parts, else a bunch of smaller changes.  Approach seems good enough to me, though Aaron should scroll through/make a determination after you've fixed my concerns.



================
Comment at: clang/include/clang/Basic/Attr.td:565
 class Attr {
+  // On a declaration, can this attribute be print on the left side?
+  bit CanPrintOnLeftSide = 1;
----------------



================
Comment at: clang/include/clang/Basic/Attr.td:566
+  // On a declaration, can this attribute be print on the left side?
+  bit CanPrintOnLeftSide = 1;
+  // On a declaration, must this attribute be print on the left side?
----------------



================
Comment at: clang/include/clang/Basic/Attr.td:567
+  bit CanPrintOnLeftSide = 1;
+  // On a declaration, must this attribute be print on the left side?
+  bit MustPrintOnLeftSide = 0;
----------------



================
Comment at: clang/include/clang/Basic/Attr.td:568
+  // On a declaration, must this attribute be print on the left side?
+  bit MustPrintOnLeftSide = 0;
   // The various ways in which an attribute can be spelled in source
----------------



================
Comment at: clang/lib/AST/DeclPrinter.cpp:263
+static bool canPrintOnLeftSide(const Attr *A) {
+  if (A->isStandardAttributeSyntax()) {
+    return false;
----------------
Don't include curley brackets here.


================
Comment at: clang/lib/AST/DeclPrinter.cpp:280
+static bool mustPrintOnLeftSide(const Attr *A) {
+  if (A->isDeclspecAttribute()) {
+    return true;
----------------
Same here, don't use curleys on single-liners.


================
Comment at: clang/lib/AST/DeclPrinter.cpp:298
+
+      AttrPrintLoc attrloc = AttrPrintLoc::Right;
+      if (mustPrintOnLeftSide(A)) {
----------------



================
Comment at: clang/lib/AST/DeclPrinter.cpp:306
+        // side so that GCC accept our dumps as well.
+        if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D);
+            FD && FD->isThisDeclarationADefinition()) {
----------------



================
Comment at: clang/lib/AST/DeclPrinter.cpp:314
+          // printing on the left side for readbility.
+        } else if (const VarDecl *VD = dyn_cast<VarDecl>(D);
+                   VD && VD->getInit() &&
----------------



================
Comment at: clang/lib/AST/DeclPrinter.cpp:1003
 
-  printDeclType(T, (isa<ParmVarDecl>(D) && Policy.CleanUglifiedParameters &&
-                    D->getIdentifier())
-                       ? D->getIdentifier()->deuglifiedName()
-                       : D->getName());
+  std::string Name;
+
----------------
Instead of making this a std::string, make it a StringRef and just assign it on line 1005, it looks like both sides of that return a StringRef (deuglifiedName and getName), and your += is unnecessary (since there is nothing in Name here anyway).


================
Comment at: clang/utils/TableGen/TableGen.cpp:282
                    "Generate riscv_vector_builtin_sema.inc for clang"),
-        clEnumValN(GenRISCVSiFiveVectorBuiltins, "gen-riscv-sifive-vector-builtins",
+        clEnumValN(GenRISCVSiFiveVectorBuiltins,
+                   "gen-riscv-sifive-vector-builtins",
----------------
Unrelated changes, please remove.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141714/new/

https://reviews.llvm.org/D141714