[clang-tools-extra] f8a469f - [clang-doc] Move file layout to the generators.
Brett Wilson via cfe-commits
cfe-commits at lists.llvm.org
Tue Nov 22 10:27:59 PST 2022
Author: Brett Wilson
Date: 2022-11-22T10:27:29-08:00
New Revision: f8a469fc572778d05b72f34a772082cf3abd3cda
URL: https://github.com/llvm/llvm-project/commit/f8a469fc572778d05b72f34a772082cf3abd3cda
DIFF: https://github.com/llvm/llvm-project/commit/f8a469fc572778d05b72f34a772082cf3abd3cda.diff
LOG: [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
Added:
Modified:
clang-tools-extra/clang-doc/Generators.h
clang-tools-extra/clang-doc/HTMLGenerator.cpp
clang-tools-extra/clang-doc/MDGenerator.cpp
clang-tools-extra/clang-doc/YAMLGenerator.cpp
clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
clang-tools-extra/test/clang-doc/single-file-public.cpp
clang-tools-extra/test/clang-doc/single-file.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-doc/Generators.h b/clang-tools-extra/clang-doc/Generators.h
index 89c6b34c43844..ba0ef64d3d0f5 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<std::unique_ptr<doc::Info>> 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 6f1d45551bc78..11a78ba60e35c 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<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 *HTMLGenerator::Format = "html";
+llvm::Error
+HTMLGenerator::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 nexessary 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() + ".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
+ // <DOCTYPE> and <title>, 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 0c5f163045986..8261e14c3efb0 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 fcca0a60a9440..b54486102ccc3 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 7531f219a6a40..83bfea027bbce 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 20cf18cd31b4a..62e35fa14d6e7 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 -regextype sed -regex ".*[0-9A-F]\{40\}.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 2ceab4f43bb9a..3703f5d2209f4 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);
More information about the cfe-commits
mailing list