brettw created this revision.
brettw added a reviewer: paulkirth.
brettw added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
brettw requested review of this revision.
Herald added subscribers: cfe-commits, ilya-biryukov.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138073

Files:
  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

Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===================================================================
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ 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 @@
   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 @@
         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 @@
         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 @@
   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) {
Index: clang-tools-extra/clang-doc/YAMLGenerator.cpp
===================================================================
--- clang-tools-extra/clang-doc/YAMLGenerator.cpp
+++ clang-tools-extra/clang-doc/YAMLGenerator.cpp
@@ -292,12 +292,48 @@
 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);
Index: clang-tools-extra/clang-doc/MDGenerator.cpp
===================================================================
--- clang-tools-extra/clang-doc/MDGenerator.cpp
+++ clang-tools-extra/clang-doc/MDGenerator.cpp
@@ -356,18 +356,83 @@
   }
   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 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() + ".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::createResources(ClangDocContext &CDCtx) {
+  // Write an all_files.md
+  auto Err = serializeIndex(CDCtx);
+  if (Err)
+    return Err;
+
+  // Generate the index page.
+  Err = genIndex(CDCtx);
+  if (Err)
+    return Err;
+
+  return llvm::Error::success();
+}
+
 llvm::Error MDGenerator::generateDocForInfo(Info *I, llvm::raw_ostream &OS,
                                             const ClangDocContext &CDCtx) {
   switch (I->IT) {
@@ -393,20 +458,6 @@
   return llvm::Error::success();
 }
 
-llvm::Error MDGenerator::createResources(ClangDocContext &CDCtx) {
-  // Write an all_files.md
-  auto Err = serializeIndex(CDCtx);
-  if (Err)
-    return Err;
-
-  // Generate the index page.
-  Err = genIndex(CDCtx);
-  if (Err)
-    return Err;
-
-  return llvm::Error::success();
-}
-
 static GeneratorRegistry::Add<MDGenerator> MD(MDGenerator::Format,
                                               "Generator for MD output.");
 
Index: clang-tools-extra/clang-doc/HTMLGenerator.cpp
===================================================================
--- clang-tools-extra/clang-doc/HTMLGenerator.cpp
+++ 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,68 @@
 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 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 separatelky 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;
Index: clang-tools-extra/clang-doc/Generators.h
===================================================================
--- clang-tools-extra/clang-doc/Generators.h
+++ clang-tools-extra/clang-doc/Generators.h
@@ -25,15 +25,23 @@
 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);
 };
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to