[PATCH] D50927: [Sema] Remove location from implicit capture init expr

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith accepted this revision.
rsmith added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/lib/Sema/SemaLambda.cpp:1422-1424
   auto Entity = InitializedEntity::InitializeLambdaCapture(
       Var->getIdentifier(), Field->getType(), Loc);
   InitializationKind InitKind = InitializationKind::CreateDirect(Loc, Loc, Loc);
----------------
vsk wrote:
> rsmith wrote:
> > Should these locations also be updated to `InitLoc`? If we're modeling the initialization as happening at the capture-default, we should do that consistently.
> I've revised the patch to use one location consistently here. The tradeoff is that a few diagnostics now point to CaptureDefaultLoc instead of within the lambda body, but I'm happy to defer to more experienced Sema hands.
Yes, the changed diagnostics are confusing, but the old diagnostics were also not great. I think the right thing to do is to keep the "used here" diagnostic in its current location, and to add a separate note "implicitly capturing local variable 'x' first used here" (or similar) at the point of capture.

The way to do that is to push a `CodeSynthesisContext` for the duration where you're synthesizing code to initialize the lambda capture. (That'll insert a note at the right place in the note stack.) You'll need to add a new value to the `CodeSynthesisContext` enumeration and update `Sema::PrintInstantiationStack` to produce the note.

I'm fine with this being done in a follow-up commit (and I'm approving this on that basis), but I don't think that the new diagnostic is really good enough for the long term (while the old one was bad, the new one no longer gives any clue as to why we're calling a copy constructor or for what).


https://reviews.llvm.org/D50927