[clang-tools-extra] [clang-tidy] Add bugprone-move-shared-pointer-contents check. (PR #67467)

2025-01-03 Thread Denis Mikhailov via cfe-commits

denzor200 wrote:

> Wouldn't that be a runtime check, and thus not something clang-tidy can do 
> without significant data flow analysis effort?

I'm not experienced enough in clang-tidy internals, but I believe it will not 
be a serious challenge to check for having `assert(sp.use_count() == 1)` one 
line before `*sp` being moved.

I suppose everybody agree that this snippet
```
assert(sp.use_count() == 1);
auto y = std::move(*sp);
```
..is better than that one:
```
// NOLINTNEXTLINE(clang-tidy-bugprone-move-shared-pointer-contents)
auto y = std::move(*sp);
```



https://github.com/llvm/llvm-project/pull/67467
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add bugprone-move-shared-pointer-contents check. (PR #67467)

2024-12-29 Thread Denis Mikhailov via cfe-commits

denzor200 wrote:

Suppose this check should be silent for a shared_ptr with `use_count() == 1`. 
Is it possible to change implementation in order to following this way?

https://github.com/llvm/llvm-project/pull/67467
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add bugprone-smartptr-reset-ambiguous-call check (PR #121291)

2024-12-30 Thread Denis Mikhailov via cfe-commits


@@ -0,0 +1,34 @@
+//===--- SmartptrResetAmbiguousCallCheck.h - clang-tidy -*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef 
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SMARTPTRRESETAMBIGUOUSCALLCHECK_H
+#define 
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SMARTPTRRESETAMBIGUOUSCALLCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// Finds potentially erroneous calls to 'reset' method on smart pointers when
+/// the pointee type also has a 'reset' method
+///
+/// For the user-facing documentation see:
+/// 
http://clang.llvm.org/extra/clang-tidy/checks/bugprone/smartptr-reset-ambiguous-call.html
+class SmartptrResetAmbiguousCallCheck : public ClangTidyCheck {
+public:
+  SmartptrResetAmbiguousCallCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+return LangOpts.CPlusPlus;

denzor200 wrote:

I disagree, the code issue this check is hitting for also relevant for C++03. 
Think about `boost::shared_ptr`, for example.

https://github.com/llvm/llvm-project/pull/121291
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Added bugprone-exception-rethrow check (PR #86448)

2024-12-21 Thread Denis Mikhailov via cfe-commits


@@ -0,0 +1,103 @@
+// RUN: %check_clang_tidy %s bugprone-exception-rethrow %t -- -- -fexceptions
+
+struct exception {};
+
+namespace std {
+  template 
+  T&& move(T &&x) {
+return static_cast(x);
+  }
+}
+
+void correct() {
+  try {
+  throw exception();
+  } catch(const exception &) {
+  throw;
+  }
+}
+
+void correct2() {
+  try {
+  throw exception();
+  } catch(const exception &e) {
+  try {
+throw exception();
+  } catch(...) {}
+  }
+}
+
+void not_correct() {
+  try {
+  throw exception();
+  } catch(const exception &e) {
+  throw e;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: throwing a copy of the caught 
'exception' exception, remove the argument to throw the original exception 
object [bugprone-exception-rethrow]
+  }
+}
+
+void not_correct2() {
+  try {
+  throw exception();
+  } catch(const exception &e) {
+  throw (e);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: throwing a copy of the caught 
'exception' exception, remove the argument to throw the original exception 
object [bugprone-exception-rethrow]
+  }
+}
+
+void not_correct3() {
+  try {
+  throw exception();
+  } catch(const exception &e) {
+  throw exception(e);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: throwing a copy of the caught 
'exception' exception, remove the argument to throw the original exception 
object [bugprone-exception-rethrow]
+  }
+}
+
+void not_correct4() {
+  try {
+  throw exception();
+  } catch(exception &e) {
+  throw std::move(e);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: throwing a copy of the caught 
'exception' exception, remove the argument to throw the original exception 
object [bugprone-exception-rethrow]
+  }
+}
+
+void not_correct5() {
+  try {
+  throw 5;
+  } catch(const int &e) {
+  throw e;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: throwing a copy of the caught 
'int' exception, remove the argument to throw the original exception object 
[bugprone-exception-rethrow]
+  }
+}
+

denzor200 wrote:

```
void not_correct6() {
  try {
  throw exception();
  } catch(const exception &e) {
  exception e1 = e;
  throw e1;
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: throwing a copy of the caught 
'exception' exception, remove the argument to throw the original exception 
object [bugprone-exception-rethrow]
  }
}
```
please

https://github.com/llvm/llvm-project/pull/86448
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add readability-string-view-substr check (PR #120055)

2024-12-21 Thread Denis Mikhailov via cfe-commits


@@ -0,0 +1,108 @@
+// RUN: %check_clang_tidy %s readability-stringview-substr %t
+
+namespace std {
+template 
+class basic_string_view {
+public:
+  using size_type = unsigned long;
+  static constexpr size_type npos = -1;
+
+  basic_string_view(const char*) {}
+  basic_string_view substr(size_type pos, size_type count = npos) const { 
return *this; }
+  void remove_prefix(size_type n) {}
+  void remove_suffix(size_type n) {}
+  size_type length() const { return 0; }
+  basic_string_view& operator=(const basic_string_view&) { return *this; }
+};
+
+using string_view = basic_string_view;
+} // namespace std
+
+void test_basic() {
+  std::string_view sv("test");
+  std::string_view sv1("test");
+  std::string_view sv2("test");
+
+  // Should match: remove_prefix
+  sv = sv.substr(5);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer 'remove_prefix' over 
'substr' for removing characters from the start [readability-stringview-substr]
+  // CHECK-FIXES: sv.remove_prefix(5)
+
+  // Should match: remove_suffix
+  sv = sv.substr(0, sv.length() - 3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer 'remove_suffix' over 
'substr' for removing characters from the end [readability-stringview-substr]
+  // CHECK-FIXES: sv.remove_suffix(3)
+
+  // Should match: remove_suffix with complex expression
+  sv = sv.substr(0, sv.length() - (3 + 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer 'remove_suffix' over 
'substr' for removing characters from the end [readability-stringview-substr]
+  // CHECK-FIXES: sv.remove_suffix((3 + 2))
+}
+
+void test_copies() {
+  std::string_view sv("test");
+  std::string_view sv1("test");
+  std::string_view sv2("test");
+
+  // Should match: remove redundant self copies
+  sv = sv.substr(0, sv.length());
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: redundant self-copy 
[readability-stringview-substr]
+
+  sv = sv.substr(0, sv.length() - 0);
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: redundant self-copy 
[readability-stringview-substr]
+
+  // Should match: simplify copies between different variables
+  sv1 = sv.substr(0, sv.length());
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer direct copy over substr 
[readability-stringview-substr]
+  // CHECK-FIXES: sv1 = sv
+
+  sv2 = sv.substr(0, sv.length() - 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer direct copy over substr 
[readability-stringview-substr]
+  // CHECK-FIXES: sv2 = sv
+}
+
+void test_zero_forms() {
+  std::string_view sv("test");
+  const int kZero = 0;
+  constexpr std::string_view::size_type Zero = 0;
+  #define START_POS 0
+
+  // Should match: various forms of zero in first argument
+  sv = sv.substr(kZero, sv.length() - 3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer 'remove_suffix' over 
'substr' for removing characters from the end [readability-stringview-substr]
+  // CHECK-FIXES: sv.remove_suffix(3)
+
+  sv = sv.substr(Zero, sv.length() - 3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer 'remove_suffix' over 
'substr' for removing characters from the end [readability-stringview-substr]
+  // CHECK-FIXES: sv.remove_suffix(3)
+
+  sv = sv.substr(START_POS, sv.length() - 3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer 'remove_suffix' over 
'substr' for removing characters from the end [readability-stringview-substr]
+  // CHECK-FIXES: sv.remove_suffix(3)
+
+  sv = sv.substr((0), sv.length() - 3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer 'remove_suffix' over 
'substr' for removing characters from the end [readability-stringview-substr]
+  // CHECK-FIXES: sv.remove_suffix(3)
+
+  sv = sv.substr(0u, sv.length() - 3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer 'remove_suffix' over 
'substr' for removing characters from the end [readability-stringview-substr]
+  // CHECK-FIXES: sv.remove_suffix(3)
+
+  sv = sv.substr(0UL, sv.length() - 3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer 'remove_suffix' over 
'substr' for removing characters from the end [readability-stringview-substr]
+  // CHECK-FIXES: sv.remove_suffix(3)
+}
+
+void test_should_not_match() {
+  std::string_view sv("test");
+  std::string_view sv1("test");
+  std::string_view sv2("test");
+
+  // No match: substr used for prefix or mid-view
+  sv = sv.substr(1, sv.length() - 3); // No warning
+
+  // No match: Different string_views
+  sv = sv2.substr(0, sv2.length() - 3); // No warning

denzor200 wrote:

Why no match? I don't see any trouble with
```
sv = sv2;
sv.remove_suffix(3);
```
Of couse still no match for const version of `sv`

https://github.com/llvm/llvm-project/pull/120055
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add readability-string-view-substr check (PR #120055)

2024-12-21 Thread Denis Mikhailov via cfe-commits


@@ -0,0 +1,108 @@
+// RUN: %check_clang_tidy %s readability-stringview-substr %t
+
+namespace std {
+template 
+class basic_string_view {
+public:
+  using size_type = unsigned long;
+  static constexpr size_type npos = -1;
+
+  basic_string_view(const char*) {}
+  basic_string_view substr(size_type pos, size_type count = npos) const { 
return *this; }
+  void remove_prefix(size_type n) {}
+  void remove_suffix(size_type n) {}
+  size_type length() const { return 0; }

denzor200 wrote:

Please add `size_type size() const { return 0; }` and unit-tests for it

https://github.com/llvm/llvm-project/pull/120055
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add readability-use-span-first-last check (PR #118074)

2024-12-21 Thread Denis Mikhailov via cfe-commits


@@ -0,0 +1,149 @@
+// RUN: %check_clang_tidy -std=c++20 %s readability-use-span-first-last %t
+
+namespace std {
+template 
+class span {
+  T* ptr;
+  __SIZE_TYPE__ len;
+
+public:
+  span(T* p, __SIZE_TYPE__ l) : ptr(p), len(l) {}
+  
+  span subspan(__SIZE_TYPE__ offset) const {
+return span(ptr + offset, len - offset);
+  }
+  
+  span subspan(__SIZE_TYPE__ offset, __SIZE_TYPE__ count) const {
+return span(ptr + offset, count);
+  }
+
+  span first(__SIZE_TYPE__ count) const {
+return span(ptr, count);
+  }
+
+  span last(__SIZE_TYPE__ count) const {
+return span(ptr + (len - count), count);
+  }
+
+  __SIZE_TYPE__ size() const { return len; }
+};
+} // namespace std
+
+// Add here, right after the std namespace closes:
+namespace std::ranges {
+  template
+  __SIZE_TYPE__ size(const span& s) { return s.size(); }
+}
+
+void test() {
+  int arr[] = {1, 2, 3, 4, 5};
+  std::span s(arr, 5);
+
+  auto sub1 = s.subspan(0, 3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: prefer 'span::first()' over 
'subspan()'
+  // CHECK-FIXES: auto sub1 = s.first(3);
+
+  auto sub2 = s.subspan(s.size() - 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: prefer 'span::last()' over 
'subspan()'
+  // CHECK-FIXES: auto sub2 = s.last(2);
+
+  __SIZE_TYPE__ n = 2;
+  auto sub3 = s.subspan(0, n);
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: prefer 'span::first()' over 
'subspan()'
+  // CHECK-FIXES: auto sub3 = s.first(n);
+
+  auto sub4 = s.subspan(1, 2);  // No warning
+  auto sub5 = s.subspan(2); // No warning
+
+
+#define ZERO 0
+#define TWO 2
+#define SIZE_MINUS(s, n) s.size() - n
+#define MAKE_SUBSPAN(obj, n) obj.subspan(0, n)
+#define MAKE_LAST_N(obj, n) obj.subspan(obj.size() - n)
+
+  auto sub6 = s.subspan(SIZE_MINUS(s, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: prefer 'span::last()' over 
'subspan()'
+  // CHECK-FIXES: auto sub6 = s.last(2);
+
+  auto sub7 = MAKE_SUBSPAN(s, 3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: prefer 'span::first()' over 
'subspan()'
+  // CHECK-FIXES: auto sub7 = s.first(3);
+
+  auto sub8 = MAKE_LAST_N(s, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: prefer 'span::last()' over 
'subspan()'
+  // CHECK-FIXES: auto sub8 = s.last(2);
+
+}
+
+template 
+void testTemplate() {
+  T arr[] = {1, 2, 3, 4, 5};
+  std::span s(arr, 5);
+
+  auto sub1 = s.subspan(0, 3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: prefer 'span::first()' over 
'subspan()'
+  // CHECK-FIXES: auto sub1 = s.first(3);
+
+  auto sub2 = s.subspan(s.size() - 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: prefer 'span::last()' over 
'subspan()'
+  // CHECK-FIXES: auto sub2 = s.last(2);
+
+  __SIZE_TYPE__ n = 2;
+  auto sub3 = s.subspan(0, n);
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: prefer 'span::first()' over 
'subspan()'
+  // CHECK-FIXES: auto sub3 = s.first(n);
+
+  auto sub4 = s.subspan(1, 2);  // No warning
+  auto sub5 = s.subspan(2); // No warning
+
+  auto complex = s.subspan(0 + (s.size() - 2), 3);  // No warning
+
+  auto complex2 = s.subspan(100 + (s.size() - 2));  // No warning
+}
+
+// Test instantiation
+void testInt() {
+  testTemplate();
+}
+
+void test_ranges() {
+  int arr[] = {1, 2, 3, 4, 5};
+  std::span s(arr, 5);
+
+  auto sub1 = s.subspan(std::ranges::size(s) - 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: prefer 'span::last()' over 
'subspan()'
+  // CHECK-FIXES: auto sub1 = s.last(2);
+
+  __SIZE_TYPE__ n = 2;
+  auto sub2 = s.subspan(std::ranges::size(s) - n);
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: prefer 'span::last()' over 
'subspan()'
+  // CHECK-FIXES: auto sub2 = s.last(n);
+}
+
+void test_different_spans() {
+  int arr1[] = {1, 2, 3, 4, 5};
+  int arr2[] = {6, 7, 8, 9, 10};
+  std::span s1(arr1, 5);
+  std::span s2(arr2, 5);
+
+  // These should NOT trigger warnings as they use size() from a different span
+  auto sub1 = s1.subspan(s2.size() - 2); // No warning
+  auto sub2 = s2.subspan(s1.size() - 3); // No warning
+  
+  // Also check with std::ranges::size
+  auto sub3 = s1.subspan(std::ranges::size(s2) - 2);  // No warning
+  auto sub4 = s2.subspan(std::ranges::size(s1) - 3);  // No warning
+
+  // Mixed usage should also not trigger
+  auto sub5 = s1.subspan(s2.size() - s1.size());  // No warning
+  
+  // Verify that correct usage still triggers warnings
+  auto good1 = s1.subspan(s1.size() - 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer 'span::last()' over 
'subspan()'
+  // CHECK-FIXES: auto good1 = s1.last(2);
+  
+  auto good2 = s2.subspan(std::ranges::size(s2) - 3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer 'span::last()' over 
'subspan()'
+  // CHECK-FIXES: auto good2 = s2.last(3);
+}

denzor200 wrote:

please add `test_span_of_bytes`:
```
  std::byte arr[] = {0x1, 0x2, 0x3, 0x4, 0x5};
  std::span s(arr, 5);

  // ...

   auto sub2 = s.subspan(s.size_bytes() - 2);
  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: prefer 'span::last()'

[clang-tools-extra] [clang-tidy] Add bugprone-smartptr-reset-ambiguous-call check (PR #121291)

2025-02-21 Thread Denis Mikhailov via cfe-commits


@@ -0,0 +1,130 @@
+//===--- AmbiguousSmartptrResetCallCheck.cpp - clang-tidy 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "AmbiguousSmartptrResetCallCheck.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Lex/Lexer.h"
+#include 
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+namespace {
+
+AST_MATCHER(CXXMethodDecl, hasOnlyDefaultParameters) {
+  for (const auto *Param : Node.parameters()) {
+if (!Param->hasDefaultArg())
+  return false;
+  }
+
+  return true;
+}
+
+const auto DefaultSmartPointers = "::std::shared_ptr;::std::unique_ptr;"
+  "::boost::shared_ptr;::boost::scoped_ptr";

denzor200 wrote:

Since this check is unable to handle the issue with `::boost::scoped_ptr`, here 
I requested another check to get rid of scoped_ptr: 
https://github.com/llvm/llvm-project/issues/128179

https://github.com/llvm/llvm-project/pull/121291
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add bugprone-smartptr-reset-ambiguous-call check (PR #121291)

2025-02-20 Thread Denis Mikhailov via cfe-commits


@@ -0,0 +1,48 @@
+.. title:: clang-tidy - readability-ambiguous-smartptr-reset-call
+
+readability-ambiguous-smartptr-reset-call
+=
+
+Finds potentially erroneous calls to ``reset`` method on smart pointers when
+the pointee type also has a ``reset`` method. Having a ``reset`` method in
+both classes makes it easy to accidentally make the pointer null when
+intending to reset the underlying object.
+
+.. code-block:: c++
+
+  struct Resettable {
+void reset() { /* Own reset logic */ }
+  };
+
+  auto ptr = std::make_unique();
+
+  ptr->reset();  // Calls underlying reset method
+  ptr.reset();   // Makes the pointer null
+
+Both calls are valid C++ code, but the second one might not be what the
+developer intended, as it destroys the pointed-to object rather than resetting
+its state. It's easy to make such a typo because the difference between
+``.`` and ``->`` is really small.
+
+The recommended approach is to make the intent explicit by using either member
+access or direct assignment:
+
+.. code-block:: c++
+
+  std::unique_ptr ptr = std::make_unique();
+
+  (*ptr).reset();  // Clearly calls underlying reset method
+  ptr = nullptr;   // Clearly makes the pointer null
+
+The default smart pointers that are considered are ``std::unique_ptr``,
+``std::shared_ptr``, ``boost::unique_ptr``, ``boost::shared_ptr``. To specify
+other smart pointers or other classes use the :option:`SmartPointers` option.
+
+Options
+---
+
+.. option:: SmartPointers
+
+Semicolon-separated list of fully qualified class names of custom smart
+pointers. Default value is `::std::unique_ptr;::std::shared_ptr;

denzor200 wrote:

> consider adding std::optional here also (even that is not smart ptr)

Strictly disagree with you, optional-like classes are not assignable with 
`nullptr`.
Here is no place for them in this check otherwise it will produce wrong fix 
hints:

```
{
  std::optional o;
  o.reset(); // should produce `o = std::nullopt;` but `o = nullptr;` does
}
{
  boost::optional o;
  o.reset(); // should produce `o = boost::none;` but `o = nullptr;` does
}

```

https://godbolt.org/z/YM79G8zdP


https://github.com/llvm/llvm-project/pull/121291
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add bugprone-smartptr-reset-ambiguous-call check (PR #121291)

2025-02-21 Thread Denis Mikhailov via cfe-commits


@@ -0,0 +1,130 @@
+//===--- AmbiguousSmartptrResetCallCheck.cpp - clang-tidy 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "AmbiguousSmartptrResetCallCheck.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Lex/Lexer.h"
+#include 
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+namespace {
+
+AST_MATCHER(CXXMethodDecl, hasOnlyDefaultParameters) {
+  for (const auto *Param : Node.parameters()) {
+if (!Param->hasDefaultArg())
+  return false;
+  }
+
+  return true;
+}
+
+const auto DefaultSmartPointers = "::std::shared_ptr;::std::unique_ptr;"
+  "::boost::shared_ptr;::boost::scoped_ptr";

denzor200 wrote:

yet another one wrong fix hint, `::boost::scoped_ptr` doesn't have `operator=` 
at all :(
No way to reset this pointer without `reset`, except the crutch like calling 
`swap` method with a reference to default initialized object passed.

https://github.com/llvm/llvm-project/pull/121291
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add bugprone-smartptr-reset-ambiguous-call check (PR #121291)

2025-02-21 Thread Denis Mikhailov via cfe-commits

https://github.com/denzor200 requested changes to this pull request.


https://github.com/llvm/llvm-project/pull/121291
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add bugprone-smartptr-reset-ambiguous-call check (PR #121291)

2025-02-20 Thread Denis Mikhailov via cfe-commits


@@ -0,0 +1,131 @@
+//===--- AmbiguousSmartptrResetCallCheck.cpp - clang-tidy 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "AmbiguousSmartptrResetCallCheck.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Lex/Lexer.h"
+#include 
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::readability {
+
+namespace {
+
+AST_MATCHER(CXXMethodDecl, hasOnlyDefaultParameters) {
+  for (const auto *Param : Node.parameters()) {
+if (!Param->hasDefaultArg())
+  return false;
+  }
+
+  return true;
+}
+
+const auto DefaultSmartPointers =
+"::std::shared_ptr;::std::unique_ptr;::std::optional;"

denzor200 wrote:

remove `std::optional` from this check

https://github.com/llvm/llvm-project/pull/121291
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add bugprone-smartptr-reset-ambiguous-call check (PR #121291)

2025-02-20 Thread Denis Mikhailov via cfe-commits

https://github.com/denzor200 requested changes to this pull request.


https://github.com/llvm/llvm-project/pull/121291
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add bugprone-smartptr-reset-ambiguous-call check (PR #121291)

2025-02-23 Thread Denis Mikhailov via cfe-commits

https://github.com/denzor200 approved this pull request.


https://github.com/llvm/llvm-project/pull/121291
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add new check bugprone-capture-this-by-field (PR #130297)

2025-03-07 Thread Denis Mikhailov via cfe-commits


@@ -0,0 +1,99 @@
+//===--- CaptureThisByFieldCheck.cpp - clang-tidy 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "CaptureThisByFieldCheck.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchersMacros.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER(CXXRecordDecl, correctHandleCaptureThisLambda) {
+  // unresolved
+  if (Node.needsOverloadResolutionForCopyConstructor() &&
+  Node.needsImplicitCopyConstructor())
+return false;
+  if (Node.needsOverloadResolutionForMoveConstructor() &&
+  Node.needsImplicitMoveConstructor())
+return false;
+  if (Node.needsOverloadResolutionForCopyAssignment() &&
+  Node.needsImplicitCopyAssignment())
+return false;
+  if (Node.needsOverloadResolutionForMoveAssignment() &&
+  Node.needsImplicitMoveAssignment())
+return false;
+  // default but not deleted
+  if (Node.hasSimpleCopyConstructor())
+return false;
+  if (Node.hasSimpleMoveConstructor())
+return false;
+  if (Node.hasSimpleCopyAssignment())
+return false;
+  if (Node.hasSimpleMoveAssignment())
+return false;
+
+  for (CXXConstructorDecl const *C : Node.ctors()) {
+if (C->isCopyOrMoveConstructor() && C->isDefaulted() && !C->isDeleted())
+  return false;
+  }
+  for (CXXMethodDecl const *M : Node.methods()) {
+if (M->isCopyAssignmentOperator() && M->isDefaulted() && !M->isDeleted())
+  return false;
+if (M->isMoveAssignmentOperator() && M->isDefaulted() && !M->isDeleted())
+  return false;
+  }
+  // FIXME: find ways to identifier correct handle capture this lambda
+  return true;
+}
+
+} // namespace
+
+void CaptureThisByFieldCheck::registerMatchers(MatchFinder *Finder) {
+  auto IsStdFunctionField =
+  fieldDecl(hasType(cxxRecordDecl(hasName("::std::function"

denzor200 wrote:

Needs an option to add a custom function class, `boost::function` for example

https://github.com/llvm/llvm-project/pull/130297
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add new check bugprone-capture-this-by-field (PR #130297)

2025-03-07 Thread Denis Mikhailov via cfe-commits


@@ -118,6 +119,8 @@ class BugproneModule : public ClangTidyModule {
 CheckFactories.registerCheck(
 "bugprone-bool-pointer-implicit-conversion");
 CheckFactories.registerCheck("bugprone-branch-clone");
+CheckFactories.registerCheck(
+"bugprone-capture-this-by-field");

denzor200 wrote:

`bugprone-capturing-this-by-field` ?

https://github.com/llvm/llvm-project/pull/130297
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add new check bugprone-capture-this-by-field (PR #130297)

2025-03-07 Thread Denis Mikhailov via cfe-commits

https://github.com/denzor200 requested changes to this pull request.


https://github.com/llvm/llvm-project/pull/130297
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add new check bugprone-capture-this-by-field (PR #130297)

2025-03-12 Thread Denis Mikhailov via cfe-commits


@@ -0,0 +1,144 @@
+// RUN: %check_clang_tidy -std=c++11-or-later %s 
bugprone-capturing-this-by-field %t -- -config="{CheckOptions: 
{bugprone-capturing-this-by-field.FunctionWrapperTypes: 
'::std::function;::Fn'}}" --
+
+namespace std {
+
+template
+class function;
+
+template
+class function {
+public:
+  function() noexcept;
+  template function(F &&);
+};
+
+} // namespace std
+
+struct Fn {
+  template Fn(F &&);
+};
+
+struct Basic {
+  Basic() : Captured([this]() { static_cast(this); }) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: using lambda expressions to 
capture 'this' and storing it in class member
+  std::function Captured;
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that 
stores captured 'this'
+};
+
+struct AssignCapture {
+  AssignCapture() : Captured([Self = this]() { static_cast(Self); }) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: using lambda expressions to 
capture 'this' and storing it in class member
+  std::function Captured;
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that 
stores captured 'this'
+};
+
+struct DeleteMoveAndCopy {
+  DeleteMoveAndCopy() : Captured([this]() { static_cast(this); }) {}
+  DeleteMoveAndCopy(DeleteMoveAndCopy const&) = delete;
+  DeleteMoveAndCopy(DeleteMoveAndCopy &&) = delete;
+  DeleteMoveAndCopy& operator=(DeleteMoveAndCopy const&) = delete;
+  DeleteMoveAndCopy& operator=(DeleteMoveAndCopy &&) = delete;
+  std::function Captured;
+};
+
+struct DeleteCopyImplicitDisabledMove {
+  DeleteCopyImplicitDisabledMove() : Captured([this]() { 
static_cast(this); }) {}
+  DeleteCopyImplicitDisabledMove(DeleteCopyImplicitDisabledMove const&) = 
delete;
+  DeleteCopyImplicitDisabledMove& operator=(DeleteCopyImplicitDisabledMove 
const&) = delete;

denzor200 wrote:

We don't really need to add boost headers at all, since noncopyable is simplest 
and tidy to implement by ourself:
```
namespace boost {
  class noncopyable
  {
  protected:
  noncopyable() {}
  ~noncopyable() {}
  private:
  noncopyable( const noncopyable& );
  noncopyable& operator=( const noncopyable& );
  };
}
``` 

https://github.com/llvm/llvm-project/pull/130297
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add new check bugprone-capture-this-by-field (PR #130297)

2025-03-12 Thread Denis Mikhailov via cfe-commits


@@ -0,0 +1,44 @@
+//===--- CapturingThisByFieldCheck.h - clang-tidy ---*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CAPTURINGTHISBYFIELDCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CAPTURINGTHISBYFIELDCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "clang/AST/ASTTypeTraits.h"
+#include 
+
+namespace clang::tidy::bugprone {
+
+/// Finds lambda captures that capture the ``this`` pointer and store it as
+/// class members without handle the copy and move constructors and the
+/// assignments.
+///
+/// For the user-facing documentation see:
+/// 
http://clang.llvm.org/extra/clang-tidy/checks/bugprone/capturing-this-by-field.html
+class CapturingThisByFieldCheck : public ClangTidyCheck {
+public:
+  CapturingThisByFieldCheck(StringRef Name, ClangTidyContext *Context);
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+return LangOpts.CPlusPlus11;

denzor200 wrote:

Since this check doesn't cove binds, let's stay it as it is.

https://github.com/llvm/llvm-project/pull/130297
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add new check bugprone-capture-this-by-field (PR #130297)

2025-03-12 Thread Denis Mikhailov via cfe-commits


@@ -0,0 +1,144 @@
+// RUN: %check_clang_tidy -std=c++11-or-later %s 
bugprone-capturing-this-by-field %t -- -config="{CheckOptions: 
{bugprone-capturing-this-by-field.FunctionWrapperTypes: 
'::std::function;::Fn'}}" --
+
+namespace std {
+
+template
+class function;
+
+template
+class function {
+public:
+  function() noexcept;
+  template function(F &&);
+};
+
+} // namespace std
+
+struct Fn {
+  template Fn(F &&);
+};
+
+struct Basic {
+  Basic() : Captured([this]() { static_cast(this); }) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: using lambda expressions to 
capture 'this' and storing it in class member
+  std::function Captured;
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that 
stores captured 'this'
+};
+
+struct AssignCapture {
+  AssignCapture() : Captured([Self = this]() { static_cast(Self); }) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: using lambda expressions to 
capture 'this' and storing it in class member
+  std::function Captured;
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: note: 'std::function' that 
stores captured 'this'
+};
+
+struct DeleteMoveAndCopy {
+  DeleteMoveAndCopy() : Captured([this]() { static_cast(this); }) {}
+  DeleteMoveAndCopy(DeleteMoveAndCopy const&) = delete;
+  DeleteMoveAndCopy(DeleteMoveAndCopy &&) = delete;
+  DeleteMoveAndCopy& operator=(DeleteMoveAndCopy const&) = delete;
+  DeleteMoveAndCopy& operator=(DeleteMoveAndCopy &&) = delete;
+  std::function Captured;
+};
+
+struct DeleteCopyImplicitDisabledMove {
+  DeleteCopyImplicitDisabledMove() : Captured([this]() { 
static_cast(this); }) {}
+  DeleteCopyImplicitDisabledMove(DeleteCopyImplicitDisabledMove const&) = 
delete;
+  DeleteCopyImplicitDisabledMove& operator=(DeleteCopyImplicitDisabledMove 
const&) = delete;

denzor200 wrote:

That was C++03 way to implement `boost::noncopyable`, I think we should provide 
another test(possible in separate .cpp) with C++11 version of 
`boost::noncopyable`:
```
namespace boost {
  class noncopyable
  {
  protected:
  constexpr noncopyable() = default;
  ~noncopyable() = default;
  noncopyable( const noncopyable& ) = delete;
  noncopyable& operator=( const noncopyable& ) = delete;
  };
}
```

https://github.com/llvm/llvm-project/pull/130297
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add new check bugprone-capture-this-by-field (PR #130297)

2025-03-12 Thread Denis Mikhailov via cfe-commits

https://github.com/denzor200 edited 
https://github.com/llvm/llvm-project/pull/130297
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add new check bugprone-capture-this-by-field (PR #130297)

2025-03-13 Thread Denis Mikhailov via cfe-commits


@@ -0,0 +1,144 @@
+// RUN: %check_clang_tidy -std=c++11-or-later %s 
bugprone-capturing-this-by-field %t -- -config="{CheckOptions: 
{bugprone-capturing-this-by-field.FunctionWrapperTypes: 
'::std::function;::Fn'}}" --

denzor200 wrote:

Created issues about bind and Boost's lambdas:
https://github.com/llvm/llvm-project/issues/131220
https://github.com/llvm/llvm-project/issues/131231

https://github.com/llvm/llvm-project/pull/130297
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add new check bugprone-capture-this-by-field (PR #130297)

2025-03-12 Thread Denis Mikhailov via cfe-commits

https://github.com/denzor200 edited 
https://github.com/llvm/llvm-project/pull/130297
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add new check bugprone-capture-this-by-field (PR #130297)

2025-03-12 Thread Denis Mikhailov via cfe-commits


@@ -0,0 +1,44 @@
+//===--- CapturingThisByFieldCheck.h - clang-tidy ---*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CAPTURINGTHISBYFIELDCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CAPTURINGTHISBYFIELDCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "clang/AST/ASTTypeTraits.h"
+#include 
+
+namespace clang::tidy::bugprone {
+
+/// Finds lambda captures that capture the ``this`` pointer and store it as
+/// class members without handle the copy and move constructors and the
+/// assignments.
+///
+/// For the user-facing documentation see:
+/// 
http://clang.llvm.org/extra/clang-tidy/checks/bugprone/capturing-this-by-field.html
+class CapturingThisByFieldCheck : public ClangTidyCheck {
+public:
+  CapturingThisByFieldCheck(StringRef Name, ClangTidyContext *Context);
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+return LangOpts.CPlusPlus11;

denzor200 wrote:

Agree about lambda, but in C++03 mode we could use bind instead of lambda to 
reproduce the same problem.

https://github.com/llvm/llvm-project/pull/130297
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add new check bugprone-capture-this-by-field (PR #130297)

2025-03-12 Thread Denis Mikhailov via cfe-commits


@@ -0,0 +1,144 @@
+// RUN: %check_clang_tidy -std=c++11-or-later %s 
bugprone-capturing-this-by-field %t -- -config="{CheckOptions: 
{bugprone-capturing-this-by-field.FunctionWrapperTypes: 
'::std::function;::Fn'}}" --

denzor200 wrote:

Oke, don't worry about bind and Boost's lambdas. I will create issues about all 
of them later.

https://github.com/llvm/llvm-project/pull/130297
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add new check bugprone-capture-this-by-field (PR #130297)

2025-03-15 Thread Denis Mikhailov via cfe-commits

denzor200 wrote:

LGTM, please provide the change for one comment I left

https://github.com/llvm/llvm-project/pull/130297
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add new check bugprone-capture-this-by-field (PR #130297)

2025-03-15 Thread Denis Mikhailov via cfe-commits

https://github.com/denzor200 approved this pull request.


https://github.com/llvm/llvm-project/pull/130297
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add new check bugprone-capture-this-by-field (PR #130297)

2025-03-15 Thread Denis Mikhailov via cfe-commits


@@ -0,0 +1,36 @@
+.. title:: clang-tidy - bugprone-capturing-this-in-member-variable
+
+bugprone-capturing-this-in-member-variable
+==
+
+Finds lambda captures that capture the ``this`` pointer and store it as class
+members without handle the copy and move constructors and the assignments.
+
+Capture this in a lambda and store it as a class member is dangerous because 
the
+lambda can outlive the object it captures. Especially when the object is copied
+or moved, the captured ``this`` pointer will be implicitly propagated to the
+new object. Most of the time, people will believe that the captured ``this``
+pointer points to the new object, which will lead to bugs.
+
+.. code-block:: c++
+
+  struct C {
+C() : Captured([this]() -> C const * { return this; }) {}
+std::function Captured;
+  };
+
+  void foo() {
+C v1{};
+C v2 = v1; // v2.Captured capture v1's 'this' pointer
+assert(v2.Captured() == v1.Captured()); // v2.Captured capture v1's 'this' 
pointer
+assert(v2.Captured() == &v2); // assertion failed.
+  }
+
+Possible fixes include refactoring the function object into a class member

denzor200 wrote:

And the rest of the users who really want the class to be copyable will 
refactor the code in the way you suggested.

https://github.com/llvm/llvm-project/pull/130297
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Add new check bugprone-capture-this-by-field (PR #130297)

2025-03-15 Thread Denis Mikhailov via cfe-commits


@@ -0,0 +1,36 @@
+.. title:: clang-tidy - bugprone-capturing-this-in-member-variable
+
+bugprone-capturing-this-in-member-variable
+==
+
+Finds lambda captures that capture the ``this`` pointer and store it as class
+members without handle the copy and move constructors and the assignments.
+
+Capture this in a lambda and store it as a class member is dangerous because 
the
+lambda can outlive the object it captures. Especially when the object is copied
+or moved, the captured ``this`` pointer will be implicitly propagated to the
+new object. Most of the time, people will believe that the captured ``this``
+pointer points to the new object, which will lead to bugs.
+
+.. code-block:: c++
+
+  struct C {
+C() : Captured([this]() -> C const * { return this; }) {}
+std::function Captured;
+  };
+
+  void foo() {
+C v1{};
+C v2 = v1; // v2.Captured capture v1's 'this' pointer
+assert(v2.Captured() == v1.Captured()); // v2.Captured capture v1's 'this' 
pointer
+assert(v2.Captured() == &v2); // assertion failed.
+  }
+
+Possible fixes include refactoring the function object into a class member

denzor200 wrote:

The first possible fix is ​​to make the class completely uncopyable. In 90% of 
cases, the real reason for this warning is that someone forgot to disallow 
copying.

https://github.com/llvm/llvm-project/pull/130297
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang-tidy] Improve `bugprone-capturing-this-in-member-variable` check: add support of `bind` functions. (PR #132635)

2025-03-24 Thread Denis Mikhailov via cfe-commits


@@ -64,16 +64,21 @@ AST_MATCHER(CXXRecordDecl, correctHandleCaptureThisLambda) {
 
 constexpr const char *DefaultFunctionWrapperTypes =
 "::std::function;::std::move_only_function;::boost::function";
+constexpr const char *DefaultBindFunctions = "::std::bind;::boost::bind";

denzor200 wrote:

Please add `::std::bind_front` and `::std::bind_back`: 
https://en.cppreference.com/w/cpp/utility/functional/bind_front
There are also Boost analogue for them - `::boost::compat::bind_front` and 
`::boost::compat::bind_back`:
https://www.boost.org/doc/libs/1_86_0/libs/compat/doc/html/compat.html#bind_front
https://www.boost.org/doc/libs/1_86_0/libs/compat/doc/html/compat.html#bind_back

https://github.com/llvm/llvm-project/pull/132635
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits