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

[Giuliano Belinassi via Phabricator via cfe-commits]

giulianobelinassi added a comment.

Hi, Alan. Thanks for your review again!

With regard to middle, the patch sent in D145269 <https://reviews.llvm.org/D145269> may answer your questions. Basically functions like:

  int* f(void) __attribute__((unused));

would be output as

  int* __attribute__((unused)) f(void);

if SIDE_MIDDLE is specified.

The case I want to avoid is:

  int f(void) __attribute__((unused))
    int i;
  { return 0; }

Which is not clear where should the attribute be applied to. A left-to-right parser that accepts attributes on the right side of a function declaration may apply this to f, but if it don't then it will be applied to i. This is why GCC rejects this kind of attribute output on the right side of functions.
Hence, I thought it would be better to always output variable as

  int __attribute__((unused)) var;

when possible. I also found that __declspec attributes is also accepted this way. But if that changes the meaning of things (I looked into generated assembly and could not find such case) then I think dumping it on the right side may be a better option, and we only have to be careful with functions declarations, those being dumped as.

  __attribute__((unused)) f(void) {}

-------------

As for changing the `__declspec` locations, I thought it was fine as it is also accepted by clang. But looking at MSVC documentation it says that outputing it at the begining is recommended so I think it may be better to always output __declspecs on the left side?

-------------

Can you explain a bit more about what warning you're seeing and under what condition?

Basically if you change this test in order for it to intentionally fail because of output mismatch, this warning is generated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141714