diff --git a/clang/include/clang/Lex/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h --- a/clang/include/clang/Lex/HeaderSearch.h +++ b/clang/include/clang/Lex/HeaderSearch.h @@ -303,6 +303,12 @@ SystemDirIdx++; } + /// 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> P) { SystemHeaderPrefixes.assign(P.begin(), P.end()); diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp --- a/clang/lib/Lex/HeaderSearch.cpp +++ b/clang/lib/Lex/HeaderSearch.cpp @@ -721,7 +721,8 @@ } static bool isFrameworkStylePath(StringRef Path, bool &IsPrivateHeader, - SmallVectorImpl &FrameworkName) { + SmallVectorImpl &FrameworkName, + SmallVectorImpl &IncludeSpelling) { using namespace llvm::sys; path::const_iterator I = path::begin(Path); path::const_iterator E = path::end(Path); @@ -737,15 +738,22 @@ // 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; } @@ -760,20 +768,24 @@ 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); @@ -1841,9 +1853,9 @@ 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)) @@ -1878,26 +1890,48 @@ 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. @@ -1910,8 +1944,39 @@ 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, MainIsPrivateHeader; + SmallString<128> FrameworkName, IncludeSpelling, MainFrameworkName, + MainIncludeSpelling; + if (BestPrefixIsFramework && + isFrameworkStylePath(Filename, IsPrivateHeader, FrameworkName, + IncludeSpelling)) { + // Prefer using `` only if the main file is in `UIKit`. + if (isFrameworkStylePath(MainFile, MainIsPrivateHeader, MainFrameworkName, + MainIncludeSpelling) && + MainFrameworkName == FrameworkName) { + Filename = IncludeSpelling; + } else { + // Prefer using `` for non-`UIKit` main files. + // + // This assumes the framework has an umbrella header of the same name. + // There's not much more we can do without hitting the filesystem, + // especially if there's no modulemap. + IncludeSpelling.clear(); + if (IsPrivateHeader) + llvm::sys::path::append(IncludeSpelling, FrameworkName, + FrameworkName + "_Private.h"); + else + llvm::sys::path::append(IncludeSpelling, FrameworkName, + FrameworkName + ".h"); + Filename = IncludeSpelling; + } + } return path::convert_to_slash(Filename); } diff --git a/clang/test/Modules/double-quotes.m b/clang/test/Modules/double-quotes.m --- 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: // CHECK: double-quoted include "B.h" in framework header, expected angle-bracketed instead +// CHECK: #include "B.h" +// CHECK: ^~~~~ +// CHECK: // 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: #import "A.h" #import diff --git a/clang/unittests/Lex/HeaderSearchTest.cpp b/clang/unittests/Lex/HeaderSearchTest.cpp --- a/clang/unittests/Lex/HeaderSearchTest.cpp +++ b/clang/unittests/Lex/HeaderSearchTest.cpp @@ -47,6 +47,15 @@ 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 Buf, bool isAngled = false) { @@ -155,6 +164,44 @@ "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/AppKit.h"); + EXPECT_TRUE(IsSystem); +} + +TEST_F(HeaderSearchTest, SdkIntraFramework) { + 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=*/"", + "/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/" + "Frameworks/AppKit.framework/Headers/NSViewController.h", + &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 struct NullTerminatedFile : public FileTy {