walrus marked an inline comment as done.
walrus added inline comments.
================
Comment at: clang/lib/AST/StmtPrinter.cpp:2011
+ Expr *Init = D->getInit();
+ if (D->getInitStyle() == VarDecl::CallInit && !isa<ParenListExpr>(Init))
+ OS << "(";
----------------
kadircet wrote:
> walrus wrote:
> > kadircet wrote:
> > > what about having a `Pre` and `Post` print blocks, set to `"(" and ")"`
> > > or `" = ` and ""` respectively?
> > >
> > > that way we could get rid of the second if block below.
> > >
> > >
> > > also i don't follow why blacklisting for parenlistexpr is needed here (i
> > > can see that it will end up printing double parens, but so is
> > > `ParenExpr`s, and I think both of them shouldn't be showing up as
> > > initializer exprs of captured variables), could you elaborate with a
> > > comment and a test case?
> > I think you're right that skipping `ParenListExpr` is wrong here. I took
> > this part of code from DeclPrinter. The `ParenListExpr`s are skipped when
> > printing variable declarations, but I think it is not applicable when
> > printing lambda expression captures.
> >
> > Honestly, I didn't get about `Pre` and `Post` blocks. I know it is
> > supported when printing types, but I could not find how to do this for
> > expressions.
> i meant something like below, (modulo naming):
> ```
> llvm::StringRef Pre;
> llvm::StringRef Post;
> if(Style == CallInit) {
> Pre = "(";
> Post = ")";
> }
> else if (Style == CInit)
> Pre = " = ";
>
> OS << Pre;
> PrintExpr(Init):
> OS << Post;
> ```
Thank you for your suggestion - this makes code more clear. Regarding
`ParenListExpr` exception - I investigated this case and found that this is
necessary to prevent double braces when capturing variables of dependent
unresolved types, e.g.:
```
template <typename T>
void foo(T var) {
auto lambda = [m(var)] {};
}
```
Here expression `m(var)` resolves in `ParenListExpr` which will print braces on
its own.
I added a couple of test cases to validate this behavior.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83855/new/
https://reviews.llvm.org/D83855
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
