================ Comment at: clang-tidy/llvm/IncludeOrderCheck.cpp:31 @@ +30,3 @@ + // Report all includes in the main file. + if (SM.isInMainFile(HashLoc)) + Check.addIncludeDirective(HashLoc, FilenameRange, FileName, IsAngled); ---------------- Daniel Jasper wrote: > If we only look at includes in the main file, will we ever get any warnings > about the include order in headers? I'm not entirely sure if the strategy is to run clang-tidy on the header file to get results or if we want to also have them when running it just on the .cpp file.
But since header results are filtered by default it should be fine to remove this check. ================ Comment at: clang-tidy/llvm/IncludeOrderCheck.cpp:44 @@ +43,3 @@ + // onEndOfTranslationUnit is not called. + Finder->addMatcher(ast_matchers::decl(), this); +} ---------------- Daniel Jasper wrote: > Maybe use translationUnitDecl() so that the empty callback isn't called so > often? Probably doesn't matter, though. > > Also, couldn't you do that based on PPCallbacks::FileChanged() (when it > leaves the main file)? There is no tranlationUnitDecl() matcher (yet). Reusing PPCallbacks is a good idea, I'll try that. ================ Comment at: clang-tidy/llvm/IncludeOrderCheck.cpp:104 @@ +103,3 @@ + + // Purge includes after the first decl in the translation unit. + IncludeDirectives.erase( ---------------- Alexander Kornienko wrote: > How about a pure-lexer approach to finding the end of includes block? Just > lex everything in between #include directives until you find a non-comment > token that is not a part of a preprocessor directive? > > BTW, is this needed at all if you only sort includes within each block? It should be fine to even sort clusters of #includes in the middle of the code in LLVM, but I have to double check that. ================ Comment at: clang-tidy/llvm/IncludeOrderCheck.h:47 @@ +46,3 @@ + bool IsAngled; ///< true if this was an include with angle brackets + bool IsMainModule; ///< true if this was the first include in a file + }; ---------------- Alexander Kornienko wrote: > It's a bit confusing. What if the first include in a file was just an > arbitrary header, which happened to be included first? What if the first > include isn't special (e.g. we're analyzing a header file)? > > Should we try to do filename matching? I tried to do filename matching (even with some fuzzyness) and it just doesn't work for LLVM. e.g. in lib/Analysis many passes have "Passes.h" at the top which is totally unrelated to the filename :( http://reviews.llvm.org/D4741 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
