kbobyrev added a comment. Thanks! The code looks right, my comments are mostly for the comments around to validate my mental model of the code structure and behavior.
================ Comment at: clang/lib/Sema/SemaExpr.cpp:17413 + StringRef Separator = LSI->NumExplicitCaptures > 0 ? ", " : ""; + if (Var->getDeclName().isIdentifier() && !Var->getName().empty()) { + SourceLocation VarInsertLoc = LSI->IntroducerRange.getEnd(); ---------------- Would be useful to have high-level comments for all three cases you have here. This one is for individual variables, right? ================ Comment at: clang/lib/Sema/SemaExpr.cpp:17422 + << Var << /*reference*/ 1 + << FixItHint::CreateInsertion(VarInsertLoc, FixBuffer); + } ---------------- Not having `return;` after this one or the other ones throws me off-guard and I came to the realization that what you intend here is: the function can issue all `FixItHint`s - for capturing a variable by value, reference and default captures by value and reference. So, in total there could be 4 `FixItHint` so that the user could choose between all of those and chose the one they want. Is that correct? I think this is something worth documenting. ================ Comment at: clang/lib/Sema/SemaExpr.cpp:17425 + + // Default capture mode must be specified first. + SourceLocation DefaultInsertLoc = ---------------- This comment seems a bit off to me? Could you please expand? (I also don't understand how this relates to the next line). ================ Comment at: clang/lib/Sema/SemaExpr.cpp:17620 Diag(Var->getLocation(), diag::note_previous_decl) << Var; - if (cast<LambdaScopeInfo>(CSI)->Lambda) + if (cast<LambdaScopeInfo>(CSI)->Lambda) { Diag(cast<LambdaScopeInfo>(CSI)->Lambda->getBeginLoc(), ---------------- now there are 3 `cast<LambdaScopeInfo>`, maybe introduce a variable? would've been much better with C++ init + condition but still Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96975/new/ https://reviews.llvm.org/D96975 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits