Skip to content

Conversation

evelez7
Copy link
Member

@evelez7 evelez7 commented Aug 26, 2025

The previous limit did not take into account filename extensions. Since
the extensions we use (.yaml, .json, .html, .md) are at most 5
characters, we can lower the limit by 5.

Also add some tests to make sure the rules are observed and don't
explicitly crash clang-doc.

@evelez7 evelez7 marked this pull request as ready for review August 26, 2025 22:33
Copy link
Member Author

evelez7 commented Aug 26, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@llvmbot
Copy link
Member

llvmbot commented Aug 26, 2025

@llvm/pr-subscribers-clang-tools-extra

Author: Erick Velez (evelez7)

Changes

The previous limit did not take into account filename extensions. Since
the extensions we use (.yaml, .json, .html, .md) are at most 5
characters, we can lower the limit by 5.

Also add some tests to make sure the rules are observed and don't
explicitly crash clang-doc.


Full diff: https://github.com/llvm/llvm-project/pull/155511.diff

2 Files Affected:

  • (modified) clang-tools-extra/clang-doc/Serialize.cpp (+1-1)
  • (added) clang-tools-extra/test/clang-doc/long-name.cpp (+14)
diff --git a/clang-tools-extra/clang-doc/Serialize.cpp b/clang-tools-extra/clang-doc/Serialize.cpp
index bcab4f1b8a729..fdcf3b29fb754 100644
--- a/clang-tools-extra/clang-doc/Serialize.cpp
+++ b/clang-tools-extra/clang-doc/Serialize.cpp
@@ -778,7 +778,7 @@ static void populateSymbolInfo(SymbolInfo &I, const T *D, const FullComment *C,
     Mangler->mangleCXXVTable(CXXD, MangledStream);
   else
     MangledStream << D->getNameAsString();
-  if (MangledName.size() > 255)
+  if (MangledName.size() > 250)
     // File creation fails if the mangled name is too long, so default to the
     // USR. We should look for a better check since filesystems differ in
     // maximum filename length
diff --git a/clang-tools-extra/test/clang-doc/long-name.cpp b/clang-tools-extra/test/clang-doc/long-name.cpp
new file mode 100644
index 0000000000000..b33337588da19
--- /dev/null
+++ b/clang-tools-extra/test/clang-doc/long-name.cpp
@@ -0,0 +1,14 @@
+// RUN: rm -rf %t && mkdir -p %t
+// RUN: clang-doc --output=%t --format=mustache --executor=standalone %s
+// RUN: ls %t/json | FileCheck %s -check-prefix=CHECK-JSON
+// RUN: ls %t/html | FileCheck %s -check-prefix=CHECK-HTML
+
+struct ThisStructHasANameThatResultsInAMangledNameThatIsExactly250CharactersLongThatIsSupposedToTestTheFilenameLengthLimitsWithinClangDocInOrdertoSeeifclangdocwillcrashornotdependingonthelengthofthestructIfTheLengthIsTooLongThenClangDocWillCrashAnd12 {};
+
+// This name is 1 character over the limit, so it will be serialized as a USR.
+struct ThisStructHasANameThatResultsInAMangledNameThatIsExactly251CharactersLongThatIsSupposedToTestTheFilenameLengthLimitsWithinClangDocInOrdertoSeeifclangdocwillcrashornotdependingonthelengthofthestructIfTheLengthIsTooLongThenClangDocWillCrashAnd123 {};
+
+// CHECK-JSON: ThisStructHasANameThatResultsInAMangledNameThatIsExactly250CharactersLongThatIsSupposedToTestTheFilenameLengthLimitsWithinClangDocInOrdertoSeeifclangdocwillcrashornotdependingonthelengthofthestructIfTheLengthIsTooLongThenClangDocWillCrashAnd12.json
+// CHECK-JSON: {{[0-9A-F]*}}.json
+// CHECK-HTML: ThisStructHasANameThatResultsInAMangledNameThatIsExactly250CharactersLongThatIsSupposedToTestTheFilenameLengthLimitsWithinClangDocInOrdertoSeeifclangdocwillcrashornotdependingonthelengthofthestructIfTheLengthIsTooLongThenClangDocWillCrashAnd12.html
+// CHECK-HTML: {{[0-9A-F]*}}.html

Copy link
Contributor

@ilovepi ilovepi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I thought we had a unit test for this? Am I misremembering?

@ilovepi
Copy link
Contributor

ilovepi commented Aug 26, 2025

LGTM, but I thought we had a unit test for this? Am I misremembering?

Looking at the source: I am wrong and we never had one.

The previous limit did not take into account filename extensions. Since
the extensions we use (.yaml, .json, .html, .md) are at most 5
characters, we can lower the limit by 5.

Also add some tests to make sure the rules are observed and don't
explicitly crash clang-doc.
@evelez7 evelez7 force-pushed the users/evelez7/clang-doc-long-name-bug branch from 509159b to fc4ad9d Compare August 26, 2025 23:14
Copy link
Member Author

evelez7 commented Aug 27, 2025

Merge activity

  • Aug 27, 2:59 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Aug 27, 3:01 PM UTC: @evelez7 merged this pull request with Graphite.

@evelez7 evelez7 merged commit 5136ef9 into main Aug 27, 2025
9 checks passed
@evelez7 evelez7 deleted the users/evelez7/clang-doc-long-name-bug branch August 27, 2025 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants