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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
