[PATCH] D157394: [clang][DeclPrinter] Improve AST print of function attributes

Wed Aug 9 10:42:36 PDT 2023

aaron.ballman added a comment.

In D157394#4572777 <https://reviews.llvm.org/D157394#4572777>, @giulianobelinassi wrote:

> This patch do not address attributes in variables nor the __declspec case, as D141714 <https://reviews.llvm.org/D141714> do. His patch looks cleaner and I can surely coordinate with @strimo378 to also fix those cases, but I want an answer to the tblgen question first to see if we decide to drop the tblgen solution or not.

Yeah, the two patches are trying to solve the same problem but for different parts of the language.

In terms of the tablegen approach, what you have in D141714 <https://reviews.llvm.org/D141714> is along the same lines of what I was thinking of, though I've not had the chance to give it a thorough review yet. It keeps all of the knowledge about the specifics of the attributes in Attr.td instead of putting special per-attribute logic into the declaration printer, which is what I was mostly concerned about.

================
Comment at: clang/lib/AST/DeclPrinter.cpp:259
+    case attr::Final:
+    case attr::Override:
+      return AttrLocation::AfterDecl;
----------------
giulianobelinassi wrote:
> @aaron.ballman @erichkeane 
> 
> Is this enough to describe the position of the attribute kind, or should it go for a tblgen approach, as I did in D141714? This here looks much cleaner than the tblgen IMHO.
Oofda, I forgot that we model those as keyword attributes. `override` and `final` (and other keyword attributes) are a bit trickier than just "before" or "after" because there's a very particular order in which those can be parsed grammatically, especially for functions: https://godbolt.org/z/ad5qecbrj

Thinking out loud: perhaps an approach to solving this is to sort the attributes by source location. So all the before attributes are written in the same order as they were in source, as are all of the after attributes. So long as we're skipping over implicit attributes, this seems like it should keep the order of the attributes correct.

A different alternative that's possibly more work than it's worth: the decl printer could dispatch to a table-genned function to ask the attribute "do you want to be printed now?" based on what part of the declaration we've already printed. This would require the decl printer to know e.g., "I've finished printing the parameter list but haven't yet printed cv qualifiers" and pass that information along to the attribute; individual attributes with special rules could supply special logic in Attr.td to say "the attribute should be printed after the function parameter list and after the cv and ref qualifiers, but before the override/final keywords" while most attributes can hopefully get away with whatever the default behavior is coming out of tablegen.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157394