deannagarcia added inline comments.
================
Comment at: clang-tidy/abseil/AbseilMatcher.h:32
+ auto Filename = FileEntry->getName();
+ llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|"
+ "synchronization|types|utiliy)");
----------------
lebedev.ri wrote:
> hokein wrote:
> > lebedev.ri wrote:
> > > hokein wrote:
> > > > The regex seems incomplete, we are missing `algorithm`, `time`
> > > > subdirectory.
> > > So what happens if open the namespace in a file that is located in my
> > > personal `absl/base` directory?
> > > It will be false-negatively not reported?
> > Yes, I'd say this is a known limitation.
> Similarly, the check will break (start producing false-positives) as soon as
> a new directory is added to abseil proper.
> I don't have any reliable idea on how to do this better, but the current
> solution seems rather suboptimal..
We are aware that there will be a false-negative if code opens namespace in the
absl directories, however we think that is pretty rare and that users shouldn't
be doing that anyway. No matter how we do this there will be a way for users to
circumvent the check, and we will allow those users to do it because it will be
their code that breaks in the end.
================
Comment at: clang-tidy/abseil/AbseilMatcher.h:32
+ auto Filename = FileEntry->getName();
+ llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|"
+ "synchronization|types|utiliy)");
----------------
deannagarcia wrote:
> lebedev.ri wrote:
> > hokein wrote:
> > > lebedev.ri wrote:
> > > > hokein wrote:
> > > > > The regex seems incomplete, we are missing `algorithm`, `time`
> > > > > subdirectory.
> > > > So what happens if open the namespace in a file that is located in my
> > > > personal `absl/base` directory?
> > > > It will be false-negatively not reported?
> > > Yes, I'd say this is a known limitation.
> > Similarly, the check will break (start producing false-positives) as soon
> > as a new directory is added to abseil proper.
> > I don't have any reliable idea on how to do this better, but the current
> > solution seems rather suboptimal..
> We are aware that there will be a false-negative if code opens namespace in
> the absl directories, however we think that is pretty rare and that users
> shouldn't be doing that anyway. No matter how we do this there will be a way
> for users to circumvent the check, and we will allow those users to do it
> because it will be their code that breaks in the end.
The false-positive with new directories is something we thought about, but new
directories aren't added to absl very often so we thought it wouldn't be too
hard to add them to this regex when they are.
https://reviews.llvm.org/D50580
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
