Skip to content

Commit 0e95921

Browse files
balazskebalazske
committed
[clang-tidy] Improve check cert-dcl58-cpp.
Detect template specializations that should be handled specially. In some cases it is allowed to extend the `std` namespace with template specializations. Reviewed By: aaron.ballman Differential Revision: https://reviews.llvm.org/D129353
1 parent cb2c8f6 commit 0e95921

File tree

5 files changed

+426
-37
lines changed

5 files changed

+426
-37
lines changed

‎clang-tools-extra/clang-tidy/cert/DontModifyStdNamespaceCheck.cpp

Lines changed: 108 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,37 +9,126 @@
99
#include "DontModifyStdNamespaceCheck.h"
1010
#include "clang/AST/ASTContext.h"
1111
#include "clang/ASTMatchers/ASTMatchFinder.h"
12+
#include "clang/ASTMatchers/ASTMatchersInternal.h"
1213

14+
using namespace clang;
1315
using namespace clang::ast_matchers;
1416

17+
namespace {
18+
19+
AST_POLYMORPHIC_MATCHER_P(
20+
hasAnyTemplateArgumentIncludingPack,
21+
AST_POLYMORPHIC_SUPPORTED_TYPES(ClassTemplateSpecializationDecl,
22+
TemplateSpecializationType, FunctionDecl),
23+
clang::ast_matchers::internal::Matcher<TemplateArgument>, InnerMatcher) {
24+
ArrayRef<TemplateArgument> Args =
25+
clang::ast_matchers::internal::getTemplateSpecializationArgs(Node);
26+
for (const auto &Arg : Args) {
27+
if (Arg.getKind() != TemplateArgument::Pack)
28+
continue;
29+
ArrayRef<TemplateArgument> PackArgs = Arg.getPackAsArray();
30+
if (matchesFirstInRange(InnerMatcher, PackArgs.begin(), PackArgs.end(),
31+
Finder, Builder) != PackArgs.end())
32+
return true;
33+
}
34+
return matchesFirstInRange(InnerMatcher, Args.begin(), Args.end(), Finder,
35+
Builder) != Args.end();
36+
}
37+
38+
} // namespace
39+
1540
namespace clang {
1641
namespace tidy {
1742
namespace cert {
1843

1944
void DontModifyStdNamespaceCheck::registerMatchers(MatchFinder *Finder) {
20-
Finder->addMatcher(
21-
namespaceDecl(unless(isExpansionInSystemHeader()),
22-
hasAnyName("std", "posix"),
23-
has(decl(unless(anyOf(
24-
functionDecl(isExplicitTemplateSpecialization()),
25-
cxxRecordDecl(isExplicitTemplateSpecialization()))))))
26-
.bind("nmspc"),
27-
this);
28-
}
45+
auto HasStdParent =
46+
hasDeclContext(namespaceDecl(hasAnyName("std", "posix"),
47+
unless(hasParent(namespaceDecl())))
48+
.bind("nmspc"));
49+
auto UserDefinedType = qualType(
50+
hasUnqualifiedDesugaredType(tagType(unless(hasDeclaration(tagDecl(
51+
hasAncestor(namespaceDecl(hasAnyName("std", "posix"),
52+
unless(hasParent(namespaceDecl()))))))))));
53+
auto HasNoProgramDefinedTemplateArgument = unless(
54+
hasAnyTemplateArgumentIncludingPack(refersToType(UserDefinedType)));
55+
auto InsideStdClassOrClassTemplateSpecialization = hasDeclContext(
56+
anyOf(cxxRecordDecl(HasStdParent),
57+
classTemplateSpecializationDecl(
58+
HasStdParent, HasNoProgramDefinedTemplateArgument)));
2959

30-
void DontModifyStdNamespaceCheck::check(
31-
const MatchFinder::MatchResult &Result) {
32-
const auto *N = Result.Nodes.getNodeAs<NamespaceDecl>("nmspc");
60+
// Try to follow exactly CERT rule DCL58-CPP (this text is taken from C++
61+
// standard into the CERT rule):
62+
// "
63+
// 1 The behavior of a C++ program is undefined if it adds declarations or
64+
// definitions to namespace std or to a namespace within namespace std unless
65+
// otherwise specified. A program may add a template specialization for any
66+
// standard library template to namespace std only if the declaration depends
67+
// on a user-defined type and the specialization meets the standard library
68+
// requirements for the original template and is not explicitly prohibited. 2
69+
// The behavior of a C++ program is undefined if it declares — an explicit
70+
// specialization of any member function of a standard library class template,
71+
// or — an explicit specialization of any member function template of a
72+
// standard library class or class template, or — an explicit or partial
73+
// specialization of any member class template of a standard library class or
74+
// class template.
75+
// "
76+
// The "standard library requirements" and explicit prohibition are not
77+
// checked.
3378

34-
// Only consider top level namespaces.
35-
if (N->getParent() != Result.Context->getTranslationUnitDecl())
36-
return;
79+
auto BadNonTemplateSpecializationDecl =
80+
decl(unless(anyOf(functionDecl(isExplicitTemplateSpecialization()),
81+
varDecl(isExplicitTemplateSpecialization()),
82+
cxxRecordDecl(isExplicitTemplateSpecialization()))),
83+
HasStdParent);
84+
auto BadClassTemplateSpec = classTemplateSpecializationDecl(
85+
HasNoProgramDefinedTemplateArgument, HasStdParent);
86+
auto BadInnerClassTemplateSpec = classTemplateSpecializationDecl(
87+
InsideStdClassOrClassTemplateSpecialization);
88+
auto BadFunctionTemplateSpec =
89+
functionDecl(unless(cxxMethodDecl()), isExplicitTemplateSpecialization(),
90+
HasNoProgramDefinedTemplateArgument, HasStdParent);
91+
auto BadMemberFunctionSpec =
92+
cxxMethodDecl(isExplicitTemplateSpecialization(),
93+
InsideStdClassOrClassTemplateSpecialization);
3794

38-
diag(N->getLocation(),
39-
"modification of %0 namespace can result in undefined behavior")
40-
<< N;
95+
Finder->addMatcher(decl(anyOf(BadNonTemplateSpecializationDecl,
96+
BadClassTemplateSpec, BadInnerClassTemplateSpec,
97+
BadFunctionTemplateSpec, BadMemberFunctionSpec),
98+
unless(isExpansionInSystemHeader()))
99+
.bind("decl"),
100+
this);
41101
}
42-
43102
} // namespace cert
44103
} // namespace tidy
45104
} // namespace clang
105+
106+
static const NamespaceDecl *getTopLevelLexicalNamespaceDecl(const Decl *D) {
107+
const NamespaceDecl *LastNS = nullptr;
108+
while (D) {
109+
if (const auto *NS = dyn_cast<NamespaceDecl>(D))
110+
LastNS = NS;
111+
D = dyn_cast_or_null<Decl>(D->getLexicalDeclContext());
112+
}
113+
return LastNS;
114+
}
115+
116+
void clang::tidy::cert::DontModifyStdNamespaceCheck::check(
117+
const MatchFinder::MatchResult &Result) {
118+
const auto *D = Result.Nodes.getNodeAs<Decl>("decl");
119+
const auto *NS = Result.Nodes.getNodeAs<NamespaceDecl>("nmspc");
120+
if (!D || !NS)
121+
return;
122+
123+
diag(D->getLocation(),
124+
"modification of %0 namespace can result in undefined behavior")
125+
<< NS;
126+
// 'NS' is not always the namespace declaration that lexically contains 'D',
127+
// try to find such a namespace.
128+
if (const NamespaceDecl *LexNS = getTopLevelLexicalNamespaceDecl(D)) {
129+
assert(NS->getCanonicalDecl() == LexNS->getCanonicalDecl() &&
130+
"Mismatch in found namespace");
131+
diag(LexNS->getLocation(), "%0 namespace opened here", DiagnosticIDs::Note)
132+
<< LexNS;
133+
}
134+
}

