[PATCH] D96975: [Sema] Add some basic lambda capture fix-its

2021-03-11 Thread Nathan James via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGcb559c8d5ebe: [Sema] Add some basic lambda capture fix-its (authored by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96975/new/ https://reviews.

[PATCH] D96975: [Sema] Add some basic lambda capture fix-its

2021-03-08 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 329075. njames93 added a comment. Prevent explicit 'this' capture fix-it if default capture mode is '=' and not in c++20 mode. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96975/new/ https://reviews.llvm.org

[PATCH] D96975: [Sema] Add some basic lambda capture fix-its

2021-03-08 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 329066. njames93 added a comment. Update to not offer Default capture fix if other variables are also captured. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96975/new/ https://reviews.llvm.org/D96975 Files:

[PATCH] D96975: [Sema] Add some basic lambda capture fix-its

2021-03-08 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:17489 + : false; +// We can't use default capture by copy if any captures already specified +// capture by copy. njames93 wrote: > sammccall wrote: >

[PATCH] D96975: [Sema] Add some basic lambda capture fix-its

2021-03-08 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:17444 +if (CXXRecordDecl *RD = T->getAsCXXRecordDecl()) { + if (RD->hasSimpleCopyConstructor()) +return true; sammccall wrote: > I suspect you need to handle the case where R

[PATCH] D96975: [Sema] Add some basic lambda capture fix-its

2021-03-08 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 329011. njames93 marked 8 inline comments as done. njames93 added a comment. Address more comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96975/new/ https://reviews.llvm.org/D96975 Files: clang/incl

[PATCH] D96975: [Sema] Add some basic lambda capture fix-its

2021-03-08 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:17453 +} +return T.isPODType(Sema.getASTContext()); + }(); sammccall wrote: > POD is rarely precisely the right concept (and deprecated for that reason). > Is isTriviallyCopyableType

[PATCH] D96975: [Sema] Add some basic lambda capture fix-its

2021-03-08 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:17457 + SmallString<32> FixBuffer; + StringRef Separator = LSI->NumExplicitCaptures > 0 ? ", " : ""; + if (Var->getDeclName().isIdentifier() && !Var->getName().empty()) { sammccall wrote:

[PATCH] D96975: [Sema] Add some basic lambda capture fix-its

2021-03-08 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Nice fixes! A few drive-by comments from me, up to you though. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7459-7460 "capture-default specified">; + def note_lambda_variable_capture_fixit : Note< +"capture variable %0 by %se

[PATCH] D96975: [Sema] Add some basic lambda capture fix-its

2021-03-08 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:17447 + if (RD->hasUserDeclaredCopyConstructor()) +for (CXXConstructorDecl *Ctor : RD->ctors()) { + if (Ctor->isCopyConstructor()) (braces are not needed here, too) Re

[PATCH] D96975: [Sema] Add some basic lambda capture fix-its

2021-03-08 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 328967. njames93 marked 6 inline comments as done. njames93 added a comment. Address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96975/new/ https://reviews.llvm.org/D96975 Files: clang/include/c

[PATCH] D96975: [Sema] Add some basic lambda capture fix-its

2021-03-08 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev accepted this revision. kbobyrev added a comment. This revision is now accepted and ready to land. LGTM, thanks for implementing this and apologies for the review delay (was a tough week for me & my team). The code is very easy to follow now and the only suggestions are stylistic nits.

[PATCH] D96975: [Sema] Add some basic lambda capture fix-its

2021-03-07 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 328859. njames93 added a comment. Add check to prevent emitting copy capture fixes for non-copyable types Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96975/new/ https://reviews.llvm.org/D96975 Files: clan

[PATCH] D96975: [Sema] Add some basic lambda capture fix-its

2021-03-05 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 328663. njames93 marked 5 inline comments as done. njames93 added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96975/new/ https://reviews.llvm.org/D96975 Files: clang/include/cl

[PATCH] D96975: [Sema] Add some basic lambda capture fix-its

2021-03-05 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7462 + def note_lambda_default_capture_fixit : Note< +"capture all by %select{value|reference}0">; def note_lambda_decl : Note<"lambda expression begins here">; a

[PATCH] D96975: [Sema] Add some basic lambda capture fix-its

2021-03-05 Thread Kirill Bobyrev via Phabricator via cfe-commits
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 ? ", " : "

[PATCH] D96975: [Sema] Add some basic lambda capture fix-its

2021-03-01 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev added a comment. In D96975#2594147 , @njames93 wrote: > Ping Hi Nathan! Thanks again for this patch and sorry for not taking time to review it yet. It is a busy week for me and my team, so I wasn't able to do much coding/reviewing, apologies fo

[PATCH] D96975: [Sema] Add some basic lambda capture fix-its

2021-03-01 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Ping 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-

[PATCH] D96975: [Sema] Add some basic lambda capture fix-its

2021-02-19 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7459-7460 "capture-default specified">; + def note_lambda_variable_capture_fixit : Note< +"capture variable %0 by %select{value|reference}1">; + def note_lambda_default_capture

[PATCH] D96975: [Sema] Add some basic lambda capture fix-its

2021-02-19 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 324981. njames93 added a comment. Restrict availability of default capture fixit based on init captures already specified. `[=, A]() {}` and `[&, &A]() {}` are both invalid as explicit captures can't be specified with the same type as default capture. So w

[PATCH] D96975: [Sema] Add some basic lambda capture fix-its

2021-02-18 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:1229-1235 +static void buildLambdaThisCaptureFixit(Sema &Sema, LambdaScopeInfo *LSI) { + SourceLocation DiagLoc = LSI->IntroducerRange.getEnd(); + assert(!LSI->isCXXThisCaptured()); + Sema.Diag(DiagLoc,

[PATCH] D96975: [Sema] Add some basic lambda capture fix-its

2021-02-18 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 324823. njames93 added a comment. Fix tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96975/new/ https://reviews.llvm.org/D96975 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Sema/

[PATCH] D96975: [Sema] Add some basic lambda capture fix-its

2021-02-18 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 324741. njames93 added a comment. Remove default capture by value fixit if 'this' is explicitly captured pre c++20. Fix default capture insertion loc to be at the start of the capture list. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D96975: [Sema] Add some basic lambda capture fix-its

2021-02-18 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision. njames93 added reviewers: rsmith, sammccall, kbobyrev. Herald added subscribers: usaxena95, kadircet. njames93 requested review of this revision. Herald added subscribers: cfe-commits, ilya-biryukov. Herald added a project: clang. Adds fix-its when users forget to e