ccotter marked an inline comment as done and an inline comment as not done.
ccotter added inline comments.


================
Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp:9
+    auto explicit_this_capture = [=, this]() { };
+    // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture this 
should not specify a capture default 
[cppcoreguidelines-avoid-capture-this-with-capture-default]
+    // CHECK-FIXES: [this]() { };
----------------
ccotter wrote:
> ccotter wrote:
> > ccotter wrote:
> > > carlosgalvezp wrote:
> > > > carlosgalvezp wrote:
> > > > > carlosgalvezp wrote:
> > > > > > "default capture"?
> > > > > I find the check name a bit unintuitive. If you are up for a rename 
> > > > > (you can use `rename_check.py`), I would consider renaming to 
> > > > > something like 
> > > > > `cppcoreguidelines-avoid-default-capture-when-capturing-this`
> > > > > 
> > > > > Like, what should be avoided is not "capturing this", it's using a 
> > > > > default capture.
> > > > > 
> > > > > Would be good to get other reviewers opinion before spending time on 
> > > > > renaming.
> > > > Maybe put it within quotes so clarify it's a C++ keyword? Either 
> > > > backticks `this` or single quotes 'this' would work I think, unless we 
> > > > have some other convention.
> > > I updated all references to capture default to say "capture default" as 
> > > this is how it is spelled in the standard. The CppCoreGuideline for F.54 
> > > does use "default capture" though, so I'll open an issue seeing if that 
> > > wording should instead say "capture default." Also for reference, `git 
> > > grep` within the llvm-project repo shows
> > > 
> > > ```
> > > $ git grep -i 'capture default' | wc -l
> > >       43
> > > $ git grep -i 'default capture' | wc -l
> > >       54
> > > $ git grep -i 'capture.default' | wc -l # E.g., 'capture-default' or 
> > > 'capture default'
> > >      105
> > > ```
> > Updated with a single quote. I didn't find any clang-tidy diagnostics 
> > emitting backticks, but saw usage of single quotes when referring to 
> > identifiers like variable or class names.
> +1 - I admit, I struggled naming this check. More feedback welcome on the name
https://github.com/isocpp/CppCoreGuidelines/pull/2016 for "capture default" vs 
"default capture"


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141133/new/

https://reviews.llvm.org/D141133

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to