intel/llvm
1
0
Fork 0
mirror of https://github.com/intel/llvm.git synced 2026-01-20 19:07:53 +08:00
Code Issues Packages Projects Releases Wiki Activity

[clang][HeaderSearch] Treat framework headers as Angled for suggestPath

Browse Source 
- Rename the IsSystem flag to be IsAngled since that's how callers
  actually use the flag.

- Since frameworks by convention use <> style includes, make sure
  we treat them as Angled

Also update clangd's custom logic for frameworks accordingly.

Differential Revision: https://reviews.llvm.org/D156704
This commit is contained in:
David Goldman
2023-07-31 11:36:22 -04:00
parent e7e4ed0d7a
commit 9fe632ba18
10 changed files with 76 additions and 75 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
clang-tools-extra/clang-include-fixer/IncludeFixer.cpp
View File

@@ -314,11 +314,11 @@ std::string IncludeFixerSemaSource::minimizeInclude(
if (!Entry)
return std::string(Include);
bool IsSystem = false;
bool IsAngled = false;
std::string Suggestion =
HeaderSearch.suggestPathToFileForDiagnostics(*Entry, "", &IsSystem);
HeaderSearch.suggestPathToFileForDiagnostics(*Entry, "", &IsAngled);
return IsSystem ? '<' + Suggestion + '>' : '"' + Suggestion + '"';
return IsAngled ? '<' + Suggestion + '>' : '"' + Suggestion + '"';
}
/// Get the include fixer context for the queried symbol.

6
clang-tools-extra/clangd/Headers.cpp
View File

