[clang] Control spacing for attribute printing (PR #174197)

[Kim Gräsman via cfe-commits]

================
@@ -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