aaron.ballman added inline comments.
================
Comment at: clang-tidy/cert/DontModifyStdNamespaceCheck.cpp:34
+
+ diag(N->getLocation(),
+ "modification of '%0' namespace can result in undefined behavior")
----------------
I think this will still trigger false positives because there are times when it
is permissible to modify the std namespace outside of a system header. 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/DontModifyStdNamespaceCheck.cpp:36
+ "modification of '%0' namespace can result in undefined behavior")
+ << N->getName();
+}
----------------
No need to call `getName()` -- you can pass `N` directly. When you correct
this, also remove the explicit quotes from the diagnostic (they're added
automatically by the diagnostics engine).
================
Comment at: docs/clang-tidy/checks/cert-dcl58-cpp.rst:8
+behavior.
+This check warns for such modifications.
+
----------------
You should add a link to the CERT guideline this addresses. See
docs/clang-tidy/checks/cert-dcl50-cpp.rst as an example.
https://reviews.llvm.org/D23421
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
