[clang] 0cf7e61 - [clang][HeaderSearch] Support framework includes in suggestPath...
David Goldman via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 10 09:28:13 PST 2022
Author: David Goldman
Date: 2022-01-10T12:25:53-05:00
New Revision: 0cf7e61a42c7a08b0332204f1b60aa9fd1562089
URL: https://github.com/llvm/llvm-project/commit/0cf7e61a42c7a08b0332204f1b60aa9fd1562089
DIFF: https://github.com/llvm/llvm-project/commit/0cf7e61a42c7a08b0332204f1b60aa9fd1562089.diff
LOG: [clang][HeaderSearch] Support framework includes in suggestPath...
Clang will now search through the framework includes to identify
the framework include path to a file, and then suggest a framework
style include spelling for the file.
Differential Revision: https://reviews.llvm.org/D115183
Added:
Modified:
clang/include/clang/Lex/HeaderSearch.h
clang/lib/Lex/HeaderSearch.cpp
clang/test/Modules/double-quotes.m
clang/unittests/Lex/HeaderSearchTest.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Lex/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h
index 0482bf6c053f..e056f009eae9 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -286,6 +286,12 @@ class HeaderSearch {
/// Add an additional search path.
void AddSearchPath(const DirectoryLookup &dir, bool isAngled);
+ /// Add an additional system search path.
+ void AddSystemSearchPath(const DirectoryLookup &dir) {
+ SearchDirs.push_back(dir);
+ SearchDirsUsage.push_back(false);
+ }
+
/// Set the list of system header prefixes.
void SetSystemHeaderPrefixes(ArrayRef<std::pair<std::string, bool>> P) {
SystemHeaderPrefixes.assign(P.begin(), P.end());
diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index f9d61ecef796..d4de4ed156b6 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -745,7 +745,8 @@ static const char *copyString(StringRef Str, llvm::BumpPtrAllocator &Alloc) {
}
static bool isFrameworkStylePath(StringRef Path, bool &IsPrivateHeader,
- SmallVectorImpl<char> &FrameworkName) {
+ SmallVectorImpl<char> &FrameworkName,
+ SmallVectorImpl<char> &IncludeSpelling) {
using namespace llvm::sys;
path::const_iterator I = path::begin(Path);
path::const_iterator E = path::end(Path);
@@ -761,15 +762,22 @@ static bool isFrameworkStylePath(StringRef Path, bool &IsPrivateHeader,
// and some other variations among these lines.
int FoundComp = 0;
while (I != E) {
- if (*I == "Headers")
+ if (*I == "Headers") {
++FoundComp;
- if (I->endswith(".framework")) {
- FrameworkName.append(I->begin(), I->end());
- ++FoundComp;
- }
- if (*I == "PrivateHeaders") {
+ } else if (*I == "PrivateHeaders") {
++FoundComp;
IsPrivateHeader = true;
+ } else if (I->endswith(".framework")) {
+ StringRef Name = I->drop_back(10); // Drop .framework
+ // Need to reset the strings and counter to support nested frameworks.
+ FrameworkName.clear();
+ FrameworkName.append(Name.begin(), Name.end());
+ IncludeSpelling.clear();
+ IncludeSpelling.append(Name.begin(), Name.end());
+ FoundComp = 1;
+ } else if (FoundComp >= 2) {
+ IncludeSpelling.push_back('/');
+ IncludeSpelling.append(I->begin(), I->end());
}
++I;
}
@@ -784,20 +792,24 @@ diagnoseFrameworkInclude(DiagnosticsEngine &Diags, SourceLocation IncludeLoc,
bool FoundByHeaderMap = false) {
bool IsIncluderPrivateHeader = false;
SmallString<128> FromFramework, ToFramework;
- if (!isFrameworkStylePath(Includer, IsIncluderPrivateHeader, FromFramework))
+ SmallString<128> FromIncludeSpelling, ToIncludeSpelling;
+ if (!isFrameworkStylePath(Includer, IsIncluderPrivateHeader, FromFramework,
+ FromIncludeSpelling))
return;
bool IsIncludeePrivateHeader = false;
- bool IsIncludeeInFramework = isFrameworkStylePath(
- IncludeFE->getName(), IsIncludeePrivateHeader, ToFramework);
+ bool IsIncludeeInFramework =
+ isFrameworkStylePath(IncludeFE->getName(), IsIncludeePrivateHeader,
+ ToFramework, ToIncludeSpelling);
if (!isAngled && !FoundByHeaderMap) {
SmallString<128> NewInclude("<");
if (IsIncludeeInFramework) {
- NewInclude += ToFramework.str().drop_back(10); // drop .framework
- NewInclude += "/";
+ NewInclude += ToIncludeSpelling;
+ NewInclude += ">";
+ } else {
+ NewInclude += IncludeFilename;
+ NewInclude += ">";
}
- NewInclude += IncludeFilename;
- NewInclude += ">";
Diags.Report(IncludeLoc, diag::warn_quoted_include_in_framework_header)
<< IncludeFilename
<< FixItHint::CreateReplacement(IncludeLoc, NewInclude);
@@ -1865,9 +1877,9 @@ std::string HeaderSearch::suggestPathToFileForDiagnostics(
using namespace llvm::sys;
unsigned BestPrefixLength = 0;
- // Checks whether Dir and File shares a common prefix, if they do and that's
- // the longest prefix we've seen so for it returns true and updates the
- // BestPrefixLength accordingly.
+ // Checks whether `Dir` is a strict path prefix of `File`. If so and that's
+ // the longest prefix we've seen so for it, returns true and updates the
+ // `BestPrefixLength` accordingly.
auto CheckDir = [&](llvm::StringRef Dir) -> bool {
llvm::SmallString<32> DirPath(Dir.begin(), Dir.end());
if (!WorkingDir.empty() && !path::is_absolute(Dir))
@@ -1902,26 +1914,48 @@ std::string HeaderSearch::suggestPathToFileForDiagnostics(
path::is_separator(NI->front()) && path::is_separator(DI->front()))
continue;
+ // Special case Apple .sdk folders since the search path is typically a
+ // symlink like `iPhoneSimulator14.5.sdk` while the file is instead
+ // located in `iPhoneSimulator.sdk` (the real folder).
+ if (NI->endswith(".sdk") && DI->endswith(".sdk")) {
+ StringRef NBasename = path::stem(*NI);
+ StringRef DBasename = path::stem(*DI);
+ if (DBasename.startswith(NBasename))
+ continue;
+ }
+
if (*NI != *DI)
break;
}
return false;
};
+ bool BestPrefixIsFramework = false;
for (unsigned I = 0; I != SearchDirs.size(); ++I) {
- // FIXME: Support this search within frameworks.
- if (!SearchDirs[I].isNormalDir())
- continue;
-
- StringRef Dir = SearchDirs[I].getDir()->getName();
- if (CheckDir(Dir) && IsSystem)
- *IsSystem = BestPrefixLength ? I >= SystemDirIdx : false;
+ if (SearchDirs[I].isNormalDir()) {
+ StringRef Dir = SearchDirs[I].getDir()->getName();
+ if (CheckDir(Dir)) {
+ if (IsSystem)
+ *IsSystem = BestPrefixLength ? I >= SystemDirIdx : false;
+ BestPrefixIsFramework = false;
+ }
+ } else if (SearchDirs[I].isFramework()) {
+ StringRef Dir = SearchDirs[I].getFrameworkDir()->getName();
+ if (CheckDir(Dir)) {
+ if (IsSystem)
+ *IsSystem = BestPrefixLength ? I >= SystemDirIdx : false;
+ BestPrefixIsFramework = true;
+ }
+ }
}
// Try to shorten include path using TUs directory, if we couldn't find any
// suitable prefix in include search paths.
- if (!BestPrefixLength && CheckDir(path::parent_path(MainFile)) && IsSystem)
- *IsSystem = false;
+ if (!BestPrefixLength && CheckDir(path::parent_path(MainFile))) {
+ if (IsSystem)
+ *IsSystem = false;
+ BestPrefixIsFramework = false;
+ }
// Try resolving resulting filename via reverse search in header maps,
// key from header name is user prefered name for the include file.
@@ -1934,8 +1968,19 @@ std::string HeaderSearch::suggestPathToFileForDiagnostics(
SearchDirs[I].getHeaderMap()->reverseLookupFilename(Filename);
if (!SpelledFilename.empty()) {
Filename = SpelledFilename;
+ BestPrefixIsFramework = false;
break;
}
}
+
+ // If the best prefix is a framework path, we need to compute the proper
+ // include spelling for the framework header.
+ bool IsPrivateHeader;
+ SmallString<128> FrameworkName, IncludeSpelling;
+ if (BestPrefixIsFramework &&
+ isFrameworkStylePath(Filename, IsPrivateHeader, FrameworkName,
+ IncludeSpelling)) {
+ Filename = IncludeSpelling;
+ }
return path::convert_to_slash(Filename);
}
diff --git a/clang/test/Modules/double-quotes.m b/clang/test/Modules/double-quotes.m
index 99187fc26654..a8790305b5fa 100644
--- a/clang/test/Modules/double-quotes.m
+++ b/clang/test/Modules/double-quotes.m
@@ -24,8 +24,17 @@
// because they only show up under the module A building context.
// RUN: FileCheck --input-file=%t/stderr %s
// CHECK: double-quoted include "A0.h" in framework header, expected angle-bracketed instead
+// CHECK: #include "A0.h"
+// CHECK: ^~~~~~
+// CHECK: <A/A0.h>
// CHECK: double-quoted include "B.h" in framework header, expected angle-bracketed instead
+// CHECK: #include "B.h"
+// CHECK: ^~~~~
+// CHECK: <B.h>
// CHECK: double-quoted include "B.h" in framework header, expected angle-bracketed instead
+// CHECK: #import "B.h" // Included from Z.h & A.h
+// CHECK: ^~~~~
+// CHECK: <B.h>
#import "A.h"
#import <Z/Z.h>
diff --git a/clang/unittests/Lex/HeaderSearchTest.cpp b/clang/unittests/Lex/HeaderSearchTest.cpp
index 6ceec7c9cc0f..a98f90320be8 100644
--- a/clang/unittests/Lex/HeaderSearchTest.cpp
+++ b/clang/unittests/Lex/HeaderSearchTest.cpp
@@ -47,6 +47,15 @@ class HeaderSearchTest : public ::testing::Test {
Search.AddSearchPath(DL, /*isAngled=*/false);
}
+ void addSystemFrameworkSearchDir(llvm::StringRef Dir) {
+ VFS->addFile(Dir, 0, llvm::MemoryBuffer::getMemBuffer(""), /*User=*/None,
+ /*Group=*/None, llvm::sys::fs::file_type::directory_file);
+ auto DE = FileMgr.getOptionalDirectoryRef(Dir);
+ assert(DE);
+ auto DL = DirectoryLookup(*DE, SrcMgr::C_System, /*isFramework=*/true);
+ Search.AddSystemSearchPath(DL);
+ }
+
void addHeaderMap(llvm::StringRef Filename,
std::unique_ptr<llvm::MemoryBuffer> Buf,
bool isAngled = false) {
@@ -155,6 +164,29 @@ TEST_F(HeaderSearchTest, IncludeFromSameDirectory) {
"y/z/t.h");
}
+TEST_F(HeaderSearchTest, SdkFramework) {
+ addSystemFrameworkSearchDir(
+ "/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk/Frameworks/");
+ bool IsSystem = false;
+ EXPECT_EQ(Search.suggestPathToFileForDiagnostics(
+ "/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/"
+ "Frameworks/AppKit.framework/Headers/NSView.h",
+ /*WorkingDir=*/"",
+ /*MainFile=*/"", &IsSystem),
+ "AppKit/NSView.h");
+ EXPECT_TRUE(IsSystem);
+}
+
+TEST_F(HeaderSearchTest, NestedFramework) {
+ addSystemFrameworkSearchDir("/Platforms/MacOSX/Frameworks");
+ EXPECT_EQ(Search.suggestPathToFileForDiagnostics(
+ "/Platforms/MacOSX/Frameworks/AppKit.framework/Frameworks/"
+ "Sub.framework/Headers/Sub.h",
+ /*WorkingDir=*/"",
+ /*MainFile=*/""),
+ "Sub/Sub.h");
+}
+
// Helper struct with null terminator character to make MemoryBuffer happy.
template <class FileTy, class PaddingTy>
struct NullTerminatedFile : public FileTy {
More information about the cfe-commits
mailing list