[clang-tools-extra] r278546 - Analyze include order on a per-file basis.
Zachary Turner via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 12 11:38:27 PDT 2016
Author: zturner
Date: Fri Aug 12 13:38:26 2016
New Revision: 278546
URL: http://llvm.org/viewvc/llvm-project?rev=278546&view=rev
Log:
Analyze include order on a per-file basis.
The include order check would get notified of all include
directives in a depth-first manner. This created the
possibility of an include directive from a header file
interfering with the sort order of a set of two distinct
blocks from the top level cpp file, if that include directive
was on just the right line.
With this patch we bucket the include directives by the file
in which they appear in and process one bucket at a time,
so that directives from different files do not get mixed
together into the same list.
Reviewed By: alexfh
Differential Revision: https://reviews.llvm.org/D23434
Added:
clang-tools-extra/trunk/test/clang-tidy/Inputs/Headers/cross-file-a.h
clang-tools-extra/trunk/test/clang-tidy/Inputs/Headers/cross-file-b.h
clang-tools-extra/trunk/test/clang-tidy/Inputs/Headers/cross-file-c.h
clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/llvm/IncludeOrderCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/llvm-include-order.cpp
Modified: clang-tools-extra/trunk/clang-tidy/llvm/IncludeOrderCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/llvm/IncludeOrderCheck.cpp?rev=278546&r1=278545&r2=278546&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/llvm/IncludeOrderCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/llvm/IncludeOrderCheck.cpp Fri Aug 12 13:38:26 2016
@@ -12,6 +12,8 @@
#include "clang/Lex/PPCallbacks.h"
#include "clang/Lex/Preprocessor.h"
+#include <map>
+
namespace clang {
namespace tidy {
namespace llvm {
@@ -37,7 +39,9 @@ private:
bool IsAngled; ///< true if this was an include with angle brackets
bool IsMainModule; ///< true if this was the first include in a file
};
- std::vector<IncludeDirective> IncludeDirectives;
+
+ typedef std::vector<IncludeDirective> FileIncludes;
+ std::map<clang::FileID, FileIncludes> IncludeDirectives;
bool LookForMainModule;
ClangTidyCheck &Check;
@@ -80,7 +84,9 @@ void IncludeOrderPPCallbacks::InclusionD
ID.IsMainModule = true;
LookForMainModule = false;
}
- IncludeDirectives.push_back(std::move(ID));
+
+ // Bucket the include directives by the id of the file they were declared in.
+ IncludeDirectives[SM.getFileID(HashLoc)].push_back(std::move(ID));
}
void IncludeOrderPPCallbacks::EndOfMainFile() {
@@ -95,69 +101,73 @@ void IncludeOrderPPCallbacks::EndOfMainF
// FIXME: We should be more careful about sorting below comments as we don't
// know if the comment refers to the next include or the whole block that
// follows.
- std::vector<unsigned> Blocks(1, 0);
- for (unsigned I = 1, E = IncludeDirectives.size(); I != E; ++I)
- if (SM.getExpansionLineNumber(IncludeDirectives[I].Loc) !=
- SM.getExpansionLineNumber(IncludeDirectives[I - 1].Loc) + 1)
- Blocks.push_back(I);
- Blocks.push_back(IncludeDirectives.size()); // Sentinel value.
-
- // Get a vector of indices.
- std::vector<unsigned> IncludeIndices;
- for (unsigned I = 0, E = IncludeDirectives.size(); I != E; ++I)
- IncludeIndices.push_back(I);
-
- // Sort the includes. We first sort by priority, then lexicographically.
- for (unsigned BI = 0, BE = Blocks.size() - 1; BI != BE; ++BI)
- std::sort(IncludeIndices.begin() + Blocks[BI],
- IncludeIndices.begin() + Blocks[BI + 1],
- [this](unsigned LHSI, unsigned RHSI) {
- IncludeDirective &LHS = IncludeDirectives[LHSI];
- IncludeDirective &RHS = IncludeDirectives[RHSI];
-
- int PriorityLHS =
- getPriority(LHS.Filename, LHS.IsAngled, LHS.IsMainModule);
- int PriorityRHS =
- getPriority(RHS.Filename, RHS.IsAngled, RHS.IsMainModule);
-
- return std::tie(PriorityLHS, LHS.Filename) <
- std::tie(PriorityRHS, RHS.Filename);
- });
-
- // Emit a warning for each block and fixits for all changes within that block.
- for (unsigned BI = 0, BE = Blocks.size() - 1; BI != BE; ++BI) {
- // Find the first include that's not in the right position.
- unsigned I, E;
- for (I = Blocks[BI], E = Blocks[BI + 1]; I != E; ++I)
- if (IncludeIndices[I] != I)
- break;
-
- if (I == E)
- continue;
-
- // Emit a warning.
- auto D = Check.diag(IncludeDirectives[I].Loc,
- "#includes are not sorted properly");
-
- // Emit fix-its for all following includes in this block.
- for (; I != E; ++I) {
- if (IncludeIndices[I] == I)
- continue;
- const IncludeDirective &CopyFrom = IncludeDirectives[IncludeIndices[I]];
-
- SourceLocation FromLoc = CopyFrom.Range.getBegin();
- const char *FromData = SM.getCharacterData(FromLoc);
- unsigned FromLen = std::strcspn(FromData, "\n");
+ for (auto &Bucket : IncludeDirectives) {
+ auto &FileDirectives = Bucket.second;
+ std::vector<unsigned> Blocks(1, 0);
+ for (unsigned I = 1, E = FileDirectives.size(); I != E; ++I)
+ if (SM.getExpansionLineNumber(FileDirectives[I].Loc) !=
+ SM.getExpansionLineNumber(FileDirectives[I - 1].Loc) + 1)
+ Blocks.push_back(I);
+ Blocks.push_back(FileDirectives.size()); // Sentinel value.
+
+ // Get a vector of indices.
+ std::vector<unsigned> IncludeIndices;
+ for (unsigned I = 0, E = FileDirectives.size(); I != E; ++I)
+ IncludeIndices.push_back(I);
+
+ // Sort the includes. We first sort by priority, then lexicographically.
+ for (unsigned BI = 0, BE = Blocks.size() - 1; BI != BE; ++BI)
+ std::sort(IncludeIndices.begin() + Blocks[BI],
+ IncludeIndices.begin() + Blocks[BI + 1],
+ [&FileDirectives](unsigned LHSI, unsigned RHSI) {
+ IncludeDirective &LHS = FileDirectives[LHSI];
+ IncludeDirective &RHS = FileDirectives[RHSI];
+
+ int PriorityLHS =
+ getPriority(LHS.Filename, LHS.IsAngled, LHS.IsMainModule);
+ int PriorityRHS =
+ getPriority(RHS.Filename, RHS.IsAngled, RHS.IsMainModule);
+
+ return std::tie(PriorityLHS, LHS.Filename) <
+ std::tie(PriorityRHS, RHS.Filename);
+ });
+
+ // Emit a warning for each block and fixits for all changes within that
+ // block.
+ for (unsigned BI = 0, BE = Blocks.size() - 1; BI != BE; ++BI) {
+ // Find the first include that's not in the right position.
+ unsigned I, E;
+ for (I = Blocks[BI], E = Blocks[BI + 1]; I != E; ++I)
+ if (IncludeIndices[I] != I)
+ break;
- StringRef FixedName(FromData, FromLen);
+ if (I == E)
+ continue;
- SourceLocation ToLoc = IncludeDirectives[I].Range.getBegin();
- const char *ToData = SM.getCharacterData(ToLoc);
- unsigned ToLen = std::strcspn(ToData, "\n");
- auto ToRange =
- CharSourceRange::getCharRange(ToLoc, ToLoc.getLocWithOffset(ToLen));
+ // Emit a warning.
+ auto D = Check.diag(FileDirectives[I].Loc,
+ "#includes are not sorted properly");
+
+ // Emit fix-its for all following includes in this block.
+ for (; I != E; ++I) {
+ if (IncludeIndices[I] == I)
+ continue;
+ const IncludeDirective &CopyFrom = FileDirectives[IncludeIndices[I]];
+
+ SourceLocation FromLoc = CopyFrom.Range.getBegin();
+ const char *FromData = SM.getCharacterData(FromLoc);
+ unsigned FromLen = std::strcspn(FromData, "\n");
+
+ StringRef FixedName(FromData, FromLen);
+
+ SourceLocation ToLoc = FileDirectives[I].Range.getBegin();
+ const char *ToData = SM.getCharacterData(ToLoc);
+ unsigned ToLen = std::strcspn(ToData, "\n");
+ auto ToRange =
+ CharSourceRange::getCharRange(ToLoc, ToLoc.getLocWithOffset(ToLen));
- D << FixItHint::CreateReplacement(ToRange, FixedName);
+ D << FixItHint::CreateReplacement(ToRange, FixedName);
+ }
}
}
Added: clang-tools-extra/trunk/test/clang-tidy/Inputs/Headers/cross-file-a.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/Inputs/Headers/cross-file-a.h?rev=278546&view=auto
==============================================================================
(empty)
Added: clang-tools-extra/trunk/test/clang-tidy/Inputs/Headers/cross-file-b.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/Inputs/Headers/cross-file-b.h?rev=278546&view=auto
==============================================================================
(empty)
Added: clang-tools-extra/trunk/test/clang-tidy/Inputs/Headers/cross-file-c.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/Inputs/Headers/cross-file-c.h?rev=278546&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/Inputs/Headers/cross-file-c.h (added)
+++ clang-tools-extra/trunk/test/clang-tidy/Inputs/Headers/cross-file-c.h Fri Aug 12 13:38:26 2016
@@ -0,0 +1,41 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+// The line number of the following include should match the location of the
+// corresponding comment in llvm-include-order.cpp
+#include "cross-file-b.h"
Added: clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp?rev=278546&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp Fri Aug 12 13:38:26 2016
@@ -0,0 +1,17 @@
+// RUN: clang-tidy -checks=-*,modernize-use-nullptr %s -- clang-cl /DTEST1 /DFOO=foo /DBAR=bar | FileCheck -implicit-check-not="{{warning|error}}:" %s
+int *a = 0;
+// CHECK: :[[@LINE-1]]:10: warning: use nullptr
+#ifdef TEST1
+int *b = 0;
+// CHECK: :[[@LINE-1]]:10: warning: use nullptr
+#endif
+#define foo 1
+#define bar 1
+#if FOO
+int *c = 0;
+// CHECK: :[[@LINE-1]]:10: warning: use nullptr
+#endif
+#if BAR
+int *d = 0;
+// CHECK: :[[@LINE-1]]:10: warning: use nullptr
+#endif
\ No newline at end of file
Modified: clang-tools-extra/trunk/test/clang-tidy/llvm-include-order.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/llvm-include-order.cpp?rev=278546&r1=278545&r2=278546&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/llvm-include-order.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/llvm-include-order.cpp Fri Aug 12 13:38:26 2016
@@ -35,3 +35,8 @@
// 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"
+// This line number should correspond to the position of the #include in cross-file-c.h
+#include "cross-file-a.h"
More information about the cfe-commits
mailing list