malcolm.parsons added inline comments.
Comment at: lib/Sema/SemaLambda.cpp:1567
+if (!CurHasPreviousCapture && !IsLast) {
+ // If there are no captures preceding this capture, remove the
+ // following comma.
In clang-tidy
malcolm.parsons added a comment.
In https://reviews.llvm.org/D48845#1158103, @alexshap wrote:
> I'm kind of interested in this fixit, but one thought which i have - probably
> it should be more conservative (i.e. fix captures by reference, integral
> types, etc) (since the code might rely on
This revision was automatically updated to reflect the committed changes.
Closed by commit rC337148: [Sema] Add fixit for unused lambda captures
(authored by alexshap, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D48845?vs=155616=155624#toc
Repository:
rC Clang
acomminos updated this revision to Diff 155616.
acomminos added a comment.
Remove `const` qualifier for SourceRange.
Repository:
rC Clang
https://reviews.llvm.org/D48845
Files:
include/clang/Sema/DeclSpec.h
include/clang/Sema/ScopeInfo.h
include/clang/Sema/Sema.h
aaron.ballman accepted this revision.
aaron.ballman added a comment.
Aside from a small nit in the comments, LGTM.
Comment at: include/clang/Sema/Sema.h:5608
+ /// diagnostic is emitted.
+ bool DiagnoseUnusedLambdaCapture(const SourceRange CaptureRange,
+
alexshap accepted this revision.
alexshap added a comment.
This revision is now accepted and ready to land.
to me LG
Repository:
rC Clang
https://reviews.llvm.org/D48845
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
acomminos updated this revision to Diff 155472.
acomminos marked an inline comment as done.
acomminos added a comment.
Use source ranges instead of a pair of source locations for explicit lambda
captures.
Repository:
rC Clang
https://reviews.llvm.org/D48845
Files:
acomminos marked an inline comment as done.
acomminos added inline comments.
Comment at: include/clang/Sema/DeclSpec.h:2552-2553
ParsedType InitCaptureType;
+SourceLocation LocStart;
+SourceLocation LocEnd;
+
aaron.ballman wrote:
> aaron.ballman
aaron.ballman added inline comments.
Comment at: include/clang/Sema/DeclSpec.h:2552-2553
ParsedType InitCaptureType;
+SourceLocation LocStart;
+SourceLocation LocEnd;
+
aaron.ballman wrote:
> How does `LocStart` relate to the existing source
acomminos updated this revision to Diff 155446.
acomminos marked 2 inline comments as done.
acomminos added a comment.
Add test for stateful initializer expressions not being removed, propagate
whether or not a diagnostic actually get emitted.
Repository:
rC Clang
acomminos updated this revision to Diff 155430.
acomminos marked 2 inline comments as done.
acomminos added a comment.
Elide braces in single-line conditional.
Repository:
rC Clang
https://reviews.llvm.org/D48845
Files:
include/clang/Sema/DeclSpec.h
include/clang/Sema/ScopeInfo.h
acomminos updated this revision to Diff 155428.
acomminos marked 2 inline comments as done.
acomminos added a comment.
Thanks! Updated to be more explicit about location names, add more tests for
VLA and *this captures, and fix an issue with VLA range captures invalidating
the capture range
aaron.ballman added inline comments.
Comment at: include/clang/Sema/DeclSpec.h:2552-2553
ParsedType InitCaptureType;
+SourceLocation LocStart;
+SourceLocation LocEnd;
+
How does `LocStart` relate to the existing source location `Loc`? I think this
acomminos updated this revision to Diff 155246.
acomminos added a comment.
Thanks for the feedback! This diff switches to using a source range for
captures provided by the parser, which is more accurate, future-proof, and
correctly handles macros.
Repository:
rC Clang
alexshap added a comment.
> Are you talking about a more conservative warning or a more conservative
> fixit? If it doesn't make sense for us to have a fixit for a particular
> capture, does it make sense for us to have a warning for that >capture in the
> first place?
to be honest i'm more
arphaman added a comment.
Thanks for working on this! Please upload the patch with the full context (git
diff -U9). It helps the reviewers :)
In https://reviews.llvm.org/D48845#1158103, @alexshap wrote:
> I'm kind of interested in this fixit, but one thought which i have - probably
> it
alexshap added a comment.
I'm kind of interested in this fixit, but one thought which i have - probably
it should be more conservative (i.e. fix captures by reference, integral types,
etc) (since the code might rely on side-effects of copy-ctors/move-ctors or
extend the lifetime of an object),
alexshap added inline comments.
Comment at: lib/Sema/SemaLambda.cpp:1548
+ // Find the end of the explicit capture for use in fixits.
+ SourceLocation EndLoc;
+ if (From.isThisCapture() && From.isCopyCapture()) {
alexshap wrote:
> maybe these
alexshap added inline comments.
Comment at: lib/Sema/SemaLambda.cpp:1548
+ // Find the end of the explicit capture for use in fixits.
+ SourceLocation EndLoc;
+ if (From.isThisCapture() && From.isCopyCapture()) {
maybe these lines 1548 -1559 can
acomminos updated this revision to Diff 154313.
acomminos added a comment.
Add additional tests to ensure that explicit capture ranges are predicted
correctly.
Repository:
rC Clang
https://reviews.llvm.org/D48845
Files:
include/clang/Sema/Sema.h
lib/Sema/SemaLambda.cpp
acomminos updated this revision to Diff 154307.
acomminos added a comment.
Handle initialization expressions and dereferenced `this` in lambda captures.
An alternative to handling various kinds of explicit captures would be
propagating the source range for each lambda capture from the parser to
acomminos planned changes to this revision.
acomminos added a comment.
Ah yes, thanks for pointing this out. Some additional logic is going to be
necessary to handle capture initializers correctly- I'll look into exposing
full source ranges in LambdaCapture to make this more consistent across
bkramer added inline comments.
Comment at: test/FixIt/fixit-unused-lambda-capture.cpp:31
+ // CHECK: [=,] { return i; };
+}
This needs tests for:
* capture initializers `[c = foo()] {};`
* Capturing this `[this] {};`
* Capturing *this `[*this] {};`
* VLA
acomminos updated this revision to Diff 153968.
acomminos retitled this revision from "[Sema] Add fixit for
-Wno-unused-lambda-capture" to "[Sema] Add fixit for unused lambda captures".
acomminos edited the summary of this revision.
acomminos changed the visibility from "Custom Policy" to "Public
24 matches
Mail list logo
