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

2025-03-11 Thread Piotr Zegar via cfe-commits

PiotrZSL wrote:

@vbvictor If you don't want this to be merged as 
"70346889+vbvic...@users.noreply.github.com.", please change your email privacy 
settings.

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-03-11 Thread via cfe-commits

EugeneZelenko wrote:

> @HerrCai0907, could you please address [#121291 
> (comment)](https://github.com/llvm/llvm-project/pull/121291#issuecomment-2702153354).
>  Thank you! Can I ask someone to grant me write access to repository? Since 
> you basically the only one who merges PRs of other people, this could lower 
> amount of work you need to do. Also, I'd be happy to help new contributors in 
> merging their PRs.

If you have at least three merged pull requests, you could request commit 
access using [other 
ones](https://github.com/llvm/llvm-project/issues/?q=is%3Aissue%20state%3Aopen%20%20label%3Ainfra%3Acommit-access-request)
 as 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] Add bugprone-smartptr-reset-ambiguous-call check (PR #121291)

2025-03-11 Thread Piotr Zegar via cfe-commits

https://github.com/PiotrZSL edited 
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-03-10 Thread via cfe-commits

github-actions[bot] wrote:



@vbvictor Congratulations on having your first Pull Request (PR) merged into 
the LLVM Project!

Your changes will be combined with recent changes from other authors, then 
tested by our [build bots](https://lab.llvm.org/buildbot/). If there is a 
problem with a build, you may receive a report in an email or a comment on this 
PR.

Please check whether problems have been caused by your change specifically, as 
the builds can include changes from many authors. It is not uncommon for your 
change to be included in a build that fails due to someone else's changes, or 
infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail 
[here](https://llvm.org/docs/MyFirstTypoFix.html#myfirsttypofix-issues-after-landing-your-pr).

If your change does cause a problem, it may be reverted, or you can revert it 
yourself. This is a normal part of [LLVM 
development](https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy).
 You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are 
working as expected, well done!


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-03-10 Thread Piotr Zegar via cfe-commits

https://github.com/PiotrZSL closed 
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-03-09 Thread Baranov Victor via cfe-commits

vbvictor wrote:

Ping @HerrCai0907, could you please address 
https://github.com/llvm/llvm-project/pull/121291#issuecomment-2702153354. Thank 
you!
Can I ask someone to grant me write access to repository?
Since you basically the only one who merges PRs of other people, this could 
lower amount of work you need to do.
Also, I'd be happy to help new contributors in merging their PRs. 

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-03-05 Thread Baranov Victor via cfe-commits

vbvictor wrote:

Since all approved are made, can we land this PR?
@HerrCai0907

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-03-01 Thread Baranov Victor via cfe-commits

https://github.com/vbvictor updated 
https://github.com/llvm/llvm-project/pull/121291

>From 099aacc263f70301efefa722d49874a47b0054a9 Mon Sep 17 00:00:00 2001
From: Victor Baranov 
Date: Sat, 1 Mar 2025 18:05:54 +0300
Subject: [PATCH] add ambigous-smartptr-reset-call check

---
 .../AmbiguousSmartptrResetCallCheck.cpp   | 126 ++
 .../AmbiguousSmartptrResetCallCheck.h |  37 ++
 .../clang-tidy/readability/CMakeLists.txt |   1 +
 .../readability/ReadabilityTidyModule.cpp |   3 +
 clang-tools-extra/docs/ReleaseNotes.rst   |   6 +
 .../docs/clang-tidy/checks/list.rst   |   1 +
 .../ambiguous-smartptr-reset-call.rst |  62 +++
 ...us-smartptr-reset-call-custom-pointers.cpp |  52 +++
 .../ambiguous-smartptr-reset-call.cpp | 397 ++
 9 files changed, 685 insertions(+)
 create mode 100644 
clang-tools-extra/clang-tidy/readability/AmbiguousSmartptrResetCallCheck.cpp
 create mode 100644 
clang-tools-extra/clang-tidy/readability/AmbiguousSmartptrResetCallCheck.h
 create mode 100644 
clang-tools-extra/docs/clang-tidy/checks/readability/ambiguous-smartptr-reset-call.rst
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/readability/ambiguous-smartptr-reset-call-custom-pointers.cpp
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/readability/ambiguous-smartptr-reset-call.cpp

diff --git 
a/clang-tools-extra/clang-tidy/readability/AmbiguousSmartptrResetCallCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/AmbiguousSmartptrResetCallCheck.cpp
new file mode 100644
index 0..5f36c3976fc69
--- /dev/null
+++ 
b/clang-tools-extra/clang-tidy/readability/AmbiguousSmartptrResetCallCheck.cpp
@@ -0,0 +1,126 @@
+//===--- 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"
+
+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";
+} // namespace
+
+AmbiguousSmartptrResetCallCheck::AmbiguousSmartptrResetCallCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  SmartPointers(utils::options::parseStringList(
+  Options.get("SmartPointers", DefaultSmartPointers))) {}
+
+void AmbiguousSmartptrResetCallCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "SmartPointers",
+utils::options::serializeStringList(SmartPointers));
+}
+
+void AmbiguousSmartptrResetCallCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IsSmartptr = hasAnyName(SmartPointers);
+
+  const auto ResetMethod =
+  cxxMethodDecl(hasName("reset"), hasOnlyDefaultParameters());
+
+  const auto TypeWithReset =
+  anyOf(cxxRecordDecl(
+anyOf(hasMethod(ResetMethod),
+  isDerivedFrom(cxxRecordDecl(hasMethod(ResetMethod),
+classTemplateSpecializationDecl(
+hasSpecializedTemplate(classTemplateDecl(has(ResetMethod);
+
+  const auto SmartptrWithReset = expr(hasType(hasUnqualifiedDesugaredType(
+  recordType(hasDeclaration(classTemplateSpecializationDecl(
+  IsSmartptr,
+  hasTemplateArgument(
+  0, templateArgument(refersToType(hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(TypeWithReset;
+
+  Finder->addMatcher(
+  cxxMemberCallExpr(
+  callee(ResetMethod),
+  unless(hasAnyArgument(expr(unless(cxxDefaultArgExpr(),
+  anyOf(on(cxxOperatorCallExpr(hasOverloadedOperatorName("->"),
+   hasArgument(0, SmartptrWithReset))
+   .bind("ArrowOp")),
+on(SmartptrWithReset)))
+  .bind("MemberCall"),
+  this);
+}
+
+void AmbiguousSmartptrResetCallCheck::check(
+const MatchFinder::MatchResult &Result) {
+  const auto *MemberCall =
+  Result.Nodes.getNodeAs("MemberCall");
+  assert(MemberCall);
+
+  if (const auto *Arrow =
+  Result.Nodes.getNodeAs("ArrowOp")) {
+const CharSourceRange SmartptrSourceRange =
+Lexer::getAsCharRange(Arrow->getArg(0)->getSourceRange(),
+   

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

2025-02-26 Thread Baranov Victor via cfe-commits

https://github.com/vbvictor edited 
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-26 Thread Baranov Victor via cfe-commits


@@ -0,0 +1,129 @@
+//===--- 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"
+
+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";
+} // namespace
+
+AmbiguousSmartptrResetCallCheck::AmbiguousSmartptrResetCallCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  SmartPointers(utils::options::parseStringList(
+  Options.get("SmartPointers", DefaultSmartPointers))) {}
+
+void AmbiguousSmartptrResetCallCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "SmartPointers",
+utils::options::serializeStringList(SmartPointers));
+}
+
+void AmbiguousSmartptrResetCallCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IsSmartptr = hasAnyName(SmartPointers);
+
+  const auto ResetMethod =
+  cxxMethodDecl(hasName("reset"), hasOnlyDefaultParameters());
+
+  const auto TypeWithReset =
+  anyOf(cxxRecordDecl(hasMethod(ResetMethod)),
+classTemplateSpecializationDecl(
+hasSpecializedTemplate(classTemplateDecl(has(ResetMethod);
+
+  const auto SmartptrWithReset = expr(hasType(hasUnqualifiedDesugaredType(
+  recordType(hasDeclaration(classTemplateSpecializationDecl(
+  IsSmartptr,
+  hasTemplateArgument(
+  0, templateArgument(refersToType(hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(TypeWithReset;
+
+  Finder->addMatcher(
+  cxxMemberCallExpr(
+  callee(ResetMethod),
+  unless(hasAnyArgument(expr(unless(cxxDefaultArgExpr(),
+  anyOf(on(SmartptrWithReset),
+on(cxxOperatorCallExpr(hasOverloadedOperatorName("->"),
+   hasArgument(0, SmartptrWithReset))
+   .bind("ArrowOp"
+  .bind("MemberCall"),
+  this);
+}
+
+void AmbiguousSmartptrResetCallCheck::check(
+const MatchFinder::MatchResult &Result) {
+
+  if (const auto *Arrow =
+  Result.Nodes.getNodeAs("ArrowOp")) {
+const auto *ObjectResetCall =
+Result.Nodes.getNodeAs("MemberCall");
+
+const CharSourceRange SmartptrSourceRange =
+Lexer::getAsCharRange(Arrow->getArg(0)->getSourceRange(),

vbvictor wrote:

Binding to variable caused test-failures, and since it isn't a "hot path" I'd 
prefer to leave as is.

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-26 Thread Baranov Victor via cfe-commits


@@ -0,0 +1,68 @@
+// RUN: %check_clang_tidy %s readability-ambiguous-smartptr-reset-call %t \

vbvictor wrote:

I'd prefer to leave two separate files, they are easier to read and maintain 
imho.

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 Baranov Victor via cfe-commits

https://github.com/vbvictor edited 
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 Baranov Victor via cfe-commits


@@ -0,0 +1,57 @@
+.. 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 and classes that are considered are
+``std::unique_ptr``, ``std::shared_ptr``, ``boost::shared_ptr``. To specify
+other smart pointers or other classes use the :option:`SmartPointers` option.
+
+
+.. note::
+
+  The check may emit invalid fix-its and misleading warning messages when
+  specifying custom smart pointers or other classes in the
+  :option:`SmartPointers` option. For example, ``boost::scoped_ptr`` does not
+  have an ``operator=`` which makes fix-its invalid.
+
+
+Options
+---
+
+.. option:: SmartPointers
+
+Semicolon-separated list of fully qualified class names of custom smart
+pointers. Default value is `::std::unique_ptr;::std::shared_ptr;
+::boost::shared_ptr`.

vbvictor wrote:

As I said in 
https://github.com/llvm/llvm-project/pull/121291#issuecomment-2676086944, I 
think it's better to provide default pointers whose fixes will be 100% correct 
(hopefully).
You think we should put values that provide incorrect fix-its sometimes?

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 Piotr Zegar via cfe-commits


@@ -0,0 +1,57 @@
+.. 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 and classes that are considered are
+``std::unique_ptr``, ``std::shared_ptr``, ``boost::shared_ptr``. To specify
+other smart pointers or other classes use the :option:`SmartPointers` option.
+
+
+.. note::
+
+  The check may emit invalid fix-its and misleading warning messages when
+  specifying custom smart pointers or other classes in the
+  :option:`SmartPointers` option. For example, ``boost::scoped_ptr`` does not
+  have an ``operator=`` which makes fix-its invalid.
+
+
+Options
+---
+
+.. option:: SmartPointers
+
+Semicolon-separated list of fully qualified class names of custom smart
+pointers. Default value is `::std::unique_ptr;::std::shared_ptr;
+::boost::shared_ptr`.

PiotrZSL wrote:

That's not about fixes, it is about default settings for configuration option.

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 Baranov Victor via cfe-commits


@@ -0,0 +1,57 @@
+.. 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 and classes that are considered are
+``std::unique_ptr``, ``std::shared_ptr``, ``boost::shared_ptr``. To specify
+other smart pointers or other classes use the :option:`SmartPointers` option.
+
+
+.. note::
+
+  The check may emit invalid fix-its and misleading warning messages when
+  specifying custom smart pointers or other classes in the
+  :option:`SmartPointers` option. For example, ``boost::scoped_ptr`` does not
+  have an ``operator=`` which makes fix-its invalid.
+
+
+Options
+---
+
+.. option:: SmartPointers
+
+Semicolon-separated list of fully qualified class names of custom smart
+pointers. Default value is `::std::unique_ptr;::std::shared_ptr;
+::boost::shared_ptr`.

vbvictor wrote:

I think here my code is correct since `::boost::scoped_ptr` does not have 
`operator=` which causes fix-its to be invalid.

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 Piotr Zegar via cfe-commits


@@ -0,0 +1,68 @@
+// RUN: %check_clang_tidy %s readability-ambiguous-smartptr-reset-call %t \

PiotrZSL wrote:

you could merge this file into main test, just have 2 RUN commands there and 
specify custom suffix for this one (you may need to include current default 
values also)

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 Piotr Zegar via cfe-commits


@@ -0,0 +1,129 @@
+//===--- 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"
+
+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";
+} // namespace
+
+AmbiguousSmartptrResetCallCheck::AmbiguousSmartptrResetCallCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  SmartPointers(utils::options::parseStringList(
+  Options.get("SmartPointers", DefaultSmartPointers))) {}
+
+void AmbiguousSmartptrResetCallCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "SmartPointers",
+utils::options::serializeStringList(SmartPointers));
+}
+
+void AmbiguousSmartptrResetCallCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IsSmartptr = hasAnyName(SmartPointers);
+
+  const auto ResetMethod =
+  cxxMethodDecl(hasName("reset"), hasOnlyDefaultParameters());
+
+  const auto TypeWithReset =
+  anyOf(cxxRecordDecl(hasMethod(ResetMethod)),
+classTemplateSpecializationDecl(
+hasSpecializedTemplate(classTemplateDecl(has(ResetMethod);
+
+  const auto SmartptrWithReset = expr(hasType(hasUnqualifiedDesugaredType(
+  recordType(hasDeclaration(classTemplateSpecializationDecl(
+  IsSmartptr,
+  hasTemplateArgument(
+  0, templateArgument(refersToType(hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(TypeWithReset;
+
+  Finder->addMatcher(
+  cxxMemberCallExpr(
+  callee(ResetMethod),
+  unless(hasAnyArgument(expr(unless(cxxDefaultArgExpr(),
+  anyOf(on(SmartptrWithReset),
+on(cxxOperatorCallExpr(hasOverloadedOperatorName("->"),
+   hasArgument(0, SmartptrWithReset))
+   .bind("ArrowOp"
+  .bind("MemberCall"),
+  this);
+}
+
+void AmbiguousSmartptrResetCallCheck::check(
+const MatchFinder::MatchResult &Result) {
+
+  if (const auto *Arrow =
+  Result.Nodes.getNodeAs("ArrowOp")) {
+const auto *ObjectResetCall =
+Result.Nodes.getNodeAs("MemberCall");

PiotrZSL wrote:

duplicated member call, just move it before if and use in both if's

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 Piotr Zegar via cfe-commits


@@ -0,0 +1,57 @@
+.. 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 and classes that are considered are
+``std::unique_ptr``, ``std::shared_ptr``, ``boost::shared_ptr``. To specify
+other smart pointers or other classes use the :option:`SmartPointers` option.
+
+

PiotrZSL wrote:

Mention in documentation that fixes are enabled via `--fix-notes`

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 Piotr Zegar via cfe-commits


@@ -0,0 +1,57 @@
+.. 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 and classes that are considered are
+``std::unique_ptr``, ``std::shared_ptr``, ``boost::shared_ptr``. To specify
+other smart pointers or other classes use the :option:`SmartPointers` option.
+
+
+.. note::
+
+  The check may emit invalid fix-its and misleading warning messages when
+  specifying custom smart pointers or other classes in the
+  :option:`SmartPointers` option. For example, ``boost::scoped_ptr`` does not
+  have an ``operator=`` which makes fix-its invalid.
+
+
+Options
+---
+
+.. option:: SmartPointers
+
+Semicolon-separated list of fully qualified class names of custom smart
+pointers. Default value is `::std::unique_ptr;::std::shared_ptr;
+::boost::shared_ptr`.

PiotrZSL wrote:

```suggestion
::boost::scoped_ptr`.
```

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 Piotr Zegar via cfe-commits

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

In general looks fine.
Few nits.

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 Piotr Zegar via cfe-commits

https://github.com/PiotrZSL edited 
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 Piotr Zegar via cfe-commits


@@ -0,0 +1,129 @@
+//===--- 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"
+
+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";
+} // namespace
+
+AmbiguousSmartptrResetCallCheck::AmbiguousSmartptrResetCallCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  SmartPointers(utils::options::parseStringList(
+  Options.get("SmartPointers", DefaultSmartPointers))) {}
+
+void AmbiguousSmartptrResetCallCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "SmartPointers",
+utils::options::serializeStringList(SmartPointers));
+}
+
+void AmbiguousSmartptrResetCallCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IsSmartptr = hasAnyName(SmartPointers);
+
+  const auto ResetMethod =
+  cxxMethodDecl(hasName("reset"), hasOnlyDefaultParameters());
+
+  const auto TypeWithReset =
+  anyOf(cxxRecordDecl(hasMethod(ResetMethod)),
+classTemplateSpecializationDecl(
+hasSpecializedTemplate(classTemplateDecl(has(ResetMethod);
+
+  const auto SmartptrWithReset = expr(hasType(hasUnqualifiedDesugaredType(
+  recordType(hasDeclaration(classTemplateSpecializationDecl(
+  IsSmartptr,
+  hasTemplateArgument(
+  0, templateArgument(refersToType(hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(TypeWithReset;
+
+  Finder->addMatcher(
+  cxxMemberCallExpr(
+  callee(ResetMethod),
+  unless(hasAnyArgument(expr(unless(cxxDefaultArgExpr(),
+  anyOf(on(SmartptrWithReset),
+on(cxxOperatorCallExpr(hasOverloadedOperatorName("->"),
+   hasArgument(0, SmartptrWithReset))
+   .bind("ArrowOp"
+  .bind("MemberCall"),
+  this);
+}
+
+void AmbiguousSmartptrResetCallCheck::check(
+const MatchFinder::MatchResult &Result) {
+
+  if (const auto *Arrow =
+  Result.Nodes.getNodeAs("ArrowOp")) {
+const auto *ObjectResetCall =
+Result.Nodes.getNodeAs("MemberCall");
+
+const CharSourceRange SmartptrSourceRange =
+Lexer::getAsCharRange(Arrow->getArg(0)->getSourceRange(),

PiotrZSL wrote:

maybe just 'bind' an object to some variable and read it like an MemberCall

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 Piotr Zegar via cfe-commits


@@ -0,0 +1,68 @@
+// RUN: %check_clang_tidy %s readability-ambiguous-smartptr-reset-call %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  {readability-ambiguous-smartptr-reset-call.SmartPointers: 
"::std::unique_ptr;::other_ptr"}}' \
+// RUN: --fix-notes --
+
+namespace std {
+
+template 

PiotrZSL wrote:

consider including 
clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/smart-ptr/shared_ptr.h
 and 
clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/smart-ptr/unique_ptr.h
 instead

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 bugprone-smartptr-reset-ambiguous-call check (PR #121291)

2025-02-22 Thread Baranov Victor via cfe-commits

vbvictor wrote:

@PiotrZSL, @denzor200 
I think we should provide 100% valid fix-its for default `SmartPointers`. That 
will be `std::shared_ptr`, `std::unique_ptr`, `boost::shared_ptr`. I created a 
note in docs that warns users about inconvenience that can happen when 
non-default smart pointers are specified.
May you please review one more time.
If everything is good i'd need help in merging this pr since i don't have write 
access to main


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-22 Thread Baranov Victor via cfe-commits

https://github.com/vbvictor updated 
https://github.com/llvm/llvm-project/pull/121291

>From 42e03bb9cc9bd815476b0a3f06ac5f58826e3708 Mon Sep 17 00:00:00 2001
From: Victor Baranov 
Date: Fri, 31 Jan 2025 19:29:05 +0300
Subject: [PATCH 1/8] [clang-tidy] add new check bugprone-reset-ambiguous-call

---
 .../bugprone/BugproneTidyModule.cpp   |   3 +
 .../clang-tidy/bugprone/CMakeLists.txt|   1 +
 .../SmartptrResetAmbiguousCallCheck.cpp   | 146 +++
 .../SmartptrResetAmbiguousCallCheck.h |  37 +++
 clang-tools-extra/docs/ReleaseNotes.rst   |   6 +
 .../smartptr-reset-ambiguous-call.rst |  47 
 .../docs/clang-tidy/checks/list.rst   |   1 +
 ...r-reset-ambiguous-call-custom-pointers.cpp |  72 ++
 .../smartptr-reset-ambiguous-call.cpp | 239 ++
 9 files changed, 552 insertions(+)
 create mode 100644 
clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp
 create mode 100644 
clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.h
 create mode 100644 
clang-tools-extra/docs/clang-tidy/checks/bugprone/smartptr-reset-ambiguous-call.rst
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call-custom-pointers.cpp
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call.cpp

diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp 
b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index c5f0b5b28418f..a01d0e384ab73 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -64,6 +64,7 @@
 #include "SignedCharMisuseCheck.h"
 #include "SizeofContainerCheck.h"
 #include "SizeofExpressionCheck.h"
+#include "SmartptrResetAmbiguousCallCheck.h"
 #include "SpuriouslyWakeUpFunctionsCheck.h"
 #include "StandaloneEmptyCheck.h"
 #include "StringConstructorCheck.h"
@@ -207,6 +208,8 @@ class BugproneModule : public ClangTidyModule {
 "bugprone-sizeof-container");
 CheckFactories.registerCheck(
 "bugprone-sizeof-expression");
+CheckFactories.registerCheck(
+"bugprone-smartptr-reset-ambiguous-call");
 CheckFactories.registerCheck(
 "bugprone-spuriously-wake-up-functions");
 CheckFactories.registerCheck(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index e8309c68b7fca..7fd6f4994f537 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -65,6 +65,7 @@ add_clang_library(clangTidyBugproneModule STATIC
   SizeofContainerCheck.cpp
   SizeofExpressionCheck.cpp
   SmartPtrArrayMismatchCheck.cpp
+  SmartptrResetAmbiguousCallCheck.cpp
   SpuriouslyWakeUpFunctionsCheck.cpp
   StandaloneEmptyCheck.cpp
   StringConstructorCheck.cpp
diff --git 
a/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp
new file mode 100644
index 0..326a5665179d7
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp
@@ -0,0 +1,146 @@
+//===--- SmartptrResetAmbiguousCallCheck.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 "SmartptrResetAmbiguousCallCheck.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"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+
+  return true;
+}
+
+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";
+} // namespace
+
+SmartptrResetAmbiguousCallCheck::SmartptrResetAmbiguousCallCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  SmartPointers(utils::options::parseStringList(
+  Options.get("SmartPointers", DefaultSmartPointers))) {}
+
+void SmartptrResetAmbiguousCallCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "SmartPointers",
+utils::options::seriali

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

2025-02-21 Thread Baranov Victor 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";

vbvictor wrote:

I think we should stick to 100% correct fix-its for default-defined types and 
warn users about incorrect behavoir of other types that lack `operator=` or 
`operator=` doesn't expect `std::nullptr_t`.

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 Baranov Victor 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";

vbvictor wrote:

Thank you for pointing that out. I have written a note in documentation that 
says the check may not provide correct fix-its on user-defined types. I guess I 
will also include `::boost::scoped_ptr` as an example.
As for `::boost::scoped_ptr` the correct fix-it would be `ptr.reset(nullptr)` 
or `ptr.reset(0)`

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:

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-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-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 Baranov Victor via cfe-commits

https://github.com/vbvictor updated 
https://github.com/llvm/llvm-project/pull/121291

>From 42e03bb9cc9bd815476b0a3f06ac5f58826e3708 Mon Sep 17 00:00:00 2001
From: Victor Baranov 
Date: Fri, 31 Jan 2025 19:29:05 +0300
Subject: [PATCH 1/7] [clang-tidy] add new check bugprone-reset-ambiguous-call

---
 .../bugprone/BugproneTidyModule.cpp   |   3 +
 .../clang-tidy/bugprone/CMakeLists.txt|   1 +
 .../SmartptrResetAmbiguousCallCheck.cpp   | 146 +++
 .../SmartptrResetAmbiguousCallCheck.h |  37 +++
 clang-tools-extra/docs/ReleaseNotes.rst   |   6 +
 .../smartptr-reset-ambiguous-call.rst |  47 
 .../docs/clang-tidy/checks/list.rst   |   1 +
 ...r-reset-ambiguous-call-custom-pointers.cpp |  72 ++
 .../smartptr-reset-ambiguous-call.cpp | 239 ++
 9 files changed, 552 insertions(+)
 create mode 100644 
clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp
 create mode 100644 
clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.h
 create mode 100644 
clang-tools-extra/docs/clang-tidy/checks/bugprone/smartptr-reset-ambiguous-call.rst
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call-custom-pointers.cpp
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call.cpp

diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp 
b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index c5f0b5b28418f..a01d0e384ab73 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -64,6 +64,7 @@
 #include "SignedCharMisuseCheck.h"
 #include "SizeofContainerCheck.h"
 #include "SizeofExpressionCheck.h"
+#include "SmartptrResetAmbiguousCallCheck.h"
 #include "SpuriouslyWakeUpFunctionsCheck.h"
 #include "StandaloneEmptyCheck.h"
 #include "StringConstructorCheck.h"
@@ -207,6 +208,8 @@ class BugproneModule : public ClangTidyModule {
 "bugprone-sizeof-container");
 CheckFactories.registerCheck(
 "bugprone-sizeof-expression");
+CheckFactories.registerCheck(
+"bugprone-smartptr-reset-ambiguous-call");
 CheckFactories.registerCheck(
 "bugprone-spuriously-wake-up-functions");
 CheckFactories.registerCheck(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index e8309c68b7fca..7fd6f4994f537 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -65,6 +65,7 @@ add_clang_library(clangTidyBugproneModule STATIC
   SizeofContainerCheck.cpp
   SizeofExpressionCheck.cpp
   SmartPtrArrayMismatchCheck.cpp
+  SmartptrResetAmbiguousCallCheck.cpp
   SpuriouslyWakeUpFunctionsCheck.cpp
   StandaloneEmptyCheck.cpp
   StringConstructorCheck.cpp
diff --git 
a/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp
new file mode 100644
index 0..326a5665179d7
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp
@@ -0,0 +1,146 @@
+//===--- SmartptrResetAmbiguousCallCheck.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 "SmartptrResetAmbiguousCallCheck.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"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+
+  return true;
+}
+
+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";
+} // namespace
+
+SmartptrResetAmbiguousCallCheck::SmartptrResetAmbiguousCallCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  SmartPointers(utils::options::parseStringList(
+  Options.get("SmartPointers", DefaultSmartPointers))) {}
+
+void SmartptrResetAmbiguousCallCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "SmartPointers",
+utils::options::seriali

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

2025-02-20 Thread Baranov Victor via cfe-commits


@@ -0,0 +1,49 @@
+.. 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 and classes that are considered are
+``std::unique_ptr``, ``std::shared_ptr``, ``std::optional``,
+``boost::shared_ptr``, ``boost::scoped_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;
+::std::optional;::boost::unique_ptr;::boost::shared_ptr`.

vbvictor wrote:

Done

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 Baranov Victor via cfe-commits

https://github.com/vbvictor updated 
https://github.com/llvm/llvm-project/pull/121291

>From 42e03bb9cc9bd815476b0a3f06ac5f58826e3708 Mon Sep 17 00:00:00 2001
From: Victor Baranov 
Date: Fri, 31 Jan 2025 19:29:05 +0300
Subject: [PATCH 1/6] [clang-tidy] add new check bugprone-reset-ambiguous-call

---
 .../bugprone/BugproneTidyModule.cpp   |   3 +
 .../clang-tidy/bugprone/CMakeLists.txt|   1 +
 .../SmartptrResetAmbiguousCallCheck.cpp   | 146 +++
 .../SmartptrResetAmbiguousCallCheck.h |  37 +++
 clang-tools-extra/docs/ReleaseNotes.rst   |   6 +
 .../smartptr-reset-ambiguous-call.rst |  47 
 .../docs/clang-tidy/checks/list.rst   |   1 +
 ...r-reset-ambiguous-call-custom-pointers.cpp |  72 ++
 .../smartptr-reset-ambiguous-call.cpp | 239 ++
 9 files changed, 552 insertions(+)
 create mode 100644 
clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp
 create mode 100644 
clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.h
 create mode 100644 
clang-tools-extra/docs/clang-tidy/checks/bugprone/smartptr-reset-ambiguous-call.rst
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call-custom-pointers.cpp
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call.cpp

diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp 
b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index c5f0b5b28418f..a01d0e384ab73 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -64,6 +64,7 @@
 #include "SignedCharMisuseCheck.h"
 #include "SizeofContainerCheck.h"
 #include "SizeofExpressionCheck.h"
+#include "SmartptrResetAmbiguousCallCheck.h"
 #include "SpuriouslyWakeUpFunctionsCheck.h"
 #include "StandaloneEmptyCheck.h"
 #include "StringConstructorCheck.h"
@@ -207,6 +208,8 @@ class BugproneModule : public ClangTidyModule {
 "bugprone-sizeof-container");
 CheckFactories.registerCheck(
 "bugprone-sizeof-expression");
+CheckFactories.registerCheck(
+"bugprone-smartptr-reset-ambiguous-call");
 CheckFactories.registerCheck(
 "bugprone-spuriously-wake-up-functions");
 CheckFactories.registerCheck(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index e8309c68b7fca..7fd6f4994f537 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -65,6 +65,7 @@ add_clang_library(clangTidyBugproneModule STATIC
   SizeofContainerCheck.cpp
   SizeofExpressionCheck.cpp
   SmartPtrArrayMismatchCheck.cpp
+  SmartptrResetAmbiguousCallCheck.cpp
   SpuriouslyWakeUpFunctionsCheck.cpp
   StandaloneEmptyCheck.cpp
   StringConstructorCheck.cpp
diff --git 
a/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp
new file mode 100644
index 0..326a5665179d7
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp
@@ -0,0 +1,146 @@
+//===--- SmartptrResetAmbiguousCallCheck.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 "SmartptrResetAmbiguousCallCheck.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"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+
+  return true;
+}
+
+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";
+} // namespace
+
+SmartptrResetAmbiguousCallCheck::SmartptrResetAmbiguousCallCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  SmartPointers(utils::options::parseStringList(
+  Options.get("SmartPointers", DefaultSmartPointers))) {}
+
+void SmartptrResetAmbiguousCallCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "SmartPointers",
+utils::options::seriali

[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-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-19 Thread Baranov Victor via cfe-commits

https://github.com/vbvictor edited 
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-18 Thread Baranov Victor via cfe-commits


@@ -0,0 +1,146 @@
+//===--- SmartptrResetAmbiguousCallCheck.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 "SmartptrResetAmbiguousCallCheck.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"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+
+  return true;
+}
+
+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";
+} // namespace
+
+SmartptrResetAmbiguousCallCheck::SmartptrResetAmbiguousCallCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  SmartPointers(utils::options::parseStringList(
+  Options.get("SmartPointers", DefaultSmartPointers))) {}
+
+void SmartptrResetAmbiguousCallCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "SmartPointers",
+utils::options::serializeStringList(SmartPointers));
+}
+
+void SmartptrResetAmbiguousCallCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IsSmartptr = hasAnyName(SmartPointers);
+
+  const auto ResetMethod =
+  cxxMethodDecl(hasName("reset"), hasOnlyDefaultParameters());
+
+  const auto TypeWithReset =
+  anyOf(cxxRecordDecl(hasMethod(ResetMethod)),
+classTemplateSpecializationDecl(
+hasSpecializedTemplate(classTemplateDecl(has(ResetMethod);
+
+  const auto SmartptrWithBugproneReset = classTemplateSpecializationDecl(
+  IsSmartptr,
+  hasTemplateArgument(
+  0, templateArgument(refersToType(hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(TypeWithReset)));
+
+  // Find a.reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(callee(memberExpr(member(hasName("reset",
+everyArgumentMatches(cxxDefaultArgExpr()),
+on(expr(hasType(SmartptrWithBugproneReset
+  .bind("smartptrResetCall"),
+  this);
+
+  // Find a->reset() calls
+  Finder->addMatcher(

vbvictor wrote:

With some modifications made this work,
changed `thisPointerType()` to `on(expr())` because it failed to match some 
cases with nested ptr's.
`std::unique_ptr>> nested_ptr;`

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-18 Thread Baranov Victor 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;

vbvictor wrote:

Synced code and docs, sorry for inconvenience.
Also added tests to cover all listed types.

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-18 Thread Baranov Victor 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

vbvictor wrote:

 Fixed

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-18 Thread Baranov Victor via cfe-commits


@@ -0,0 +1,239 @@
+// RUN: %check_clang_tidy %s readability-ambiguous-smartptr-reset-call %t 
--fix-notes --
+
+namespace std {
+
+template 
+struct unique_ptr {
+  T& operator*() const;
+  T* operator->() const;
+  void reset(T* p = nullptr);
+};
+
+template 
+struct shared_ptr {
+  T& operator*() const;
+  T* operator->() const;
+  void reset();
+  void reset(T*);
+};
+
+} // namespace std
+
+struct Resettable {
+  void reset();
+  void doSomething();
+};
+
+struct ResettableWithParam {
+  void reset(int a);
+  void doSomething();
+};
+
+struct ResettableWithDefaultParams {
+  void reset(int a = 0, double b = 0.0);
+  void doSomething();
+};
+
+struct NonResettable {
+  void doSomething();
+};
+
+void Positive() {
+  std::unique_ptr u;
+  u.reset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a 
smart pointer with pointee that also has a 'reset()' method, prefer more 
explicit approach
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 
'nullptr' here
+  // CHECK-FIXES: u = nullptr;
+  u->reset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a 
pointee of a smart pointer, prefer more explicit approach
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart 
pointer to call 'reset' method of the pointee here
+  // CHECK-FIXES: (*u).reset();
+
+  std::shared_ptr s;
+  s.reset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a 
smart pointer with pointee that also has a 'reset()' method, prefer more 
explicit approach
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 
'nullptr' here
+  // CHECK-FIXES: s = nullptr;
+  s->reset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a 
pointee of a smart pointer, prefer more explicit approach
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart 
pointer to call 'reset' method of the pointee here
+  // CHECK-FIXES: (*s).reset();
+
+  std::unique_ptr> uu_ptr;
+  uu_ptr.reset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a 
smart pointer with pointee that also has a 'reset()' method, prefer more 
explicit approach
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 
'nullptr' here
+  // CHECK-FIXES: uu_ptr = nullptr;
+  uu_ptr->reset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a 
pointee of a smart pointer, prefer more explicit approach
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart 
pointer to call 'reset' method of the pointee here
+  // CHECK-FIXES: (*uu_ptr).reset();
+
+  std::unique_ptr> su_ptr;
+  su_ptr.reset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a 
smart pointer with pointee that also has a 'reset()' method, prefer more 
explicit approach
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 
'nullptr' here
+  // CHECK-FIXES: su_ptr = nullptr;
+  su_ptr->reset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a 
pointee of a smart pointer, prefer more explicit approach
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart 
pointer to call 'reset' method of the pointee here
+  // CHECK-FIXES: (*su_ptr).reset();
+
+  std::unique_ptr rd;
+  rd.reset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a 
smart pointer with pointee that also has a 'reset()' method, prefer more 
explicit approach
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 
'nullptr' here
+  // CHECK-FIXES: rd = nullptr;
+  rd->reset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a 
pointee of a smart pointer, prefer more explicit approach
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart 
pointer to call 'reset' method of the pointee here
+  // CHECK-FIXES: (*rd).reset();
+
+  std::unique_ptr>> nested_ptr;
+  nested_ptr.reset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a 
smart pointer with pointee that also has a 'reset()' method, prefer more 
explicit approach
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 
'nullptr' here
+  // CHECK-FIXES: nested_ptr = nullptr;
+  nested_ptr->reset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a 
pointee of a smart pointer, prefer more explicit approach
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart 
pointer to call 'reset' method of the pointee here
+  // CHECK-FIXES: (*nested_ptr).reset();
+  (*nested_ptr).reset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a 
smart pointer with pointee that also has a 'reset()' method, prefer more 
explicit approach
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigni

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

2025-02-18 Thread Baranov Victor via cfe-commits

https://github.com/vbvictor updated 
https://github.com/llvm/llvm-project/pull/121291

>From 42e03bb9cc9bd815476b0a3f06ac5f58826e3708 Mon Sep 17 00:00:00 2001
From: Victor Baranov 
Date: Fri, 31 Jan 2025 19:29:05 +0300
Subject: [PATCH 1/5] [clang-tidy] add new check bugprone-reset-ambiguous-call

---
 .../bugprone/BugproneTidyModule.cpp   |   3 +
 .../clang-tidy/bugprone/CMakeLists.txt|   1 +
 .../SmartptrResetAmbiguousCallCheck.cpp   | 146 +++
 .../SmartptrResetAmbiguousCallCheck.h |  37 +++
 clang-tools-extra/docs/ReleaseNotes.rst   |   6 +
 .../smartptr-reset-ambiguous-call.rst |  47 
 .../docs/clang-tidy/checks/list.rst   |   1 +
 ...r-reset-ambiguous-call-custom-pointers.cpp |  72 ++
 .../smartptr-reset-ambiguous-call.cpp | 239 ++
 9 files changed, 552 insertions(+)
 create mode 100644 
clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp
 create mode 100644 
clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.h
 create mode 100644 
clang-tools-extra/docs/clang-tidy/checks/bugprone/smartptr-reset-ambiguous-call.rst
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call-custom-pointers.cpp
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call.cpp

diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp 
b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index c5f0b5b28418f..a01d0e384ab73 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -64,6 +64,7 @@
 #include "SignedCharMisuseCheck.h"
 #include "SizeofContainerCheck.h"
 #include "SizeofExpressionCheck.h"
+#include "SmartptrResetAmbiguousCallCheck.h"
 #include "SpuriouslyWakeUpFunctionsCheck.h"
 #include "StandaloneEmptyCheck.h"
 #include "StringConstructorCheck.h"
@@ -207,6 +208,8 @@ class BugproneModule : public ClangTidyModule {
 "bugprone-sizeof-container");
 CheckFactories.registerCheck(
 "bugprone-sizeof-expression");
+CheckFactories.registerCheck(
+"bugprone-smartptr-reset-ambiguous-call");
 CheckFactories.registerCheck(
 "bugprone-spuriously-wake-up-functions");
 CheckFactories.registerCheck(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index e8309c68b7fca..7fd6f4994f537 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -65,6 +65,7 @@ add_clang_library(clangTidyBugproneModule STATIC
   SizeofContainerCheck.cpp
   SizeofExpressionCheck.cpp
   SmartPtrArrayMismatchCheck.cpp
+  SmartptrResetAmbiguousCallCheck.cpp
   SpuriouslyWakeUpFunctionsCheck.cpp
   StandaloneEmptyCheck.cpp
   StringConstructorCheck.cpp
diff --git 
a/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp
new file mode 100644
index 0..326a5665179d7
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp
@@ -0,0 +1,146 @@
+//===--- SmartptrResetAmbiguousCallCheck.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 "SmartptrResetAmbiguousCallCheck.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"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+
+  return true;
+}
+
+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";
+} // namespace
+
+SmartptrResetAmbiguousCallCheck::SmartptrResetAmbiguousCallCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  SmartPointers(utils::options::parseStringList(
+  Options.get("SmartPointers", DefaultSmartPointers))) {}
+
+void SmartptrResetAmbiguousCallCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "SmartPointers",
+utils::options::seriali

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

2025-02-17 Thread Baranov Victor via cfe-commits

https://github.com/vbvictor updated 
https://github.com/llvm/llvm-project/pull/121291

>From 42e03bb9cc9bd815476b0a3f06ac5f58826e3708 Mon Sep 17 00:00:00 2001
From: Victor Baranov 
Date: Fri, 31 Jan 2025 19:29:05 +0300
Subject: [PATCH 1/4] [clang-tidy] add new check bugprone-reset-ambiguous-call

---
 .../bugprone/BugproneTidyModule.cpp   |   3 +
 .../clang-tidy/bugprone/CMakeLists.txt|   1 +
 .../SmartptrResetAmbiguousCallCheck.cpp   | 146 +++
 .../SmartptrResetAmbiguousCallCheck.h |  37 +++
 clang-tools-extra/docs/ReleaseNotes.rst   |   6 +
 .../smartptr-reset-ambiguous-call.rst |  47 
 .../docs/clang-tidy/checks/list.rst   |   1 +
 ...r-reset-ambiguous-call-custom-pointers.cpp |  72 ++
 .../smartptr-reset-ambiguous-call.cpp | 239 ++
 9 files changed, 552 insertions(+)
 create mode 100644 
clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp
 create mode 100644 
clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.h
 create mode 100644 
clang-tools-extra/docs/clang-tidy/checks/bugprone/smartptr-reset-ambiguous-call.rst
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call-custom-pointers.cpp
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call.cpp

diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp 
b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index c5f0b5b28418f..a01d0e384ab73 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -64,6 +64,7 @@
 #include "SignedCharMisuseCheck.h"
 #include "SizeofContainerCheck.h"
 #include "SizeofExpressionCheck.h"
+#include "SmartptrResetAmbiguousCallCheck.h"
 #include "SpuriouslyWakeUpFunctionsCheck.h"
 #include "StandaloneEmptyCheck.h"
 #include "StringConstructorCheck.h"
@@ -207,6 +208,8 @@ class BugproneModule : public ClangTidyModule {
 "bugprone-sizeof-container");
 CheckFactories.registerCheck(
 "bugprone-sizeof-expression");
+CheckFactories.registerCheck(
+"bugprone-smartptr-reset-ambiguous-call");
 CheckFactories.registerCheck(
 "bugprone-spuriously-wake-up-functions");
 CheckFactories.registerCheck(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index e8309c68b7fca..7fd6f4994f537 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -65,6 +65,7 @@ add_clang_library(clangTidyBugproneModule STATIC
   SizeofContainerCheck.cpp
   SizeofExpressionCheck.cpp
   SmartPtrArrayMismatchCheck.cpp
+  SmartptrResetAmbiguousCallCheck.cpp
   SpuriouslyWakeUpFunctionsCheck.cpp
   StandaloneEmptyCheck.cpp
   StringConstructorCheck.cpp
diff --git 
a/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp
new file mode 100644
index 0..326a5665179d7
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp
@@ -0,0 +1,146 @@
+//===--- SmartptrResetAmbiguousCallCheck.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 "SmartptrResetAmbiguousCallCheck.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"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+
+  return true;
+}
+
+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";
+} // namespace
+
+SmartptrResetAmbiguousCallCheck::SmartptrResetAmbiguousCallCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  SmartPointers(utils::options::parseStringList(
+  Options.get("SmartPointers", DefaultSmartPointers))) {}
+
+void SmartptrResetAmbiguousCallCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "SmartPointers",
+utils::options::seriali

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

2025-02-17 Thread Piotr Zegar 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

PiotrZSL wrote:

boost does not have unique_ptr, it has boost::scoped_ptr

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-17 Thread Piotr Zegar via cfe-commits


@@ -0,0 +1,239 @@
+// RUN: %check_clang_tidy %s readability-ambiguous-smartptr-reset-call %t 
--fix-notes --
+
+namespace std {
+
+template 
+struct unique_ptr {
+  T& operator*() const;
+  T* operator->() const;
+  void reset(T* p = nullptr);
+};
+
+template 
+struct shared_ptr {
+  T& operator*() const;
+  T* operator->() const;
+  void reset();
+  void reset(T*);
+};
+
+} // namespace std
+
+struct Resettable {
+  void reset();
+  void doSomething();
+};
+
+struct ResettableWithParam {
+  void reset(int a);
+  void doSomething();
+};
+
+struct ResettableWithDefaultParams {
+  void reset(int a = 0, double b = 0.0);
+  void doSomething();
+};
+
+struct NonResettable {
+  void doSomething();
+};
+
+void Positive() {
+  std::unique_ptr u;
+  u.reset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a 
smart pointer with pointee that also has a 'reset()' method, prefer more 
explicit approach
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 
'nullptr' here
+  // CHECK-FIXES: u = nullptr;
+  u->reset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a 
pointee of a smart pointer, prefer more explicit approach
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart 
pointer to call 'reset' method of the pointee here
+  // CHECK-FIXES: (*u).reset();
+
+  std::shared_ptr s;
+  s.reset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a 
smart pointer with pointee that also has a 'reset()' method, prefer more 
explicit approach
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 
'nullptr' here
+  // CHECK-FIXES: s = nullptr;
+  s->reset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a 
pointee of a smart pointer, prefer more explicit approach
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart 
pointer to call 'reset' method of the pointee here
+  // CHECK-FIXES: (*s).reset();
+
+  std::unique_ptr> uu_ptr;
+  uu_ptr.reset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a 
smart pointer with pointee that also has a 'reset()' method, prefer more 
explicit approach
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 
'nullptr' here
+  // CHECK-FIXES: uu_ptr = nullptr;
+  uu_ptr->reset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a 
pointee of a smart pointer, prefer more explicit approach
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart 
pointer to call 'reset' method of the pointee here
+  // CHECK-FIXES: (*uu_ptr).reset();
+
+  std::unique_ptr> su_ptr;
+  su_ptr.reset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a 
smart pointer with pointee that also has a 'reset()' method, prefer more 
explicit approach
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 
'nullptr' here
+  // CHECK-FIXES: su_ptr = nullptr;
+  su_ptr->reset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a 
pointee of a smart pointer, prefer more explicit approach
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart 
pointer to call 'reset' method of the pointee here
+  // CHECK-FIXES: (*su_ptr).reset();
+
+  std::unique_ptr rd;
+  rd.reset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a 
smart pointer with pointee that also has a 'reset()' method, prefer more 
explicit approach
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 
'nullptr' here
+  // CHECK-FIXES: rd = nullptr;
+  rd->reset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a 
pointee of a smart pointer, prefer more explicit approach
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart 
pointer to call 'reset' method of the pointee here
+  // CHECK-FIXES: (*rd).reset();
+
+  std::unique_ptr>> nested_ptr;
+  nested_ptr.reset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a 
smart pointer with pointee that also has a 'reset()' method, prefer more 
explicit approach
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigning the pointer to 
'nullptr' here
+  // CHECK-FIXES: nested_ptr = nullptr;
+  nested_ptr->reset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a 
pointee of a smart pointer, prefer more explicit approach
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider dereferencing smart 
pointer to call 'reset' method of the pointee here
+  // CHECK-FIXES: (*nested_ptr).reset();
+  (*nested_ptr).reset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ambiguous call to 'reset()' on a 
smart pointer with pointee that also has a 'reset()' method, prefer more 
explicit approach
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: note: consider assigni

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

2025-02-17 Thread Piotr Zegar 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;

PiotrZSL wrote:

out of sync, change also DefaultSmartPointers to match this, consider adding 
std::optional here also (even that is not smart ptr)

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-17 Thread Piotr Zegar via cfe-commits


@@ -0,0 +1,146 @@
+//===--- SmartptrResetAmbiguousCallCheck.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 "SmartptrResetAmbiguousCallCheck.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"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+
+  return true;
+}
+
+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";
+} // namespace
+
+SmartptrResetAmbiguousCallCheck::SmartptrResetAmbiguousCallCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  SmartPointers(utils::options::parseStringList(
+  Options.get("SmartPointers", DefaultSmartPointers))) {}
+
+void SmartptrResetAmbiguousCallCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "SmartPointers",
+utils::options::serializeStringList(SmartPointers));
+}
+
+void SmartptrResetAmbiguousCallCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IsSmartptr = hasAnyName(SmartPointers);
+
+  const auto ResetMethod =
+  cxxMethodDecl(hasName("reset"), hasOnlyDefaultParameters());
+
+  const auto TypeWithReset =
+  anyOf(cxxRecordDecl(hasMethod(ResetMethod)),
+classTemplateSpecializationDecl(
+hasSpecializedTemplate(classTemplateDecl(has(ResetMethod);
+
+  const auto SmartptrWithBugproneReset = classTemplateSpecializationDecl(
+  IsSmartptr,
+  hasTemplateArgument(
+  0, templateArgument(refersToType(hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(TypeWithReset)));
+
+  // Find a.reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(callee(memberExpr(member(hasName("reset",
+everyArgumentMatches(cxxDefaultArgExpr()),
+on(expr(hasType(SmartptrWithBugproneReset
+  .bind("smartptrResetCall"),
+  this);
+
+  // Find a->reset() calls
+  Finder->addMatcher(

PiotrZSL wrote:

maybe something like this: 
```
Finder->addMatcher(
  cxxMemberCallExpr(
   callee(ResetMethod),
   unless(hasAnyArgument(expr(unless(cxxDefaultArgExpr(),
   anyOf(
 thisPointerType(SmartptrWithBugproneReset),
 on(cxxOperatorCallExpr(
  hasOverloadedOperatorName("->"),
  hasArgument(
  0, 
expr(hasType(classTemplateSpecializationDecl(IsSmartptr)
   )
   )
   ), this);
```

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-17 Thread Baranov Victor via cfe-commits

vbvictor wrote:

@PiotrZSL, thank you for pointing out better naming pattern for diag-messages, 
would use it in the future.

1. Created new warning and fix-it messages.
2. Renamed check.
3. As for now, my check can support any type that will have `reset` method, 
this include `std::optional`. Maybe a more general approach would be to detect 
overlapping methods with classes that are returned via `operator*` and 
`operator->`. However, if some method is called on main object and there is no 
equivalent of `= nullptr` I don't know how user can fix such warning.

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-17 Thread Baranov Victor via cfe-commits


@@ -0,0 +1,146 @@
+//===--- SmartptrResetAmbiguousCallCheck.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 "SmartptrResetAmbiguousCallCheck.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"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+
+  return true;
+}
+
+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";
+} // namespace
+
+SmartptrResetAmbiguousCallCheck::SmartptrResetAmbiguousCallCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  SmartPointers(utils::options::parseStringList(
+  Options.get("SmartPointers", DefaultSmartPointers))) {}
+
+void SmartptrResetAmbiguousCallCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "SmartPointers",
+utils::options::serializeStringList(SmartPointers));
+}
+
+void SmartptrResetAmbiguousCallCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IsSmartptr = hasAnyName(SmartPointers);
+
+  const auto ResetMethod =
+  cxxMethodDecl(hasName("reset"), hasOnlyDefaultParameters());
+
+  const auto TypeWithReset =
+  anyOf(cxxRecordDecl(hasMethod(ResetMethod)),
+classTemplateSpecializationDecl(
+hasSpecializedTemplate(classTemplateDecl(has(ResetMethod);
+
+  const auto SmartptrWithBugproneReset = classTemplateSpecializationDecl(
+  IsSmartptr,
+  hasTemplateArgument(
+  0, templateArgument(refersToType(hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(TypeWithReset)));
+
+  // Find a.reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(callee(memberExpr(member(hasName("reset",
+everyArgumentMatches(cxxDefaultArgExpr()),
+on(expr(hasType(SmartptrWithBugproneReset
+  .bind("smartptrResetCall"),
+  this);
+
+  // Find a->reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(
+  callee(memberExpr(
+  member(ResetMethod),
+  hasObjectExpression(
+  cxxOperatorCallExpr(
+  hasOverloadedOperatorName("->"),
+  hasArgument(
+  0, expr(hasType(
+ 
classTemplateSpecializationDecl(IsSmartptr)
+  .bind("OpCall",
+  everyArgumentMatches(cxxDefaultArgExpr()))
+  .bind("objectResetCall"),
+  this);
+}
+
+void SmartptrResetAmbiguousCallCheck::check(
+const MatchFinder::MatchResult &Result) {
+
+  if (const auto *SmartptrResetCall =
+  Result.Nodes.getNodeAs("smartptrResetCall")) {
+const auto *Member = cast(SmartptrResetCall->getCallee());
+
+diag(SmartptrResetCall->getBeginLoc(),
+ "be explicit when calling 'reset()' on a smart pointer with a "
+ "pointee that has a 'reset()' method");
+
+diag(SmartptrResetCall->getBeginLoc(), "assign the pointer to 'nullptr'",
+ DiagnosticIDs::Note)
+<< FixItHint::CreateReplacement(
+   SourceRange(Member->getOperatorLoc(),
+   Member->getOperatorLoc().getLocWithOffset(0)),
+   " =")
+<< FixItHint::CreateReplacement(
+   SourceRange(Member->getMemberLoc(),
+   SmartptrResetCall->getEndLoc()),
+   " nullptr");
+return;
+  }
+
+  if (const auto *ObjectResetCall =
+  Result.Nodes.getNodeAs("objectResetCall")) {
+const auto *Arrow = Result.Nodes.getNodeAs("OpCall");
+
+const CharSourceRange SmartptrSourceRange =
+Lexer::getAsCharRange(Arrow->getArg(0)->getSourceRange(),
+  *Result.SourceManager, getLangOpts());
+
+diag(ObjectResetCall->getBeginLoc(),
+ "be explicit when calling 'reset()' on a pointee of a smart pointer");

vbvictor wrote:

Done

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

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

2025-02-17 Thread Baranov Victor via cfe-commits


@@ -0,0 +1,47 @@
+.. title:: clang-tidy - bugprone-smartptr-reset-ambiguous-call
+
+bugprone-smartptr-reset-ambiguous-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``. To specify other smart pointers or other classes use the
+:option:`SmartPointers` option.
+
+Options
+---
+
+.. option:: SmartPointers
+
+Semicolon-separated list of class names of custom smart pointers.

vbvictor wrote:

Done

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-17 Thread Baranov Victor via cfe-commits


@@ -0,0 +1,146 @@
+//===--- SmartptrResetAmbiguousCallCheck.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 "SmartptrResetAmbiguousCallCheck.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"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+
+  return true;
+}
+
+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";
+} // namespace
+
+SmartptrResetAmbiguousCallCheck::SmartptrResetAmbiguousCallCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  SmartPointers(utils::options::parseStringList(
+  Options.get("SmartPointers", DefaultSmartPointers))) {}
+
+void SmartptrResetAmbiguousCallCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "SmartPointers",
+utils::options::serializeStringList(SmartPointers));
+}
+
+void SmartptrResetAmbiguousCallCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IsSmartptr = hasAnyName(SmartPointers);
+
+  const auto ResetMethod =
+  cxxMethodDecl(hasName("reset"), hasOnlyDefaultParameters());
+
+  const auto TypeWithReset =
+  anyOf(cxxRecordDecl(hasMethod(ResetMethod)),
+classTemplateSpecializationDecl(
+hasSpecializedTemplate(classTemplateDecl(has(ResetMethod);
+
+  const auto SmartptrWithBugproneReset = classTemplateSpecializationDecl(
+  IsSmartptr,
+  hasTemplateArgument(
+  0, templateArgument(refersToType(hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(TypeWithReset)));
+
+  // Find a.reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(callee(memberExpr(member(hasName("reset",
+everyArgumentMatches(cxxDefaultArgExpr()),
+on(expr(hasType(SmartptrWithBugproneReset
+  .bind("smartptrResetCall"),
+  this);
+
+  // Find a->reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(
+  callee(memberExpr(
+  member(ResetMethod),
+  hasObjectExpression(
+  cxxOperatorCallExpr(
+  hasOverloadedOperatorName("->"),
+  hasArgument(
+  0, expr(hasType(
+ 
classTemplateSpecializationDecl(IsSmartptr)
+  .bind("OpCall",
+  everyArgumentMatches(cxxDefaultArgExpr()))
+  .bind("objectResetCall"),
+  this);
+}
+
+void SmartptrResetAmbiguousCallCheck::check(
+const MatchFinder::MatchResult &Result) {
+
+  if (const auto *SmartptrResetCall =
+  Result.Nodes.getNodeAs("smartptrResetCall")) {
+const auto *Member = cast(SmartptrResetCall->getCallee());
+
+diag(SmartptrResetCall->getBeginLoc(),
+ "be explicit when calling 'reset()' on a smart pointer with a "
+ "pointee that has a 'reset()' method");
+
+diag(SmartptrResetCall->getBeginLoc(), "assign the pointer to 'nullptr'",

vbvictor wrote:

Done

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-17 Thread Baranov Victor via cfe-commits


@@ -0,0 +1,146 @@
+//===--- SmartptrResetAmbiguousCallCheck.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 "SmartptrResetAmbiguousCallCheck.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"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+
+  return true;
+}
+
+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";
+} // namespace
+
+SmartptrResetAmbiguousCallCheck::SmartptrResetAmbiguousCallCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  SmartPointers(utils::options::parseStringList(
+  Options.get("SmartPointers", DefaultSmartPointers))) {}
+
+void SmartptrResetAmbiguousCallCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "SmartPointers",
+utils::options::serializeStringList(SmartPointers));
+}
+
+void SmartptrResetAmbiguousCallCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IsSmartptr = hasAnyName(SmartPointers);
+
+  const auto ResetMethod =
+  cxxMethodDecl(hasName("reset"), hasOnlyDefaultParameters());
+
+  const auto TypeWithReset =
+  anyOf(cxxRecordDecl(hasMethod(ResetMethod)),
+classTemplateSpecializationDecl(
+hasSpecializedTemplate(classTemplateDecl(has(ResetMethod);
+
+  const auto SmartptrWithBugproneReset = classTemplateSpecializationDecl(
+  IsSmartptr,
+  hasTemplateArgument(
+  0, templateArgument(refersToType(hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(TypeWithReset)));
+
+  // Find a.reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(callee(memberExpr(member(hasName("reset",
+everyArgumentMatches(cxxDefaultArgExpr()),
+on(expr(hasType(SmartptrWithBugproneReset
+  .bind("smartptrResetCall"),
+  this);
+
+  // Find a->reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(
+  callee(memberExpr(
+  member(ResetMethod),
+  hasObjectExpression(
+  cxxOperatorCallExpr(
+  hasOverloadedOperatorName("->"),
+  hasArgument(
+  0, expr(hasType(
+ 
classTemplateSpecializationDecl(IsSmartptr)
+  .bind("OpCall",
+  everyArgumentMatches(cxxDefaultArgExpr()))
+  .bind("objectResetCall"),
+  this);
+}
+
+void SmartptrResetAmbiguousCallCheck::check(
+const MatchFinder::MatchResult &Result) {
+
+  if (const auto *SmartptrResetCall =
+  Result.Nodes.getNodeAs("smartptrResetCall")) {
+const auto *Member = cast(SmartptrResetCall->getCallee());
+
+diag(SmartptrResetCall->getBeginLoc(),
+ "be explicit when calling 'reset()' on a smart pointer with a "
+ "pointee that has a 'reset()' method");

vbvictor wrote:

Done

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-17 Thread Baranov Victor via cfe-commits


@@ -0,0 +1,146 @@
+//===--- SmartptrResetAmbiguousCallCheck.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 "SmartptrResetAmbiguousCallCheck.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"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+
+  return true;
+}
+
+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";
+} // namespace
+
+SmartptrResetAmbiguousCallCheck::SmartptrResetAmbiguousCallCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  SmartPointers(utils::options::parseStringList(
+  Options.get("SmartPointers", DefaultSmartPointers))) {}
+
+void SmartptrResetAmbiguousCallCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "SmartPointers",
+utils::options::serializeStringList(SmartPointers));
+}
+
+void SmartptrResetAmbiguousCallCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IsSmartptr = hasAnyName(SmartPointers);
+
+  const auto ResetMethod =
+  cxxMethodDecl(hasName("reset"), hasOnlyDefaultParameters());
+
+  const auto TypeWithReset =
+  anyOf(cxxRecordDecl(hasMethod(ResetMethod)),
+classTemplateSpecializationDecl(
+hasSpecializedTemplate(classTemplateDecl(has(ResetMethod);
+
+  const auto SmartptrWithBugproneReset = classTemplateSpecializationDecl(
+  IsSmartptr,
+  hasTemplateArgument(
+  0, templateArgument(refersToType(hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(TypeWithReset)));
+
+  // Find a.reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(callee(memberExpr(member(hasName("reset",
+everyArgumentMatches(cxxDefaultArgExpr()),
+on(expr(hasType(SmartptrWithBugproneReset
+  .bind("smartptrResetCall"),
+  this);
+
+  // Find a->reset() calls
+  Finder->addMatcher(

vbvictor wrote:

I Could not find a good way to use "anyOf" in this context.
If I write like this, it gives CE.
```cpp
Finder->addMatcher(anyOf(...));
```
If I write:
```cpp
Finder->addMatcher(
cxxMemberCallExpr(
   anyOf(...))
.bind("MemberCall"));
```
I get only one bind-name for `cxxMemberCallExpr` which complicates futher 
processing.
If there aren't any performance issues, I'd like to leave as is.

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-17 Thread Baranov Victor via cfe-commits

https://github.com/vbvictor updated 
https://github.com/llvm/llvm-project/pull/121291

>From 42e03bb9cc9bd815476b0a3f06ac5f58826e3708 Mon Sep 17 00:00:00 2001
From: Victor Baranov 
Date: Fri, 31 Jan 2025 19:29:05 +0300
Subject: [PATCH 1/3] [clang-tidy] add new check bugprone-reset-ambiguous-call

---
 .../bugprone/BugproneTidyModule.cpp   |   3 +
 .../clang-tidy/bugprone/CMakeLists.txt|   1 +
 .../SmartptrResetAmbiguousCallCheck.cpp   | 146 +++
 .../SmartptrResetAmbiguousCallCheck.h |  37 +++
 clang-tools-extra/docs/ReleaseNotes.rst   |   6 +
 .../smartptr-reset-ambiguous-call.rst |  47 
 .../docs/clang-tidy/checks/list.rst   |   1 +
 ...r-reset-ambiguous-call-custom-pointers.cpp |  72 ++
 .../smartptr-reset-ambiguous-call.cpp | 239 ++
 9 files changed, 552 insertions(+)
 create mode 100644 
clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp
 create mode 100644 
clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.h
 create mode 100644 
clang-tools-extra/docs/clang-tidy/checks/bugprone/smartptr-reset-ambiguous-call.rst
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call-custom-pointers.cpp
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call.cpp

diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp 
b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index c5f0b5b28418f..a01d0e384ab73 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -64,6 +64,7 @@
 #include "SignedCharMisuseCheck.h"
 #include "SizeofContainerCheck.h"
 #include "SizeofExpressionCheck.h"
+#include "SmartptrResetAmbiguousCallCheck.h"
 #include "SpuriouslyWakeUpFunctionsCheck.h"
 #include "StandaloneEmptyCheck.h"
 #include "StringConstructorCheck.h"
@@ -207,6 +208,8 @@ class BugproneModule : public ClangTidyModule {
 "bugprone-sizeof-container");
 CheckFactories.registerCheck(
 "bugprone-sizeof-expression");
+CheckFactories.registerCheck(
+"bugprone-smartptr-reset-ambiguous-call");
 CheckFactories.registerCheck(
 "bugprone-spuriously-wake-up-functions");
 CheckFactories.registerCheck(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index e8309c68b7fca..7fd6f4994f537 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -65,6 +65,7 @@ add_clang_library(clangTidyBugproneModule STATIC
   SizeofContainerCheck.cpp
   SizeofExpressionCheck.cpp
   SmartPtrArrayMismatchCheck.cpp
+  SmartptrResetAmbiguousCallCheck.cpp
   SpuriouslyWakeUpFunctionsCheck.cpp
   StandaloneEmptyCheck.cpp
   StringConstructorCheck.cpp
diff --git 
a/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp
new file mode 100644
index 0..326a5665179d7
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp
@@ -0,0 +1,146 @@
+//===--- SmartptrResetAmbiguousCallCheck.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 "SmartptrResetAmbiguousCallCheck.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"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+
+  return true;
+}
+
+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";
+} // namespace
+
+SmartptrResetAmbiguousCallCheck::SmartptrResetAmbiguousCallCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  SmartPointers(utils::options::parseStringList(
+  Options.get("SmartPointers", DefaultSmartPointers))) {}
+
+void SmartptrResetAmbiguousCallCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "SmartPointers",
+utils::options::seriali

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

2025-02-15 Thread Piotr Zegar via cfe-commits


@@ -0,0 +1,146 @@
+//===--- SmartptrResetAmbiguousCallCheck.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 "SmartptrResetAmbiguousCallCheck.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"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+
+  return true;
+}
+
+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";
+} // namespace
+
+SmartptrResetAmbiguousCallCheck::SmartptrResetAmbiguousCallCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  SmartPointers(utils::options::parseStringList(
+  Options.get("SmartPointers", DefaultSmartPointers))) {}
+
+void SmartptrResetAmbiguousCallCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "SmartPointers",
+utils::options::serializeStringList(SmartPointers));
+}
+
+void SmartptrResetAmbiguousCallCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IsSmartptr = hasAnyName(SmartPointers);
+
+  const auto ResetMethod =
+  cxxMethodDecl(hasName("reset"), hasOnlyDefaultParameters());
+
+  const auto TypeWithReset =
+  anyOf(cxxRecordDecl(hasMethod(ResetMethod)),
+classTemplateSpecializationDecl(
+hasSpecializedTemplate(classTemplateDecl(has(ResetMethod);
+
+  const auto SmartptrWithBugproneReset = classTemplateSpecializationDecl(
+  IsSmartptr,
+  hasTemplateArgument(
+  0, templateArgument(refersToType(hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(TypeWithReset)));
+
+  // Find a.reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(callee(memberExpr(member(hasName("reset",
+everyArgumentMatches(cxxDefaultArgExpr()),
+on(expr(hasType(SmartptrWithBugproneReset
+  .bind("smartptrResetCall"),
+  this);
+
+  // Find a->reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(
+  callee(memberExpr(
+  member(ResetMethod),
+  hasObjectExpression(
+  cxxOperatorCallExpr(
+  hasOverloadedOperatorName("->"),
+  hasArgument(
+  0, expr(hasType(
+ 
classTemplateSpecializationDecl(IsSmartptr)
+  .bind("OpCall",
+  everyArgumentMatches(cxxDefaultArgExpr()))
+  .bind("objectResetCall"),
+  this);
+}
+
+void SmartptrResetAmbiguousCallCheck::check(
+const MatchFinder::MatchResult &Result) {
+
+  if (const auto *SmartptrResetCall =
+  Result.Nodes.getNodeAs("smartptrResetCall")) {
+const auto *Member = cast(SmartptrResetCall->getCallee());
+
+diag(SmartptrResetCall->getBeginLoc(),
+ "be explicit when calling 'reset()' on a smart pointer with a "
+ "pointee that has a 'reset()' method");

PiotrZSL wrote:

consider format "whats wrong, how to fix". In this case warning probably could 
start with `ambiguous call to `reset()` on a smart pointer with pointee that 
also has a 'reset()' method, prefer more explicit approach`

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-15 Thread Piotr Zegar via cfe-commits


@@ -0,0 +1,146 @@
+//===--- SmartptrResetAmbiguousCallCheck.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 "SmartptrResetAmbiguousCallCheck.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"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+
+  return true;
+}
+
+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";
+} // namespace
+
+SmartptrResetAmbiguousCallCheck::SmartptrResetAmbiguousCallCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  SmartPointers(utils::options::parseStringList(
+  Options.get("SmartPointers", DefaultSmartPointers))) {}
+
+void SmartptrResetAmbiguousCallCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "SmartPointers",
+utils::options::serializeStringList(SmartPointers));
+}
+
+void SmartptrResetAmbiguousCallCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IsSmartptr = hasAnyName(SmartPointers);
+
+  const auto ResetMethod =
+  cxxMethodDecl(hasName("reset"), hasOnlyDefaultParameters());
+
+  const auto TypeWithReset =
+  anyOf(cxxRecordDecl(hasMethod(ResetMethod)),
+classTemplateSpecializationDecl(
+hasSpecializedTemplate(classTemplateDecl(has(ResetMethod);
+
+  const auto SmartptrWithBugproneReset = classTemplateSpecializationDecl(
+  IsSmartptr,
+  hasTemplateArgument(
+  0, templateArgument(refersToType(hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(TypeWithReset)));
+
+  // Find a.reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(callee(memberExpr(member(hasName("reset",
+everyArgumentMatches(cxxDefaultArgExpr()),
+on(expr(hasType(SmartptrWithBugproneReset
+  .bind("smartptrResetCall"),
+  this);
+
+  // Find a->reset() calls
+  Finder->addMatcher(

PiotrZSL wrote:

you could merge both addMatcher by using "anyOf".


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-15 Thread Piotr Zegar via cfe-commits

https://github.com/PiotrZSL edited 
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-15 Thread Piotr Zegar via cfe-commits


@@ -0,0 +1,146 @@
+//===--- SmartptrResetAmbiguousCallCheck.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 "SmartptrResetAmbiguousCallCheck.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"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+
+  return true;
+}
+
+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";
+} // namespace
+
+SmartptrResetAmbiguousCallCheck::SmartptrResetAmbiguousCallCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  SmartPointers(utils::options::parseStringList(
+  Options.get("SmartPointers", DefaultSmartPointers))) {}
+
+void SmartptrResetAmbiguousCallCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "SmartPointers",
+utils::options::serializeStringList(SmartPointers));
+}
+
+void SmartptrResetAmbiguousCallCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IsSmartptr = hasAnyName(SmartPointers);
+
+  const auto ResetMethod =
+  cxxMethodDecl(hasName("reset"), hasOnlyDefaultParameters());
+
+  const auto TypeWithReset =
+  anyOf(cxxRecordDecl(hasMethod(ResetMethod)),
+classTemplateSpecializationDecl(
+hasSpecializedTemplate(classTemplateDecl(has(ResetMethod);
+
+  const auto SmartptrWithBugproneReset = classTemplateSpecializationDecl(
+  IsSmartptr,
+  hasTemplateArgument(
+  0, templateArgument(refersToType(hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(TypeWithReset)));
+
+  // Find a.reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(callee(memberExpr(member(hasName("reset",
+everyArgumentMatches(cxxDefaultArgExpr()),
+on(expr(hasType(SmartptrWithBugproneReset
+  .bind("smartptrResetCall"),
+  this);
+
+  // Find a->reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(
+  callee(memberExpr(
+  member(ResetMethod),
+  hasObjectExpression(
+  cxxOperatorCallExpr(
+  hasOverloadedOperatorName("->"),
+  hasArgument(
+  0, expr(hasType(
+ 
classTemplateSpecializationDecl(IsSmartptr)
+  .bind("OpCall",
+  everyArgumentMatches(cxxDefaultArgExpr()))
+  .bind("objectResetCall"),
+  this);
+}
+
+void SmartptrResetAmbiguousCallCheck::check(
+const MatchFinder::MatchResult &Result) {
+
+  if (const auto *SmartptrResetCall =
+  Result.Nodes.getNodeAs("smartptrResetCall")) {
+const auto *Member = cast(SmartptrResetCall->getCallee());
+
+diag(SmartptrResetCall->getBeginLoc(),
+ "be explicit when calling 'reset()' on a smart pointer with a "
+ "pointee that has a 'reset()' method");
+
+diag(SmartptrResetCall->getBeginLoc(), "assign the pointer to 'nullptr'",
+ DiagnosticIDs::Note)
+<< FixItHint::CreateReplacement(
+   SourceRange(Member->getOperatorLoc(),
+   Member->getOperatorLoc().getLocWithOffset(0)),
+   " =")
+<< FixItHint::CreateReplacement(
+   SourceRange(Member->getMemberLoc(),
+   SmartptrResetCall->getEndLoc()),
+   " nullptr");
+return;
+  }
+
+  if (const auto *ObjectResetCall =
+  Result.Nodes.getNodeAs("objectResetCall")) {
+const auto *Arrow = Result.Nodes.getNodeAs("OpCall");
+
+const CharSourceRange SmartptrSourceRange =
+Lexer::getAsCharRange(Arrow->getArg(0)->getSourceRange(),
+  *Result.SourceManager, getLangOpts());
+
+diag(ObjectResetCall->getBeginLoc(),
+ "be explicit when calling 'reset()' on a pointee of a smart pointer");

PiotrZSL wrote:

same comments related to msg as before

https://github.com/llvm/llvm-project/pull/121291
___
cfe-commits mailing list
cfe-commits@list

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

2025-02-15 Thread Piotr Zegar via cfe-commits


@@ -0,0 +1,146 @@
+//===--- SmartptrResetAmbiguousCallCheck.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 "SmartptrResetAmbiguousCallCheck.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"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+
+  return true;
+}
+
+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";
+} // namespace
+
+SmartptrResetAmbiguousCallCheck::SmartptrResetAmbiguousCallCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  SmartPointers(utils::options::parseStringList(
+  Options.get("SmartPointers", DefaultSmartPointers))) {}
+
+void SmartptrResetAmbiguousCallCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "SmartPointers",
+utils::options::serializeStringList(SmartPointers));
+}
+
+void SmartptrResetAmbiguousCallCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IsSmartptr = hasAnyName(SmartPointers);
+
+  const auto ResetMethod =
+  cxxMethodDecl(hasName("reset"), hasOnlyDefaultParameters());
+
+  const auto TypeWithReset =
+  anyOf(cxxRecordDecl(hasMethod(ResetMethod)),
+classTemplateSpecializationDecl(
+hasSpecializedTemplate(classTemplateDecl(has(ResetMethod);
+
+  const auto SmartptrWithBugproneReset = classTemplateSpecializationDecl(
+  IsSmartptr,
+  hasTemplateArgument(
+  0, templateArgument(refersToType(hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(TypeWithReset)));
+
+  // Find a.reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(callee(memberExpr(member(hasName("reset",
+everyArgumentMatches(cxxDefaultArgExpr()),
+on(expr(hasType(SmartptrWithBugproneReset
+  .bind("smartptrResetCall"),
+  this);
+
+  // Find a->reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(
+  callee(memberExpr(
+  member(ResetMethod),
+  hasObjectExpression(
+  cxxOperatorCallExpr(
+  hasOverloadedOperatorName("->"),
+  hasArgument(
+  0, expr(hasType(
+ 
classTemplateSpecializationDecl(IsSmartptr)
+  .bind("OpCall",
+  everyArgumentMatches(cxxDefaultArgExpr()))
+  .bind("objectResetCall"),
+  this);
+}
+
+void SmartptrResetAmbiguousCallCheck::check(
+const MatchFinder::MatchResult &Result) {
+
+  if (const auto *SmartptrResetCall =
+  Result.Nodes.getNodeAs("smartptrResetCall")) {
+const auto *Member = cast(SmartptrResetCall->getCallee());
+
+diag(SmartptrResetCall->getBeginLoc(),
+ "be explicit when calling 'reset()' on a smart pointer with a "
+ "pointee that has a 'reset()' method");
+
+diag(SmartptrResetCall->getBeginLoc(), "assign the pointer to 'nullptr'",

PiotrZSL wrote:

again, `consider assigning 'nullptr' here`

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-15 Thread Piotr Zegar 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;

PiotrZSL wrote:

Ok, but then add "boost::shared_ptr" to default configuration

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-15 Thread Piotr Zegar via cfe-commits


@@ -0,0 +1,47 @@
+.. title:: clang-tidy - bugprone-smartptr-reset-ambiguous-call
+
+bugprone-smartptr-reset-ambiguous-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``. To specify other smart pointers or other classes use the
+:option:`SmartPointers` option.
+
+Options
+---
+
+.. option:: SmartPointers
+
+Semicolon-separated list of class names of custom smart pointers.

PiotrZSL wrote:

add fully qualified

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-15 Thread Piotr Zegar via cfe-commits

https://github.com/PiotrZSL commented:

1. Overall ok, please work a little bit on an warning message.
2. Probably better name would be: readability-ambiguous-smartptr-reset-call
3. Same issue exist on other objects like std::optional, and other methods like 
std::optional and call to `emplace`. Maybe more checks or more 
generic check could be constructed where configuration is an pair of "object" 
and "method".

Consider points 1 & 2. And after that it could be delivered.


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-15 Thread Baranov Victor via cfe-commits

vbvictor wrote:

@PiotrZSL, @5chmidti Ping

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-07 Thread Baranov Victor via cfe-commits

vbvictor wrote:

Ping

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-01-31 Thread Baranov Victor via cfe-commits

https://github.com/vbvictor updated 
https://github.com/llvm/llvm-project/pull/121291

>From 42e03bb9cc9bd815476b0a3f06ac5f58826e3708 Mon Sep 17 00:00:00 2001
From: Victor Baranov 
Date: Fri, 31 Jan 2025 19:29:05 +0300
Subject: [PATCH 1/2] [clang-tidy] add new check bugprone-reset-ambiguous-call

---
 .../bugprone/BugproneTidyModule.cpp   |   3 +
 .../clang-tidy/bugprone/CMakeLists.txt|   1 +
 .../SmartptrResetAmbiguousCallCheck.cpp   | 146 +++
 .../SmartptrResetAmbiguousCallCheck.h |  37 +++
 clang-tools-extra/docs/ReleaseNotes.rst   |   6 +
 .../smartptr-reset-ambiguous-call.rst |  47 
 .../docs/clang-tidy/checks/list.rst   |   1 +
 ...r-reset-ambiguous-call-custom-pointers.cpp |  72 ++
 .../smartptr-reset-ambiguous-call.cpp | 239 ++
 9 files changed, 552 insertions(+)
 create mode 100644 
clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp
 create mode 100644 
clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.h
 create mode 100644 
clang-tools-extra/docs/clang-tidy/checks/bugprone/smartptr-reset-ambiguous-call.rst
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call-custom-pointers.cpp
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call.cpp

diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp 
b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index c5f0b5b28418f8..a01d0e384ab737 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -64,6 +64,7 @@
 #include "SignedCharMisuseCheck.h"
 #include "SizeofContainerCheck.h"
 #include "SizeofExpressionCheck.h"
+#include "SmartptrResetAmbiguousCallCheck.h"
 #include "SpuriouslyWakeUpFunctionsCheck.h"
 #include "StandaloneEmptyCheck.h"
 #include "StringConstructorCheck.h"
@@ -207,6 +208,8 @@ class BugproneModule : public ClangTidyModule {
 "bugprone-sizeof-container");
 CheckFactories.registerCheck(
 "bugprone-sizeof-expression");
+CheckFactories.registerCheck(
+"bugprone-smartptr-reset-ambiguous-call");
 CheckFactories.registerCheck(
 "bugprone-spuriously-wake-up-functions");
 CheckFactories.registerCheck(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index e8309c68b7fcaa..7fd6f4994f5375 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -65,6 +65,7 @@ add_clang_library(clangTidyBugproneModule STATIC
   SizeofContainerCheck.cpp
   SizeofExpressionCheck.cpp
   SmartPtrArrayMismatchCheck.cpp
+  SmartptrResetAmbiguousCallCheck.cpp
   SpuriouslyWakeUpFunctionsCheck.cpp
   StandaloneEmptyCheck.cpp
   StringConstructorCheck.cpp
diff --git 
a/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp
new file mode 100644
index 00..326a5665179d79
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp
@@ -0,0 +1,146 @@
+//===--- SmartptrResetAmbiguousCallCheck.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 "SmartptrResetAmbiguousCallCheck.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"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+
+  return true;
+}
+
+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";
+} // namespace
+
+SmartptrResetAmbiguousCallCheck::SmartptrResetAmbiguousCallCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  SmartPointers(utils::options::parseStringList(
+  Options.get("SmartPointers", DefaultSmartPointers))) {}
+
+void SmartptrResetAmbiguousCallCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "SmartPointers",
+utils::options::s

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

2025-01-31 Thread Baranov Victor via cfe-commits

https://github.com/vbvictor updated 
https://github.com/llvm/llvm-project/pull/121291

>From 42e03bb9cc9bd815476b0a3f06ac5f58826e3708 Mon Sep 17 00:00:00 2001
From: Victor Baranov 
Date: Fri, 31 Jan 2025 19:29:05 +0300
Subject: [PATCH] [clang-tidy] add new check bugprone-reset-ambiguous-call

---
 .../bugprone/BugproneTidyModule.cpp   |   3 +
 .../clang-tidy/bugprone/CMakeLists.txt|   1 +
 .../SmartptrResetAmbiguousCallCheck.cpp   | 146 +++
 .../SmartptrResetAmbiguousCallCheck.h |  37 +++
 clang-tools-extra/docs/ReleaseNotes.rst   |   6 +
 .../smartptr-reset-ambiguous-call.rst |  47 
 .../docs/clang-tidy/checks/list.rst   |   1 +
 ...r-reset-ambiguous-call-custom-pointers.cpp |  72 ++
 .../smartptr-reset-ambiguous-call.cpp | 239 ++
 9 files changed, 552 insertions(+)
 create mode 100644 
clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp
 create mode 100644 
clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.h
 create mode 100644 
clang-tools-extra/docs/clang-tidy/checks/bugprone/smartptr-reset-ambiguous-call.rst
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call-custom-pointers.cpp
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/bugprone/smartptr-reset-ambiguous-call.cpp

diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp 
b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index c5f0b5b28418f8..a01d0e384ab737 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -64,6 +64,7 @@
 #include "SignedCharMisuseCheck.h"
 #include "SizeofContainerCheck.h"
 #include "SizeofExpressionCheck.h"
+#include "SmartptrResetAmbiguousCallCheck.h"
 #include "SpuriouslyWakeUpFunctionsCheck.h"
 #include "StandaloneEmptyCheck.h"
 #include "StringConstructorCheck.h"
@@ -207,6 +208,8 @@ class BugproneModule : public ClangTidyModule {
 "bugprone-sizeof-container");
 CheckFactories.registerCheck(
 "bugprone-sizeof-expression");
+CheckFactories.registerCheck(
+"bugprone-smartptr-reset-ambiguous-call");
 CheckFactories.registerCheck(
 "bugprone-spuriously-wake-up-functions");
 CheckFactories.registerCheck(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index e8309c68b7fcaa..7fd6f4994f5375 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -65,6 +65,7 @@ add_clang_library(clangTidyBugproneModule STATIC
   SizeofContainerCheck.cpp
   SizeofExpressionCheck.cpp
   SmartPtrArrayMismatchCheck.cpp
+  SmartptrResetAmbiguousCallCheck.cpp
   SpuriouslyWakeUpFunctionsCheck.cpp
   StandaloneEmptyCheck.cpp
   StringConstructorCheck.cpp
diff --git 
a/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp
new file mode 100644
index 00..326a5665179d79
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/SmartptrResetAmbiguousCallCheck.cpp
@@ -0,0 +1,146 @@
+//===--- SmartptrResetAmbiguousCallCheck.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 "SmartptrResetAmbiguousCallCheck.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"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+
+  return true;
+}
+
+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";
+} // namespace
+
+SmartptrResetAmbiguousCallCheck::SmartptrResetAmbiguousCallCheck(
+StringRef Name, ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  SmartPointers(utils::options::parseStringList(
+  Options.get("SmartPointers", DefaultSmartPointers))) {}
+
+void SmartptrResetAmbiguousCallCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "SmartPointers",
+utils::options::seria

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

2025-01-30 Thread via cfe-commits

EugeneZelenko wrote:

Please rebase from `main`. Release Notes were cleared after 20 branch creation.

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-01-30 Thread Baranov Victor via cfe-commits

vbvictor wrote:

Ping

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-01-25 Thread Baranov Victor via cfe-commits


@@ -0,0 +1,48 @@
+.. title:: clang-tidy - bugprone-smartptr-reset-ambiguous-call
+
+bugprone-smartptr-reset-ambiguous-call
+==
+
+Finds potentially erroneous calls to ``reset`` method on

vbvictor wrote:

Done

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-01-25 Thread Baranov Victor via cfe-commits

https://github.com/vbvictor updated 
https://github.com/llvm/llvm-project/pull/121291

>From 37dce6a7ed0cca2e9819c24f4d176c43e3c9f2ac Mon Sep 17 00:00:00 2001
From: Victor Baranov 
Date: Sun, 29 Dec 2024 15:32:22 +0300
Subject: [PATCH 1/8] [clang-tidy] Add bugprone-reset-call check

---
 .../bugprone/BugproneTidyModule.cpp   |   2 +
 .../clang-tidy/bugprone/CMakeLists.txt|   1 +
 .../clang-tidy/bugprone/ResetCallCheck.cpp| 133 +++
 .../clang-tidy/bugprone/ResetCallCheck.h  |  34 +++
 clang-tools-extra/docs/ReleaseNotes.rst   |   6 +
 .../clang-tidy/checks/bugprone/reset-call.rst |  33 +++
 .../docs/clang-tidy/checks/list.rst   |   1 +
 .../checkers/bugprone/reset-call.cpp  | 215 ++
 8 files changed, 425 insertions(+)
 create mode 100644 clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.cpp
 create mode 100644 clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.h
 create mode 100644 
clang-tools-extra/docs/clang-tidy/checks/bugprone/reset-call.rst
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/bugprone/reset-call.cpp

diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp 
b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 33ac65e715ce81..645958e47e22a5 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -57,6 +57,7 @@
 #include "PosixReturnCheck.h"
 #include "RedundantBranchConditionCheck.h"
 #include "ReservedIdentifierCheck.h"
+#include "ResetCallCheck.h"
 #include "ReturnConstRefFromParameterCheck.h"
 #include "SharedPtrArrayMismatchCheck.h"
 #include "SignalHandlerCheck.h"
@@ -144,6 +145,7 @@ class BugproneModule : public ClangTidyModule {
 "bugprone-inaccurate-erase");
 CheckFactories.registerCheck(
 "bugprone-incorrect-enable-if");
+CheckFactories.registerCheck("bugprone-reset-call");
 CheckFactories.registerCheck(
 "bugprone-return-const-ref-from-parameter");
 CheckFactories.registerCheck(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 13adad7c3dadbd..17ab5b27ec5550 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -52,6 +52,7 @@ add_clang_library(clangTidyBugproneModule STATIC
   PosixReturnCheck.cpp
   RedundantBranchConditionCheck.cpp
   ReservedIdentifierCheck.cpp
+  ResetCallCheck.cpp
   ReturnConstRefFromParameterCheck.cpp
   SharedPtrArrayMismatchCheck.cpp
   SignalHandlerCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.cpp
new file mode 100644
index 00..305ac8d51adf3e
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.cpp
@@ -0,0 +1,133 @@
+//===--- ResetCallCheck.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 "ResetCallCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  if (Node.getNumArgs() == 0)
+return true;
+
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+  return true;
+}
+
+AST_MATCHER(CXXMethodDecl, hasOnlyDefaultArgs) {
+  if (Node.param_empty())
+return true;
+
+  for (const auto *Param : Node.parameters()) {
+if (!Param->hasDefaultArg())
+  return false;
+  }
+  return true;
+}
+
+} // namespace
+
+void ResetCallCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IsSmartptr = hasAnyName("::std::unique_ptr", "::std::shared_ptr");
+
+  const auto ResetMethod =
+  cxxMethodDecl(hasName("reset"), hasOnlyDefaultArgs());
+
+  const auto TypeWithReset =
+  anyOf(cxxRecordDecl(hasMethod(ResetMethod)),
+classTemplateSpecializationDecl(
+hasSpecializedTemplate(classTemplateDecl(has(ResetMethod);
+
+  const auto SmartptrWithBugproneReset = classTemplateSpecializationDecl(
+  IsSmartptr,
+  hasTemplateArgument(
+  0, templateArgument(refersToType(hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(TypeWithReset)));
+
+  // Find a.reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(callee(memberExpr(member(hasName("reset",
+ 

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

2025-01-25 Thread via cfe-commits


@@ -0,0 +1,48 @@
+.. title:: clang-tidy - bugprone-smartptr-reset-ambiguous-call
+
+bugprone-smartptr-reset-ambiguous-call
+==
+
+Finds potentially erroneous calls to ``reset`` method on

EugeneZelenko wrote:

Please fill as much as possible to 80-characters limit.

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-01-25 Thread Congcong Cai via cfe-commits

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

LGTM.

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-01-24 Thread Baranov Victor via cfe-commits


@@ -0,0 +1,150 @@
+//===--- SmartptrResetAmbiguousCallCheck.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 "SmartptrResetAmbiguousCallCheck.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"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  if (Node.getNumArgs() == 0)
+return true;
+
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+  return true;
+}
+
+AST_MATCHER(CXXMethodDecl, hasOnlyDefaultParameters) {
+  if (Node.param_empty())

vbvictor wrote:

Yes, removed useless condition.

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-01-24 Thread Baranov Victor via cfe-commits


@@ -0,0 +1,150 @@
+//===--- SmartptrResetAmbiguousCallCheck.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 "SmartptrResetAmbiguousCallCheck.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"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  if (Node.getNumArgs() == 0)
+return true;
+

vbvictor wrote:

Yes, removed useless condition.

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-01-24 Thread Baranov Victor via cfe-commits

https://github.com/vbvictor updated 
https://github.com/llvm/llvm-project/pull/121291

>From 37dce6a7ed0cca2e9819c24f4d176c43e3c9f2ac Mon Sep 17 00:00:00 2001
From: Victor Baranov 
Date: Sun, 29 Dec 2024 15:32:22 +0300
Subject: [PATCH 1/7] [clang-tidy] Add bugprone-reset-call check

---
 .../bugprone/BugproneTidyModule.cpp   |   2 +
 .../clang-tidy/bugprone/CMakeLists.txt|   1 +
 .../clang-tidy/bugprone/ResetCallCheck.cpp| 133 +++
 .../clang-tidy/bugprone/ResetCallCheck.h  |  34 +++
 clang-tools-extra/docs/ReleaseNotes.rst   |   6 +
 .../clang-tidy/checks/bugprone/reset-call.rst |  33 +++
 .../docs/clang-tidy/checks/list.rst   |   1 +
 .../checkers/bugprone/reset-call.cpp  | 215 ++
 8 files changed, 425 insertions(+)
 create mode 100644 clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.cpp
 create mode 100644 clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.h
 create mode 100644 
clang-tools-extra/docs/clang-tidy/checks/bugprone/reset-call.rst
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/bugprone/reset-call.cpp

diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp 
b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 33ac65e715ce81..645958e47e22a5 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -57,6 +57,7 @@
 #include "PosixReturnCheck.h"
 #include "RedundantBranchConditionCheck.h"
 #include "ReservedIdentifierCheck.h"
+#include "ResetCallCheck.h"
 #include "ReturnConstRefFromParameterCheck.h"
 #include "SharedPtrArrayMismatchCheck.h"
 #include "SignalHandlerCheck.h"
@@ -144,6 +145,7 @@ class BugproneModule : public ClangTidyModule {
 "bugprone-inaccurate-erase");
 CheckFactories.registerCheck(
 "bugprone-incorrect-enable-if");
+CheckFactories.registerCheck("bugprone-reset-call");
 CheckFactories.registerCheck(
 "bugprone-return-const-ref-from-parameter");
 CheckFactories.registerCheck(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 13adad7c3dadbd..17ab5b27ec5550 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -52,6 +52,7 @@ add_clang_library(clangTidyBugproneModule STATIC
   PosixReturnCheck.cpp
   RedundantBranchConditionCheck.cpp
   ReservedIdentifierCheck.cpp
+  ResetCallCheck.cpp
   ReturnConstRefFromParameterCheck.cpp
   SharedPtrArrayMismatchCheck.cpp
   SignalHandlerCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.cpp
new file mode 100644
index 00..305ac8d51adf3e
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.cpp
@@ -0,0 +1,133 @@
+//===--- ResetCallCheck.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 "ResetCallCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  if (Node.getNumArgs() == 0)
+return true;
+
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+  return true;
+}
+
+AST_MATCHER(CXXMethodDecl, hasOnlyDefaultArgs) {
+  if (Node.param_empty())
+return true;
+
+  for (const auto *Param : Node.parameters()) {
+if (!Param->hasDefaultArg())
+  return false;
+  }
+  return true;
+}
+
+} // namespace
+
+void ResetCallCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IsSmartptr = hasAnyName("::std::unique_ptr", "::std::shared_ptr");
+
+  const auto ResetMethod =
+  cxxMethodDecl(hasName("reset"), hasOnlyDefaultArgs());
+
+  const auto TypeWithReset =
+  anyOf(cxxRecordDecl(hasMethod(ResetMethod)),
+classTemplateSpecializationDecl(
+hasSpecializedTemplate(classTemplateDecl(has(ResetMethod);
+
+  const auto SmartptrWithBugproneReset = classTemplateSpecializationDecl(
+  IsSmartptr,
+  hasTemplateArgument(
+  0, templateArgument(refersToType(hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(TypeWithReset)));
+
+  // Find a.reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(callee(memberExpr(member(hasName("reset",
+ 

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

2025-01-24 Thread Congcong Cai via cfe-commits


@@ -0,0 +1,150 @@
+//===--- SmartptrResetAmbiguousCallCheck.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 "SmartptrResetAmbiguousCallCheck.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"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  if (Node.getNumArgs() == 0)
+return true;
+

HerrCai0907 wrote:

looks like useless since it is return at the end if it is empty.

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-01-24 Thread Congcong Cai via cfe-commits


@@ -0,0 +1,150 @@
+//===--- SmartptrResetAmbiguousCallCheck.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 "SmartptrResetAmbiguousCallCheck.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"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  if (Node.getNumArgs() == 0)
+return true;
+
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+  return true;
+}
+
+AST_MATCHER(CXXMethodDecl, hasOnlyDefaultParameters) {
+  if (Node.param_empty())

HerrCai0907 wrote:

same

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-01-24 Thread Baranov Victor via cfe-commits

vbvictor wrote:

Thank you everyone for the feedback!

@PiotrZSL I tried using TK_IgnoreUnlessSpelledInSource with 
`cxxDependentScopeMemberExpr` ast-matcher but failed to write a working matcher 
case with nested template parameters:
```cpp

template 
void TemplatePositiveTest() {
  std::unique_ptr u_ptr;

  u_ptr.reset();
  u_ptr->reset();
}

void instantiate() {
  TemplatePositiveTest>();
}
```

As for now, I'm thinking about leaving it as is and make an NFC change in the 
future when I get more familiar with matchers.
Apart from TK_IgnoreUnlessSpelledInSource I fixed your comments.

@5chmidti I fixed all your comments and added better _warning_ and _note_ 
messages:

```cpp
  s.reset(); // warning: be explicit when calling 'reset()' on a smart pointer 
with a pointee that has a 'reset()' method
 // note: assign the pointer to 'nullptr'
  s->reset(); // warning: be explicit when calling 'reset()' on a pointee of a 
smart pointer
  // note: use dereference to call 'reset' method of the pointee
```

@EugeneZelenko Fixed all pr comments.

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-01-23 Thread Baranov Victor via cfe-commits


@@ -0,0 +1,47 @@
+.. title:: clang-tidy - bugprone-smartptr-reset-ambiguous-call
+
+bugprone-smartptr-reset-ambiguous-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``. To specify other smart pointers or
+other classes use the :option:`SmartPointers` option.
+
+Options
+---
+
+.. option:: SmartPointers
+
+Semicolon-separated list of class names of custom smart pointers.

vbvictor wrote:

Done

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-01-23 Thread Baranov Victor via cfe-commits

https://github.com/vbvictor updated 
https://github.com/llvm/llvm-project/pull/121291

>From 37dce6a7ed0cca2e9819c24f4d176c43e3c9f2ac Mon Sep 17 00:00:00 2001
From: Victor Baranov 
Date: Sun, 29 Dec 2024 15:32:22 +0300
Subject: [PATCH 1/6] [clang-tidy] Add bugprone-reset-call check

---
 .../bugprone/BugproneTidyModule.cpp   |   2 +
 .../clang-tidy/bugprone/CMakeLists.txt|   1 +
 .../clang-tidy/bugprone/ResetCallCheck.cpp| 133 +++
 .../clang-tidy/bugprone/ResetCallCheck.h  |  34 +++
 clang-tools-extra/docs/ReleaseNotes.rst   |   6 +
 .../clang-tidy/checks/bugprone/reset-call.rst |  33 +++
 .../docs/clang-tidy/checks/list.rst   |   1 +
 .../checkers/bugprone/reset-call.cpp  | 215 ++
 8 files changed, 425 insertions(+)
 create mode 100644 clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.cpp
 create mode 100644 clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.h
 create mode 100644 
clang-tools-extra/docs/clang-tidy/checks/bugprone/reset-call.rst
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/bugprone/reset-call.cpp

diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp 
b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 33ac65e715ce81..645958e47e22a5 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -57,6 +57,7 @@
 #include "PosixReturnCheck.h"
 #include "RedundantBranchConditionCheck.h"
 #include "ReservedIdentifierCheck.h"
+#include "ResetCallCheck.h"
 #include "ReturnConstRefFromParameterCheck.h"
 #include "SharedPtrArrayMismatchCheck.h"
 #include "SignalHandlerCheck.h"
@@ -144,6 +145,7 @@ class BugproneModule : public ClangTidyModule {
 "bugprone-inaccurate-erase");
 CheckFactories.registerCheck(
 "bugprone-incorrect-enable-if");
+CheckFactories.registerCheck("bugprone-reset-call");
 CheckFactories.registerCheck(
 "bugprone-return-const-ref-from-parameter");
 CheckFactories.registerCheck(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 13adad7c3dadbd..17ab5b27ec5550 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -52,6 +52,7 @@ add_clang_library(clangTidyBugproneModule STATIC
   PosixReturnCheck.cpp
   RedundantBranchConditionCheck.cpp
   ReservedIdentifierCheck.cpp
+  ResetCallCheck.cpp
   ReturnConstRefFromParameterCheck.cpp
   SharedPtrArrayMismatchCheck.cpp
   SignalHandlerCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.cpp
new file mode 100644
index 00..305ac8d51adf3e
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.cpp
@@ -0,0 +1,133 @@
+//===--- ResetCallCheck.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 "ResetCallCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  if (Node.getNumArgs() == 0)
+return true;
+
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+  return true;
+}
+
+AST_MATCHER(CXXMethodDecl, hasOnlyDefaultArgs) {
+  if (Node.param_empty())
+return true;
+
+  for (const auto *Param : Node.parameters()) {
+if (!Param->hasDefaultArg())
+  return false;
+  }
+  return true;
+}
+
+} // namespace
+
+void ResetCallCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IsSmartptr = hasAnyName("::std::unique_ptr", "::std::shared_ptr");
+
+  const auto ResetMethod =
+  cxxMethodDecl(hasName("reset"), hasOnlyDefaultArgs());
+
+  const auto TypeWithReset =
+  anyOf(cxxRecordDecl(hasMethod(ResetMethod)),
+classTemplateSpecializationDecl(
+hasSpecializedTemplate(classTemplateDecl(has(ResetMethod);
+
+  const auto SmartptrWithBugproneReset = classTemplateSpecializationDecl(
+  IsSmartptr,
+  hasTemplateArgument(
+  0, templateArgument(refersToType(hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(TypeWithReset)));
+
+  // Find a.reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(callee(memberExpr(member(hasName("reset",
+ 

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

2025-01-23 Thread via cfe-commits


@@ -0,0 +1,47 @@
+.. title:: clang-tidy - bugprone-smartptr-reset-ambiguous-call
+
+bugprone-smartptr-reset-ambiguous-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``. To specify other smart pointers or
+other classes use the :option:`SmartPointers` option.
+
+Options
+---
+
+.. option:: SmartPointers
+
+Semicolon-separated list of class names of custom smart pointers.

EugeneZelenko wrote:

Please add default value.

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-01-23 Thread Baranov Victor via cfe-commits

https://github.com/vbvictor updated 
https://github.com/llvm/llvm-project/pull/121291

>From 37dce6a7ed0cca2e9819c24f4d176c43e3c9f2ac Mon Sep 17 00:00:00 2001
From: Victor Baranov 
Date: Sun, 29 Dec 2024 15:32:22 +0300
Subject: [PATCH 1/5] [clang-tidy] Add bugprone-reset-call check

---
 .../bugprone/BugproneTidyModule.cpp   |   2 +
 .../clang-tidy/bugprone/CMakeLists.txt|   1 +
 .../clang-tidy/bugprone/ResetCallCheck.cpp| 133 +++
 .../clang-tidy/bugprone/ResetCallCheck.h  |  34 +++
 clang-tools-extra/docs/ReleaseNotes.rst   |   6 +
 .../clang-tidy/checks/bugprone/reset-call.rst |  33 +++
 .../docs/clang-tidy/checks/list.rst   |   1 +
 .../checkers/bugprone/reset-call.cpp  | 215 ++
 8 files changed, 425 insertions(+)
 create mode 100644 clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.cpp
 create mode 100644 clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.h
 create mode 100644 
clang-tools-extra/docs/clang-tidy/checks/bugprone/reset-call.rst
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/bugprone/reset-call.cpp

diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp 
b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 33ac65e715ce81..645958e47e22a5 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -57,6 +57,7 @@
 #include "PosixReturnCheck.h"
 #include "RedundantBranchConditionCheck.h"
 #include "ReservedIdentifierCheck.h"
+#include "ResetCallCheck.h"
 #include "ReturnConstRefFromParameterCheck.h"
 #include "SharedPtrArrayMismatchCheck.h"
 #include "SignalHandlerCheck.h"
@@ -144,6 +145,7 @@ class BugproneModule : public ClangTidyModule {
 "bugprone-inaccurate-erase");
 CheckFactories.registerCheck(
 "bugprone-incorrect-enable-if");
+CheckFactories.registerCheck("bugprone-reset-call");
 CheckFactories.registerCheck(
 "bugprone-return-const-ref-from-parameter");
 CheckFactories.registerCheck(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 13adad7c3dadbd..17ab5b27ec5550 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -52,6 +52,7 @@ add_clang_library(clangTidyBugproneModule STATIC
   PosixReturnCheck.cpp
   RedundantBranchConditionCheck.cpp
   ReservedIdentifierCheck.cpp
+  ResetCallCheck.cpp
   ReturnConstRefFromParameterCheck.cpp
   SharedPtrArrayMismatchCheck.cpp
   SignalHandlerCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.cpp
new file mode 100644
index 00..305ac8d51adf3e
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.cpp
@@ -0,0 +1,133 @@
+//===--- ResetCallCheck.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 "ResetCallCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  if (Node.getNumArgs() == 0)
+return true;
+
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+  return true;
+}
+
+AST_MATCHER(CXXMethodDecl, hasOnlyDefaultArgs) {
+  if (Node.param_empty())
+return true;
+
+  for (const auto *Param : Node.parameters()) {
+if (!Param->hasDefaultArg())
+  return false;
+  }
+  return true;
+}
+
+} // namespace
+
+void ResetCallCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IsSmartptr = hasAnyName("::std::unique_ptr", "::std::shared_ptr");
+
+  const auto ResetMethod =
+  cxxMethodDecl(hasName("reset"), hasOnlyDefaultArgs());
+
+  const auto TypeWithReset =
+  anyOf(cxxRecordDecl(hasMethod(ResetMethod)),
+classTemplateSpecializationDecl(
+hasSpecializedTemplate(classTemplateDecl(has(ResetMethod);
+
+  const auto SmartptrWithBugproneReset = classTemplateSpecializationDecl(
+  IsSmartptr,
+  hasTemplateArgument(
+  0, templateArgument(refersToType(hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(TypeWithReset)));
+
+  // Find a.reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(callee(memberExpr(member(hasName("reset",
+ 

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

2025-01-09 Thread Baranov Victor via cfe-commits


@@ -0,0 +1,34 @@
+.. title:: clang-tidy - bugprone-smartptr-reset-ambiguous-call
+
+bugprone-smartptr-reset-ambiguous-call
+==
+
+Finds potentially erroneous calls to ``reset`` method on
+smart pointers when the pointee type also has a ``reset`` method.
+Having ``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 typo because the difference between ``.`` and ``->`` is 
really small.

vbvictor wrote:

Done

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-01-09 Thread Baranov Victor via cfe-commits


@@ -144,6 +145,8 @@ class BugproneModule : public ClangTidyModule {
 "bugprone-inaccurate-erase");
 CheckFactories.registerCheck(
 "bugprone-incorrect-enable-if");
+CheckFactories.registerCheck(
+"bugprone-smartptr-reset-ambiguous-call");

vbvictor wrote:

Done

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-01-09 Thread Baranov Victor via cfe-commits


@@ -0,0 +1,134 @@
+//===--- SmartptrResetAmbiguousCallCheck.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 "SmartptrResetAmbiguousCallCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  if (Node.getNumArgs() == 0)
+return true;
+
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+  return true;
+}
+
+AST_MATCHER(CXXMethodDecl, hasOnlyDefaultArgs) {
+  if (Node.param_empty())
+return true;
+
+  for (const auto *Param : Node.parameters()) {
+if (!Param->hasDefaultArg())
+  return false;
+  }
+  return true;
+}
+
+} // namespace
+
+void SmartptrResetAmbiguousCallCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IsSmartptr = hasAnyName("::std::unique_ptr", "::std::shared_ptr");
+
+  const auto ResetMethod =
+  cxxMethodDecl(hasName("reset"), hasOnlyDefaultArgs());
+
+  const auto TypeWithReset =
+  anyOf(cxxRecordDecl(hasMethod(ResetMethod)),
+classTemplateSpecializationDecl(
+hasSpecializedTemplate(classTemplateDecl(has(ResetMethod);
+
+  const auto SmartptrWithBugproneReset = classTemplateSpecializationDecl(
+  IsSmartptr,
+  hasTemplateArgument(
+  0, templateArgument(refersToType(hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(TypeWithReset)));
+
+  // Find a.reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(callee(memberExpr(member(hasName("reset",
+everyArgumentMatches(cxxDefaultArgExpr()),
+on(expr(hasType(SmartptrWithBugproneReset
+  .bind("smartptrResetCall"),
+  this);
+
+  // Find a->reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(
+  callee(memberExpr(
+  member(ResetMethod),
+  hasObjectExpression(
+  cxxOperatorCallExpr(
+  hasOverloadedOperatorName("->"),
+  hasArgument(
+  0, expr(hasType(
+ 
classTemplateSpecializationDecl(IsSmartptr)
+  .bind("OpCall",
+  everyArgumentMatches(cxxDefaultArgExpr()))
+  .bind("objectResetCall"),
+  this);
+}
+
+void SmartptrResetAmbiguousCallCheck::check(
+const MatchFinder::MatchResult &Result) {
+  const auto *SmartptrResetCall =
+  Result.Nodes.getNodeAs("smartptrResetCall");
+  const auto *ObjectResetCall =
+  Result.Nodes.getNodeAs("objectResetCall");
+
+  if (SmartptrResetCall) {
+const auto *Member = cast(SmartptrResetCall->getCallee());
+
+auto DiagBuilder = diag(SmartptrResetCall->getBeginLoc(),
+"bugprone 'reset()' call on smart pointer");
+
+DiagBuilder << FixItHint::CreateReplacement(
+SourceRange(Member->getOperatorLoc(),
+Member->getOperatorLoc().getLocWithOffset(0)),
+" =");
+
+DiagBuilder << FixItHint::CreateReplacement(
+SourceRange(Member->getMemberLoc(), SmartptrResetCall->getEndLoc()),
+" nullptr");
+  }
+
+  if (ObjectResetCall) {
+const auto *Arrow = Result.Nodes.getNodeAs("OpCall");
+
+const auto SmartptrSourceRange =

vbvictor wrote:

Done

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-01-09 Thread Baranov Victor via cfe-commits


@@ -0,0 +1,134 @@
+//===--- SmartptrResetAmbiguousCallCheck.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 "SmartptrResetAmbiguousCallCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  if (Node.getNumArgs() == 0)
+return true;
+
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+  return true;
+}
+
+AST_MATCHER(CXXMethodDecl, hasOnlyDefaultArgs) {
+  if (Node.param_empty())
+return true;
+
+  for (const auto *Param : Node.parameters()) {
+if (!Param->hasDefaultArg())
+  return false;
+  }
+  return true;
+}
+
+} // namespace
+
+void SmartptrResetAmbiguousCallCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IsSmartptr = hasAnyName("::std::unique_ptr", "::std::shared_ptr");
+
+  const auto ResetMethod =
+  cxxMethodDecl(hasName("reset"), hasOnlyDefaultArgs());
+
+  const auto TypeWithReset =
+  anyOf(cxxRecordDecl(hasMethod(ResetMethod)),
+classTemplateSpecializationDecl(
+hasSpecializedTemplate(classTemplateDecl(has(ResetMethod);
+
+  const auto SmartptrWithBugproneReset = classTemplateSpecializationDecl(
+  IsSmartptr,
+  hasTemplateArgument(
+  0, templateArgument(refersToType(hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(TypeWithReset)));
+
+  // Find a.reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(callee(memberExpr(member(hasName("reset",
+everyArgumentMatches(cxxDefaultArgExpr()),
+on(expr(hasType(SmartptrWithBugproneReset
+  .bind("smartptrResetCall"),
+  this);
+
+  // Find a->reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(
+  callee(memberExpr(
+  member(ResetMethod),
+  hasObjectExpression(
+  cxxOperatorCallExpr(
+  hasOverloadedOperatorName("->"),
+  hasArgument(
+  0, expr(hasType(
+ 
classTemplateSpecializationDecl(IsSmartptr)
+  .bind("OpCall",
+  everyArgumentMatches(cxxDefaultArgExpr()))
+  .bind("objectResetCall"),
+  this);
+}
+
+void SmartptrResetAmbiguousCallCheck::check(
+const MatchFinder::MatchResult &Result) {
+  const auto *SmartptrResetCall =
+  Result.Nodes.getNodeAs("smartptrResetCall");
+  const auto *ObjectResetCall =
+  Result.Nodes.getNodeAs("objectResetCall");

vbvictor wrote:

Done

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-01-09 Thread Baranov Victor via cfe-commits


@@ -0,0 +1,34 @@
+.. title:: clang-tidy - bugprone-smartptr-reset-ambiguous-call
+
+bugprone-smartptr-reset-ambiguous-call
+==
+
+Finds potentially erroneous calls to ``reset`` method on
+smart pointers when the pointee type also has a ``reset`` method.
+Having ``reset`` method in both classes makes it easy to accidentally

vbvictor wrote:

Done

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-01-09 Thread Baranov Victor via cfe-commits


@@ -0,0 +1,34 @@
+.. title:: clang-tidy - bugprone-smartptr-reset-ambiguous-call
+
+bugprone-smartptr-reset-ambiguous-call
+==
+
+Finds potentially erroneous calls to ``reset`` method on
+smart pointers when the pointee type also has a ``reset`` method.
+Having ``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 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:

vbvictor wrote:

Done

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-01-09 Thread Baranov Victor 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;

vbvictor wrote:

I added an option to specify custom smart pointers. Now `boost::shared_ptr` can 
be valid pointer for this check, so I did not change to `CPlusPlus11`.

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-01-09 Thread Baranov Victor via cfe-commits


@@ -0,0 +1,134 @@
+//===--- SmartptrResetAmbiguousCallCheck.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 "SmartptrResetAmbiguousCallCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  if (Node.getNumArgs() == 0)
+return true;
+
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+  return true;
+}
+
+AST_MATCHER(CXXMethodDecl, hasOnlyDefaultArgs) {
+  if (Node.param_empty())
+return true;
+
+  for (const auto *Param : Node.parameters()) {
+if (!Param->hasDefaultArg())
+  return false;
+  }
+  return true;
+}
+
+} // namespace
+
+void SmartptrResetAmbiguousCallCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IsSmartptr = hasAnyName("::std::unique_ptr", "::std::shared_ptr");
+
+  const auto ResetMethod =
+  cxxMethodDecl(hasName("reset"), hasOnlyDefaultArgs());
+
+  const auto TypeWithReset =
+  anyOf(cxxRecordDecl(hasMethod(ResetMethod)),
+classTemplateSpecializationDecl(
+hasSpecializedTemplate(classTemplateDecl(has(ResetMethod);
+
+  const auto SmartptrWithBugproneReset = classTemplateSpecializationDecl(
+  IsSmartptr,
+  hasTemplateArgument(
+  0, templateArgument(refersToType(hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(TypeWithReset)));
+
+  // Find a.reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(callee(memberExpr(member(hasName("reset",
+everyArgumentMatches(cxxDefaultArgExpr()),
+on(expr(hasType(SmartptrWithBugproneReset
+  .bind("smartptrResetCall"),
+  this);
+
+  // Find a->reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(
+  callee(memberExpr(
+  member(ResetMethod),
+  hasObjectExpression(
+  cxxOperatorCallExpr(
+  hasOverloadedOperatorName("->"),
+  hasArgument(
+  0, expr(hasType(
+ 
classTemplateSpecializationDecl(IsSmartptr)
+  .bind("OpCall",
+  everyArgumentMatches(cxxDefaultArgExpr()))
+  .bind("objectResetCall"),
+  this);
+}
+
+void SmartptrResetAmbiguousCallCheck::check(
+const MatchFinder::MatchResult &Result) {
+  const auto *SmartptrResetCall =
+  Result.Nodes.getNodeAs("smartptrResetCall");
+  const auto *ObjectResetCall =
+  Result.Nodes.getNodeAs("objectResetCall");
+
+  if (SmartptrResetCall) {
+const auto *Member = cast(SmartptrResetCall->getCallee());
+
+auto DiagBuilder = diag(SmartptrResetCall->getBeginLoc(),
+"bugprone 'reset()' call on smart pointer");
+
+DiagBuilder << FixItHint::CreateReplacement(
+SourceRange(Member->getOperatorLoc(),
+Member->getOperatorLoc().getLocWithOffset(0)),
+" =");
+
+DiagBuilder << FixItHint::CreateReplacement(
+SourceRange(Member->getMemberLoc(), SmartptrResetCall->getEndLoc()),
+" nullptr");
+  }

vbvictor wrote:

Done

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-01-09 Thread Baranov Victor via cfe-commits

https://github.com/vbvictor updated 
https://github.com/llvm/llvm-project/pull/121291

>From 37dce6a7ed0cca2e9819c24f4d176c43e3c9f2ac Mon Sep 17 00:00:00 2001
From: Victor Baranov 
Date: Sun, 29 Dec 2024 15:32:22 +0300
Subject: [PATCH 1/4] [clang-tidy] Add bugprone-reset-call check

---
 .../bugprone/BugproneTidyModule.cpp   |   2 +
 .../clang-tidy/bugprone/CMakeLists.txt|   1 +
 .../clang-tidy/bugprone/ResetCallCheck.cpp| 133 +++
 .../clang-tidy/bugprone/ResetCallCheck.h  |  34 +++
 clang-tools-extra/docs/ReleaseNotes.rst   |   6 +
 .../clang-tidy/checks/bugprone/reset-call.rst |  33 +++
 .../docs/clang-tidy/checks/list.rst   |   1 +
 .../checkers/bugprone/reset-call.cpp  | 215 ++
 8 files changed, 425 insertions(+)
 create mode 100644 clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.cpp
 create mode 100644 clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.h
 create mode 100644 
clang-tools-extra/docs/clang-tidy/checks/bugprone/reset-call.rst
 create mode 100644 
clang-tools-extra/test/clang-tidy/checkers/bugprone/reset-call.cpp

diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp 
b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 33ac65e715ce81..645958e47e22a5 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -57,6 +57,7 @@
 #include "PosixReturnCheck.h"
 #include "RedundantBranchConditionCheck.h"
 #include "ReservedIdentifierCheck.h"
+#include "ResetCallCheck.h"
 #include "ReturnConstRefFromParameterCheck.h"
 #include "SharedPtrArrayMismatchCheck.h"
 #include "SignalHandlerCheck.h"
@@ -144,6 +145,7 @@ class BugproneModule : public ClangTidyModule {
 "bugprone-inaccurate-erase");
 CheckFactories.registerCheck(
 "bugprone-incorrect-enable-if");
+CheckFactories.registerCheck("bugprone-reset-call");
 CheckFactories.registerCheck(
 "bugprone-return-const-ref-from-parameter");
 CheckFactories.registerCheck(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 13adad7c3dadbd..17ab5b27ec5550 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -52,6 +52,7 @@ add_clang_library(clangTidyBugproneModule STATIC
   PosixReturnCheck.cpp
   RedundantBranchConditionCheck.cpp
   ReservedIdentifierCheck.cpp
+  ResetCallCheck.cpp
   ReturnConstRefFromParameterCheck.cpp
   SharedPtrArrayMismatchCheck.cpp
   SignalHandlerCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.cpp
new file mode 100644
index 00..305ac8d51adf3e
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/ResetCallCheck.cpp
@@ -0,0 +1,133 @@
+//===--- ResetCallCheck.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 "ResetCallCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  if (Node.getNumArgs() == 0)
+return true;
+
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+  return true;
+}
+
+AST_MATCHER(CXXMethodDecl, hasOnlyDefaultArgs) {
+  if (Node.param_empty())
+return true;
+
+  for (const auto *Param : Node.parameters()) {
+if (!Param->hasDefaultArg())
+  return false;
+  }
+  return true;
+}
+
+} // namespace
+
+void ResetCallCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IsSmartptr = hasAnyName("::std::unique_ptr", "::std::shared_ptr");
+
+  const auto ResetMethod =
+  cxxMethodDecl(hasName("reset"), hasOnlyDefaultArgs());
+
+  const auto TypeWithReset =
+  anyOf(cxxRecordDecl(hasMethod(ResetMethod)),
+classTemplateSpecializationDecl(
+hasSpecializedTemplate(classTemplateDecl(has(ResetMethod);
+
+  const auto SmartptrWithBugproneReset = classTemplateSpecializationDecl(
+  IsSmartptr,
+  hasTemplateArgument(
+  0, templateArgument(refersToType(hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(TypeWithReset)));
+
+  // Find a.reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(callee(memberExpr(member(hasName("reset",
+ 

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

2024-12-31 Thread Baranov Victor via cfe-commits

https://github.com/vbvictor reopened 
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)

2024-12-31 Thread Baranov Victor via cfe-commits

vbvictor wrote:

> I think that this check is one that we don't want to emit fixes for, at least 
> not in the main diagnostic, but maybe as a fixit attached to a note

> What do others think?



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)

2024-12-31 Thread Baranov Victor via cfe-commits

https://github.com/vbvictor closed 
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)

2024-12-30 Thread Baranov Victor via cfe-commits

https://github.com/vbvictor edited 
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)

2024-12-30 Thread Baranov Victor via cfe-commits

https://github.com/vbvictor edited 
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)

2024-12-30 Thread via cfe-commits


@@ -0,0 +1,134 @@
+//===--- SmartptrResetAmbiguousCallCheck.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 "SmartptrResetAmbiguousCallCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+AST_MATCHER_P(CallExpr, everyArgumentMatches,
+  ast_matchers::internal::Matcher, InnerMatcher) {
+  if (Node.getNumArgs() == 0)
+return true;
+
+  for (const auto *Arg : Node.arguments()) {
+if (!InnerMatcher.matches(*Arg, Finder, Builder))
+  return false;
+  }
+  return true;
+}
+
+AST_MATCHER(CXXMethodDecl, hasOnlyDefaultArgs) {
+  if (Node.param_empty())
+return true;
+
+  for (const auto *Param : Node.parameters()) {
+if (!Param->hasDefaultArg())
+  return false;
+  }
+  return true;
+}
+
+} // namespace
+
+void SmartptrResetAmbiguousCallCheck::registerMatchers(MatchFinder *Finder) {
+  const auto IsSmartptr = hasAnyName("::std::unique_ptr", "::std::shared_ptr");
+
+  const auto ResetMethod =
+  cxxMethodDecl(hasName("reset"), hasOnlyDefaultArgs());
+
+  const auto TypeWithReset =
+  anyOf(cxxRecordDecl(hasMethod(ResetMethod)),
+classTemplateSpecializationDecl(
+hasSpecializedTemplate(classTemplateDecl(has(ResetMethod);
+
+  const auto SmartptrWithBugproneReset = classTemplateSpecializationDecl(
+  IsSmartptr,
+  hasTemplateArgument(
+  0, templateArgument(refersToType(hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(TypeWithReset)));
+
+  // Find a.reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(callee(memberExpr(member(hasName("reset",
+everyArgumentMatches(cxxDefaultArgExpr()),
+on(expr(hasType(SmartptrWithBugproneReset
+  .bind("smartptrResetCall"),
+  this);
+
+  // Find a->reset() calls
+  Finder->addMatcher(
+  cxxMemberCallExpr(
+  callee(memberExpr(
+  member(ResetMethod),
+  hasObjectExpression(
+  cxxOperatorCallExpr(
+  hasOverloadedOperatorName("->"),
+  hasArgument(
+  0, expr(hasType(
+ 
classTemplateSpecializationDecl(IsSmartptr)
+  .bind("OpCall",
+  everyArgumentMatches(cxxDefaultArgExpr()))
+  .bind("objectResetCall"),
+  this);
+}
+
+void SmartptrResetAmbiguousCallCheck::check(
+const MatchFinder::MatchResult &Result) {
+  const auto *SmartptrResetCall =
+  Result.Nodes.getNodeAs("smartptrResetCall");
+  const auto *ObjectResetCall =
+  Result.Nodes.getNodeAs("objectResetCall");
+
+  if (SmartptrResetCall) {
+const auto *Member = cast(SmartptrResetCall->getCallee());
+
+auto DiagBuilder = diag(SmartptrResetCall->getBeginLoc(),
+"bugprone 'reset()' call on smart pointer");
+
+DiagBuilder << FixItHint::CreateReplacement(
+SourceRange(Member->getOperatorLoc(),
+Member->getOperatorLoc().getLocWithOffset(0)),
+" =");
+
+DiagBuilder << FixItHint::CreateReplacement(
+SourceRange(Member->getMemberLoc(), SmartptrResetCall->getEndLoc()),
+" nullptr");
+  }
+
+  if (ObjectResetCall) {
+const auto *Arrow = Result.Nodes.getNodeAs("OpCall");
+
+const auto SmartptrSourceRange =

EugeneZelenko wrote:

`auto` should not be used here - type is not spelled explicitly.

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)

2024-12-30 Thread Baranov Victor 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;

vbvictor wrote:

As for now, this check only matches `::std::unique_ptr` and 
`::std::shared_ptr`, so this comment is valid.
In the future, I think a list of smartptr-like classes should be an option for 
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)

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


  1   2   >