https://github.com/cyndyishida updated https://github.com/llvm/llvm-project/pull/81701
>From 9d0da0010f145b23ce2b7e5b6183558609600789 Mon Sep 17 00:00:00 2001 From: Cyndy Ishida <cyndy_ish...@apple.com> Date: Tue, 13 Feb 2024 18:22:23 -0800 Subject: [PATCH 1/4] [clang][InstallAPI] Add input file support to library This patch adds support for expected InstallAPI inputs. InstallAPI accepts a well defined filelist of headers and how those headers represent a single library. InstallAPI captures header files to determine linkable symbols to then compare against what was compiled in a binary dylib and generate TBD files. --- clang/include/clang/InstallAPI/FileList.h | 53 +++++ clang/include/clang/InstallAPI/HeaderFile.h | 72 +++++++ clang/lib/ExtractAPI/CMakeLists.txt | 1 + clang/lib/ExtractAPI/ExtractAPIConsumer.cpp | 7 +- clang/lib/InstallAPI/CMakeLists.txt | 2 + clang/lib/InstallAPI/FileList.cpp | 196 ++++++++++++++++++ clang/lib/InstallAPI/HeaderFile.cpp | 37 ++++ clang/unittests/CMakeLists.txt | 1 + clang/unittests/InstallAPI/CMakeLists.txt | 9 + clang/unittests/InstallAPI/FileListTest.cpp | 140 +++++++++++++ clang/unittests/InstallAPI/HeaderFileTest.cpp | 89 ++++++++ 11 files changed, 603 insertions(+), 4 deletions(-) create mode 100644 clang/include/clang/InstallAPI/FileList.h create mode 100644 clang/include/clang/InstallAPI/HeaderFile.h create mode 100644 clang/lib/InstallAPI/FileList.cpp create mode 100644 clang/lib/InstallAPI/HeaderFile.cpp create mode 100644 clang/unittests/InstallAPI/CMakeLists.txt create mode 100644 clang/unittests/InstallAPI/FileListTest.cpp create mode 100644 clang/unittests/InstallAPI/HeaderFileTest.cpp diff --git a/clang/include/clang/InstallAPI/FileList.h b/clang/include/clang/InstallAPI/FileList.h new file mode 100644 index 00000000000000..01ec9b35e31490 --- /dev/null +++ b/clang/include/clang/InstallAPI/FileList.h @@ -0,0 +1,53 @@ +//===- InstallAPI/FileList.h ------------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +/// +/// The JSON file list parser is used to communicate input to InstallAPI. +/// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_INSTALLAPI_FILELIST_H +#define LLVM_CLANG_INSTALLAPI_FILELIST_H + +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/FileManager.h" +#include "clang/InstallAPI/HeaderFile.h" +#include "llvm/Support/Error.h" +#include "llvm/Support/MemoryBuffer.h" + +namespace clang { +namespace installapi { + +/// JSON decoder for InstallAPI Inputs. +class FileListReader { + + class Implementation; + Implementation &Impl; + + FileListReader(std::unique_ptr<llvm::MemoryBuffer> InputBuffer, + llvm::Error &Err); + +public: + /// Decode JSON input and append header input into destination container. + /// Headers are loaded in the order they appear in the JSON input. + /// + /// \param InputBuffer JSON input data. + /// \param Destination Container to load headers into. + static llvm::Error + loadHeaders(std::unique_ptr<llvm::MemoryBuffer> InputBuffer, + HeaderSeq &Destination); + + ~FileListReader(); + + FileListReader(const FileListReader &) = delete; + FileListReader &operator=(const FileListReader &) = delete; +}; + +} // namespace installapi +} // namespace clang + +#endif // LLVM_CLANG_INSTALLAPI_FILELIST_H diff --git a/clang/include/clang/InstallAPI/HeaderFile.h b/clang/include/clang/InstallAPI/HeaderFile.h new file mode 100644 index 00000000000000..6ccd944f8b01be --- /dev/null +++ b/clang/include/clang/InstallAPI/HeaderFile.h @@ -0,0 +1,72 @@ +//===- InstallAPI/HeaderFile.h ----------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +/// +/// Representations of a library's headers for InstallAPI. +/// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_INSTALLAPI_HEADERFILE_H +#define LLVM_CLANG_INSTALLAPI_HEADERFILE_H + +#include "clang/Basic/LangStandard.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/Regex.h" +#include <optional> +#include <string> + +namespace clang::installapi { +enum class HeaderType { + /// Represents declarations accessible to all clients. + Public, + /// Represents declarations accessible to a disclosed set of clients. + Private, + /// Represents declarations only accessible as implementation details to the + /// input library. + Project, +}; + +class HeaderFile { + /// Full input path to header. + std::string FullPath; + /// Access level of header. + HeaderType Type; + /// Expected way header will be included by clients. + std::string IncludeName; + /// Supported language mode for header. + std::optional<clang::Language> Language; + +public: + HeaderFile(StringRef FullPath, HeaderType Type, + StringRef IncludeName = StringRef(), + std::optional<clang::Language> Language = std::nullopt) + : FullPath(FullPath), Type(Type), IncludeName(IncludeName), + Language(Language) {} + + static llvm::Regex getFrameworkIncludeRule(); + + bool operator==(const HeaderFile &Other) const { + return std::tie(Type, FullPath, IncludeName, Language) == + std::tie(Other.Type, Other.FullPath, Other.IncludeName, + Other.Language); + } +}; + +/// Assemble expected way header will be included by clients. +/// As in what maps inside the brackets of `#include <IncludeName.h>` +/// For example, +/// "/System/Library/Frameworks/Foo.framework/Headers/Foo.h" returns +/// "Foo/Foo.h" +/// +/// \param FullPath Path to the header file which includes the library +/// structure. +std::optional<std::string> createIncludeHeaderName(const StringRef FullPath); +using HeaderSeq = std::vector<HeaderFile>; + +} // namespace clang::installapi + +#endif // LLVM_CLANG_INSTALLAPI_HEADERFILE_H diff --git a/clang/lib/ExtractAPI/CMakeLists.txt b/clang/lib/ExtractAPI/CMakeLists.txt index b43fe742478ce2..f0e3e06c0ec17e 100644 --- a/clang/lib/ExtractAPI/CMakeLists.txt +++ b/clang/lib/ExtractAPI/CMakeLists.txt @@ -17,5 +17,6 @@ add_clang_library(clangExtractAPI clangBasic clangFrontend clangIndex + clangInstallAPI clangLex ) diff --git a/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp b/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp index fd62d841197d9f..275f49be22e15a 100644 --- a/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp +++ b/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp @@ -30,6 +30,7 @@ #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/FrontendOptions.h" #include "clang/Frontend/MultiplexConsumer.h" +#include "clang/InstallAPI/HeaderFile.h" #include "clang/Lex/MacroInfo.h" #include "clang/Lex/PPCallbacks.h" #include "clang/Lex/Preprocessor.h" @@ -61,9 +62,6 @@ std::optional<std::string> getRelativeIncludeName(const CompilerInstance &CI, "CompilerInstance does not have a FileNamager!"); using namespace llvm::sys; - // Matches framework include patterns - const llvm::Regex Rule("/(.+)\\.framework/(.+)?Headers/(.+)"); - const auto &FS = CI.getVirtualFileSystem(); SmallString<128> FilePath(File.begin(), File.end()); @@ -147,7 +145,8 @@ std::optional<std::string> getRelativeIncludeName(const CompilerInstance &CI, // include name `<Framework/Header.h>` if (Entry.IsFramework) { SmallVector<StringRef, 4> Matches; - Rule.match(File, &Matches); + clang::installapi::HeaderFile::getFrameworkIncludeRule().match( + File, &Matches); // Returned matches are always in stable order. if (Matches.size() != 4) return std::nullopt; diff --git a/clang/lib/InstallAPI/CMakeLists.txt b/clang/lib/InstallAPI/CMakeLists.txt index 1476b737c5e61c..6c9cb4b559f67d 100644 --- a/clang/lib/InstallAPI/CMakeLists.txt +++ b/clang/lib/InstallAPI/CMakeLists.txt @@ -5,6 +5,8 @@ set(LLVM_LINK_COMPONENTS add_clang_library(clangInstallAPI Context.cpp + FileList.cpp + HeaderFile.cpp LINK_LIBS clangAST diff --git a/clang/lib/InstallAPI/FileList.cpp b/clang/lib/InstallAPI/FileList.cpp new file mode 100644 index 00000000000000..6b307682346e2d --- /dev/null +++ b/clang/lib/InstallAPI/FileList.cpp @@ -0,0 +1,196 @@ +//===- FileList.cpp ---------------------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "clang/InstallAPI/FileList.h" +#include "clang/Basic/DiagnosticFrontend.h" +#include "clang/InstallAPI/FileList.h" +#include "llvm/ADT/StringSwitch.h" +#include "llvm/Support/Error.h" +#include "llvm/Support/JSON.h" +#include "llvm/TextAPI/TextAPIError.h" +#include <optional> + +// clang-format off +/* +InstallAPI JSON Input Format specification. + +{ + "headers" : [ # Required: Key must exist. + { # Optional: May contain 0 or more header inputs. + "path" : "/usr/include/mach-o/dlfn.h", # Required: Path should point to destination + # location where applicable. + "type" : "public", # Required: Maps to HeaderType for header. + "language": "c++" # Optional: Language mode for header. + } + ], + "version" : "3" # Required: Version 3 supports language mode + & project header input. +} +*/ +// clang-format on + +using namespace llvm; +using namespace llvm::json; +using namespace llvm::MachO; +using namespace clang::installapi; + +class FileListReader::Implementation { +private: + Expected<StringRef> parseString(const Object *Obj, StringRef Key, + StringRef Error); + Expected<StringRef> parsePath(const Object *Obj); + Expected<HeaderType> parseType(const Object *Obj); + std::optional<clang::Language> parseLanguage(const Object *Obj); + Error parseHeaders(Array &Headers); + +public: + std::unique_ptr<MemoryBuffer> InputBuffer; + unsigned Version; + HeaderSeq HeaderList; + + Error parse(StringRef Input); +}; + +Expected<StringRef> +FileListReader::Implementation::parseString(const Object *Obj, StringRef Key, + StringRef Error) { + auto Str = Obj->getString(Key); + if (!Str) + return make_error<StringError>(Error, inconvertibleErrorCode()); + return *Str; +} + +Expected<HeaderType> +FileListReader::Implementation::parseType(const Object *Obj) { + auto TypeStr = + parseString(Obj, "type", "required field 'type' not specified"); + if (!TypeStr) + return TypeStr.takeError(); + + if (*TypeStr == "public") + return HeaderType::Public; + else if (*TypeStr == "private") + return HeaderType::Private; + else if (*TypeStr == "project" && Version >= 2) + return HeaderType::Project; + + return make_error<TextAPIError>(TextAPIErrorCode::InvalidInputFormat, + "unsupported header type"); +} + +Expected<StringRef> +FileListReader::Implementation::parsePath(const Object *Obj) { + auto Path = parseString(Obj, "path", "required field 'path' not specified"); + if (!Path) + return Path.takeError(); + + return *Path; +} + +std::optional<clang::Language> +FileListReader::Implementation::parseLanguage(const Object *Obj) { + auto Language = Obj->getString("language"); + if (!Language) + return std::nullopt; + + return StringSwitch<clang::Language>(*Language) + .Case("c", clang::Language::C) + .Case("c++", clang::Language::CXX) + .Case("objective-c", clang::Language::ObjC) + .Case("objective-c++", clang::Language::ObjCXX) + .Default(clang::Language::Unknown); +} + +Error FileListReader::Implementation::parseHeaders(Array &Headers) { + for (const auto &H : Headers) { + auto *Obj = H.getAsObject(); + if (!Obj) + return make_error<StringError>("expect a JSON object", + inconvertibleErrorCode()); + auto Type = parseType(Obj); + if (!Type) + return Type.takeError(); + auto Path = parsePath(Obj); + if (!Path) + return Path.takeError(); + auto Language = parseLanguage(Obj); + + StringRef PathStr = *Path; + if (*Type == HeaderType::Project) { + HeaderList.emplace_back( + HeaderFile{PathStr, *Type, /*IncludeName=*/"", Language}); + continue; + } + auto IncludeName = createIncludeHeaderName(PathStr); + HeaderList.emplace_back(PathStr, *Type, + IncludeName.has_value() ? IncludeName.value() : "", + Language); + } + + return Error::success(); +} + +Error FileListReader::Implementation::parse(StringRef Input) { + auto Val = json::parse(Input); + if (!Val) + return Val.takeError(); + + auto *Root = Val->getAsObject(); + if (!Root) + return make_error<StringError>("not a JSON object", + inconvertibleErrorCode()); + + auto VersionStr = Root->getString("version"); + if (!VersionStr) + return make_error<TextAPIError>(TextAPIErrorCode::InvalidInputFormat, + "required field 'version' not specified"); + if (VersionStr->getAsInteger(10, Version)) + return make_error<TextAPIError>(TextAPIErrorCode::InvalidInputFormat, + "invalid version number"); + + if (Version < 1 || Version > 3) + return make_error<TextAPIError>(TextAPIErrorCode::InvalidInputFormat, + "unsupported version"); + + // Not specifying any header files should be atypical, but valid. + auto Headers = Root->getArray("headers"); + if (!Headers) + return Error::success(); + + Error Err = parseHeaders(*Headers); + if (Err) + return Err; + + return Error::success(); +} + +FileListReader::FileListReader(std::unique_ptr<MemoryBuffer> InputBuffer, + Error &Error) + : Impl(*new FileListReader::Implementation()) { + ErrorAsOutParameter ErrorAsOutParam(&Error); + Impl.InputBuffer = std::move(InputBuffer); + + Error = Impl.parse(Impl.InputBuffer->getBuffer()); +} + +llvm::Error +FileListReader::loadHeaders(std::unique_ptr<MemoryBuffer> InputBuffer, + HeaderSeq &Destination) { + llvm::Error Err = Error::success(); + std::unique_ptr<FileListReader> Reader( + new FileListReader(std::move(InputBuffer), Err)); + if (Err) + return Err; + + Destination.reserve(Destination.size() + Reader->Impl.HeaderList.size()); + llvm::move(Reader->Impl.HeaderList, std::back_inserter(Destination)); + + return Err; +} + +FileListReader::~FileListReader() { delete &Impl; } diff --git a/clang/lib/InstallAPI/HeaderFile.cpp b/clang/lib/InstallAPI/HeaderFile.cpp new file mode 100644 index 00000000000000..9c42a2d1d3b44a --- /dev/null +++ b/clang/lib/InstallAPI/HeaderFile.cpp @@ -0,0 +1,37 @@ +//===- HeaderFile.cpp ------------------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "clang/InstallAPI/HeaderFile.h" + +using namespace llvm; +namespace clang::installapi { + +llvm::Regex getFrameworkIncludeRule() { + return llvm::Regex("/(.+)\\.framework/(.+)?Headers/(.+)"); +} + +std::optional<std::string> createIncludeHeaderName(const StringRef FullPath) { + // Headers in usr(/local)*/include. + std::string Pattern = "/include/"; + auto PathPrefix = FullPath.find(Pattern); + if (PathPrefix != StringRef::npos) { + PathPrefix += Pattern.size(); + return FullPath.drop_front(PathPrefix).str(); + } + + // Framework Headers. + SmallVector<StringRef, 4> Matches; + getFrameworkIncludeRule().match(FullPath, &Matches); + // Returned matches are always in stable order. + if (Matches.size() != 4) + return std::nullopt; + + return Matches[1].drop_front(Matches[1].rfind('/') + 1).str() + "/" + + Matches[3].str(); +} +} // namespace clang::installapi diff --git a/clang/unittests/CMakeLists.txt b/clang/unittests/CMakeLists.txt index f4e4f585bdd800..37ca3107b54774 100644 --- a/clang/unittests/CMakeLists.txt +++ b/clang/unittests/CMakeLists.txt @@ -51,5 +51,6 @@ endif() add_subdirectory(DirectoryWatcher) add_subdirectory(Rename) add_subdirectory(Index) +add_subdirectory(InstallAPI) add_subdirectory(Serialization) add_subdirectory(Support) diff --git a/clang/unittests/InstallAPI/CMakeLists.txt b/clang/unittests/InstallAPI/CMakeLists.txt new file mode 100644 index 00000000000000..b70b7c136e64a6 --- /dev/null +++ b/clang/unittests/InstallAPI/CMakeLists.txt @@ -0,0 +1,9 @@ +add_clang_unittest(InstallAPITests + HeaderFileTest.cpp + FileListTest.cpp + ) + +clang_target_link_libraries(InstallAPITests + PRIVATE + clangInstallAPI + ) diff --git a/clang/unittests/InstallAPI/FileListTest.cpp b/clang/unittests/InstallAPI/FileListTest.cpp new file mode 100644 index 00000000000000..7e4ee715a26204 --- /dev/null +++ b/clang/unittests/InstallAPI/FileListTest.cpp @@ -0,0 +1,140 @@ +//===- unittests/InstallAPI/FileList.cpp - File List Tests ---------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "clang/InstallAPI/FileList.h" +#include "clang/InstallAPI/HeaderFile.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/MemoryBuffer.h" +#include "gtest/gtest.h" + +using namespace llvm; +using namespace clang::installapi; + +static void testValidFileList(std::string Input, HeaderSeq &Expected) { + auto InputBuf = MemoryBuffer::getMemBuffer(Input); + HeaderSeq Headers; + llvm::Error Err = FileListReader::loadHeaders(std::move(InputBuf), Headers); + ASSERT_FALSE(Err); + EXPECT_EQ(Expected.size(), Headers.size()); + EXPECT_EQ(Headers, Expected); +} + +TEST(FileListReader, Version3) { + static const char Input[] = R"({ + "version" : "3", + "headers" : [ + { + "type" : "public", + "path" : "/tmp/dst/usr/include/foo.h", + "language" : "objective-c" + }, + { + "type" : "private", + "path" : "/tmp/dst/usr/local/include/bar.h", + "language" : "objective-c++" + }, + { + "type" : "project", + "path" : "/tmp/src/baz.h" + } + ] + })"; + + HeaderSeq Expected = { + {"/tmp/dst/usr/include/foo.h", HeaderType::Public, "foo.h", + clang::Language::ObjC}, + {"/tmp/dst/usr/local/include/bar.h", HeaderType::Private, "bar.h", + clang::Language::ObjCXX}, + {"/tmp/src/baz.h", HeaderType::Project, "", std::nullopt}}; + + testValidFileList(Input, Expected); +} + +TEST(FileList, Version1) { + static const char Input[] = R"({ + "version" : "1", + "headers" : [ + { + "type" : "public", + "path" : "/usr/include/foo.h" + }, + { + "type" : "private", + "path" : "/usr/local/include/bar.h" + } + ] + })"; + + HeaderSeq Expected = { + {"/usr/include/foo.h", HeaderType::Public, "foo.h", std::nullopt}, + {"/usr/local/include/bar.h", HeaderType::Private, "bar.h", std::nullopt}, + }; + + testValidFileList(Input, Expected); +} + +TEST(FileListReader, Version2) { + static const auto Input = R"({ + "version" : "2", + "headers" : [ + { + "type" : "public", + "path" : "/usr/include/foo.h" + }, + { + "type" : "project", + "path" : "src/bar.h" + } + ] + })"; + HeaderSeq Expected = { + {"/usr/include/foo.h", HeaderType::Public, "foo.h", std::nullopt}, + {"src/bar.h", HeaderType::Project, "", std::nullopt}, + }; + + testValidFileList(Input, Expected); +} + +TEST(FileList, MissingVersion) { + static const char Input[] = R"({ + "headers" : [ + { + "type" : "public", + "path" : "/usr/include/foo.h" + }, + { + "type" : "private", + "path" : "/usr/local/include/bar.h" + } + ] + })"; + auto InputBuf = MemoryBuffer::getMemBuffer(Input); + HeaderSeq Headers; + llvm::Error Err = FileListReader::loadHeaders(std::move(InputBuf), Headers); + EXPECT_FALSE(!Err); + EXPECT_STREQ("invalid input format: required field 'version' not specified\n", + toString(std::move(Err)).c_str()); +} + +TEST(FileList, InvalidTypes) { + static const char Input[] = R"({ + "version" : "1", + "headers" : [ + { + "type" : "project", + "path" : "/usr/include/foo.h" + } + ] + })"; + auto InputBuf = MemoryBuffer::getMemBuffer(Input); + HeaderSeq Headers; + llvm::Error Err = FileListReader::loadHeaders(std::move(InputBuf), Headers); + EXPECT_FALSE(!Err); + EXPECT_STREQ("invalid input format: unsupported header type\n", + toString(std::move(Err)).c_str()); +} diff --git a/clang/unittests/InstallAPI/HeaderFileTest.cpp b/clang/unittests/InstallAPI/HeaderFileTest.cpp new file mode 100644 index 00000000000000..7c0b7c5db36892 --- /dev/null +++ b/clang/unittests/InstallAPI/HeaderFileTest.cpp @@ -0,0 +1,89 @@ +//===- unittests/InstallAPI/HeaderFile.cpp - HeaderFile Test --------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "clang/InstallAPI/HeaderFile.h" +#include "gtest/gtest.h" + +using namespace llvm; +using namespace clang::installapi; + +TEST(HeaderFile, FrameworkIncludes) { + const char *Path = "/System/Library/Frameworks/Foo.framework/Headers/Foo.h"; + std::optional<std::string> IncludeName = createIncludeHeaderName(Path); + EXPECT_TRUE(IncludeName.has_value()); + EXPECT_STREQ(IncludeName.value().c_str(), "Foo/Foo.h"); + + Path = "/System/Library/Frameworks/Foo.framework/Frameworks/Bar.framework/" + "Headers/SimpleBar.h"; + IncludeName = createIncludeHeaderName(Path); + EXPECT_TRUE(IncludeName.has_value()); + EXPECT_STREQ(IncludeName.value().c_str(), "Bar/SimpleBar.h"); + + Path = "/tmp/Foo.framework/Versions/A/Headers/SimpleFoo.h"; + IncludeName = createIncludeHeaderName(Path); + EXPECT_TRUE(IncludeName.has_value()); + EXPECT_STREQ(IncludeName.value().c_str(), "Foo/SimpleFoo.h"); + + Path = "/System/Library/PrivateFrameworks/Foo.framework/Headers/Foo.h"; + IncludeName = createIncludeHeaderName(Path); + EXPECT_TRUE(IncludeName.has_value()); + EXPECT_STREQ(IncludeName.value().c_str(), "Foo/Foo.h"); + + Path = "/AppleInternal/Developer/Library/Frameworks/" + "HelloFramework.framework/Headers/HelloFramework.h"; + IncludeName = createIncludeHeaderName(Path); + EXPECT_TRUE(IncludeName.has_value()); + EXPECT_STREQ(IncludeName.value().c_str(), "HelloFramework/HelloFramework.h"); + + Path = "/tmp/BuildProducts/Foo.framework/Versions/A/" + "PrivateHeaders/Foo+Private.h"; + IncludeName = createIncludeHeaderName(Path); + EXPECT_TRUE(IncludeName.has_value()); + EXPECT_STREQ(IncludeName.value().c_str(), "Foo/Foo+Private.h"); + + Path = "/Applications/Xcode.app/Contents/Developer/SDKS/MacOS.sdk/System/" + "Library/Frameworks/Foo.framework/PrivateHeaders/Foo_Private.h"; + IncludeName = createIncludeHeaderName(Path); + EXPECT_TRUE(IncludeName.has_value()); + EXPECT_STREQ(IncludeName.value().c_str(), "Foo/Foo_Private.h"); + + Path = + "/System/Library/PrivateFrameworks/Foo.framework/PrivateHeaders/Foo.hpp"; + IncludeName = createIncludeHeaderName(Path); + EXPECT_TRUE(IncludeName.has_value()); + EXPECT_STREQ(IncludeName.value().c_str(), "Foo/Foo.hpp"); + + Path = "/Applications/Xcode.app/Contents/Developer/SDKS/MacOS.sdk/System/" + "Library/Frameworks/Foo.framework/Headers/BarDir/Bar.h"; + IncludeName = createIncludeHeaderName(Path); + EXPECT_TRUE(IncludeName.has_value()); + EXPECT_STREQ(IncludeName.value().c_str(), "Foo/BarDir/Bar.h"); +} + +TEST(HeaderFile, DylibIncludes) { + const char *Path = "/usr/include/foo.h"; + std::optional<std::string> IncludeName = createIncludeHeaderName(Path); + EXPECT_TRUE(IncludeName.has_value()); + EXPECT_STREQ(IncludeName.value().c_str(), "foo.h"); + + Path = "/tmp/BuildProducts/usr/include/a/A.h"; + IncludeName = createIncludeHeaderName(Path); + EXPECT_TRUE(IncludeName.has_value()); + EXPECT_STREQ(IncludeName.value().c_str(), "a/A.h"); + + Path = "/Applications/Xcode.app/Contents/Developer/SDKS/MacOS.sdk/" + "usr/include/simd/types.h"; + IncludeName = createIncludeHeaderName(Path); + EXPECT_TRUE(IncludeName.has_value()); + EXPECT_STREQ(IncludeName.value().c_str(), "simd/types.h"); + + Path = "/usr/local/include/hidden/A.h"; + IncludeName = createIncludeHeaderName(Path); + EXPECT_TRUE(IncludeName.has_value()); + EXPECT_STREQ(IncludeName.value().c_str(), "hidden/A.h"); +} >From 88485b85c70f4d9bba2b8559e5479fe2a3e92480 Mon Sep 17 00:00:00 2001 From: Cyndy Ishida <cyndy_ish...@apple.com> Date: Wed, 14 Feb 2024 16:11:32 -0800 Subject: [PATCH 2/4] Add missing scope identifer --- clang/lib/InstallAPI/HeaderFile.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/InstallAPI/HeaderFile.cpp b/clang/lib/InstallAPI/HeaderFile.cpp index 9c42a2d1d3b44a..c2d8372741ee07 100644 --- a/clang/lib/InstallAPI/HeaderFile.cpp +++ b/clang/lib/InstallAPI/HeaderFile.cpp @@ -11,7 +11,7 @@ using namespace llvm; namespace clang::installapi { -llvm::Regex getFrameworkIncludeRule() { +llvm::Regex HeaderFile::getFrameworkIncludeRule() { return llvm::Regex("/(.+)\\.framework/(.+)?Headers/(.+)"); } @@ -26,7 +26,7 @@ std::optional<std::string> createIncludeHeaderName(const StringRef FullPath) { // Framework Headers. SmallVector<StringRef, 4> Matches; - getFrameworkIncludeRule().match(FullPath, &Matches); + HeaderFile::getFrameworkIncludeRule().match(FullPath, &Matches); // Returned matches are always in stable order. if (Matches.size() != 4) return std::nullopt; >From d34370745c2c4719b7592eb4d0359d4fc52de518 Mon Sep 17 00:00:00 2001 From: Cyndy Ishida <cyndy_ish...@apple.com> Date: Wed, 14 Feb 2024 16:39:45 -0800 Subject: [PATCH 3/4] Cleanup Filelist Reader Interface --- clang/include/clang/InstallAPI/FileList.h | 13 +------ clang/lib/InstallAPI/FileList.cpp | 41 +++++++++-------------- 2 files changed, 17 insertions(+), 37 deletions(-) diff --git a/clang/include/clang/InstallAPI/FileList.h b/clang/include/clang/InstallAPI/FileList.h index 01ec9b35e31490..460af003b6a0ac 100644 --- a/clang/include/clang/InstallAPI/FileList.h +++ b/clang/include/clang/InstallAPI/FileList.h @@ -22,15 +22,7 @@ namespace clang { namespace installapi { -/// JSON decoder for InstallAPI Inputs. class FileListReader { - - class Implementation; - Implementation &Impl; - - FileListReader(std::unique_ptr<llvm::MemoryBuffer> InputBuffer, - llvm::Error &Err); - public: /// Decode JSON input and append header input into destination container. /// Headers are loaded in the order they appear in the JSON input. @@ -41,10 +33,7 @@ class FileListReader { loadHeaders(std::unique_ptr<llvm::MemoryBuffer> InputBuffer, HeaderSeq &Destination); - ~FileListReader(); - - FileListReader(const FileListReader &) = delete; - FileListReader &operator=(const FileListReader &) = delete; + FileListReader() = delete; }; } // namespace installapi diff --git a/clang/lib/InstallAPI/FileList.cpp b/clang/lib/InstallAPI/FileList.cpp index 6b307682346e2d..ae5a4a47c0bb55 100644 --- a/clang/lib/InstallAPI/FileList.cpp +++ b/clang/lib/InstallAPI/FileList.cpp @@ -39,7 +39,8 @@ using namespace llvm::json; using namespace llvm::MachO; using namespace clang::installapi; -class FileListReader::Implementation { +namespace { +class Implementation { private: Expected<StringRef> parseString(const Object *Obj, StringRef Key, StringRef Error); @@ -57,16 +58,14 @@ class FileListReader::Implementation { }; Expected<StringRef> -FileListReader::Implementation::parseString(const Object *Obj, StringRef Key, - StringRef Error) { +Implementation::parseString(const Object *Obj, StringRef Key, StringRef Error) { auto Str = Obj->getString(Key); if (!Str) return make_error<StringError>(Error, inconvertibleErrorCode()); return *Str; } -Expected<HeaderType> -FileListReader::Implementation::parseType(const Object *Obj) { +Expected<HeaderType> Implementation::parseType(const Object *Obj) { auto TypeStr = parseString(Obj, "type", "required field 'type' not specified"); if (!TypeStr) @@ -83,8 +82,7 @@ FileListReader::Implementation::parseType(const Object *Obj) { "unsupported header type"); } -Expected<StringRef> -FileListReader::Implementation::parsePath(const Object *Obj) { +Expected<StringRef> Implementation::parsePath(const Object *Obj) { auto Path = parseString(Obj, "path", "required field 'path' not specified"); if (!Path) return Path.takeError(); @@ -93,7 +91,7 @@ FileListReader::Implementation::parsePath(const Object *Obj) { } std::optional<clang::Language> -FileListReader::Implementation::parseLanguage(const Object *Obj) { +Implementation::parseLanguage(const Object *Obj) { auto Language = Obj->getString("language"); if (!Language) return std::nullopt; @@ -106,7 +104,7 @@ FileListReader::Implementation::parseLanguage(const Object *Obj) { .Default(clang::Language::Unknown); } -Error FileListReader::Implementation::parseHeaders(Array &Headers) { +Error Implementation::parseHeaders(Array &Headers) { for (const auto &H : Headers) { auto *Obj = H.getAsObject(); if (!Obj) @@ -135,7 +133,7 @@ Error FileListReader::Implementation::parseHeaders(Array &Headers) { return Error::success(); } -Error FileListReader::Implementation::parse(StringRef Input) { +Error Implementation::parse(StringRef Input) { auto Val = json::parse(Input); if (!Val) return Val.takeError(); @@ -168,29 +166,22 @@ Error FileListReader::Implementation::parse(StringRef Input) { return Error::success(); } - -FileListReader::FileListReader(std::unique_ptr<MemoryBuffer> InputBuffer, - Error &Error) - : Impl(*new FileListReader::Implementation()) { - ErrorAsOutParameter ErrorAsOutParam(&Error); - Impl.InputBuffer = std::move(InputBuffer); - - Error = Impl.parse(Impl.InputBuffer->getBuffer()); -} +} // namespace llvm::Error FileListReader::loadHeaders(std::unique_ptr<MemoryBuffer> InputBuffer, HeaderSeq &Destination) { llvm::Error Err = Error::success(); - std::unique_ptr<FileListReader> Reader( - new FileListReader(std::move(InputBuffer), Err)); + Implementation Impl; + ErrorAsOutParameter ErrorAsOutParam(&Err); + Impl.InputBuffer = std::move(InputBuffer); + + Err = Impl.parse(Impl.InputBuffer->getBuffer()); if (Err) return Err; - Destination.reserve(Destination.size() + Reader->Impl.HeaderList.size()); - llvm::move(Reader->Impl.HeaderList, std::back_inserter(Destination)); + Destination.reserve(Destination.size() + Impl.HeaderList.size()); + llvm::move(Impl.HeaderList, std::back_inserter(Destination)); return Err; } - -FileListReader::~FileListReader() { delete &Impl; } >From 01469bb6abde6811d587d274b731c5dfa1658366 Mon Sep 17 00:00:00 2001 From: Cyndy Ishida <cyndy_ish...@apple.com> Date: Wed, 14 Feb 2024 18:32:28 -0800 Subject: [PATCH 4/4] Fix Tests for windows speculatively --- clang/unittests/InstallAPI/FileListTest.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/clang/unittests/InstallAPI/FileListTest.cpp b/clang/unittests/InstallAPI/FileListTest.cpp index 7e4ee715a26204..dae515cecbfa8d 100644 --- a/clang/unittests/InstallAPI/FileListTest.cpp +++ b/clang/unittests/InstallAPI/FileListTest.cpp @@ -19,7 +19,8 @@ static void testValidFileList(std::string Input, HeaderSeq &Expected) { auto InputBuf = MemoryBuffer::getMemBuffer(Input); HeaderSeq Headers; llvm::Error Err = FileListReader::loadHeaders(std::move(InputBuf), Headers); - ASSERT_FALSE(Err); + EXPECT_FALSE(!!Err); + EXPECT_EQ(Expected.size(), Headers.size()); EXPECT_EQ(Headers, Expected); } @@ -116,7 +117,7 @@ TEST(FileList, MissingVersion) { auto InputBuf = MemoryBuffer::getMemBuffer(Input); HeaderSeq Headers; llvm::Error Err = FileListReader::loadHeaders(std::move(InputBuf), Headers); - EXPECT_FALSE(!Err); + EXPECT_TRUE(!!Err); EXPECT_STREQ("invalid input format: required field 'version' not specified\n", toString(std::move(Err)).c_str()); } @@ -134,7 +135,7 @@ TEST(FileList, InvalidTypes) { auto InputBuf = MemoryBuffer::getMemBuffer(Input); HeaderSeq Headers; llvm::Error Err = FileListReader::loadHeaders(std::move(InputBuf), Headers); - EXPECT_FALSE(!Err); + EXPECT_TRUE(!!Err); EXPECT_STREQ("invalid input format: unsupported header type\n", toString(std::move(Err)).c_str()); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