@@ -286,11 +286,11 @@ IncludeInserter::calculateIncludePath(const HeaderFile &InsertedHeader,
assert(InsertedHeader.valid());
if (InsertedHeader.Verbatim)
return InsertedHeader.File;
bool IsSystem = false;
bool IsAngled = false;
std::string Suggested;
if (HeaderSearchInfo) {
Suggested = HeaderSearchInfo->suggestPathToFileForDiagnostics(
InsertedHeader.File, BuildDir, IncludingFile, &IsSystem);
InsertedHeader.File, BuildDir, IncludingFile, &IsAngled);
} else {
// Calculate include relative to including file only.
StringRef IncludingDir = llvm::sys::path::parent_path(IncludingFile);
@@ -303,7 +303,7 @@ IncludeInserter::calculateIncludePath(const HeaderFile &InsertedHeader,
// FIXME: should we allow (some limited number of) "../header.h"?
if (llvm::sys::path::is_absolute(Suggested))
return std::nullopt;
if (IsSystem)
if (IsAngled)
Suggested = "<" + Suggested + ">";
else
Suggested = "\"" + Suggested + "\"";

42
clang-tools-extra/clangd/index/SymbolCollector.cpp
View File

@@ -328,42 +328,33 @@ private:
// <Foundation/Foundation_Private.h> instead of
// <Foundation/NSObject_Private.h> which should be used instead of directly
// importing the header.
std::optional<std::string> getFrameworkUmbrellaSpelling(
llvm::StringRef Framework, SrcMgr::CharacteristicKind HeadersDirKind,
const HeaderSearch &HS, FrameworkHeaderPath &HeaderPath) {
std::optional<std::string>
getFrameworkUmbrellaSpelling(llvm::StringRef Framework,
const HeaderSearch &HS,
FrameworkHeaderPath &HeaderPath) {
auto Res = CacheFrameworkToUmbrellaHeaderSpelling.try_emplace(Framework);
auto *CachedSpelling = &Res.first->second;
if (!Res.second) {
return HeaderPath.IsPrivateHeader ? CachedSpelling->PrivateHeader
: CachedSpelling->PublicHeader;
}
bool IsSystem = isSystem(HeadersDirKind);
SmallString<256> UmbrellaPath(HeaderPath.HeadersParentDir);
llvm::sys::path::append(UmbrellaPath, "Headers", Framework + ".h");
llvm::vfs::Status Status;
auto StatErr = HS.getFileMgr().getNoncachedStatValue(UmbrellaPath, Status);
if (!StatErr) {
if (IsSystem)
CachedSpelling->PublicHeader = llvm::formatv("<{0}/{0}.h>", Framework);
else
CachedSpelling->PublicHeader =
llvm::formatv("\"{0}/{0}.h\"", Framework);
}
if (!StatErr)
CachedSpelling->PublicHeader = llvm::formatv("<{0}/{0}.h>", Framework);
UmbrellaPath = HeaderPath.HeadersParentDir;
llvm::sys::path::append(UmbrellaPath, "PrivateHeaders",
Framework + "_Private.h");
StatErr = HS.getFileMgr().getNoncachedStatValue(UmbrellaPath, Status);
if (!StatErr) {
if (IsSystem)
CachedSpelling->PrivateHeader =
llvm::formatv("<{0}/{0}_Private.h>", Framework);
else
CachedSpelling->PrivateHeader =
llvm::formatv("\"{0}/{0}_Private.h\"", Framework);
}
if (!StatErr)
CachedSpelling->PrivateHeader =
llvm::formatv("<{0}/{0}_Private.h>", Framework);
return HeaderPath.IsPrivateHeader ? CachedSpelling->PrivateHeader
: CachedSpelling->PublicHeader;
}
@@ -386,21 +377,14 @@ private:
CachePathToFrameworkSpelling.erase(Res.first);
return std::nullopt;
}
auto DirKind = HS.getFileDirFlavor(FE);
if (auto UmbrellaSpelling =
getFrameworkUmbrellaSpelling(Framework, DirKind, HS, *HeaderPath)) {
getFrameworkUmbrellaSpelling(Framework, HS, *HeaderPath)) {
*CachedHeaderSpelling = *UmbrellaSpelling;
return llvm::StringRef(*CachedHeaderSpelling);
}
if (isSystem(DirKind))
*CachedHeaderSpelling =
llvm::formatv("<{0}/{1}>", Framework, HeaderPath->HeaderSubpath)
.str();
else
*CachedHeaderSpelling =
llvm::formatv("\"{0}/{1}\"", Framework, HeaderPath->HeaderSubpath)
.str();
*CachedHeaderSpelling =
llvm::formatv("<{0}/{1}>", Framework, HeaderPath->HeaderSubpath).str();
return llvm::StringRef(*CachedHeaderSpelling);
}

18
clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
View File

@@ -702,9 +702,9 @@ TEST_F(SymbolCollectorTest, ObjCFrameworkIncludeHeader) {
EXPECT_THAT(
Symbols,
UnorderedElementsAre(
AllOf(qName("NSObject"), includeHeader("\"Foundation/NSObject.h\"")),
AllOf(qName("NSObject"), includeHeader("<Foundation/NSObject.h>")),
AllOf(qName("PrivateClass"),
includeHeader("\"Foundation/NSObject+Private.h\"")),
includeHeader("<Foundation/NSObject+Private.h>")),
AllOf(qName("Container"))));
// After adding the umbrella headers, we should use that spelling instead.
@@ -722,13 +722,13 @@ TEST_F(SymbolCollectorTest, ObjCFrameworkIncludeHeader) {
"Foundation_Private.h"),
0, llvm::MemoryBuffer::getMemBuffer(PrivateUmbrellaHeader));
runSymbolCollector(Header, Main, {"-F", FrameworksPath, "-xobjective-c++"});
EXPECT_THAT(Symbols,
UnorderedElementsAre(
AllOf(qName("NSObject"),
includeHeader("\"Foundation/Foundation.h\"")),
AllOf(qName("PrivateClass"),
includeHeader("\"Foundation/Foundation_Private.h\"")),
AllOf(qName("Container"))));
EXPECT_THAT(
Symbols,
UnorderedElementsAre(
AllOf(qName("NSObject"), includeHeader("<Foundation/Foundation.h>")),
AllOf(qName("PrivateClass"),
includeHeader("<Foundation/Foundation_Private.h>")),
AllOf(qName("Container"))));
runSymbolCollector(Header, Main,
{"-iframework", FrameworksPath, "-xobjective-c++"});

6
clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
View File

