[clang-tools-extra] r245042 - [clang-tidy] Fix IncludeInserter/IncludeSorter bug.
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 14 05:33:25 PDT 2015
Author: alexfh
Date: Fri Aug 14 07:33:25 2015
New Revision: 245042
URL: http://llvm.org/viewvc/llvm-project?rev=245042&view=rev
Log:
[clang-tidy] Fix IncludeInserter/IncludeSorter bug.
If there weren't any includes in the file, or all of them had 'IncludeKind'
greater than the desired one, then 'CreateIncludeInsertion' didn't work.
http://reviews.llvm.org/D12017
Patch by Angel Garcia!
Modified:
clang-tools-extra/trunk/clang-tidy/IncludeInserter.cpp
clang-tools-extra/trunk/clang-tidy/IncludeSorter.cpp
clang-tools-extra/trunk/clang-tidy/IncludeSorter.h
clang-tools-extra/trunk/unittests/clang-tidy/IncludeInserterTest.cpp
Modified: clang-tools-extra/trunk/clang-tidy/IncludeInserter.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/IncludeInserter.cpp?rev=245042&r1=245041&r2=245042&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/IncludeInserter.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/IncludeInserter.cpp Fri Aug 14 07:33:25 2015
@@ -26,7 +26,7 @@ public:
StringRef /*SearchPath*/, StringRef /*RelativePath*/,
const Module * /*ImportedModule*/) override {
Inserter->AddInclude(FileNameRef, IsAngled, HashLocation,
- FileNameRange.getEnd());
+ FileNameRange.getEnd());
}
private:
@@ -54,7 +54,14 @@ IncludeInserter::CreateIncludeInsertion(
return llvm::None;
}
if (IncludeSorterByFile.find(FileID) == IncludeSorterByFile.end()) {
- return llvm::None;
+ // This may happen if there have been no preprocessor directives in this
+ // file.
+ IncludeSorterByFile.insert(std::make_pair(
+ FileID,
+ llvm::make_unique<IncludeSorter>(
+ &SourceMgr, &LangOpts, FileID,
+ SourceMgr.getFilename(SourceMgr.getLocForStartOfFile(FileID)),
+ Style)));
}
return IncludeSorterByFile[FileID]->CreateIncludeInsertion(Header, IsAngled);
}
Modified: clang-tools-extra/trunk/clang-tidy/IncludeSorter.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/IncludeSorter.cpp?rev=245042&r1=245041&r2=245042&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/IncludeSorter.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/IncludeSorter.cpp Fri Aug 14 07:33:25 2015
@@ -111,14 +111,20 @@ void IncludeSorter::AddInclude(StringRef
Optional<FixItHint> IncludeSorter::CreateIncludeInsertion(StringRef FileName,
bool IsAngled) {
- if (SourceLocations.empty()) {
- return None;
- }
std::string IncludeStmt =
IsAngled ? llvm::Twine("#include <" + FileName + ">\n").str()
: llvm::Twine("#include \"" + FileName + "\"\n").str();
+ if (SourceLocations.empty()) {
+ // If there are no includes in this file, add it in the first line.
+ // FIXME: insert after the file comment or the header guard, if present.
+ IncludeStmt.append("\n");
+ return FixItHint::CreateInsertion(
+ SourceMgr->getLocForStartOfFile(CurrentFileID), IncludeStmt);
+ }
+
auto IncludeKind =
DetermineIncludeKind(CanonicalFile, FileName, IsAngled, Style);
+
if (!IncludeBucket[IncludeKind].empty()) {
for (const std::string &IncludeEntry : IncludeBucket[IncludeKind]) {
if (FileName < IncludeEntry) {
@@ -135,21 +141,37 @@ Optional<FixItHint> IncludeSorter::Creat
return FixItHint::CreateInsertion(LastIncludeLocation.getEnd(),
IncludeStmt);
}
+ // Find the non-empty include bucket to be sorted directly above
+ // 'IncludeKind'. If such a bucket exists, we'll want to sort the include
+ // after that bucket. If no such bucket exists, find the first non-empty
+ // include bucket in the file. In that case, we'll want to sort the include
+ // before that bucket.
IncludeKinds NonEmptyKind = IK_InvalidInclude;
- for (int i = IncludeKind - 1; i >= 0; --i) {
+ for (int i = IK_InvalidInclude - 1; i >= 0; --i) {
if (!IncludeBucket[i].empty()) {
NonEmptyKind = static_cast<IncludeKinds>(i);
- break;
+ if (NonEmptyKind < IncludeKind)
+ break;
}
}
if (NonEmptyKind == IK_InvalidInclude) {
return llvm::None;
}
- const std::string &LastInclude = IncludeBucket[NonEmptyKind].back();
- SourceRange LastIncludeLocation = IncludeLocations[LastInclude].back();
+
+ if (NonEmptyKind < IncludeKind) {
+ // Create a block after.
+ const std::string &LastInclude = IncludeBucket[NonEmptyKind].back();
+ SourceRange LastIncludeLocation = IncludeLocations[LastInclude].back();
+ IncludeStmt = '\n' + IncludeStmt;
+ return FixItHint::CreateInsertion(LastIncludeLocation.getEnd(),
+ IncludeStmt);
+ }
+ // Create a block before.
+ const std::string &FirstInclude = IncludeBucket[NonEmptyKind][0];
+ SourceRange FirstIncludeLocation = IncludeLocations[FirstInclude].back();
IncludeStmt.append("\n");
- return FixItHint::CreateInsertion(
- LastIncludeLocation.getEnd().getLocWithOffset(1), IncludeStmt);
+ return FixItHint::CreateInsertion(FirstIncludeLocation.getBegin(),
+ IncludeStmt);
}
std::vector<FixItHint> IncludeSorter::GetEdits() {
Modified: clang-tools-extra/trunk/clang-tidy/IncludeSorter.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/IncludeSorter.h?rev=245042&r1=245041&r2=245042&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/IncludeSorter.h (original)
+++ clang-tools-extra/trunk/clang-tidy/IncludeSorter.h Fri Aug 14 07:33:25 2015
@@ -21,27 +21,23 @@ namespace tidy {
// the necessary commands to sort the inclusions according to the precedence
// enocded in IncludeKinds.
class IncludeSorter {
- public:
+public:
// Supported include styles.
- enum IncludeStyle {
- IS_LLVM = 0,
- IS_Google = 1
- };
+ enum IncludeStyle { IS_LLVM = 0, IS_Google = 1 };
// The classifications of inclusions, in the order they should be sorted.
enum IncludeKinds {
- IK_MainTUInclude = 0, // e.g. #include "foo.h" when editing foo.cc
- IK_CSystemInclude = 1, // e.g. #include <stdio.h>
- IK_CXXSystemInclude = 2, // e.g. #include <vector>
- IK_NonSystemInclude = 3, // e.g. #include "bar.h"
- IK_InvalidInclude = 4 // total number of valid IncludeKinds
+ IK_MainTUInclude = 0, // e.g. #include "foo.h" when editing foo.cc
+ IK_CSystemInclude = 1, // e.g. #include <stdio.h>
+ IK_CXXSystemInclude = 2, // e.g. #include <vector>
+ IK_NonSystemInclude = 3, // e.g. #include "bar.h"
+ IK_InvalidInclude = 4 // total number of valid IncludeKinds
};
// IncludeSorter constructor; takes the FileID and name of the file to be
// processed by the sorter.
- IncludeSorter(const SourceManager* SourceMgr,
- const LangOptions* LangOpts, const FileID FileID,
- StringRef FileName, IncludeStyle Style);
+ IncludeSorter(const SourceManager *SourceMgr, const LangOptions *LangOpts,
+ const FileID FileID, StringRef FileName, IncludeStyle Style);
// Returns the SourceManager-specific file ID for the file being handled by
// the sorter.
@@ -58,18 +54,17 @@ class IncludeSorter {
// Creates a quoted inclusion directive in the right sort order. Returns None
// on error or if header inclusion directive for header already exists.
- Optional<FixItHint> CreateIncludeInsertion(StringRef FileName,
- bool IsAngled);
+ Optional<FixItHint> CreateIncludeInsertion(StringRef FileName, bool IsAngled);
- private:
+private:
typedef SmallVector<SourceRange, 1> SourceRangeVector;
// Creates a fix-it for the given replacements.
// Takes the the source location that will be replaced, and the new text.
- FixItHint CreateFixIt(SourceRange EditRange, const std::string& NewText);
+ FixItHint CreateFixIt(SourceRange EditRange, const std::string &NewText);
- const SourceManager* SourceMgr;
- const LangOptions* LangOpts;
+ const SourceManager *SourceMgr;
+ const LangOptions *LangOpts;
const IncludeStyle Style;
FileID CurrentFileID;
// The file name stripped of common suffixes.
@@ -82,6 +77,6 @@ class IncludeSorter {
SmallVector<std::string, 1> IncludeBucket[IK_InvalidInclude];
};
-} // namespace tidy
-} // namespace clang
+} // namespace tidy
+} // namespace clang
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_INCLUDESORTER_H
Modified: clang-tools-extra/trunk/unittests/clang-tidy/IncludeInserterTest.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/IncludeInserterTest.cpp?rev=245042&r1=245041&r2=245042&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clang-tidy/IncludeInserterTest.cpp (original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/IncludeInserterTest.cpp Fri Aug 14 07:33:25 2015
@@ -71,6 +71,13 @@ public:
bool IsAngledInclude() const override { return false; }
};
+class CSystemIncludeInserterCheck : public IncludeInserterCheckBase {
+public:
+ using IncludeInserterCheckBase::IncludeInserterCheckBase;
+ StringRef HeaderToInclude() const override { return "stdlib.h"; }
+ bool IsAngledInclude() const override { return true; }
+};
+
class CXXSystemIncludeInserterCheck : public IncludeInserterCheckBase {
public:
CXXSystemIncludeInserterCheck(StringRef CheckName, ClangTidyContext *Context)
@@ -414,6 +421,72 @@ void foo() {
"insert_includes_test_header.cc",
1));
}
+
+TEST(IncludeInserterTest, InsertCXXSystemIncludeBeforeNonSystemInclude) {
+ const char *PreCode = R"(
+#include "path/to/a/header.h"
+
+void foo() {
+ int a = 0;
+})";
+ const char *PostCode = R"(
+#include <set>
+
+#include "path/to/a/header.h"
+
+void foo() {
+ int a = 0;
+})";
+
+ EXPECT_EQ(PostCode, runCheckOnCode<CXXSystemIncludeInserterCheck>(
+ PreCode, "devtools/cymbal/clang_tidy/tests/"
+ "insert_includes_test_header.cc",
+ 1));
+}
+
+TEST(IncludeInserterTest, InsertCSystemIncludeBeforeCXXSystemInclude) {
+ const char *PreCode = R"(
+#include <set>
+
+#include "path/to/a/header.h"
+
+void foo() {
+ int a = 0;
+})";
+ const char *PostCode = R"(
+#include <stdlib.h>
+
+#include <set>
+
+#include "path/to/a/header.h"
+
+void foo() {
+ int a = 0;
+})";
+
+ EXPECT_EQ(PostCode, runCheckOnCode<CSystemIncludeInserterCheck>(
+ PreCode, "devtools/cymbal/clang_tidy/tests/"
+ "insert_includes_test_header.cc",
+ 1));
+}
+
+TEST(IncludeInserterTest, InsertIncludeIfThereWasNoneBefore) {
+ const char *PreCode = R"(
+void foo() {
+ int a = 0;
+})";
+ const char *PostCode = R"(#include <set>
+
+
+void foo() {
+ int a = 0;
+})";
+
+ EXPECT_EQ(PostCode, runCheckOnCode<CXXSystemIncludeInserterCheck>(
+ PreCode, "devtools/cymbal/clang_tidy/tests/"
+ "insert_includes_test_header.cc",
+ 1));
+}
} // namespace
} // namespace tidy
More information about the cfe-commits
mailing list