Skip to content

[clang-tidy] add 'IgnoreMarcos' option to 'special-member-functions' check #143550

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
merged 3 commits into from
Jun 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ using namespace clang::ast_matchers;

namespace clang::tidy::cppcoreguidelines {

namespace {
AST_MATCHER(CXXRecordDecl, isInMacro) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(For another commit) This comes up often, maybe we can create an AST matcher for this that can be reused?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I was thinking lately about it.
I've seen some matchers use only Node.getBeginLoc().isMacroID() others use Node.getBeginLoc().isMacroID() && Node.getEndLoc().isMacroID() so we need to decide on the wanted behavior.
I guess we should place both Begin and End inside a macro

return Node.getBeginLoc().isMacroID() && Node.getEndLoc().isMacroID();
}
} // namespace

SpecialMemberFunctionsCheck::SpecialMemberFunctionsCheck(
StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context), AllowMissingMoveFunctions(Options.get(
Expand All @@ -26,7 +32,8 @@ SpecialMemberFunctionsCheck::SpecialMemberFunctionsCheck(
AllowMissingMoveFunctionsWhenCopyIsDeleted(
Options.get("AllowMissingMoveFunctionsWhenCopyIsDeleted", false)),
AllowImplicitlyDeletedCopyOrMove(
Options.get("AllowImplicitlyDeletedCopyOrMove", false)) {}
Options.get("AllowImplicitlyDeletedCopyOrMove", false)),
IgnoreMacros(Options.get("IgnoreMacros", true)) {}

void SpecialMemberFunctionsCheck::storeOptions(
ClangTidyOptions::OptionMap &Opts) {
Expand All @@ -36,6 +43,7 @@ void SpecialMemberFunctionsCheck::storeOptions(
AllowMissingMoveFunctionsWhenCopyIsDeleted);
Options.store(Opts, "AllowImplicitlyDeletedCopyOrMove",
AllowImplicitlyDeletedCopyOrMove);
Options.store(Opts, "IgnoreMacros", IgnoreMacros);
}

std::optional<TraversalKind>
Expand All @@ -45,11 +53,12 @@ SpecialMemberFunctionsCheck::getCheckTraversalKind() const {
}

void SpecialMemberFunctionsCheck::registerMatchers(MatchFinder *Finder) {
auto IsNotImplicitOrDeleted = anyOf(unless(isImplicit()), isDeleted());
const auto IsNotImplicitOrDeleted = anyOf(unless(isImplicit()), isDeleted());
const ast_matchers::internal::Matcher<CXXRecordDecl> Anything = anything();

Finder->addMatcher(
cxxRecordDecl(
unless(isImplicit()),
unless(isImplicit()), IgnoreMacros ? unless(isInMacro()) : Anything,
eachOf(has(cxxDestructorDecl(unless(isImplicit())).bind("dtor")),
has(cxxConstructorDecl(isCopyConstructor(),
IsNotImplicitOrDeleted)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ class SpecialMemberFunctionsCheck : public ClangTidyCheck {
const bool AllowMissingMoveFunctionsWhenCopyIsDeleted;
const bool AllowImplicitlyDeletedCopyOrMove;
ClassDefiningSpecialMembersMap ClassWithSpecialMembers;
const bool IgnoreMacros;
};

} // namespace clang::tidy::cppcoreguidelines
Expand Down
8 changes: 8 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,10 @@ Changes in existing checks
<clang-tidy/checks/cppcoreguidelines/avoid-goto>` check by adding the option
`IgnoreMacros` to ignore ``goto`` labels defined in macros.

- Improved :doc:`cppcoreguidelines-special-member-functions
<clang-tidy/checks/cppcoreguidelines/special-member-functions>` check by
adding the option `IgnoreMacros` to ignore classes defined in macros.

- Improved :doc:`google-readability-namespace-comments
<clang-tidy/checks/google/readability-namespace-comments>` check by adding
the option `AllowOmittingNamespaceComments` to accept if a namespace comment
Expand All @@ -216,6 +220,10 @@ Changes in existing checks
<clang-tidy/checks/hicpp/avoid-goto>` check by adding the option
`IgnoreMacros` to ignore ``goto`` labels defined in macros.

- Improved :doc:`hicpp-special-member-functions
<clang-tidy/checks/hicpp/special-member-functions>` check by adding the
option `IgnoreMacros` to ignore classes defined in macros.

- Improved :doc:`llvm-namespace-comment
<clang-tidy/checks/llvm/namespace-comment>` check by adding the option
`AllowOmittingNamespaceComments` to accept if a namespace comment is omitted
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,3 +85,8 @@ Options
struct A : boost::noncopyable {
~A() { std::cout << "dtor\n"; }
};

.. option:: IgnoreMacros

If set to `true`, the check will not give warnings for classes defined
inside macros. Default is `true`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
// RUN: %check_clang_tidy %s cppcoreguidelines-special-member-functions %t -- -config="{CheckOptions: {cppcoreguidelines-special-member-functions.IgnoreMacros: false}}" --

class DefinesDestructor {
~DefinesDestructor();
};
// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDestructor' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]

class DefinesDefaultedDestructor {
~DefinesDefaultedDestructor() = default;
};
// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesDefaultedDestructor' defines a default destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]

class DefinesCopyConstructor {
DefinesCopyConstructor(const DefinesCopyConstructor &);
};
// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'DefinesCopyConstructor' defines a copy constructor but does not define a destructor, a copy assignment operator, a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]

class DefinesNothing {
};

class DefinesEverything {
DefinesEverything(const DefinesEverything &);
DefinesEverything &operator=(const DefinesEverything &);
DefinesEverything(DefinesEverything &&);
DefinesEverything &operator=(DefinesEverything &&);
~DefinesEverything();
};

#define DEFINE_DESTRUCTOR_ONLY(ClassName) \
class ClassName { \
~ClassName(); \
};

#define DEFINE_COPY_CTOR_ONLY(ClassName) \
class ClassName { \
ClassName(const ClassName &); \
};

#define DEFINE_CLASS_WITH_DTOR(ClassName) \
class ClassName { \
~ClassName(); \
};

DEFINE_DESTRUCTOR_ONLY(MacroDefinedClass1)
// CHECK-MESSAGES: [[@LINE-1]]:24: warning: class 'MacroDefinedClass1' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator
DEFINE_COPY_CTOR_ONLY(MacroDefinedClass2)
// CHECK-MESSAGES: [[@LINE-1]]:23: warning: class 'MacroDefinedClass2' defines a copy constructor but does not define a destructor, a copy assignment operator, a move constructor or a move assignment operator
DEFINE_CLASS_WITH_DTOR(MacroDefinedClass3)
// CHECK-MESSAGES: [[@LINE-1]]:24: warning: class 'MacroDefinedClass3' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator

// Test partial macro expansion
#define CLASS_NAME MacroNamedClass
class CLASS_NAME {
~MacroNamedClass();
};
// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'MacroNamedClass' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator

Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,29 @@ struct TemplateClass {
// This should not cause problems.
TemplateClass<int> InstantiationWithInt;
TemplateClass<double> InstantiationWithDouble;

#define DEFINE_DESTRUCTOR_ONLY(ClassName) \
class ClassName { \
~ClassName(); \
};

#define DEFINE_COPY_CTOR_ONLY(ClassName) \
class ClassName { \
ClassName(const ClassName &); \
};

#define DEFINE_CLASS_WITH_DTOR(ClassName) \
class ClassName { \
~ClassName(); \
};

DEFINE_DESTRUCTOR_ONLY(MacroDefinedClass1)
DEFINE_COPY_CTOR_ONLY(MacroDefinedClass2)
DEFINE_CLASS_WITH_DTOR(MacroDefinedClass3)

// Test partial macro expansion
#define CLASS_NAME MacroNamedClass
class CLASS_NAME {
~MacroNamedClass();
};
// CHECK-MESSAGES: [[@LINE-3]]:7: warning: class 'MacroNamedClass' defines a destructor but does not define a copy constructor, a copy assignment operator, a move constructor or a move assignment operator
Loading