dgoldman updated this revision to Diff 398178.
dgoldman added a comment.
Don't suggest umbrella headers
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/test/Modules/double-quotes.m
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,29 @@
"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 {
Index: clang/test/Modules/double-quotes.m
===================================================================
--- clang/test/Modules/double-quotes.m
+++ 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>
Index: clang/lib/Lex/HeaderSearch.cpp
===================================================================
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -721,7 +721,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);
@@ -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,19 @@
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);
}
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
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
