Don't allow llvm-include-order to intermingle includes from different files.

[zturner]

Summary

See llvm.org/pr28943 for some context.

I'm not sure if this is the right fix, but it seems to pass the test suite for me.

[zturner]

Fixed the test for this. Confirmed that it fails before the patch and succeeds after the patch.

[zturner]

Seems like dropping all non-main-file include directives is wrong because then it won't be able to detect errors in "non user code". Seems like the real fix is to bucket the include directives by file in which they were declared in, and then process each bucket in order. This way we process all include directives, including those in non-user code, but block analysis is done at a per-file level.

[djasper]

I think we should entirely drop this implementation of the check and let it just check #includes with clang-format. clang-format's implementation isn't a strict superset, e.g. won't sort between multiple blocks, but that's intentional for now.

[alexfh]

The patch LG.

In D23434#513533, @djasper wrote:

I think we should entirely drop this implementation of the check and let it just check #includes with clang-format. clang-format's implementation isn't a strict superset, e.g. won't sort between multiple blocks, but that's intentional for now.

Since sorting across blocks is frequently needed to make include order consistent with the LLVM coding standards, it makes sense to keep this check until clang-format is able to sort across blocks. Especially, since Zachary is specifically interested in this functionality.

[djasper]

I think we got confused. We once had tried to write an experimental separate check to comply with Google's style guide. If you want to fiddle around with that, contact me, I can send you pointers. But as I mentioned we moved away from that. And I think it makes more sense to re-create the sort-across-blocks functionality in clang-format and not in clang-tidy.

[alexfh]

In D23434#513839, @djasper wrote:

I think we got confused. We once had tried to write an experimental separate check to comply with Google's style guide. If you want to fiddle around with that, contact me, I can send you pointers. But as I mentioned we moved away from that. And I think it makes more sense to re-create the sort-across-blocks functionality in clang-format and not in clang-tidy.

Yep, we definitely got confused. That experimental check actually implemented cross-block sorting, but this caused a bunch of issues. Which makes me think that proper implementation of cross-block include sorting is challenging be it in clang-format or clang-tidy. Clang-format probably makes it even more complex, since a higher safety of transformations is expected from it.

clang-tidy/llvm/IncludeOrderCheck.cpp

static int getPriority(StringRef Filename, bool IsAngled, bool IsMainModule) {
   // Other headers are inserted between the main module header and LLVM headers.
   return 1;
 }
 
 void IncludeOrderPPCallbacks::InclusionDirective(
     SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName,
     bool IsAngled, CharSourceRange FilenameRange, const FileEntry *File,
     StringRef SearchPath, StringRef RelativePath, const Module *Imported) {
+  if (SM.getFileID(HashLoc) != SM.getMainFileID())
+    return;
+
   // We recognize the first include as a special main module header and want
   // to leave it in the top position.
   IncludeDirective ID = {HashLoc, FilenameRange, FileName, IsAngled, false};
   if (LookForMainModule && !IsAngled) {
     ID.IsMainModule = true;
     LookForMainModule = false;
   }
   IncludeDirectives.push_back(std::move(ID));

test/clang-tidy/Inputs/Headers/cross-file-c.h

+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+#include "cross-file-b.h"

test/clang-tidy/llvm-include-order.cpp

 // CHECK-FIXES-NEXT: #endif
 
 // CHECK-MESSAGES: [[@LINE+1]]:1: warning: #includes are not sorted properly
 #include "b.h"
 #include "a.h"
 
 // CHECK-FIXES: #include "a.h"
 // CHECK-FIXES-NEXT: #include "b.h"
+
+// CHECK-MESSAGES-NOT: [[@LINE+1]]:1: warning: #includes are not sorted properly
+#include "cross-file-c.h"
+
+#include "cross-file-a.h"