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

giulianobelinassi added inline comments. Comment at: clang/lib/AST/DeclPrinter.cpp:269-276 +static bool mustPrintOnLeftSide(attr::Kind kind) { + switch (kind) { +#include "clang/Basic/AttrLeftSideMustPrintList.inc" +return true; + default: +return false; + } --

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

aaron.ballman added a comment. Comment at: clang/lib/AST/DeclPrinter.cpp:269-276 +static bool mustPrintOnLeftSide(attr::Kind kind) { + switch (kind) { +#include "clang/Basic/AttrLeftSideMustPrintList.inc" +return true; + default: +return false; + }

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

erichkeane added inline comments. Comment at: clang/lib/AST/DeclPrinter.cpp:312 + // printing on the left side for readbility. +else if (const VarDecl *VD = dyn_cast(D); + VD && VD->getInit() && giulianobelinassi wrote: > erichk

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG46f3ade5083b: Fix ast print of variables with attributes (authored by giulianobelinassi, committed by erichkeane). Changed prior to commit: https:

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

giulianobelinassi updated this revision to Diff 556197. giulianobelinassi added a comment. Fix `dangling-else` compilation warning. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141714/new/ https://reviews.llvm.org/D141714 Files: clang/include/c

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

giulianobelinassi added inline comments. Comment at: clang/lib/AST/DeclPrinter.cpp:312 + // printing on the left side for readbility. +else if (const VarDecl *VD = dyn_cast(D); + VD && VD->getInit() && erichkeane wrote: > While

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

erichkeane added inline comments. Comment at: clang/lib/AST/DeclPrinter.cpp:312 + // printing on the left side for readbility. +else if (const VarDecl *VD = dyn_cast(D); + VD && VD->getInit() && While compiling this, I discovere

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

erichkeane added a comment. In D141714#4640825 , @giulianobelinassi wrote: > In D141714#4638225 , @erichkeane > wrote: > >> Looks fine to me, please don't commit for a day or two to give >> @aaron.ballman a cha

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

giulianobelinassi added a comment. In D141714#4638225 , @erichkeane wrote: > Looks fine to me, please don't commit for a day or two to give @aaron.ballman > a chance to make a final comment. I am sorry, but how am I supposed to commit those changes to

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

erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. Looks fine to me, please don't commit for a day or two to give @aaron.ballman a chance to make a final comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

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

giulianobelinassi updated this revision to Diff 555642. giulianobelinassi added a comment. Apply @erichkeane suggestions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141714/new/ https://reviews.llvm.org/D141714 Files: clang/include/clang/Basic/

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

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

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

giulianobelinassi added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141714/new/ https://reviews.llvm.org/D141714 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.l

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

strimo378 added a comment. @aaron.ballman I had today a teams meeting with @giulianobelinassi and we discussed both patches as well as that we want to work together in improving AST print. Both patches are fine for us since they improve the attribute printing. The features of the respective oth

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

strimo378 added a comment. @giulianobelinassi I provided a similar patch as you addressed here, see https://reviews.llvm.org/D157394 . It looks like we have the same requirement that we both need compilable ast-print code. Are you interested in a teams meeting to discuss the topic and align our

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

giulianobelinassi added a comment. Hello, It took me a while, but here it is the newer version of the patch with the tablegen stuff. Please reach to me if something needs to be changed in this regard. This also improves the readability of declarations of variables that have a parenthesis cons

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

giulianobelinassi updated this revision to Diff 547477. giulianobelinassi added a comment. Herald added subscribers: wangpc, s.egerton, simoncook, asb. - Use tblgen to generate table of attributes that can or must be print on the left side of the Decl. - Use LLVM_MARK_AS_BITMASK_ENUM on AttrPrint

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

giulianobelinassi marked 4 inline comments as done and 2 inline comments as done. giulianobelinassi added a comment. I will update the patch with the suggestions and send them again. Comment at: clang/lib/AST/DeclPrinter.cpp:270 + + // FIXME: Find a way to use the AttrLis

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

aaron.ballman added inline comments. Comment at: clang/lib/AST/DeclPrinter.cpp:270 + + // FIXME: Find a way to use the AttrList.inc. We use if-else statements + // to classify each of them. erichkeane wrote: > I think this is something we need to just d

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

erichkeane added inline comments. Comment at: clang/lib/AST/DeclPrinter.cpp:270 + + // FIXME: Find a way to use the AttrList.inc. We use if-else statements + // to classify each of them. I think this is something we need to just do the right way, right

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

aaron.ballman added a comment. It looks like several comments are left unaddressed, are you planning on making the suggested changes? Comment at: clang/lib/AST/DeclPrinter.cpp:52-58 +enum AttrPrintLoc { + SIDE_NONE = 0, + SIDE_LEFT = 1, + SIDE_MIDDLE = 2, +

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

giulianobelinassi added a comment. @aaron.ballman Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141714/new/ https://reviews.llvm.org/D141714 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

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

giulianobelinassi updated this revision to Diff 519131. giulianobelinassi added a comment. Incorporate some of Aron suggestions: - Replace isDeclspecAttribute with isStandardAttributeSyntax - Avoid multiple calls to getAsFunction - Use AttrPrintLoc:: instead of referencing the value directly - Al

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

giulianobelinassi added a comment. Hi. See inline answers. I will send the new version in a few minutes. Comment at: clang/lib/AST/DeclPrinter.cpp:52-58 +enum AttrPrintLoc { + SIDE_NONE = 0, + SIDE_LEFT = 1, + SIDE_MIDDLE = 2, + SIDE_RIGHT = 4, + SI

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

aaron.ballman added inline comments. Comment at: clang/lib/AST/DeclPrinter.cpp:52-58 +enum AttrPrintLoc { + SIDE_NONE = 0, + SIDE_LEFT = 1, + SIDE_MIDDLE = 2, + SIDE_RIGHT = 4, + SIDE_ANY = SIDE_LEFT | SIDE_MIDDLE | SIDE_RIGHT, +}; ---

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

giulianobelinassi updated this revision to Diff 514473. giulianobelinassi added a comment. Herald added a reviewer: jdoerfert. Herald added subscribers: jplehr, sstefan1. Update patch with the agreements of last discussion. This new version includes code to handle the cases presented by Aron, wh

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

aaron.ballman added a comment. In D141714#4175942 , @giulianobelinassi wrote: > Hi, Aron. > > Just to make myself clear: What I need to do is that the clang dumps for C > files are also accepted by GCC as input. FWIW, that's not a supported use for `-a

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

giulianobelinassi added a comment. Hi, Aron. Just to make myself clear: What I need to do is that the clang dumps for C files are also accepted by GCC as input. Here is why I wanted to output the attribute on middle: https://godbolt.org/z/6aPc6aWcz As you can see, GCC complains of `__attribut

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

aaron.ballman added a comment. In D141714#4175263 , @giulianobelinassi wrote: > Hi, Alan. Thanks for your review again! > > With regard to middle, the patch sent in D145269 > may answer your questions. Basically > fun

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

giulianobelinassi added a comment. Hi, Alan. Thanks for your review again! With regard to middle, the patch sent in D145269 may answer your questions. Basically functions like: int* f(void) __attribute__((unused)); would be output as int* __attribute__(

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

aaron.ballman added a reviewer: erichkeane. aaron.ballman added inline comments. Comment at: clang/lib/AST/DeclPrinter.cpp:52-58 +enum AttrPrintLoc { + SIDE_NONE = 0, + SIDE_LEFT = 1, + SIDE_MIDDLE = 2, + SIDE_RIGHT = 4, + SIDE_ANY = SIDE_LEFT | SIDE_

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

giulianobelinassi updated this revision to Diff 501835. giulianobelinassi edited the summary of this revision. giulianobelinassi added a comment. Update to dump attributes right after the type specification, also fixing the __declspec case. Repository: rG LLVM Github Monorepo CHANGES SINCE LA

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

aaron.ballman added a comment. In D141714#4077631 , @giulianobelinassi wrote: >> The way Clang handles this is to mostly go back to the original source code >> (through the source manager and source location information) to grab what >> the user actual

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

giulianobelinassi added a comment. > The way Clang handles this is to mostly go back to the original source code > (through the source manager and source location information) to grab what the > user actually wrote. The pretty printing functionality loses information like > whether something

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

aaron.ballman added a comment. In D141714#4077207 , @giulianobelinassi wrote: > In D141714#4077204 , @aaron.ballman > wrote: > >> In D141714#4077199 , >> @giulianobelin

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

giulianobelinassi added a comment. In D141714#4077204 , @aaron.ballman wrote: > In D141714#4077199 , > @giulianobelinassi wrote: > >> In D141714#4077150 , >> @aaron.bal

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

aaron.ballman added a comment. In D141714#4077199 , @giulianobelinassi wrote: > In D141714#4077150 , @aaron.ballman > wrote: > >> Thank you for the fix! >> >> It looks like precommit CI found a related failure t

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

giulianobelinassi added a comment. In D141714#4077150 , @aaron.ballman wrote: > Thank you for the fix! > > It looks like precommit CI found a related failure that needs to be > addressed: > https://buildkite.com/llvm-project/premerge-checks/builds/1305

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

aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added a comment. Thank you for the fix! It looks like precommit CI found a related failure that needs to be addressed: https://buildkite.com/llvm-project/premerge-checks/builds/130589#0185ac99-1158-46b3-b6d1-52fcf5310a59 Can you also

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

giulianobelinassi created this revision. Herald added a project: All. giulianobelinassi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Previously clang AST prints the following declaration: int fun_var_unused() { int x __attribute__((u