JonasToth added inline comments.
================
Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:23
+
+ Finder->addMatcher(namespaceDecl(hasName("absl")).bind("absl_namespace"),
+ this);
----------------
hugoeg wrote:
> 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
In principle it is ok, but I don't think ASTMatchers.h is the correct place,
because it is only of use to clang-tidy.
There is a `util` directory in clang-tidy for this kind of stuff. Defining you
own matchers works the same as in ASTMatchers, you can grep through clang-tidy
checks (e.g. `AST_MATCHER`) to have some examples of private matchers.
https://reviews.llvm.org/D50580
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
