[PATCH] D53882: [clang-tidy] Adding Zircon checker for std namespace

Julie Hockett via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 30 14:58:04 PDT 2018


juliehockett added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/zircon/NoStdNamespaceCheck.cpp:71
+void NoStdNamespaceCheck::check(const MatchFinder::MatchResult &Result) {
+  if (const auto *D = Result.Nodes.getNodeAs<ValueDecl>("stdVar"))
+    diag(D->getBeginLoc(),
----------------
JonasToth wrote:
> Please create a `StringRef` for the diagnostic message and reuse that.
> 
> Did you consider merging all `Decl` classes if you just use `getNodeAs<Decl>("common_decl_name")`?
Yes, but getting the location right posed an issue there. The `std` token is not always at `D->getLocation()`.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/zircon-no-std-namespace.rst:1
+.. title:: clang-tidy - zircon-no-std-namespace
+
----------------
JonasToth wrote:
> Could you please clarify what exactly is forbidden?
> 
> __Using__ stuff from `std` like `std::string` in normal "user" code or __opening__ `std` to add stuff?
> 
> If the first thing is the case why are the variables flagged but not the functions? And adding things to namespace `std` is AFAIK UB with the exception of template-function specializations, so maybe that would be worth a general check?
Both. Documentation updated -- the uses in particular are to be flagged.


================
Comment at: clang-tools-extra/test/clang-tidy/zircon-no-std-namespace.cpp:33
+int x = func();
+std::std_int y;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of the 'std' namespace is not allowed in Zircon kernel code
----------------
JonasToth wrote:
> What about using `int64_t` and these typedefs. Are they forbidden, too?
Yes, which is why that's tested. Is there an additional test case I'm missing regarding those?


https://reviews.llvm.org/D53882





More information about the cfe-commits mailing list