Skip to content

[clang-tidy] Create bugprone-incorrect-enable-shared-from-this check #102299

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
062fcab
[clang-tidy] Add bugprone-public-enable-shared-from-this check
MichelleCDjunaidi Aug 7, 2024
acf73fd
Fix test to be compatible with llvm-lit, add docs
MichelleCDjunaidi Aug 7, 2024
52186f8
Change name to bugprone-incorrect-enable-shared-from-this.
MichelleCDjunaidi Aug 8, 2024
2f05bbc
Fix ``FixItHint`` of public inheritance, implement ``std::`` check
MichelleCDjunaidi Aug 8, 2024
772a8eb
Stated type instead of using auto in the ``check`` method
MichelleCDjunaidi Aug 8, 2024
7aa9232
Refactored const variables and cleanup newlines/formatting
MichelleCDjunaidi Aug 12, 2024
47a6f64
Fix documentation, comments, newlines, sort module registration properly
MichelleCDjunaidi Aug 13, 2024
65eb824
Switch implementation to RecursiveASTVisitor
MichelleCDjunaidi Aug 14, 2024
a61eff9
Fix test case formatting
MichelleCDjunaidi Aug 14, 2024
5d489b8
Fix check code formatting to follow LLVM style
MichelleCDjunaidi Aug 14, 2024
9bbc97b
Optimize traversal, add alias and typedef testing, remove missing std…
MichelleCDjunaidi Aug 16, 2024
3f84574
refactor: simplify to VisitCXXRecordDecl only instead of TraverseCXXR…
MichelleCDjunaidi Aug 26, 2024
7f964ee
refactor: exclude non-definitions from repeated processing
MichelleCDjunaidi Aug 26, 2024
c7d8559
more detailed diag for classes with bases from EnableSharedClassSet
MichelleCDjunaidi Aug 26, 2024
5849a10
refactor to use diag %select, fix docs typo
MichelleCDjunaidi Sep 12, 2024
15b381a
add space to code block in docs
MichelleCDjunaidi Sep 12, 2024
b95ad5d
update test CHECK-MESSAGES to check for class name
MichelleCDjunaidi Sep 12, 2024
05017de
[NFC] in progress change diag
MichelleCDjunaidi Sep 22, 2024
a2c5cff
add macros to test file and revise CHECK-MESSAGES
MichelleCDjunaidi Sep 22, 2024
fd044a2
change auto vars to explicit typing
MichelleCDjunaidi Sep 23, 2024
10c1933
update docs to fix wording
MichelleCDjunaidi Oct 9, 2024
ff2d2ef
change to matcher-based check and cleanup headers
MichelleCDjunaidi Oct 14, 2024
415fb12
synchronize check documentation wording
MichelleCDjunaidi Oct 14, 2024
9e7689b
sort ReleaseNotes.rst new checks by alphabetical
MichelleCDjunaidi Oct 25, 2024
3978355
Change fix to emit only on direct inheritance
MichelleCDjunaidi Nov 9, 2024
ebffebb
reworded first sentence of docs
MichelleCDjunaidi Nov 9, 2024
b38109c
additional macro testing
MichelleCDjunaidi Nov 9, 2024
f915a0c
refactor add newline on eof of test file
MichelleCDjunaidi Nov 9, 2024
93fa16c
refactor to only replace access specifier
MichelleCDjunaidi Nov 9, 2024
fb1b306
refactor to remove duplicated arg
MichelleCDjunaidi Nov 9, 2024
1567de5
Merge remote-tracking branch 'origin/main' into HEAD
MichelleCDjunaidi Nov 9, 2024
a10c4e1
Merge branch 'main' into clang-tidy-create-bugprone-public-enable-sha…
MichelleCDjunaidi Nov 26, 2024
a09955d
Merge branch 'main' into clang-tidy-create-bugprone-public-enable-sha…
MichelleCDjunaidi Nov 26, 2024
33c1cad
Merge remote-tracking branch 'origin/main' into clang-tidy-create-bug…
MichelleCDjunaidi Jan 1, 2025
d8f9563
exclude system header and update test flag
MichelleCDjunaidi Jan 10, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "InaccurateEraseCheck.h"
#include "IncDecInConditionsCheck.h"
#include "IncorrectEnableIfCheck.h"
#include "IncorrectEnableSharedFromThisCheck.h"
#include "IncorrectRoundingsCheck.h"
#include "InfiniteLoopCheck.h"
#include "IntegerDivisionCheck.h"
Expand Down Expand Up @@ -144,6 +145,8 @@ class BugproneModule : public ClangTidyModule {
"bugprone-inaccurate-erase");
CheckFactories.registerCheck<IncorrectEnableIfCheck>(
"bugprone-incorrect-enable-if");
CheckFactories.registerCheck<IncorrectEnableSharedFromThisCheck>(
"bugprone-incorrect-enable-shared-from-this");
CheckFactories.registerCheck<ReturnConstRefFromParameterCheck>(
"bugprone-return-const-ref-from-parameter");
CheckFactories.registerCheck<SwitchMissingDefaultCaseCheck>(
Expand Down
5 changes: 5 additions & 0 deletions clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ add_clang_library(clangTidyBugproneModule STATIC
ForwardingReferenceOverloadCheck.cpp
ImplicitWideningOfMultiplicationResultCheck.cpp
InaccurateEraseCheck.cpp
IncorrectEnableIfCheck.cpp
IncorrectEnableSharedFromThisCheck.cpp
ReturnConstRefFromParameterCheck.cpp
SuspiciousStringviewDataUsageCheck.cpp
SwitchMissingDefaultCaseCheck.cpp
IncDecInConditionsCheck.cpp
IncorrectEnableIfCheck.cpp
IncorrectRoundingsCheck.cpp
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
//===--- IncorrectEnableSharedFromThisCheck.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 "IncorrectEnableSharedFromThisCheck.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/DeclCXX.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"

using namespace clang::ast_matchers;

namespace clang::tidy::bugprone {

void IncorrectEnableSharedFromThisCheck::registerMatchers(MatchFinder *Finder) {
const auto EnableSharedFromThis =
cxxRecordDecl(hasName("enable_shared_from_this"), isInStdNamespace());
const auto QType = hasCanonicalType(hasDeclaration(
cxxRecordDecl(
anyOf(EnableSharedFromThis.bind("enable_rec"),
cxxRecordDecl(hasAnyBase(cxxBaseSpecifier(
isPublic(), hasType(hasCanonicalType(
hasDeclaration(EnableSharedFromThis))))))))
.bind("base_rec")));
Finder->addMatcher(
cxxRecordDecl(
unless(isExpansionInSystemHeader()),
hasDirectBase(cxxBaseSpecifier(unless(isPublic()), hasType(QType))
.bind("base")))
.bind("derived"),
this);
}

void IncorrectEnableSharedFromThisCheck::check(
const MatchFinder::MatchResult &Result) {
const auto *BaseSpec = Result.Nodes.getNodeAs<CXXBaseSpecifier>("base");
const auto *Base = Result.Nodes.getNodeAs<CXXRecordDecl>("base_rec");
const auto *Derived = Result.Nodes.getNodeAs<CXXRecordDecl>("derived");
const bool IsEnableSharedFromThisDirectBase =
Result.Nodes.getNodeAs<CXXRecordDecl>("enable_rec") == Base;
const bool HasWrittenAccessSpecifier =
BaseSpec->getAccessSpecifierAsWritten() != AS_none;
const auto ReplacementRange = CharSourceRange(
SourceRange(BaseSpec->getBeginLoc()), HasWrittenAccessSpecifier);
const llvm::StringRef Replacement =
HasWrittenAccessSpecifier ? "public" : "public ";
const FixItHint Hint =
IsEnableSharedFromThisDirectBase
? FixItHint::CreateReplacement(ReplacementRange, Replacement)
: FixItHint();
diag(Derived->getLocation(),
"%2 is not publicly inheriting from "
"%select{%1 which inherits from |}0'std::enable_shared_"
"from_this', "
"which will cause unintended behaviour "
"when using 'shared_from_this'; make the inheritance "
"public",
DiagnosticIDs::Warning)
<< IsEnableSharedFromThisDirectBase << Base << Derived << Hint;
}

} // namespace clang::tidy::bugprone
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
//===--- IncorrectEnableSharedFromThisCheck.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_INCORRECTENABLESHAREDFROMTHISCHECK_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INCORRECTENABLESHAREDFROMTHISCHECK_H

#include "../ClangTidyCheck.h"

namespace clang::tidy::bugprone {

/// Detect classes or structs that do not publicly inherit from
/// ``std::enable_shared_from_this``, because unintended behavior will
/// otherwise occur when calling ``shared_from_this``.
///
/// For the user-facing documentation see:
/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/incorrect-enable-shared-from-this.html
class IncorrectEnableSharedFromThisCheck : public ClangTidyCheck {
public:
IncorrectEnableSharedFromThisCheck(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.CPlusPlus11;
}
};

} // namespace clang::tidy::bugprone

#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INCORRECTENABLESHAREDFROMTHISCHECK_H
7 changes: 7 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,13 @@ New checks
Warns about code that tries to cast between pointers by means of
``std::bit_cast`` or ``memcpy``.

- New :doc:`bugprone-incorrect-enable-shared-from-this
<clang-tidy/checks/bugprone/incorrect-enable-shared-from-this>` check.

Detect classes or structs that do not publicly inherit from
``std::enable_shared_from_this``, because unintended behavior will
otherwise occur when calling ``shared_from_this``.

- New :doc:`bugprone-nondeterministic-pointer-iteration-order
<clang-tidy/checks/bugprone/nondeterministic-pointer-iteration-order>`
check.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
.. title:: clang-tidy - bugprone-incorrect-enable-shared-from-this

bugprone-incorrect-enable-shared-from-this
==========================================

Detect classes or structs that do not publicly inherit from
``std::enable_shared_from_this``, because unintended behavior will
otherwise occur when calling ``shared_from_this``.

Consider the following code:

.. code-block:: c++

#include <memory>

// private inheritance
class BadExample : std::enable_shared_from_this<BadExample> {

// ``shared_from_this``` unintended behaviour
// `libstdc++` implementation returns uninitialized ``weak_ptr``
public:
BadExample* foo() { return shared_from_this().get(); }
void bar() { return; }
};

void using_not_public() {
auto bad_example = std::make_shared<BadExample>();
auto* b_ex = bad_example->foo();
b_ex->bar();
}

Using `libstdc++` implementation, ``shared_from_this`` will throw
``std::bad_weak_ptr``. When ``using_not_public()`` is called, this code will
crash without exception handling.
1 change: 1 addition & 0 deletions clang-tools-extra/docs/clang-tidy/checks/list.rst
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ Clang-Tidy Checks
:doc:`bugprone-inaccurate-erase <bugprone/inaccurate-erase>`, "Yes"
:doc:`bugprone-inc-dec-in-conditions <bugprone/inc-dec-in-conditions>`,
:doc:`bugprone-incorrect-enable-if <bugprone/incorrect-enable-if>`, "Yes"
:doc:`bugprone-incorrect-enable-shared-from-this <bugprone/incorrect-enable-shared-from-this>`, "Yes"
:doc:`bugprone-incorrect-roundings <bugprone/incorrect-roundings>`,
:doc:`bugprone-infinite-loop <bugprone/infinite-loop>`,
:doc:`bugprone-integer-division <bugprone/integer-division>`,
Expand Down
Loading
Loading