[PATCH] D148340: [clang-tidy] Apply cppcoreguidelines-avoid-capture-default-when-capturin-this only to by-value capture default

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp:55
+        AvoidByValueCaptureDefaultWhenCapturingThisCheck>(
+        "cppcoreguidelines-avoid-by-value-capture-default-when-capturing-this");
     CheckFactories.registerCheck<AvoidCapturingLambdaCoroutinesCheck>(
----------------
PiotrZSL wrote:
> carlosgalvezp wrote:
> > PiotrZSL wrote:
> > > carlosgalvezp wrote:
> > > > PiotrZSL wrote:
> > > > > PiotrZSL wrote:
> > > > > > this name is hard to understand
> > > > > > 
> > > > > > I asked ChatGPT about it, and here are some other proposals:
> > > > > > 
> > > > > > - cppcoreguidelines-avoid-by-value-default-this-capture
> > > > > > - cppcoreguidelines-avoid-this-capture-by-value-default
> > > > > > - cppcoreguidelines-explicit-this-capture-by-value
> > > > > > - cppcoreguidelines-implicit-this-capture-by-value
> > > > > > - cppcoreguidelines-implicit-by-value-this-capture
> > > > > > - cppcoreguidelines-prefer-explicit-this-capture
> > > > > > - cppcoreguidelines-avoid-ambiguous-this-capture
> > > > > Probably something like this would be good:
> > > > > cppcoreguidelines-avoid-implicit-this-capture-by-value
> > > > Nice suggestions! I've done a bit of mix of them, let me know if you are happy with it.
> > > To be honest that "default" sounds strange. Simply I check name does not read as an "sentence", more as bunch of words, but I'm fine with this.
> > I agree, I'm not very happy either. We had quite a lot of discussion in the original patch about this. The problem is that the Standard calls it "capture default" which doesn't read like an English sentence (should be "default capture"). But it seems "capture default" is a widely used term out there and in the repo, so that's why we went for that.
> > 
> > What about the following?
> > 
> > - cppcoreguidelines-ambiguous-capture-default-by-value
> > - cppcoreguidelines-ambiguous-by-value-capture-default
> GPT: "Both of these names convey the intended meaning, but the latter option, cppcoreguidelines-ambiguous-capture-default-by-value, is slightly clearer and more readable. It emphasizes that the ambiguity is related to the "capture-default" and its "by-value" aspect. This makes it easier for someone to understand the rule's purpose at a glance."
> 
> ME: "is word ambiguous correct here ?"
> 
> GPT: "The word "ambiguous" may not be the most accurate term to describe the issue in this context. A better term might be "misleading" since the behavior of by-value capture-defaults in member functions can lead to confusion or incorrect assumptions about how data members are captured.
> 
> Thus, a more appropriate name for the guideline could be cppcoreguidelines-misleading-capture-default-by-value. This name highlights that the capture-default by value can be misleading in terms of how data members are captured within member functions."
But probably we can stay with current name, there is not much that we can do.
In future we can always change it to something else...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148340