dgoldman updated this revision to Diff 401422.
dgoldman marked 18 inline comments as done.
dgoldman added a comment.
Changes for review
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D117056/new/
https://reviews.llvm.org/D117056
Files:
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
clang/lib/Lex/HeaderSearch.cpp
Index: clang/lib/Lex/HeaderSearch.cpp
===================================================================
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1049,6 +1049,11 @@
getUniqueFrameworkName(StringRef(Filename.begin(), SlashPos));
if (CurDir->isIndexHeaderMap())
HFI.IndexHeaderMapHeader = 1;
+ } else if (CurDir->isFramework()) {
+ size_t SlashPos = Filename.find('/');
+ if (SlashPos != StringRef::npos)
+ HFI.Framework =
+ getUniqueFrameworkName(StringRef(Filename.begin(), SlashPos));
}
if (checkMSVCHeaderSearch(Diags, MSFE ? &MSFE->getFileEntry() : nullptr,
Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -663,6 +663,53 @@
QName("Cat::meow"), QName("Cat::pur")));
}
+TEST_F(SymbolCollectorTest, ObjCFrameworkIncludeHeader) {
+ CollectorOpts.CollectIncludePath = true;
+ auto FrameworksPath = testPath("Frameworks/");
+ std::string FrameworkHeader = R"(
+ __attribute((objc_root_class))
+ @interface NSObject
+ @end
+ )";
+ InMemoryFileSystem->addFile(
+ testPath("Frameworks/Foundation.framework/Headers/NSObject.h"), 0,
+ llvm::MemoryBuffer::getMemBuffer(FrameworkHeader));
+
+ std::string Header = R"(
+ #import <Foundation/NSObject.h>
+
+ @interface Container : NSObject
+ @end
+ )";
+ std::string Main = "";
+ TestFileName = testPath("test.m");
+ runSymbolCollector(Header, Main, {"-F", FrameworksPath, "-xobjective-c++"});
+ EXPECT_THAT(Symbols, UnorderedElementsAre(
+ AllOf(QName("NSObject"),
+ IncludeHeader("\"Foundation/NSObject.h\"")),
+ AllOf(QName("Container"))));
+
+ // After adding the umbrella header, we should use that spelling instead.
+ std::string UmbrellaHeader = R"(
+ #import <Foundation/NSObject.h>
+ )";
+ InMemoryFileSystem->addFile(
+ testPath("Frameworks/Foundation.framework/Headers/Foundation.h"), 0,
+ llvm::MemoryBuffer::getMemBuffer(UmbrellaHeader));
+ runSymbolCollector(Header, Main, {"-F", FrameworksPath, "-xobjective-c++"});
+ EXPECT_THAT(Symbols, UnorderedElementsAre(
+ AllOf(QName("NSObject"),
+ IncludeHeader("\"Foundation/Foundation.h\"")),
+ AllOf(QName("Container"))));
+
+ runSymbolCollector(Header, Main,
+ {"-iframework", FrameworksPath, "-xobjective-c++"});
+ EXPECT_THAT(Symbols, UnorderedElementsAre(
+ AllOf(QName("NSObject"),
+ IncludeHeader("<Foundation/Foundation.h>")),
+ AllOf(QName("Container"))));
+}
+
TEST_F(SymbolCollectorTest, Locations) {
Annotations Header(R"cpp(
// Declared in header, defined in main.
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -193,6 +193,8 @@
llvm::DenseMap<const FileEntry *, const std::string *> CacheFEToURI;
llvm::StringMap<std::string> CachePathToURI;
llvm::DenseMap<FileID, llvm::StringRef> CacheFIDToInclude;
+ llvm::StringMap<std::string> CachePathToFrameworkSpelling;
+ llvm::StringMap<std::string> CacheFrameworkToUmbrellaHeaderSpelling;
public:
HeaderFileURICache(Preprocessor *&PP, const SourceManager &SM,
@@ -249,6 +251,112 @@
return R.first->second;
}
+ struct FrameworkHeaderPath {
+ // Path to the Headers dir e.g. /Frameworks/Foundation.framework/Headers/
+ llvm::StringRef HeadersDir;
+ // Subpath relative to the Headers dir, e.g. NSObject.h
+ llvm::StringRef HeaderSubpath;
+ };
+
+ llvm::Optional<FrameworkHeaderPath>
+ splitFrameworkHeaderPath(llvm::StringRef Path) {
+ using namespace llvm::sys;
+ path::reverse_iterator I = path::rbegin(Path);
+ path::reverse_iterator Prev = I;
+ path::reverse_iterator E = path::rend(Path);
+ while (I != E) {
+ if (*I == "Headers") {
+ auto PrefixLen = Prev - E;
+ FrameworkHeaderPath HeaderPath;
+ HeaderPath.HeadersDir = Path.substr(0, PrefixLen);
+ HeaderPath.HeaderSubpath = Path.substr(PrefixLen);
+ return HeaderPath;
+ }
+ // We don't currently support handling the private headers dir, just
+ // return early.
+ if (*I == "PrivateHeaders") {
+ return llvm::None;
+ }
+ Prev = I;
+ ++I;
+ }
+ // Unexpected, must not be a framework header.
+ return llvm::None;
+ }
+
+ // Frameworks typically have an umbrella header of the same name, e.g.
+ // <Foundation/Foundation.h> instead of <Foundation/NSObject.h>. We stat
+ // to check for existence of that file.
+ llvm::Optional<llvm::StringRef>
+ getFrameworkUmbrellaSpelling(const FileEntry *FE, llvm::StringRef Framework,
+ HeaderSearch &HS,
+ FrameworkHeaderPath &HeaderPath) {
+ auto Res = CacheFrameworkToUmbrellaHeaderSpelling.try_emplace(Framework);
+ auto *CachedSpelling = &Res.first->second;
+ if (!Res.second) {
+ if (CachedSpelling->empty())
+ return llvm::None;
+ else
+ return llvm::StringRef(*CachedSpelling);
+ }
+
+ SmallString<256> UmbrellaPath(HeaderPath.HeadersDir);
+ llvm::sys::path::append(UmbrellaPath, Framework + ".h");
+
+ llvm::vfs::Status Status;
+ auto StatErr = HS.getFileMgr().getNoncachedStatValue(UmbrellaPath, Status);
+ if (StatErr) {
+ *CachedSpelling = "";
+ return llvm::None;
+ }
+ if (isSystem(HS.getFileDirFlavor(FE)))
+ *CachedSpelling = llvm::formatv("<{0}/{0}.h>", Framework);
+ else
+ *CachedSpelling = llvm::formatv("\"{0}/{0}.h\"", Framework);
+ return llvm::StringRef(*CachedSpelling);
+ }
+
+ // Compute the framework include spelling for `FE` which is in a framework
+ // named `Framework`, e.g. `NSObject.h` in framework `Foundation` would
+ // give <Foundation/Foundation.h> if the umbrella header exists, otherwise
+ // <Foundation/NSObject.h>.
+ llvm::Optional<llvm::StringRef> getFrameworkHeaderIncludeSpelling(
+ const FileEntry *FE, llvm::StringRef Framework, HeaderSearch &HS) {
+ auto Path = FE->getName();
+ auto It = CachePathToFrameworkSpelling.find(Path);
+ if (It != CachePathToFrameworkSpelling.end())
+ return llvm::StringRef(It->second);
+
+ auto HeaderPath = splitFrameworkHeaderPath(Path);
+ if (!HeaderPath) {
+ // Possible for private headers in a framework; we don't currently support
+ // them. Cache the failure as well so we won't do this again for the same
+ // header.
+ CachePathToFrameworkSpelling[Path] = "";
+ return llvm::None;
+ }
+ if (auto UmbrellaSpelling =
+ getFrameworkUmbrellaSpelling(FE, Framework, HS, *HeaderPath)) {
+ CachePathToFrameworkSpelling[Path] = UmbrellaSpelling->str();
+ return UmbrellaSpelling;
+ }
+
+ auto Res = CachePathToFrameworkSpelling.try_emplace(Path);
+ auto *CachedHeaderSpelling = &Res.first->second;
+ if (!Res.second)
+ return llvm::StringRef(*CachedHeaderSpelling);
+
+ if (isSystem(HS.getFileDirFlavor(FE)))
+ *CachedHeaderSpelling =
+ llvm::formatv("<{0}/{1}>", Framework, HeaderPath->HeaderSubpath)
+ .str();
+ else
+ *CachedHeaderSpelling =
+ llvm::formatv("\"{0}/{1}\"", Framework, HeaderPath->HeaderSubpath)
+ .str();
+ return llvm::StringRef(*CachedHeaderSpelling);
+ }
+
llvm::StringRef getIncludeHeaderUncached(FileID FID) {
const FileEntry *FE = SM.getFileEntryForID(FID);
if (!FE || FE->getName().empty())
@@ -265,6 +373,15 @@
return toURI(Canonical);
}
}
+ // Framework headers are spelled as <FrameworkName/Foo.h>, not
+ // "path/FrameworkName.framework/Headers/Foo.h".
+ auto &HS = PP->getHeaderSearchInfo();
+ if (const auto *HFI = HS.getExistingFileInfo(FE, /*WantExternal*/ false))
+ if (!HFI->Framework.empty())
+ if (auto Spelling =
+ getFrameworkHeaderIncludeSpelling(FE, HFI->Framework, HS))
+ return *Spelling;
+
if (!isSelfContainedHeader(FE, FID, PP->getSourceManager(),
PP->getHeaderSearchInfo())) {
// A .inc or .def file is often included into a real header to define
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
