aaron.ballman added inline comments.
================
Comment at: clang-tidy/fuchsia/RestrictSystemIncludesCheck.cpp:67
+ llvm::SmallDenseMap<FileID, FileIncludes> IncludeDirectives;
+ llvm::StringMap<bool> IsSystem;
+
----------------
Rather than use this `StringMap`, can you change `InclusionDirective()` to
filter out includes you don't care about? Something along the lines of:
```
FileID FID = SM.translateFile(File);
assert(FID != FileID() && "Source Manager does not know about this header
file");
bool Invalid;
const SLocEntry &Entry = SM.getSLocEntry(FID, &Invalid);
if (!Invalid && SrcMgr::isSystem(Entry.getFile().getFileCharacteristic())) {
// It's a system file.
}
```
This way you don't have to store all the path information and test against
paths in `EndOfMainFile()`, unless I'm missing something. Note, this is
untested code and perhaps that assert will fire (maybe SourceManager is unaware
of this file by the time InclusionDirective() is called).
Alternatively, you could alter InclusionDirective() to also pass in the
`CharacteristicKind` calculated by `Preprocessor::HandleIncludeDirective()` as
that seems like generally useful information for the callback to have on hand.
================
Comment at: clang-tidy/fuchsia/RestrictSystemIncludesCheck.h:1-2
+//===--- RestrictSystemIncludesCheck.h - clang-tidy---------------------*-
+//C++-*-===//
+//
----------------
This should only be on a single line -- you can remove some `-` characters.
================
Comment at: docs/clang-tidy/checks/fuchsia-restrict-system-includes.rst:32
+ A string containing a semi-colon separated list of allowed include
filenames.
+ The default is an empty string, which allows all includes.
----------------
This default seems a bit odd to me, but perhaps it's fine. What's novel is that
the check is a no-op by default, so how do Fuchsia developers get the correct
list? Or is there no canonical list and developers are expected to populate
their own manually?
https://reviews.llvm.org/D43778
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
