dgoldman updated this revision to Diff 392477.
dgoldman added a comment.
Add HeaderSearch tests
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115183/new/
https://reviews.llvm.org/D115183
Files:
clang/include/clang/Lex/HeaderSearch.h
clang/lib/Lex/HeaderSearch.cpp
clang/unittests/Lex/HeaderSearchTest.cpp
Index: clang/unittests/Lex/HeaderSearchTest.cpp
===================================================================
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ 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<llvm::MemoryBuffer> Buf,
bool isAngled = false) {
@@ -155,6 +164,34 @@
"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);
+}
+
// Helper struct with null terminator character to make MemoryBuffer happy.
template <class FileTy, class PaddingTy>
struct NullTerminatedFile : public FileTy {
Index: clang/lib/Lex/HeaderSearch.cpp
===================================================================
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -720,7 +720,8 @@
}
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);
@@ -736,15 +737,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;
}
@@ -759,20 +767,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);
@@ -1843,10 +1855,11 @@
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.
- auto CheckDir = [&](llvm::StringRef Dir) -> bool {
+ bool BestPrefixIsFramework = false;
+ // 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 IsFramework) -> bool {
llvm::SmallString<32> DirPath(Dir.begin(), Dir.end());
if (!WorkingDir.empty() && !path::is_absolute(Dir))
fs::make_absolute(WorkingDir, DirPath);
@@ -1870,6 +1883,7 @@
unsigned PrefixLength = NI - path::begin(File);
if (PrefixLength > BestPrefixLength) {
BestPrefixLength = PrefixLength;
+ BestPrefixIsFramework = IsFramework;
return true;
}
break;
@@ -1880,6 +1894,16 @@
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;
}
@@ -1887,18 +1911,22 @@
};
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, /** IsFramework= */ false) && IsSystem)
+ *IsSystem = BestPrefixLength ? I >= SystemDirIdx : false;
+ } else if (SearchDirs[I].isFramework()) {
+ StringRef Dir = SearchDirs[I].getFrameworkDir()->getName();
+ if (CheckDir(Dir, /** IsFramework= */ true) && IsSystem)
+ *IsSystem = BestPrefixLength ? I >= SystemDirIdx : false;
+ }
}
// 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)
+ if (!BestPrefixLength &&
+ CheckDir(path::parent_path(MainFile), /** IsFramework= */ false) &&
+ IsSystem)
*IsSystem = false;
// Try resolving resulting filename via reverse search in header maps,
@@ -1912,8 +1940,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 `<UIKit/UIView.h>` only if the main file is in `UIKit`.
+ if (isFrameworkStylePath(MainFile, MainIsPrivateHeader, MainFrameworkName,
+ MainIncludeSpelling) &&
+ MainFrameworkName == FrameworkName) {
+ Filename = IncludeSpelling;
+ } else {
+ // Prefer using `<UIKit/UIKit.h>` 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);
}
Index: clang/include/clang/Lex/HeaderSearch.h
===================================================================
--- clang/include/clang/Lex/HeaderSearch.h
+++ 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<std::pair<std::string, bool>> P) {
SystemHeaderPrefixes.assign(P.begin(), P.end());
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