@@ -170,10 +170,10 @@ class Reporter {
std::string spellHeader(const Header &H) {
switch (H.kind()) {
case Header::Physical: {
bool IsSystem = false;
bool IsAngled = false;
std::string Path = HS.suggestPathToFileForDiagnostics(
H.physical(), MainFE->tryGetRealPathName(), &IsSystem);
return IsSystem ? "<" + Path + ">" : "\"" + Path + "\"";
H.physical(), MainFE->tryGetRealPathName(), &IsAngled);
return IsAngled ? "<" + Path + ">" : "\"" + Path + "\"";
}
case Header::Standard:
return H.standard().name().str();

6
clang-tools-extra/include-cleaner/lib/IncludeSpeller.cpp
View File

@@ -29,10 +29,10 @@ public:
case Header::Verbatim:
return Input.H.verbatim().str();
case Header::Physical:
bool IsSystem = false;
bool IsAngled = false;
std::string FinalSpelling = Input.HS.suggestPathToFileForDiagnostics(
Input.H.physical(), Input.Main->tryGetRealPathName(), &IsSystem);
return IsSystem ? "<" + FinalSpelling + ">" : "\"" + FinalSpelling + "\"";
Input.H.physical(), Input.Main->tryGetRealPathName(), &IsAngled);
return IsAngled ? "<" + FinalSpelling + ">" : "\"" + FinalSpelling + "\"";
}
llvm_unreachable("Unknown clang::include_cleaner::Header::Kind enum");
}

8
clang/include/clang/Lex/HeaderSearch.h
View File

@@ -871,11 +871,11 @@ public:
/// MainFile location, if none of the include search directories were prefix
/// of File.
///
/// \param IsSystem If non-null, filled in to indicate whether the suggested
/// path is relative to a system header directory.
/// \param IsAngled If non-null, filled in to indicate whether the suggested
/// path should be referenced as <Header.h> instead of "Header.h".
std::string suggestPathToFileForDiagnostics(const FileEntry *File,
llvm::StringRef MainFile,
bool *IsSystem = nullptr) const;
bool *IsAngled = nullptr) const;
/// Suggest a path by which the specified file could be found, for use in
/// diagnostics to suggest a #include. Returned path will only contain forward
@@ -889,7 +889,7 @@ public:
std::string suggestPathToFileForDiagnostics(llvm::StringRef File,
llvm::StringRef WorkingDir,
llvm::StringRef MainFile,
bool *IsSystem = nullptr) const;
bool *IsAngled = nullptr) const;
void PrintStats();

19
clang/lib/Lex/HeaderSearch.cpp
View File

