[PATCH] [clang-tidy] Implement the include order checker for LLVM.

Daniel Jasper djasper at google.com
Thu Jul 31 08:31:28 PDT 2014


Just some high-level comments for now.

================
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);
----------------
If we only look at includes in the main file, will we ever get any warnings about the include order in headers?

================
Comment at: clang-tidy/llvm/IncludeOrderCheck.cpp:44
@@ +43,3 @@
+  // onEndOfTranslationUnit is not called.
+  Finder->addMatcher(ast_matchers::decl(), this);
+}
----------------
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)?

================
Comment at: clang-tidy/llvm/IncludeOrderCheck.cpp:150
@@ +149,3 @@
+
+  // Emit a warning for each block but and fixits for all changes within that
+  // block.
----------------
The "but" seems wrong.

http://reviews.llvm.org/D4741






More information about the cfe-commits mailing list