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