================
@@ -124,9 +125,11 @@ namespace {
     void printTemplateArguments(ArrayRef<TemplateArgumentLoc> Args,
                                 const TemplateParameterList *Params);
     enum class AttrPosAsWritten { Default = 0, Left, Right };
-    bool
-    prettyPrintAttributes(const Decl *D,
-                          AttrPosAsWritten Pos = AttrPosAsWritten::Default);
+    AttrVec
+    getPrintableAttributes(const Decl *D,
----------------
kimgr wrote:

> Isn't the prefix suffix determined by the position left/right? 

I don't think so. Left/Right/Default indicate which-side attributes to extract 
and print at a particular position of a decl. Your AttrPosAsWritten design 
brilliantly makes it possible for the printer to print each attribute in the 
same location as they were originally written in the source code.

But they do not necessarily imply anything about enclosing spaces, that all 
depends on the general form of the decl. For example:

* A `FunctionDecl` or `VarDecl` should never have a space before the Left 
attributes, because they are the leftmost component of the decl 
* A `CXXRecordDecl` should always have a space before Left attributes, because 
they can only occur after the tag (`class`)
...

When I've enumerated all decls with attributes, it seemed clear to me that 
there's a lot of variation (including stuff like newlines instead of spaces), 
and each decl does formatting in a way that makes sense for itself. Having the 
spaces spelled out in the "parent" decl printer makes it much more obvious 
where they are necessary or not.

>  Maybe we can have just a single argument needsSpace

The above is also why I don't like such a boolean flag (I tried it); it becomes 
harder to see where the spaces occur with:
```c++
void DeclPrinter::VisitRecordDecl(RecordDecl *D) {
  if (!Policy.SuppressSpecifiers && D->isModulePrivate())
    Out << "__module_private__ ";
  Out << D->getKindName();

  prettyPrintAttributes(D, AttrPosAsWritten::Default, /*needsSpace=*/true);
// ...

void DeclPrinter::VisitCXXRecordDecl(CXXRecordDecl *D) {
  // FIXME: add printing of pragma attributes if required.
  if (!Policy.SuppressSpecifiers && D->isModulePrivate())
    Out << "__module_private__ ";

  Out << D->getKindName() << ' ';

  // FIXME: Move before printing the decl kind to match the behavior of the
  // attribute printing for variables and function where they are printed first.
  if (prettyPrintAttributes(D, AttrPosAsWritten::Left, /*needsSpace=*/false))
    Out << ' ';
// ...
```
vs.
```c++
void DeclPrinter::VisitRecordDecl(RecordDecl *D) {
  if (!Policy.SuppressSpecifiers && D->isModulePrivate())
    Out << "__module_private__ ";
  Out << D->getKindName();

  if (std::string Attrs = prettyPrintAttributes(D); !Attrs.empty()) {
    Out << ' ' << Attrs;
  }
// ...

void DeclPrinter::VisitCXXRecordDecl(CXXRecordDecl *D) {
  // FIXME: add printing of pragma attributes if required.
  if (!Policy.SuppressSpecifiers && D->isModulePrivate())
    Out << "__module_private__ ";

  Out << D->getKindName() << ' ';

  // FIXME: Move before printing the decl kind to match the behavior of the
  // attribute printing for variables and function where they are printed first.
  if (std::string Attrs = prettyPrintAttributes(D, AttrPosAsWritten::Left); 
!Attrs.empty()) {
    Out << Attrs << ' ';
  }
// ...
```

I'll push a commit with the string style on top, I've convinced myself it will 
be nicer than the get/print pair.

https://github.com/llvm/llvm-project/pull/174197
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to