‎clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,11 @@ Changes in existing checks
183183

184184
- Don't emit an erroneous warning on self-moves.
185185

186+
- Improved :doc:`cert-dcl58-cpp
187+
<clang-tidy/checks/cert/dcl58-cpp>` check.
188+
189+
The check now detects explicit template specializations that are handled specially.
190+
186191
- Made :doc:`cert-oop57-cpp <clang-tidy/checks/cert/oop57-cpp>` more sensitive
187192
by checking for an arbitrary expression in the second argument of ``memset``.
188193

‎clang-tools-extra/docs/clang-tidy/checks/cert/dcl58-cpp.rst

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,54 @@ cert-dcl58-cpp
66
Modification of the ``std`` or ``posix`` namespace can result in undefined
77
behavior.
88
This check warns for such modifications.
9+
The ``std`` (or ``posix``) namespace is allowed to be extended with (class or
10+
function) template specializations that depend on an user-defined type (a type
11+
that is not defined in the standard system headers).
12+
13+
The check detects the following (user provided) declarations in namespace ``std`` or ``posix``:
14+
15+
- Anything that is not a template specialization.
16+
- Explicit specializations of any standard library function template or class template, if it does not have any user-defined type as template argument.
17+
- Explicit specializations of any member function of a standard library class template.
18+
- Explicit specializations of any member function template of a standard library class or class template.
19+
- Explicit or partial specialization of any member class template of a standard library class or class template.
920

