aaron.ballman added a comment.
Thank you for working on this check! There is a slight problem, however, in
that the check as-written will flag false positives because there are times
when it is permissible to modify the std namespace. The important bit of the
CERT requirement is "Do not add declarations or definitions to the standard
namespaces std or posix, or to a namespace contained therein, except for a
template specialization that depends on a user-defined type that meets the
standard library requirements for the original template." So the part that's
missing from this check is excluding template specializations that depend on
user-defined types. For instance, the following should be valid:
namespace std {
template <>
struct is_error_code_enum<llvm::object::object_error> : std::true_type {};
}
(This comes from Error.h in LLVM.)
================
Comment at: clang-tidy/cert/CERTTidyModule.cpp:37-38
@@ -35,2 +36,4 @@
// DCL
+ CheckFactories.registerCheck<DontModifyStdNamespaceCheck>(
+ "cert-msc53-cpp");
CheckFactories.registerCheck<VariadicFunctionDefCheck>(
----------------
Should put this under a `// MSC` comment rather than the `// DCL` one.
================
Comment at: clang-tidy/cert/DontModifyStdNamespaceCheck.cpp:33
@@ +32,3 @@
+
+ diag(Nmspc->getLocation(), "Modification of " + namespaceName +
+ " namespace can result to undefined
behavior");
----------------
Should use format specifiers instead of building the string manually. e.g.,
using %0. Also, diagnostics should start with a lowercase letter and "to"
should be "in".
================
Comment at: clang-tidy/cert/DontModifyStdNamespaceCheck.h:19
@@ +18,3 @@
+
+/// Modification of the std or posix namespace can result to undefined
behavior.
+/// This checker warns for such modifications.
----------------
s/to/in
================
Comment at: docs/clang-tidy/checks/list.rst:12
@@ -11,2 +11,3 @@
cert-dcl59-cpp (redirects to google-build-namespaces) <cert-dcl59-cpp>
+ cert-msc53-cpp
cert-env33-c
----------------
Should keep this in alphabetical order.
Repository:
rL LLVM
https://reviews.llvm.org/D23421
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
