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

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 24 08:04:58 PST 2023


aaron.ballman added a comment.

In D141714#4077207 <https://reviews.llvm.org/D141714#4077207>, @giulianobelinassi wrote:

> In D141714#4077204 <https://reviews.llvm.org/D141714#4077204>, @aaron.ballman wrote:
>
>> In D141714#4077199 <https://reviews.llvm.org/D141714#4077199>, @giulianobelinassi wrote:
>>
>>> In D141714#4077150 <https://reviews.llvm.org/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/130589#0185ac99-1158-46b3-b6d1-52fcf5310a59
>>>>
>>>> Can you also add a release note about the fix as well?
>>>
>>> Thank you for your review!
>>>
>>> I will update this patch once I get spare cycles to this. There is also an additonal case regarding __declspec and K&R functions that needs to be addressed as well in this patch which it currently doesn't do.
>>
>> You don't have to worry about the additional cases (unless you want to, but then they can be handled in separate patches); ast pretty printing is wrong, broken, and totally incorrect in a whole lot of places; we maintain it as a best effort as a debugging aid.
>
> That is interesting. I am developing a static analyzer which relies on this for outputing code, so I would need those issues to be fixed for that project to succeed. If you have additional cases already mapped and you want to share with me I will happily fix them as well.

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 was expanded from a macro, etc and so, if the goal is to show what the user wrote, it's likely to be a really big uphill battle to get that through the pretty printer. (The other issue is that we don't actively maintain the pretty printer, so it's quite frequently the case that newer language constructs are not well-supported by the pretty printer unless it just so happens to work by default.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141714



More information about the cfe-commits mailing list