giulianobelinassi added inline comments.
================
Comment at: clang/lib/AST/DeclPrinter.cpp:312
+ // printing on the left side for readbility.
+ else if (const VarDecl *VD = dyn_cast<VarDecl>(D);
+ VD && VD->getInit() &&
----------------
erichkeane wrote:
> While compiling this, I discovered that the lack of curley braces on the
> `else if (canPrintOnLeftSide(A))` means that this `else-if` is actually an
> else to THAT instead, despite its indention (which implies it should be
> associated with the `if (const FunctionDecl...`).
>
> Which is it? I presume the indenting is what you mean, but it hasn't been
> tested in a way that matters. Can you add a test that validates the var-decl
> printing on the LHS ONLY happens when it 'can' print on the LHS?
Sorry for the warning there. I will post a fixed version of this as soon as my
build with `-Werror=all` completes.
Just to be clear, the intended code with braces is:
```
AttrPrintLoc AttrLoc = AttrPrintLoc::Right;
if (mustPrintOnLeftSide(A))
// If we must always print on left side (e.g. declspec), then mark as
// so.
AttrLoc = AttrPrintLoc::Left;
else if (canPrintOnLeftSide(A)) {
// For functions with body defined we print the attributes on the left
// side so that GCC accept our dumps as well.
if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D);
FD && FD->isThisDeclarationADefinition())
// In case Decl is a function with a body, then attrs should be print
// on the left side.
AttrLoc = AttrPrintLoc::Left;
// In case it is a variable declaration with a ctor, then allow
// printing on the left side for readbility.
else if (const VarDecl *VD = dyn_cast<VarDecl>(D);
VD && VD->getInit() &&
VD->getInitStyle() == VarDecl::CallInit)
AttrLoc = AttrPrintLoc::Left;
}
// Only print the side matches the user requested.
if ((Loc & AttrLoc) != AttrPrintLoc::None)
A->printPretty(Out, Policy);
```
As you can see, both `if (const FunctionDecl *FD` and `else if (const VarDecl
*VD` are inside the `if (canPrintOnLeftSide())`, and the `AttrLoc` variable is
only set to `AttrPrintLoc::Left` when this function returns `true`. Hence
unless there is a bug in `canPrintOnLeftSide` or `mustPrintOnLeftSide` (which I
don't see any), I can't see a case where PrintOnLeft is false and it prints on
the left side.
Now, I could be wrong on this, but IIRC the brackets on this is optional once
the `else if` would match the preceding else-less if, which in this case would
be `if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D);`, which is what the
indentation meant.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D141714/new/
https://reviews.llvm.org/D141714
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