1021
Examples:
1122

1223
.. code-block:: c++
1324

1425
namespace std {
15-
int x; // May cause undefined behavior.
26+
int x; // warning: modification of 'std' namespace can result in undefined behavior [cert-dcl58-cpp]
1627
}
1728

29+
namespace posix::a { // warning: modification of 'posix' namespace can result in undefined behavior
30+
}
31+
32+
template <>
33+
struct ::std::hash<long> { // warning: modification of 'std' namespace can result in undefined behavior
34+
unsigned long operator()(const long &K) const {
35+
return K;
36+
}
37+
};
38+
39+
struct MyData { long data; };
40+
41+
template <>
42+
struct ::std::hash<MyData> { // no warning: specialization with user-defined type
43+
unsigned long operator()(const MyData &K) const {
44+
return K.data;
45+
}
46+
};
47+
48+
namespace std {
49+
template <>
50+
void swap<bool>(bool &a, bool &b); // warning: modification of 'std' namespace can result in undefined behavior
51+
52+
template <>
53+
bool less<void>::operator()<MyData &&, MyData &&>(MyData &&, MyData &&) const { // warning: modification of 'std' namespace can result in undefined behavior
54+
return true;
55+
}
56+
}
1857

1958
This check corresponds to the CERT C++ Coding Standard rule
2059
`DCL58-CPP. Do not modify the standard namespaces

‎clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/system-header-simulation.h

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,60 @@ using false_type = bool_constant<false>;
1818
template<class T>
1919
struct is_error_code_enum : false_type {};
2020

21-
template<class T>
21+
template <class T>
2222
void swap(T &a, T &b);
23+
24+
enum class io_errc {
25+
stream = 1,
26+
};
27+
28+
template <class... Types>
29+
class tuple;
30+
31+
template <typename T = void>
32+
class less;
33+
34+
template <>
35+
class less<void> {
36+
public:
37+
template <typename T, typename U>
38+
bool operator()(T &&Lhs, U &&Rhs) const {
39+
return static_cast<T &&>(Lhs) < static_cast<U &&>(Rhs);
40+
}
41+
template <typename A, typename B = int>
42+
struct X {};
43+
};
44+
45+
template <class Key>
46+
struct hash;
47+
48+
template <class T>
49+
class numeric_limits;
50+
51+
struct Outer {
52+
struct Inner {};
53+
};
54+
55+
namespace detail {
56+
struct X {};
57+
} // namespace detail
58+
59+
} // namespace std
60+
61+
// Template specializations that are in a system-header file.
62+
// The purpose is to test cert-dcl58-cpp (no warnings here).
63+
namespace std {
64+
template <>
65+
void swap<short>(short &, short &){};
66+
67+
template <>
68+
struct is_error_code_enum<short> : true_type {};
69+
70+
template <>
71+
bool less<void>::operator()<short &&, short &&>(short &&, short &&) const {
72+
return false;
2373
}
2474

75+
template <>
76+
struct less<void>::X<short> {};
77+
} // namespace std

0 commit comments

Comments
 (0)