aaron.ballman added a comment. In D119609#3355878 <https://reviews.llvm.org/D119609#3355878>, @junaire wrote:
> Frankly speaking, this is really a randomly written patch, and I have never > thought it would raise so much concern. But I believe I can handle it if you > guys can give me some trust and little guidance! <3 We're happy to help! > In D119609#3355409 <https://reviews.llvm.org/D119609#3355409>, @aaron.ballman > wrote: > >> Thank you for looking into this! I'm not certain this is the correct >> approach, however. The comment on the function is: >> >> /// ActOnCapScopeReturnStmt - Utility routine to type-check return >> statements >> /// for capturing scopes. >> >> however, there's not a capture scope in the test case -- the GNU expression >> statement is within the parameter list, which can't capture anything: >> https://godbolt.org/z/3KrEx8hEW. I think the issue might be whatever causes >> us to call `ActOnCapScopeReturnStmt()` in the first place. > > Yes, you're correct. From the call stack we can see: > > clang::Sema::ActOnCapScopeReturnStmt > clang::Sema::BuildReturnStmt // bad things happened here :( > clang::Sema::ActOnReturnStmt > > clang::Parser::ParseStatementOrDeclarationAfterAttributes > clang::Parser::ParseStatementOrDeclaration > clang::Parser::ParseCompoundStatementBody > clang::Parser::ParseCompoundStatement > clang::Parser::ParseParenExpression > clang::Parser::ParseCastExpression > clang::Parser::ParseCastExpression > clang::Parser::ParseAssignmentExpression > clang::Parser::ParseParameterDeclarationClause > > clang::Parser::ParseLambdaExpressionAfterIntroducer > > When clang is parsing a lambda and hit a `ReturnStmt`, it will do something > like: > > if (isa<CapturingScopeInfo>(getCurFunction())) > return ActOnCapScopeReturnStmt(ReturnLoc, RetValExp, NRInfo, > SupressSimplerImplicitMoves); > > However, in this case, though what `getCurFunction()` returns is a > `CapturingScopeInfo`, we're not finished building the lambda! so clang will > crash when we try to use its `CallOperator`: > > bool HasDeducedReturnType = > CurLambda && hasDeducedReturnType(CurLambda->CallOperator); > > So my original patch is to add an extra check when we decide to jump into > `ActOnCapScopeReturnStmt`, however, I can't pass all the tests. Then I > discovered that `CurLambda` can be `nullptr`, and we still need to call > `ActOnCapScopeReurnStmt`, so I changed my patch. I'm not really sure why this > would happen, maybe I need to dig into the code more. Thanks for the explanation! Looking at `BuildReturnStmt()`, I now see why we're getting into this capturing return statement. >> But there's a design question there as to whether expression statements in a >> lambda default parameter value makes sense *at all*, because the lambda is >> an object that can be passed around to others to call. e.g., >> https://godbolt.org/z/3Edeqv9qv > > Not sure about this, but at least gcc accepts code below: > > void g() { > auto whatever = [=](int foo = ({ return;5; })) {}; > } > > I agree to listen to what gcc folks think about it. I'm not certain they intended this to work as they give some interesting diagnostics before crashing: https://godbolt.org/z/nWTGEc1dW. I added that information to the bug report, but it seems to be gaining steam for rejecting the use of a statement expression in a lambda's default argument. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119609/new/ https://reviews.llvm.org/D119609 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
