[clang-doc] fix flaky test in clang-doc (llvm#101387)

Fixes llvm#97507 llvm#96809 introduce non-determinstic behaviour in clang-doc which caused the output to be randomly ordered which lead to randomly failing test. This patch modify clang-doc to behave deterministically again by sorting the output by location or USR.
kstoimenov · Aug 15, 2024 · a206bfb · a206bfb
1 parent c277e68
commit a206bfb
Show file tree

Hide file tree

Showing 6 changed files with 75 additions and 27 deletions.
diff --git a/clang-tools-extra/clang-doc/Representation.cpp b/clang-tools-extra/clang-doc/Representation.cpp
@@ -384,5 +384,12 @@ ClangDocContext::ClangDocContext(tooling::ExecutionContext *ECtx,
   }
 }
 
+void ScopeChildren::sort() {
+  llvm::sort(Namespaces.begin(), Namespaces.end());
+  llvm::sort(Records.begin(), Records.end());
+  llvm::sort(Functions.begin(), Functions.end());
+  llvm::sort(Enums.begin(), Enums.end());
+  llvm::sort(Typedefs.begin(), Typedefs.end());
+}
 } // namespace doc
 } // namespace clang
diff --git a/clang-tools-extra/clang-doc/Representation.h b/clang-tools-extra/clang-doc/Representation.h
@@ -104,6 +104,7 @@ struct Reference {
 
   bool mergeable(const Reference &Other);
   void merge(Reference &&I);
+  bool operator<(const Reference &Other) const { return Name < Other.Name; }
 
   /// Returns the path for this Reference relative to CurrentPath.
   llvm::SmallString<64> getRelativeFilePath(const StringRef &CurrentPath) const;
@@ -145,6 +146,8 @@ struct ScopeChildren {
   std::vector<FunctionInfo> Functions;
   std::vector<EnumInfo> Enums;
   std::vector<TypedefInfo> Typedefs;
+
+  void sort();
 };
 
 // A base struct for TypeInfos
@@ -245,6 +248,11 @@ struct Location {
            std::tie(Other.LineNumber, Other.Filename);
   }
 
+  bool operator!=(const Location &Other) const {
+    return std::tie(LineNumber, Filename) !=
+           std::tie(Other.LineNumber, Other.Filename);
+  }
+
   // This operator is used to sort a vector of Locations.
   // No specific order (attributes more important than others) is required. Any
   // sort is enough, the order is only needed to call std::unique after sorting
@@ -270,10 +278,12 @@ struct Info {
 
   virtual ~Info() = default;
 
+  Info &operator=(Info &&Other) = default;
+
   SymbolID USR =
       SymbolID(); // Unique identifier for the decl described by this Info.
-  const InfoType IT = InfoType::IT_default; // InfoType of this particular Info.
-  SmallString<16> Name;                     // Unqualified name of the decl.
+  InfoType IT = InfoType::IT_default; // InfoType of this particular Info.
+  SmallString<16> Name;               // Unqualified name of the decl.
   llvm::SmallVector<Reference, 4>
       Namespace; // List of parent namespaces for this decl.
   std::vector<CommentInfo> Description; // Comment description of this decl.
@@ -312,6 +322,20 @@ struct SymbolInfo : public Info {
 
   std::optional<Location> DefLoc;     // Location where this decl is defined.
   llvm::SmallVector<Location, 2> Loc; // Locations where this decl is declared.
+
+  bool operator<(const SymbolInfo &Other) const {
+    // Sort by declaration location since we want the doc to be
+    // generated in the order of the source code.
+    // If the declaration location is the same, or not present
+    // we sort by defined location otherwise fallback to the extracted name
+    if (Loc.size() > 0 && Other.Loc.size() > 0 && Loc[0] != Other.Loc[0])
+      return Loc[0] < Other.Loc[0];
+
+    if (DefLoc && Other.DefLoc && *DefLoc != *Other.DefLoc)
+      return *DefLoc < *Other.DefLoc;
+
+    return extractName() < Other.extractName();
+  }
 };
 
 // TODO: Expand to allow for documenting templating and default args.

diff --git a/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp b/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -205,6 +205,22 @@ llvm::Error getHtmlAssetFiles(const char *Argv0,
   return getDefaultAssetFiles(Argv0, CDCtx);
 }
 
+/// Make the output of clang-doc deterministic by sorting the children of
+/// namespaces and records.
+void sortUsrToInfo(llvm::StringMap<std::unique_ptr<doc::Info>> &USRToInfo) {
+  for (auto &I : USRToInfo) {
+    auto &Info = I.second;
+    if (Info->IT == doc::InfoType::IT_namespace) {
+      auto *Namespace = static_cast<clang::doc::NamespaceInfo *>(Info.get());
+      Namespace->Children.sort();
+    }
+    if (Info->IT == doc::InfoType::IT_record) {
+      auto *Record = static_cast<clang::doc::RecordInfo *>(Info.get());
+      Record->Children.sort();
+    }
+  }
+}
+
 int main(int argc, const char **argv) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   std::error_code OK;
@@ -341,6 +357,8 @@ Example usage for a project using a compile commands database:
   if (Error)
     return 1;
 
+  sortUsrToInfo(USRToInfo);
+
   // Ensure the root output directory exists.
   if (std::error_code Err = llvm::sys::fs::create_directories(OutDirectory);
       Err != std::error_code()) {

diff --git a/clang-tools-extra/test/clang-doc/basic-project.test b/clang-tools-extra/test/clang-doc/basic-project.test
@@ -1,6 +1,3 @@
-// See https://github.com/llvm/llvm-project/issues/97507.
-// UNSUPPORTED: target={{.*}}
-
 // RUN: rm -rf %t && mkdir -p %t/docs %t/build
 // RUN: sed 's|$test_dir|%/S|g' %S/Inputs/basic-project/database_template.json > %t/build/compile_commands.json
 // RUN: clang-doc --format=html --output=%t/docs --executor=all-TUs %t/build/compile_commands.json
@@ -61,13 +58,13 @@
 // HTML-SHAPE: <p>Defined at line 8 of file {{.*}}Shape.h</p>
 // HTML-SHAPE: <p> Provides a common interface for different types of shapes.</p>
 // HTML-SHAPE: <h2 id="Functions">Functions</h2>
-// HTML-SHAPE: <h3 id="{{([0-9A-F]{40})}}">~Shape</h3>
-// HTML-SHAPE: <p>public void ~Shape()</p>
-// HTML-SHAPE: <p>Defined at line 13 of file {{.*}}Shape.h</p>
 // HTML-SHAPE: <h3 id="{{([0-9A-F]{40})}}">area</h3>
 // HTML-SHAPE: <p>public double area()</p>
 // HTML-SHAPE: <h3 id="{{([0-9A-F]{40})}}">perimeter</h3>
 // HTML-SHAPE: <p>public double perimeter()</p>
+// HTML-SHAPE: <h3 id="{{([0-9A-F]{40})}}">~Shape</h3>
+// HTML-SHAPE: <p>public void ~Shape()</p>
+// HTML-SHAPE: <p>Defined at line 13 of file {{.*}}Shape.h</p>
 
 // HTML-CALC:  <h1>class Calculator</h1>
 // HTML-CALC:  <p>Defined at line 8 of file {{.*}}Calculator.h</p>

diff --git a/clang-tools-extra/test/clang-doc/namespace.cpp b/clang-tools-extra/test/clang-doc/namespace.cpp
@@ -272,14 +272,16 @@ namespace AnotherNamespace {
 // HTML-GLOBAL-INDEX: <h1>Global Namespace</h1>
 // HTML-GLOBAL-INDEX: <h2 id="Namespaces">Namespaces</h2>
 // HTML-GLOBAL-INDEX: <li>@nonymous_namespace</li>
-// HTML-GLOBAL-INDEX: <li>PrimaryNamespace</li>
 // HTML-GLOBAL-INDEX: <li>AnotherNamespace</li>
+// HTML-GLOBAL-INDEX: <li>PrimaryNamespace</li>
+
 
 // MD-GLOBAL-INDEX: # Global Namespace
 // MD-GLOBAL-INDEX: ## Namespaces
 // MD-GLOBAL-INDEX: * [@nonymous_namespace](..{{[\/]}}@nonymous_namespace{{[\/]}}index.md)
-// MD-GLOBAL-INDEX: * [PrimaryNamespace](..{{[\/]}}PrimaryNamespace{{[\/]}}index.md)
 // MD-GLOBAL-INDEX: * [AnotherNamespace](..{{[\/]}}AnotherNamespace{{[\/]}}index.md)
+// MD-GLOBAL-INDEX: * [PrimaryNamespace](..{{[\/]}}PrimaryNamespace{{[\/]}}index.md)
+
 
 // MD-ALL-FILES: # All Files
 // MD-ALL-FILES: ## [@nonymous_namespace](@nonymous_namespace{{[\/]}}index.md)

diff --git a/clang-tools-extra/test/clang-doc/templates.cpp b/clang-tools-extra/test/clang-doc/templates.cpp
@@ -18,6 +18,23 @@ void ParamPackFunction(T... args);
 // CHECK: ---
 // CHECK-NEXT: USR:             '{{([0-9A-F]{40})}}'
 // CHECK-NEXT: ChildFunctions:
+// CHECK-NEXT:  - USR:             '{{([0-9A-F]{40})}}'
+// CHECK-NEXT:    Name:            'ParamPackFunction'
+// CHECK-NEXT:    Location:
+// CHECK-NEXT:      - LineNumber:      16
+// CHECK-NEXT:        Filename:        '{{.*}}'
+// CHECK-NEXT:    Params:
+// CHECK-NEXT:      - Type:
+// CHECK-NEXT:          Name:            'T...'
+// CHECK-NEXT:          QualName:        'T...'
+// CHECK-NEXT:        Name:            'args'
+// CHECK-NEXT:    ReturnType:
+// CHECK-NEXT:      Type:
+// CHECK-NEXT:        Name:            'void'
+// CHECK-NEXT:        QualName:        'void'
+// CHECK-NEXT:    Template:
+// CHECK-NEXT:      Params:
+// CHECK-NEXT:        - Contents:        'class... T'
 // CHECK-NEXT:   - USR:             '{{([0-9A-F]{40})}}'
 // CHECK-NEXT:     Name:            'function'
 // CHECK-NEXT:     DefLocation:
@@ -56,21 +73,4 @@ void ParamPackFunction(T... args);
 // CHECK-NEXT:         Params:
 // CHECK-NEXT:           - Contents:        'bool'
 // CHECK-NEXT:           - Contents:        '0'
-// CHECK-NEXT:  - USR:             '{{([0-9A-F]{40})}}'
-// CHECK-NEXT:    Name:            'ParamPackFunction'
-// CHECK-NEXT:    Location:
-// CHECK-NEXT:      - LineNumber:      16
-// CHECK-NEXT:        Filename:        '{{.*}}'
-// CHECK-NEXT:    Params:
-// CHECK-NEXT:      - Type:
-// CHECK-NEXT:          Name:            'T...'
-// CHECK-NEXT:          QualName:        'T...'
-// CHECK-NEXT:        Name:            'args'
-// CHECK-NEXT:    ReturnType:
-// CHECK-NEXT:      Type:
-// CHECK-NEXT:        Name:            'void'
-// CHECK-NEXT:        QualName:        'void'
-// CHECK-NEXT:    Template:
-// CHECK-NEXT:      Params:
-// CHECK-NEXT:        - Contents:        'class... T'
 // CHECK-NEXT: ...