[PATCH] D50580: [clang-tidy] Abseil: no namespace check

Deanna Garcia via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 21 07:55:52 PDT 2018


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





More information about the cfe-commits mailing list