[clang-doc] fix names of conversions for template parameters #140856
Conversation
|
@llvm/pr-subscribers-clang-tools-extra Author: Erick Velez (evelez7) ChangesFixes #59812 The names of conversion functions of template type parameters were being emitted as "type-parameter-N-M". Now we check if the conversion type is a TemplateTypeParmType and reconstruct the source name. Full diff: https://github.com/llvm/llvm-project/pull/140856.diff 1 Files Affected:
diff --git a/clang-tools-extra/clang-doc/Serialize.cpp b/clang-tools-extra/clang-doc/Serialize.cpp
index 18db427b5239e..585a46112d43d 100644
--- a/clang-tools-extra/clang-doc/Serialize.cpp
+++ b/clang-tools-extra/clang-doc/Serialize.cpp
@@ -525,7 +525,13 @@ template <typename T>
static void populateInfo(Info &I, const T *D, const FullComment *C,
bool &IsInAnonymousNamespace) {
I.USR = getUSRForDecl(D);
- I.Name = D->getNameAsString();
+ auto ConversionDecl = dyn_cast_or_null<CXXConversionDecl>(D);
+ if (ConversionDecl && ConversionDecl->getConversionType()
+ .getTypePtr()
+ ->isTemplateTypeParmType())
+ I.Name = "operator " + ConversionDecl->getConversionType().getAsString();
+ else
+ I.Name = D->getNameAsString();
populateParentNamespaces(I.Namespace, D, IsInAnonymousNamespace);
if (C) {
I.Description.emplace_back();
|
|
Should note that #59812 also mentions the |
There was a problem hiding this comment.
LGTM, but would you mind pre-committing the test so its easy to see what's changing in the output with this patch? I've been trying to get more of that done when we're changing clang-doc output. Otherwise it's hard to know exactly what we're achieving with a particular patch. I can rubber stamp the pre-commit patch.
Could you explain what this means please? Do you mean reorganizing the PR so that the first commit is the test and then the second is the code change? |
|
Sorry, I mean committing a test w/ the current in-tree behavior, and then this patch would have your fix and a small delta to the test showing the new behavior. Some information on that can be found here: https://llvm.org/docs/TestingGuide.html#precommit-workflow-for-tests |
|
You can find a stack where I did that here: https://app.graphite.dev/github/pr/llvm/llvm-project/135705/%5Bllvm%5D%5Blto%5D-Precommit-test-for-libcall-internalization Those aren't landed, since we're hashing out the proper way to fix things, but I added a test w/ the behavior today, and then in the next patch its very easy to see what changed. |
evelez7
force-pushed
the
clang-doc-operator-names
branch
from
985ca4b to
b1a07c3
Compare
|
Opened #141168. I'll make the test changes once that's in. I can see why Graphite would be useful to learn. Makes me miss Phabricator a bit. 🥲 |
|
stacked PRs are still not as nice as phabricator or gerrit, but Graphite is a lot more firendly than I found spr, or winging it w/ github PRs. For a precommit test, it isn't too hard to upload a patch for before and then rebase the "after" patch after it lands. |
evelez7
force-pushed
the
clang-doc-operator-names
branch
from
b1a07c3 to
f2f7e1e
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/174/builds/18265 Here is the relevant piece of the build log for the reference |
…0856) Fixes llvm#59812 The names of conversion functions of template type parameters were being emitted as "type-parameter-N-M". Now we check if the conversion type is a TemplateTypeParmType and reconstruct the source name.
Fixes #59812
The names of conversion functions of template type parameters were being emitted as "type-parameter-N-M". Now we check if the conversion type is a TemplateTypeParmType and reconstruct the source name.