Changing __attribute__ AST printing location for GCC compatibility

Hi all,

I’ve noticed that option “-ast-print” creates code that is incompatible with GCC as it prints attributes at the end of function declaration. I submitted a patch at https://reviews.llvm.org/D159362 but then I noticed that this change would affect the project much more than I expected. For example it would change correct function declarations void fun() __attribute__(cold); to a different but still correct form __attribute__(cold) void fun();. It would also mean changing some tests in the project.

I wanted to ask you if that’s a change that should be made or is GCC compatibility not worth messing with existing convention?. Thanks!

Mateusz

1 Like

FWIW, there are two other patches out for changing the location of attributes in -ast-print output:

https://reviews.llvm.org/D141714
https://reviews.llvm.org/D157394

So this is something that we are interested in and may be changing in the near future.

Thanks for reply.

I’m new to the llvm community so I would like to ask you how should I proceed now? I’m worried that changing the printing order could mess with some other elements of the project so maybe I should discuss it somewhere before submitting a working patch?

Or perhaps I should change clang tests to take into account the effects my change and submit the patch?

1 Like

Welcome! :slight_smile:

Because folks are already working on fixing the same issue, I would recommend not pursuing your patch at the moment (we try to avoid multiple patches changing the same functionality at the same time – that’s very hard for anyone to reason about). Instead, I would recommend locally testing the changes from ⚙ D157394 [clang][DeclPrinter] Improve AST print of function attributes with whatever test cases you found important, and if that patch doesn’t address your needs, leave comments on the review discussing what you’d like to see supported. That review should continue to make progress and eventually will land.