cor3ntin added a comment. In D124351#4207501 <https://reviews.llvm.org/D124351#4207501>, @eandrews wrote:
> This patch causes an assertion when the attribute argument is an integer > constant expression - https://godbolt.org/z/osKx5ejMb and has resulted in > test fails downstream since any attribute which uses > `VerifyIntegerConstantExpression` now hits this assert if used with lambdas. > > The assert hit is in `getCurLambda()` : > > auto *CurLSI = dyn_cast<LambdaScopeInfo>(*I); > if (CurLSI && CurLSI->Lambda && CurLSI->CallOperator && > !CurLSI->Lambda->Encloses(CurContext) && CurLSI->AfterParameterList) { > // We have switched contexts due to template instantiation. > assert(!CodeSynthesisContexts.empty()); <----- Hits this > return nullptr; > } > > Prior to this patch, Lambda Scope Information was not populated till after > semantic analysis of attributes. This meant `CurLSI->Lambda` returned nullptr > and we never entered the `if`. This patch however populates LSI during > semantic analysis of lambda expression after introducer, which means we now > enter the `if` during semantic analysis of the attribute` > (`AfterParameterList` will be true at this point since this assert is hit way > past parsing the parameters. `CurContext` is `foo`. I don't know if > `CurContext` is right/wrong without further debugging.) > > For the godbolt test pasted above, neither before nor after this patch do we > hit the code where `CodeSynthesisContexts` is populated. I wanted to see what > code actually enters that block and so I tried deleting the code to see what > tests fails. What is interesting is that nothing does :/ So I don't know if > that means our tests are incomplete or if this code is not required. Thanks :) As far i could tell, this check is only there to catch developer mistakes. We could probably hide it under #ifndef NDEBUG - seems preferable than to remove it entirely - and then make sure AfterParameterList is appropriately set to false when parsing GNU attributes. I might have time to look into it later today if you don't. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124351/new/ https://reviews.llvm.org/D124351 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