@@ -1928,17 +1928,17 @@ void HeaderSearch::loadSubdirectoryModuleMaps(DirectoryLookup &SearchDir) {
}
std::string HeaderSearch::suggestPathToFileForDiagnostics(
const FileEntry *File, llvm::StringRef MainFile, bool *IsSystem) const {
const FileEntry *File, llvm::StringRef MainFile, bool *IsAngled) const {
// FIXME: We assume that the path name currently cached in the FileEntry is
// the most appropriate one for this analysis (and that it's spelled the
// same way as the corresponding header search path).
return suggestPathToFileForDiagnostics(File->getName(), /*WorkingDir=*/"",
MainFile, IsSystem);
MainFile, IsAngled);
}
std::string HeaderSearch::suggestPathToFileForDiagnostics(
llvm::StringRef File, llvm::StringRef WorkingDir, llvm::StringRef MainFile,
bool *IsSystem) const {
bool *IsAngled) const {
using namespace llvm::sys;
llvm::SmallString<32> FilePath = File;
@@ -1996,15 +1996,16 @@ std::string HeaderSearch::suggestPathToFileForDiagnostics(
if (DL.isNormalDir()) {
StringRef Dir = DL.getDirRef()->getName();
if (CheckDir(Dir)) {
if (IsSystem)
*IsSystem = BestPrefixLength && isSystem(DL.getDirCharacteristic());
if (IsAngled)
*IsAngled = BestPrefixLength && isSystem(DL.getDirCharacteristic());
BestPrefixIsFramework = false;
}
} else if (DL.isFramework()) {
StringRef Dir = DL.getFrameworkDirRef()->getName();
if (CheckDir(Dir)) {
if (IsSystem)
*IsSystem = BestPrefixLength && isSystem(DL.getDirCharacteristic());
// Framework includes by convention use <>.
if (IsAngled)
*IsAngled = BestPrefixLength;
BestPrefixIsFramework = true;
}
}
@@ -2013,8 +2014,8 @@ std::string HeaderSearch::suggestPathToFileForDiagnostics(
// 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))) {
if (IsSystem)
*IsSystem = false;
if (IsAngled)
*IsAngled = false;
BestPrefixIsFramework = false;
}

6
clang/lib/Sema/SemaLookup.cpp
View File

@@ -5701,10 +5701,10 @@ void Sema::diagnoseMissingImport(SourceLocation Loc, const NamedDecl *Decl,
/// suggesting the addition of a #include of the specified file.
static std::string getHeaderNameForHeader(Preprocessor &PP, const FileEntry *E,
llvm::StringRef IncludingFile) {
bool IsSystem = false;
bool IsAngled = false;
auto Path = PP.getHeaderSearchInfo().suggestPathToFileForDiagnostics(
E, IncludingFile, &IsSystem);
return (IsSystem ? '<' : '"') + Path + (IsSystem ? '>' : '"');
E, IncludingFile, &IsAngled);
return (IsAngled ? '<' : '"') + Path + (IsAngled ? '>' : '"');
}
void Sema::diagnoseMissingImport(SourceLocation UseLoc, const NamedDecl *Decl,

34
clang/unittests/Lex/HeaderSearchTest.cpp
View File

@@ -47,14 +47,18 @@ protected:
Search.AddSearchPath(DL, /*isAngled=*/false);
}
void addSystemFrameworkSearchDir(llvm::StringRef Dir) {
void addFrameworkSearchDir(llvm::StringRef Dir, bool IsSystem = true) {
VFS->addFile(
Dir, 0, llvm::MemoryBuffer::getMemBuffer(""), /*User=*/std::nullopt,
/*Group=*/std::nullopt, 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);
auto DL = DirectoryLookup(*DE, IsSystem ? SrcMgr::C_System : SrcMgr::C_User,
/*isFramework=*/true);
if (IsSystem)
Search.AddSystemSearchPath(DL);
else
Search.AddSearchPath(DL, /*isAngled=*/true);
}
void addHeaderMap(llvm::StringRef Filename,
@@ -175,20 +179,32 @@ TEST_F(HeaderSearchTest, IncludeFromSameDirectory) {
}
TEST_F(HeaderSearchTest, SdkFramework) {
addSystemFrameworkSearchDir(
addFrameworkSearchDir(
"/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.3.sdk/Frameworks/");
bool IsSystem = false;
bool IsAngled = false;
EXPECT_EQ(Search.suggestPathToFileForDiagnostics(
"/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/"
"Frameworks/AppKit.framework/Headers/NSView.h",
/*WorkingDir=*/"",
/*MainFile=*/"", &IsSystem),
/*MainFile=*/"", &IsAngled),
"AppKit/NSView.h");
EXPECT_TRUE(IsSystem);
EXPECT_TRUE(IsAngled);
addFrameworkSearchDir("/System/Developer/Library/Framworks/",
/*IsSystem*/ false);
EXPECT_EQ(Search.suggestPathToFileForDiagnostics(
"/System/Developer/Library/Framworks/"
"Foo.framework/Headers/Foo.h",
/*WorkingDir=*/"",
/*MainFile=*/"", &IsAngled),
"Foo/Foo.h");
// Expect to be true even though we passed false to IsSystem earlier since
// all frameworks should be treated as <>.
EXPECT_TRUE(IsAngled);
}
TEST_F(HeaderSearchTest, NestedFramework) {
addSystemFrameworkSearchDir("/Platforms/MacOSX/Frameworks");
addFrameworkSearchDir("/Platforms/MacOSX/Frameworks");
EXPECT_EQ(Search.suggestPathToFileForDiagnostics(
"/Platforms/MacOSX/Frameworks/AppKit.framework/Frameworks/"
"Sub.framework/Headers/Sub.h",
@@ -199,7 +215,7 @@ TEST_F(HeaderSearchTest, NestedFramework) {
TEST_F(HeaderSearchTest, HeaderFrameworkLookup) {
std::string HeaderPath = "/tmp/Frameworks/Foo.framework/Headers/Foo.h";
addSystemFrameworkSearchDir("/tmp/Frameworks");
addFrameworkSearchDir("/tmp/Frameworks");
VFS->addFile(HeaderPath, 0,
llvm::MemoryBuffer::getMemBufferCopy("", HeaderPath),
/*User=*/std::nullopt, /*Group=*/std::nullopt,