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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
