aaron.ballman added inline comments.
================
Comment at: clang-tidy/cppcoreguidelines/ProTypeReinterpretCastCheck.cpp:25
@@ -23,2 +24,3 @@
- Finder->addMatcher(cxxReinterpretCastExpr().bind("cast"), this);
+ std::vector<StringRef> Rules{"type", "type.1",
"cppcoreguidelines-pro-type-reinterpret-cast"};
+
Finder->addMatcher(cxxReinterpretCastExpr(unless(isSuppressed(Rules))).bind("cast"),
this);
----------------
Hmm, it seems like this is boilerplate we are going to want for every rule, and
that it's pretty easy to not get this right (for instance, if you change the
way the check is spelled, you have to remember to update this as well). Could
this actually be handled more transparently, by gathering this information when
the check is registered and exposing it to the check?
The checks would still need to use `unless(isSuppressed(Rules))` in some form,
but I am thinking that it would be more convenient if we could do:
`Finder->addMatcher(cxxReinterpretCastExpr(unlessSuppressed(*this)).bind("cast"),
this);`
================
Comment at: clang-tidy/cppcoreguidelines/Suppression.h:59
@@ +58,3 @@
+ anyOf(hasAncestor(attributedStmt(hasSuppressAttr(Rules))),
+ hasAncestor(decl(hasAttrs(), hasSuppressAttr(Rules)))))
+ .matches(Node, Finder, Builder);
----------------
Why is the check for `hasAttrs` required?
Also, this use of `hasAncestor()` makes this expensive to use, and that expense
may be hidden from the caller. Is there a way to structure this so that we
don't need to walk the entire ancestor tree?
https://reviews.llvm.org/D24888
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
