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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
