hugoeg added inline comments.
================ Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:23 + + Finder->addMatcher(namespaceDecl(hasName("absl")).bind("absl_namespace"), + this); ---------------- deannagarcia wrote: > aaron.ballman wrote: > > hokein wrote: > > > aaron.ballman wrote: > > > > I think this needs a `not(isExpansionInSystemHeader())` in there, as > > > > this is going to trigger on code that includes a header file using an > > > > `absl` namespace. If I'm incorrect and users typically include abseil > > > > as something other than system includes, you'll have to find a > > > > different way to solve this. > > > Yeah, we have discussed this issue internally. abseil-user code usually > > > includes abseil header like `#include > > > "whatever/abseil/base/optimization.h"`, so `IsExpansionInSystemHeader` > > > doesn't work for most cases. > > > > > > We need a way to filter out all headers being a part of abseil library. I > > > think we can create a matcher `InExpansionInAbseilHeader` -- basically > > > using `IsExpansionInFileMatching` with a regex expression that matches > > > typical abseil include path. What do you think? > > > > > > And we'll have more abseil checks (e.g. `abseil-no-internal-deps`) that > > > use `InExpansionInAbseilHeader`, we should put it to a common header. > > I think that is a sensible approach. > We will begin working on this, I think it will first be implemented in > abseil-no-internal-deps but once it looks good I will add it to this patch. I'm going to go ahead a implement this in ASTMatchers.h and include it on the patch for **abseil-no-internal-dep**s https://reviews.llvm.org/D50580 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits