[PATCH] Add readability-duplicate-include check to clang-tidy

Richard legalize at xmission.com
Tue Mar 17 09:13:23 PDT 2015


================
Comment at: clang-tidy/readability/DuplicateIncludeCheck.cpp:62
@@ +61,3 @@
+    StringRef SearchPath, StringRef RelativePath, const Module *Imported) {
+  if (!SM_.isInMainFile(HashLoc)) {
+    return;
----------------
LegalizeAdulthood wrote:
> alexfh wrote:
> > LegalizeAdulthood wrote:
> > > alexfh wrote:
> > > > What's the reason to limit the check to the main file only? I think, it should work on all headers as well. Also, sometimes it's fine to have duplicate includes even without macro definitions in between, e.g. when these #includes are in different namespaces.
> > > > 
> > > > I'd suggest using the same technique as in the IncludeOrderCheck: for each file collect all preprocessor directives sorted by SourceLocation. Then detect #include blocks (not necessarily the same way as its done in the IncludeOrderCheck. Maybe use the presense of any non-comment tokens between #includes as a boundary of blocks), and detect duplicate includes in each block.
> > > If I remove the `isInMainFile`, how do I ensure that I don't attempt to modify system headers?
> > Using `SourceLocation::isInSystemHeader()`.
> Thanks, I'll try that.  The next question that brings to mind is how do I distinguish between headers that I own and headers used from third party libraries?
> 
> For instance, suppose I run a check on a clang refactoring tool and it uses `isInSystemHeader` and starts flagging issues in the clang tooling library headers.
> 
> The `compile_commands.json` doesn't contain any information about headers in my project, only translation units in my build, so it doesn't know whether or not included headers belong to me or third-party libraries.
For the benefit of others reading this, Alex pointed out to me the `-header-filter` and `-system-headers` options to clang-tidy.  I think this means I don't need any narrowing if `isExpansinInMainFile()` in any of my matchers.  I will do some experimenting to verify that this doesn't introduce regressions.

http://reviews.llvm.org/D7982

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list