[clang-tools-extra] 48bb147 - Revert "[clang-doc] Move file layout to the generators."

Paul Kirth via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 22 11:15:23 PST 2022


Author: Paul Kirth
Date: 2022-11-22T19:14:54Z
New Revision: 48bb1471122d156bc0c4feaabc32de4487024ca5

URL: https://github.com/llvm/llvm-project/commit/48bb1471122d156bc0c4feaabc32de4487024ca5
DIFF: https://github.com/llvm/llvm-project/commit/48bb1471122d156bc0c4feaabc32de4487024ca5.diff

LOG: Revert "[clang-doc] Move file layout to the generators."

This reverts commit f8a469fc572778d05b72f34a772082cf3abd3cda.

The test in single-file-public.cpp breaks on Mac, due to an unknown
regextype in the find command.

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 ba0ef64d3d0f5..89c6b34c43844 100644
--- a/clang-tools-extra/clang-doc/Generators.h
+++ b/clang-tools-extra/clang-doc/Generators.h
@@ -25,23 +25,15 @@ class Generator {
 public:
   virtual ~Generator() = default;
 
-  // 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;
-
+  // Write out the decl info in the specified format.
+  virtual llvm::Error generateDocForInfo(Info *I, llvm::raw_ostream &OS,
+                                         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 11a78ba60e35c..6f1d45551bc78 100644
--- a/clang-tools-extra/clang-doc/HTMLGenerator.cpp
+++ b/clang-tools-extra/clang-doc/HTMLGenerator.cpp
@@ -11,7 +11,6 @@
 #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"
@@ -840,69 +839,13 @@ 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 8261e14c3efb0..0c5f163045986 100644
--- a/clang-tools-extra/clang-doc/MDGenerator.cpp
+++ b/clang-tools-extra/clang-doc/MDGenerator.cpp
@@ -356,69 +356,18 @@ 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 b54486102ccc3..fcca0a60a9440 100644
--- a/clang-tools-extra/clang-doc/YAMLGenerator.cpp
+++ b/clang-tools-extra/clang-doc/YAMLGenerator.cpp
@@ -292,48 +292,12 @@ 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 83bfea027bbce..7531f219a6a40 100644
--- a/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ b/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -43,7 +43,6 @@
 #include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/raw_ostream.h"
 #include <atomic>
-#include <mutex>
 #include <string>
 
 using namespace clang::ast_matchers;
@@ -131,6 +130,54 @@ 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;
@@ -227,11 +274,6 @@ 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;
@@ -262,17 +304,31 @@ Example usage for a project using a compile commands database:
         return;
       }
 
-      // Add a reference to this Info in the Index
-      {
-        std::lock_guard Guard(IndexMutex);
-        clang::doc::Generator::addInfoToIndex(CDCtx.Idx, Reduced.get().get());
+      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;
       }
-
-      // Save in the result map (needs a lock due to threaded access).
-      {
-        std::lock_guard Guard(USRToInfoMutex);
-        USRToInfo[Group.getKey()] = std::move(Reduced.get());
+      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();
+
+      if (auto Err = G->get()->generateDocForInfo(I, InfoOS, CDCtx))
+        llvm::errs() << toString(std::move(Err)) << "\n";
     });
   }
 
@@ -281,21 +337,6 @@ 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 62e35fa14d6e7..20cf18cd31b4a 100644
--- a/clang-tools-extra/test/clang-doc/single-file-public.cpp
+++ b/clang-tools-extra/test/clang-doc/single-file-public.cpp
@@ -3,10 +3,7 @@
 // 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
-//   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: cat %t/docs/GlobalNamespace/Record.yaml | FileCheck %s --check-prefix=CHECK
 // RUN: rm -rf %t
 
 class Record {
@@ -22,7 +19,7 @@ void Record::function_private() {}
 void Record::function_public() {}
 
 // CHECK: ---
-// CHECK-NEXT: USR:             '{{([0-9A-F]{40})}}'
+// 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: Name:            'Record'
 // CHECK-NEXT: Path:            'GlobalNamespace'
 // CHECK-NEXT: Namespace:
@@ -33,12 +30,12 @@ void Record::function_public() {}
 // CHECK-NEXT:   Filename:        '{{.*}}'
 // CHECK-NEXT: TagType:         Class
 // CHECK-NEXT: ChildFunctions:
-// CHECK-NEXT:   - USR:             '{{([0-9A-F]{40})}}'
+// 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:     Name:            'function_public'
 // CHECK-NEXT:     Namespace:
 // CHECK-NEXT:       - Type:            Record
 // CHECK-NEXT:         Name:            'Record'
-// CHECK-NEXT:         USR:             '{{([0-9A-F]{40})}}'
+// 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:       - Type:            Namespace
 // CHECK-NEXT:         Name:            'GlobalNamespace'
 // CHECK-NEXT:     DefLocation:
@@ -51,7 +48,7 @@ void Record::function_public() {}
 // CHECK-NEXT:     Parent:
 // CHECK-NEXT:         Type:            Record
 // CHECK-NEXT:         Name:            'Record'
-// CHECK-NEXT:         USR:             '{{([0-9A-F]{40})}}'
+// 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:     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 3703f5d2209f4..2ceab4f43bb9a 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/index.yaml | FileCheck %s --check-prefix=CHECK
+// RUN: cat %t/docs/GlobalNamespace/index.yaml | FileCheck %s --check-prefix=CHECK
 // RUN: rm -rf %t
 
 void function(int x);


        


More information about the cfe-commits mailing list