This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
hokein marked an inline comment as done.
Closed by commit rG84048e234f8f: [format] foo.<name>.h should be the
main-header for foo.<name>.cc (authored by hokein).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89783/new/
https://reviews.llvm.org/D89783
Files:
clang/include/clang/Tooling/Inclusions/HeaderIncludes.h
clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
clang/unittests/Format/SortIncludesTest.cpp
Index: clang/unittests/Format/SortIncludesTest.cpp
===================================================================
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -151,7 +151,7 @@
EXPECT_TRUE(sortIncludes(FmtStyle, Code, GetCodeRange(Code),
"a.cc").empty());
}
-TEST_F(SortIncludesTest, NoMainFileHeader) {
+TEST_F(SortIncludesTest, MainFileHeader) {
std::string Code = "#include <string>\n"
"\n"
"#include \"a/extra_action.proto.h\"\n";
@@ -159,6 +159,13 @@
EXPECT_TRUE(
sortIncludes(FmtStyle, Code, GetCodeRange(Code), "a/extra_action.cc")
.empty());
+
+ EXPECT_EQ("#include \"foo.bar.h\"\n"
+ "\n"
+ "#include \"a.h\"\n",
+ sort("#include \"a.h\"\n"
+ "#include \"foo.bar.h\"\n",
+ "foo.bar.cc"));
}
TEST_F(SortIncludesTest, SortedIncludesInMultipleBlocksAreMerged) {
Index: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
===================================================================
--- clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
+++ clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
@@ -190,7 +190,6 @@
IncludeCategoryManager::IncludeCategoryManager(const IncludeStyle &Style,
StringRef FileName)
: Style(Style), FileName(FileName) {
- FileStem = matchingStem(FileName);
for (const auto &Category : Style.IncludeCategories)
CategoryRegexs.emplace_back(Category.Regex, llvm::Regex::IgnoreCase);
IsMainFile = FileName.endswith(".c") || FileName.endswith(".cc") ||
@@ -234,16 +233,30 @@
if (!IncludeName.startswith("\""))
return false;
+ IncludeName =
+ IncludeName.drop_front(1).drop_back(1); // remove the surrounding "" or
<>
// Not matchingStem: implementation files may have compound extensions but
// headers may not.
- StringRef HeaderStem =
- llvm::sys::path::stem(IncludeName.drop_front(1).drop_back(
- 1) /* remove the surrounding "" or <> */);
- if (FileStem.startswith(HeaderStem) ||
- FileStem.startswith_lower(HeaderStem)) {
+ StringRef HeaderStem = llvm::sys::path::stem(IncludeName);
+ StringRef FileStem = llvm::sys::path::stem(FileName); // foo.cu for foo.cu.cc
+ StringRef MatchingFileStem = matchingStem(FileName); // foo for foo.cu.cc
+ // main-header examples:
+ // 1) foo.h => foo.cc
+ // 2) foo.h => foo.cu.cc
+ // 3) foo.proto.h => foo.proto.cc
+ //
+ // non-main-header examples:
+ // 1) foo.h => bar.cc
+ // 2) foo.proto.h => foo.cc
+ StringRef Matching;
+ if (MatchingFileStem.startswith_lower(HeaderStem))
+ Matching = MatchingFileStem; // example 1), 2)
+ else if (FileStem.equals_lower(HeaderStem))
+ Matching = FileStem; // example 3)
+ if (!Matching.empty()) {
llvm::Regex MainIncludeRegex(HeaderStem.str() + Style.IncludeIsMainRegex,
llvm::Regex::IgnoreCase);
- if (MainIncludeRegex.match(FileStem))
+ if (MainIncludeRegex.match(Matching))
return true;
}
return false;
Index: clang/include/clang/Tooling/Inclusions/HeaderIncludes.h
===================================================================
--- clang/include/clang/Tooling/Inclusions/HeaderIncludes.h
+++ clang/include/clang/Tooling/Inclusions/HeaderIncludes.h
@@ -40,8 +40,6 @@
const IncludeStyle Style;
bool IsMainFile;
std::string FileName;
- // This refers to a substring in FileName.
- StringRef FileStem;
SmallVector<llvm::Regex, 4> CategoryRegexs;
};
Index: clang/unittests/Format/SortIncludesTest.cpp
===================================================================
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -151,7 +151,7 @@
EXPECT_TRUE(sortIncludes(FmtStyle, Code, GetCodeRange(Code), "a.cc").empty());
}
-TEST_F(SortIncludesTest, NoMainFileHeader) {
+TEST_F(SortIncludesTest, MainFileHeader) {
std::string Code = "#include <string>\n"
"\n"
"#include \"a/extra_action.proto.h\"\n";
@@ -159,6 +159,13 @@
EXPECT_TRUE(
sortIncludes(FmtStyle, Code, GetCodeRange(Code), "a/extra_action.cc")
.empty());
+
+ EXPECT_EQ("#include \"foo.bar.h\"\n"
+ "\n"
+ "#include \"a.h\"\n",
+ sort("#include \"a.h\"\n"
+ "#include \"foo.bar.h\"\n",
+ "foo.bar.cc"));
}
TEST_F(SortIncludesTest, SortedIncludesInMultipleBlocksAreMerged) {
Index: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
===================================================================
--- clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
+++ clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
@@ -190,7 +190,6 @@
IncludeCategoryManager::IncludeCategoryManager(const IncludeStyle &Style,
StringRef FileName)
: Style(Style), FileName(FileName) {
- FileStem = matchingStem(FileName);
for (const auto &Category : Style.IncludeCategories)
CategoryRegexs.emplace_back(Category.Regex, llvm::Regex::IgnoreCase);
IsMainFile = FileName.endswith(".c") || FileName.endswith(".cc") ||
@@ -234,16 +233,30 @@
if (!IncludeName.startswith("\""))
return false;
+ IncludeName =
+ IncludeName.drop_front(1).drop_back(1); // remove the surrounding "" or <>
// Not matchingStem: implementation files may have compound extensions but
// headers may not.
- StringRef HeaderStem =
- llvm::sys::path::stem(IncludeName.drop_front(1).drop_back(
- 1) /* remove the surrounding "" or <> */);
- if (FileStem.startswith(HeaderStem) ||
- FileStem.startswith_lower(HeaderStem)) {
+ StringRef HeaderStem = llvm::sys::path::stem(IncludeName);
+ StringRef FileStem = llvm::sys::path::stem(FileName); // foo.cu for foo.cu.cc
+ StringRef MatchingFileStem = matchingStem(FileName); // foo for foo.cu.cc
+ // main-header examples:
+ // 1) foo.h => foo.cc
+ // 2) foo.h => foo.cu.cc
+ // 3) foo.proto.h => foo.proto.cc
+ //
+ // non-main-header examples:
+ // 1) foo.h => bar.cc
+ // 2) foo.proto.h => foo.cc
+ StringRef Matching;
+ if (MatchingFileStem.startswith_lower(HeaderStem))
+ Matching = MatchingFileStem; // example 1), 2)
+ else if (FileStem.equals_lower(HeaderStem))
+ Matching = FileStem; // example 3)
+ if (!Matching.empty()) {
llvm::Regex MainIncludeRegex(HeaderStem.str() + Style.IncludeIsMainRegex,
llvm::Regex::IgnoreCase);
- if (MainIncludeRegex.match(FileStem))
+ if (MainIncludeRegex.match(Matching))
return true;
}
return false;
Index: clang/include/clang/Tooling/Inclusions/HeaderIncludes.h
===================================================================
--- clang/include/clang/Tooling/Inclusions/HeaderIncludes.h
+++ clang/include/clang/Tooling/Inclusions/HeaderIncludes.h
@@ -40,8 +40,6 @@
const IncludeStyle Style;
bool IsMainFile;
std::string FileName;
- // This refers to a substring in FileName.
- StringRef FileStem;
SmallVector<llvm::Regex, 4> CategoryRegexs;
};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
