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

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp marked 3 inline comments as done.
carlosgalvezp added a comment.

> To be honest for me this check still looks too strict, for example it will warn when we capture this explicitly, and we don't use any class fields, but we use some local variables captured by value and for example call some other class method using that this pointer.

That's a good point, I would agree that in that case it would not be confusing. Maybe you can bring it up for discussion to them? My proposal got accepted :)



================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidValueCaptureDefaultThisCheck.h:17-25
+/// Warns when lambda specify a by-value capture default and capture ``this``.
+/// The check also offers fix-its.
 ///
-/// Capture defaults in lambas defined within member functions can be
+/// By-value capture defaults in lambas defined within member functions can be
 /// misleading about whether capturing data member is by value or reference.
 /// For example, [=] will capture local variables by value but member variables
+/// by reference. CppCoreGuideline F.54 suggests to never use by-value capture
----------------
PiotrZSL wrote:
> thats too much information for an check short description.
> 
Sure, I can fix in a separate NFC patch. I'd like to focus on matching functionality against the updated rules in this patch.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-value-capture-default-this.rst:11
 specifying the capture default ``[=]`` will still capture data members
 by reference.
 
----------------
PiotrZSL wrote:
> Note: I'm missing here explanation why.  (yes I know that this may not be related)
> 
> Maybe:
> "By-value capture-defaults in member functions can be misleading about whether data members are captured by value or reference. This occurs because specifying the capture default [=] actually captures the this pointer by value, not the data members themselves. As a result, data members are still indirectly accessed via the captured this pointer, which essentially means they are being accessed by reference. Therefore, even when using [=], data members are effectively captured by reference, which might not align with the user's expectations."
Good point, I can fix in a separate patch together with your previous comment!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148340