[PATCH] D45463: [AST] Print correct tag decl for tag specifier

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: lib/AST/DeclPrinter.cpp:180-181
     if (isFirst) {
-      if(TD)
-        SubPolicy.IncludeTagDefinition = true;
+      if (TD)
+        SubPolicy.TagSpecifierAs = TD;
       SubPolicy.SuppressSpecifiers = false;
----------------
jdenny wrote:
> rsmith wrote:
> > Instead of the changes in this patch, can you address your use case by just changing the `if` here to `if (TD && TD->isThisDeclarationADefinition())`?
> It sounds like that would fix the case of duplicated definitions.
> 
> It doesn't address the case of dropped or relocated attributes.  For example, -ast-print drops the following attribute and thus the deprecation warning:
> 
> ```
> void fn() {
>   struct T *p0;
>   struct __attribute__((deprecated)) T *p1;
> }
> ```
> 
> I don't know how clang *should* accept and interpret attributes in such cases.  However, it seems wrong for -ast-print to produce a program that clang interprets differently than the original program.
> I don't know how clang *should* accept and interpret attributes in such cases. However, it seems wrong for -ast-print to produce a program that clang interprets differently than the original program.

This has been a *very* long-standing issue with -ast-print that I'd love to see fixed. The AST printer does not print attributes in the correct locations except by accident, and it oftentimes prints with the wrong syntax, or is missing from places where it should be printed.

Long story short, I agree that this behavior is wrong and needs to be fixed, but it's also going to take a bit of work to get it right.


https://reviews.llvm.org/D45463