From 7b8c7e02122a2ea392b371e3e39b405bc98146de Mon Sep 17 00:00:00 2001 From: Brett Wilson Date: Tue, 29 Nov 2022 12:35:58 -0800 Subject: [PATCH] [clang-doc] Move file layout to the generators. Previously file naming and directory layout was handled on a per Info object basis by ClangDocMain and the generators blindly wrote to the files given. This means all generators must use the same file layout and caused problems where multiple objects mapped to the same file. The object collision problem happens most easily with template specializations because the template parameters are not part of the "name". This patch moves the responsibility for output file organization to the generators. Currently HTML and MD use the same structure as before. But they now collect all objects that map to a given file and combine them, avoiding the corruption problems. Converts the YAML generator to naming files based on USR in one directory. This is easier for downstream tools to manage and avoids the naming problems with template specializations. Since this change requires backward-incompatible output changes to referenced files anyway (since each one is now an array), this is a good time to introduce this change. Differential Revision: https://reviews.llvm.org/D138073 --- clang-tools-extra/clang-doc/Generators.h | 14 ++- clang-tools-extra/clang-doc/HTMLGenerator.cpp | 59 +++++++++++- clang-tools-extra/clang-doc/MDGenerator.cpp | 53 ++++++++++- clang-tools-extra/clang-doc/YAMLGenerator.cpp | 36 ++++++++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp | 101 ++++++--------------- .../test/clang-doc/single-file-public.cpp | 13 ++- clang-tools-extra/test/clang-doc/single-file.cpp | 2 +- 7 files changed, 196 insertions(+), 82 deletions(-) diff --git a/clang-tools-extra/clang-doc/Generators.h b/clang-tools-extra/clang-doc/Generators.h index 89c6b34..ba0ef64 100644 --- a/clang-tools-extra/clang-doc/Generators.h +++ b/clang-tools-extra/clang-doc/Generators.h @@ -25,15 +25,23 @@ class Generator { public: virtual ~Generator() = default; - // Write out the decl info in the specified format. - virtual llvm::Error generateDocForInfo(Info *I, llvm::raw_ostream &OS, - const ClangDocContext &CDCtx) = 0; + // Write out the decl info for the objects in the given map in the specified + // format. + virtual llvm::Error + generateDocs(StringRef RootDir, + llvm::StringMap> Infos, + const ClangDocContext &CDCtx) = 0; + // This function writes a file with the index previously constructed. // It can be overwritten by any of the inherited generators. // If the override method wants to run this it should call // Generator::createResources(CDCtx); virtual llvm::Error createResources(ClangDocContext &CDCtx); + // Write out one specific decl info to the destination stream. + virtual llvm::Error generateDocForInfo(Info *I, llvm::raw_ostream &OS, + const ClangDocContext &CDCtx) = 0; + static void addInfoToIndex(Index &Idx, const doc::Info *Info); }; diff --git a/clang-tools-extra/clang-doc/HTMLGenerator.cpp b/clang-tools-extra/clang-doc/HTMLGenerator.cpp index 6f1d455..11a78ba 100644 --- a/clang-tools-extra/clang-doc/HTMLGenerator.cpp +++ b/clang-tools-extra/clang-doc/HTMLGenerator.cpp @@ -11,6 +11,7 @@ #include "clang/Basic/Version.h" #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" +#include "llvm/ADT/StringSet.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/JSON.h" #include "llvm/Support/Path.h" @@ -839,13 +840,69 @@ class HTMLGenerator : public Generator { public: static const char *Format; + llvm::Error generateDocs(StringRef RootDir, + llvm::StringMap> Infos, + const ClangDocContext &CDCtx) override; + llvm::Error createResources(ClangDocContext &CDCtx) override; llvm::Error generateDocForInfo(Info *I, llvm::raw_ostream &OS, const ClangDocContext &CDCtx) override; - llvm::Error createResources(ClangDocContext &CDCtx) override; }; const char *HTMLGenerator::Format = "html"; +llvm::Error +HTMLGenerator::generateDocs(StringRef RootDir, + llvm::StringMap> Infos, + const ClangDocContext &CDCtx) { + // Track which directories we already tried to create. + llvm::StringSet<> CreatedDirs; + + // Collect all output by file name and create the nexessary directories. + llvm::StringMap> FileToInfos; + for (const auto &Group : Infos) { + doc::Info *Info = Group.getValue().get(); + + llvm::SmallString<128> Path; + llvm::sys::path::native(RootDir, Path); + llvm::sys::path::append(Path, Info->getRelativeFilePath("")); + if (CreatedDirs.find(Path) == CreatedDirs.end()) { + if (std::error_code Err = llvm::sys::fs::create_directories(Path); + Err != std::error_code()) { + return llvm::createStringError(Err, "Failed to create directory '%s'.", + Path.c_str()); + } + CreatedDirs.insert(Path); + } + + llvm::sys::path::append(Path, Info->getFileBaseName() + ".html"); + FileToInfos[Path].push_back(Info); + } + + for (const auto &Group : FileToInfos) { + std::error_code FileErr; + llvm::raw_fd_ostream InfoOS(Group.getKey(), FileErr, + llvm::sys::fs::OF_None); + if (FileErr) { + return llvm::createStringError(FileErr, "Error opening file '%s'", + Group.getKey().str().c_str()); + } + + // TODO: https://github.com/llvm/llvm-project/issues/59073 + // If there are multiple Infos for this file name (for example, template + // specializations), this will generate multiple complete web pages (with + // and , etc.) concatenated together. This generator needs + // some refactoring to be able to output the headers separately from the + // contents. + for (const auto &Info : Group.getValue()) { + if (llvm::Error Err = generateDocForInfo(Info, InfoOS, CDCtx)) { + return Err; + } + } + } + + return llvm::Error::success(); +} + llvm::Error HTMLGenerator::generateDocForInfo(Info *I, llvm::raw_ostream &OS, const ClangDocContext &CDCtx) { std::string InfoTitle; diff --git a/clang-tools-extra/clang-doc/MDGenerator.cpp b/clang-tools-extra/clang-doc/MDGenerator.cpp index 0c5f163..8261e14 100644 --- a/clang-tools-extra/clang-doc/MDGenerator.cpp +++ b/clang-tools-extra/clang-doc/MDGenerator.cpp @@ -356,18 +356,69 @@ static llvm::Error genIndex(ClangDocContext &CDCtx) { } return llvm::Error::success(); } + /// Generator for Markdown documentation. class MDGenerator : public Generator { public: static const char *Format; + llvm::Error generateDocs(StringRef RootDir, + llvm::StringMap<std::unique_ptr<doc::Info>> Infos, + const ClangDocContext &CDCtx) override; + llvm::Error createResources(ClangDocContext &CDCtx) override; llvm::Error generateDocForInfo(Info *I, llvm::raw_ostream &OS, const ClangDocContext &CDCtx) override; - llvm::Error createResources(ClangDocContext &CDCtx) override; }; const char *MDGenerator::Format = "md"; +llvm::Error +MDGenerator::generateDocs(StringRef RootDir, + llvm::StringMap<std::unique_ptr<doc::Info>> Infos, + const ClangDocContext &CDCtx) { + // Track which directories we already tried to create. + llvm::StringSet<> CreatedDirs; + + // Collect all output by file name and create the necessary directories. + llvm::StringMap<std::vector<doc::Info *>> FileToInfos; + for (const auto &Group : Infos) { + doc::Info *Info = Group.getValue().get(); + + llvm::SmallString<128> Path; + llvm::sys::path::native(RootDir, Path); + llvm::sys::path::append(Path, Info->getRelativeFilePath("")); + if (CreatedDirs.find(Path) == CreatedDirs.end()) { + if (std::error_code Err = llvm::sys::fs::create_directories(Path); + Err != std::error_code()) { + return llvm::createStringError(Err, "Failed to create directory '%s'.", + Path.c_str()); + } + CreatedDirs.insert(Path); + } + + llvm::sys::path::append(Path, Info->getFileBaseName() + ".md"); + FileToInfos[Path].push_back(Info); + } + + for (const auto &Group : FileToInfos) { + std::error_code FileErr; + llvm::raw_fd_ostream InfoOS(Group.getKey(), FileErr, + llvm::sys::fs::OF_None); + if (FileErr) { + return llvm::createStringError(FileErr, "Error opening file '%s'", + Group.getKey().str().c_str()); + } + + for (const auto &Info : Group.getValue()) { + if (llvm::Error Err = generateDocForInfo(Info, InfoOS, CDCtx)) { + return Err; + } + } + } + + return llvm::Error::success(); +} + llvm::Error MDGenerator::generateDocForInfo(Info *I, llvm::raw_ostream &OS, const ClangDocContext &CDCtx) { switch (I->IT) { diff --git a/clang-tools-extra/clang-doc/YAMLGenerator.cpp b/clang-tools-extra/clang-doc/YAMLGenerator.cpp index fcca0a6..b544861 100644 --- a/clang-tools-extra/clang-doc/YAMLGenerator.cpp +++ b/clang-tools-extra/clang-doc/YAMLGenerator.cpp @@ -292,12 +292,48 @@ class YAMLGenerator : public Generator { public: static const char *Format; + llvm::Error generateDocs(StringRef RootDir, + llvm::StringMap<std::unique_ptr<doc::Info>> Infos, + const ClangDocContext &CDCtx) override; llvm::Error generateDocForInfo(Info *I, llvm::raw_ostream &OS, const ClangDocContext &CDCtx) override; }; const char *YAMLGenerator::Format = "yaml"; +llvm::Error +YAMLGenerator::generateDocs(StringRef RootDir, + llvm::StringMap<std::unique_ptr<doc::Info>> Infos, + const ClangDocContext &CDCtx) { + for (const auto &Group : Infos) { + doc::Info *Info = Group.getValue().get(); + + // Output file names according to the USR except the global namesapce. + // Anonymous namespaces are taken care of in serialization, so here we can + // safely assume an unnamed namespace is the global one. + llvm::SmallString<128> Path; + llvm::sys::path::native(RootDir, Path); + if (Info->IT == InfoType::IT_namespace && Info->Name.empty()) { + llvm::sys::path::append(Path, "index.yaml"); + } else { + llvm::sys::path::append(Path, Group.getKey() + ".yaml"); + } + + std::error_code FileErr; + llvm::raw_fd_ostream InfoOS(Path, FileErr, llvm::sys::fs::OF_None); + if (FileErr) { + return llvm::createStringError(FileErr, "Error opening file '%s'", + Path.c_str()); + } + + if (llvm::Error Err = generateDocForInfo(Info, InfoOS, CDCtx)) { + return Err; + } + } + + return llvm::Error::success(); +} + llvm::Error YAMLGenerator::generateDocForInfo(Info *I, llvm::raw_ostream &OS, const ClangDocContext &CDCtx) { llvm::yaml::Output InfoYAML(OS); diff --git a/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp b/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp index 7531f21..83bfea0 100644 --- a/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp +++ b/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp @@ -43,6 +43,7 @@ #include "llvm/Support/ThreadPool.h" #include "llvm/Support/raw_ostream.h" #include <atomic> +#include <mutex> #include <string> using namespace clang::ast_matchers; @@ -130,54 +131,6 @@ std::string GetExecutablePath(const char *Argv0, void *MainAddr) { return llvm::sys::fs::getMainExecutable(Argv0, MainAddr); } -bool CreateDirectory(const Twine &DirName, bool ClearDirectory = false) { - std::error_code OK; - llvm::SmallString<128> DocsRootPath; - if (ClearDirectory) { - std::error_code RemoveStatus = llvm::sys::fs::remove_directories(DirName); - if (RemoveStatus != OK) { - llvm::errs() << "Unable to remove existing documentation directory for " - << DirName << ".\n"; - return true; - } - } - std::error_code DirectoryStatus = llvm::sys::fs::create_directories(DirName); - if (DirectoryStatus != OK) { - llvm::errs() << "Unable to create documentation directories.\n"; - return true; - } - return false; -} - -// A function to extract the appropriate file name for a given info's -// documentation. The path returned is a composite of the output directory, the -// info's relative path and name and the extension. The relative path should -// have been constructed in the serialization phase. -// -// Example: Given the below, the <ext> path for class C will be -// <root>/A/B/C.<ext> -// -// namespace A { -// namespace B { -// -// class C {}; -// -// } -// } -llvm::Expected<llvm::SmallString<128>> getInfoOutputFile(StringRef Root, - StringRef RelativePath, - StringRef Name, - StringRef Ext) { - llvm::SmallString<128> Path; - llvm::sys::path::native(Root, Path); - llvm::sys::path::append(Path, RelativePath); - if (CreateDirectory(Path)) - return llvm::createStringError(llvm::inconvertibleErrorCode(), - "failed to create directory"); - llvm::sys::path::append(Path, Name + Ext); - return Path; -} - int main(int argc, const char **argv) { llvm::sys::PrintStackTraceOnErrorSignal(argv[0]); std::error_code OK; @@ -274,6 +227,11 @@ Example usage for a project using a compile commands database: R.first->second.emplace_back(Value); }); + // Collects all Infos according to their unique USR value. This map is added + // to from the thread pool below and is protected by the USRToInfoMutex. + llvm::sys::Mutex USRToInfoMutex; + llvm::StringMap<std::unique_ptr<doc::Info>> USRToInfo; + // First reducing phase (reduce all decls into one info per decl). llvm::outs() << "Reducing " << USRToBitcode.size() << " infos...\n"; std::atomic<bool> Error; @@ -304,31 +262,17 @@ Example usage for a project using a compile commands database: return; } - doc::Info *I = Reduced.get().get(); - auto InfoPath = - getInfoOutputFile(OutDirectory, I->getRelativeFilePath(""), - I->getFileBaseName(), "." + Format); - if (!InfoPath) { - llvm::errs() << toString(InfoPath.takeError()) << "\n"; - Error = true; - return; - } - std::error_code FileErr; - llvm::raw_fd_ostream InfoOS(InfoPath.get(), FileErr, - llvm::sys::fs::OF_None); - if (FileErr) { - llvm::errs() << "Error opening info file " << InfoPath.get() << ": " - << FileErr.message() << "\n"; - return; - } - - IndexMutex.lock(); // Add a reference to this Info in the Index - clang::doc::Generator::addInfoToIndex(CDCtx.Idx, I); - IndexMutex.unlock(); + { + std::lock_guard Guard(IndexMutex); + clang::doc::Generator::addInfoToIndex(CDCtx.Idx, Reduced.get().get()); + } - if (auto Err = G->get()->generateDocForInfo(I, InfoOS, CDCtx)) - llvm::errs() << toString(std::move(Err)) << "\n"; + // Save in the result map (needs a lock due to threaded access). + { + std::lock_guard Guard(USRToInfoMutex); + USRToInfo[Group.getKey()] = std::move(Reduced.get()); + } }); } @@ -337,6 +281,21 @@ Example usage for a project using a compile commands database: if (Error) return 1; + // Ensure the root output directory exists. + if (std::error_code Err = llvm::sys::fs::create_directories(OutDirectory); + Err != std::error_code()) { + llvm::errs() << "Failed to create directory '" << OutDirectory << "'\n"; + return 1; + } + + // Run the generator. + llvm::outs() << "Generating docs...\n"; + if (auto Err = + G->get()->generateDocs(OutDirectory, std::move(USRToInfo), CDCtx)) { + llvm::errs() << toString(std::move(Err)) << "\n"; + return 1; + } + llvm::outs() << "Generating assets for docs...\n"; Err = G->get()->createResources(CDCtx); if (Err) { diff --git a/clang-tools-extra/test/clang-doc/single-file-public.cpp b/clang-tools-extra/test/clang-doc/single-file-public.cpp index 20cf18c..91c3146 100644 --- a/clang-tools-extra/test/clang-doc/single-file-public.cpp +++ b/clang-tools-extra/test/clang-doc/single-file-public.cpp @@ -3,7 +3,10 @@ // RUN: echo "" > %t/compile_flags.txt // RUN: cp "%s" "%t/test.cpp" // RUN: clang-doc --doxygen --public --executor=standalone -p %t %t/test.cpp -output=%t/docs -// RUN: cat %t/docs/GlobalNamespace/Record.yaml | FileCheck %s --check-prefix=CHECK +// This produces two files, index.yaml and one for the record named by its USR +// (which we don't know in advance). This checks the record file by searching +// for a name with a 40-char USR name. +// RUN: find %t/docs -regex ".*/[0-9A-F]*.yaml" -exec cat {} ";" | FileCheck %s --check-prefix=CHECK // RUN: rm -rf %t class Record { @@ -19,7 +22,7 @@ void Record::function_private() {} void Record::function_public() {} // CHECK: --- -// CHECK-NEXT: USR: '{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}' +// CHECK-NEXT: USR: '{{([0-9A-F]{40})}}' // CHECK-NEXT: Name: 'Record' // CHECK-NEXT: Path: 'GlobalNamespace' // CHECK-NEXT: Namespace: @@ -30,12 +33,12 @@ void Record::function_public() {} // CHECK-NEXT: Filename: '{{.*}}' // CHECK-NEXT: TagType: Class // CHECK-NEXT: ChildFunctions: -// CHECK-NEXT: - USR: '{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}' +// CHECK-NEXT: - USR: '{{([0-9A-F]{40})}}' // CHECK-NEXT: Name: 'function_public' // CHECK-NEXT: Namespace: // CHECK-NEXT: - Type: Record // CHECK-NEXT: Name: 'Record' -// CHECK-NEXT: USR: '{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}' +// CHECK-NEXT: USR: '{{([0-9A-F]{40})}}' // CHECK-NEXT: - Type: Namespace // CHECK-NEXT: Name: 'GlobalNamespace' // CHECK-NEXT: DefLocation: @@ -48,7 +51,7 @@ void Record::function_public() {} // CHECK-NEXT: Parent: // CHECK-NEXT: Type: Record // CHECK-NEXT: Name: 'Record' -// CHECK-NEXT: USR: '{{[0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z][0-9A-Z]}}' +// CHECK-NEXT: USR: '{{([0-9A-F]{40})}}' // CHECK-NEXT: ReturnType: // CHECK-NEXT: Type: // CHECK-NEXT: Name: 'void' diff --git a/clang-tools-extra/test/clang-doc/single-file.cpp b/clang-tools-extra/test/clang-doc/single-file.cpp index 2ceab4f..3703f5d 100644 --- a/clang-tools-extra/test/clang-doc/single-file.cpp +++ b/clang-tools-extra/test/clang-doc/single-file.cpp @@ -3,7 +3,7 @@ // RUN: echo "" > %t/compile_flags.txt // RUN: cp "%s" "%t/test.cpp" // RUN: clang-doc --doxygen --executor=standalone -p %t %t/test.cpp -output=%t/docs -// RUN: cat %t/docs/GlobalNamespace/index.yaml | FileCheck %s --check-prefix=CHECK +// RUN: cat %t/docs/index.yaml | FileCheck %s --check-prefix=CHECK // RUN: rm -rf %t void function(int x); -- 2.7.4