[libcxxabi][ItaniumDemangle] Separate GtIsGt counter into more states #166577
+54
−22
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently `OutputBuffer::GtIsGt` is used to tell us if we're inside template arguments and have printed a '(' without a closing ')'. If so, we don't need to quote '<' when printing it as part of a binary expression inside a template argument. Otherwise we need to. E.g.,
```
foo<a<(b < c)>> // Quotes around binary expression needed.
```
LLDB's `TrackingOutputBuffer` has heuristics that rely on checking whether we are inside template arguments, regardless of the current parentheses depth. We've been using `isGtInsideTemplateArgs` for this, but that isn't correct. Resulting in us incorrectly tracking the basename of function like:
```
void func<(foo::Enum)1>()
```
Here `GtIsGt > 0` despite us being inside template arguments (because we incremented it when seeing '(').
This patch adds a `isInsideTemplateArgs` API which LLDB will use to more accurately track parts of the demangled name.
To make sure this API doesn't go untested in the actual libcxxabi test-suite, I changed the existing `GtIsGt` logic to use it. Also renamed the various variables/APIs involved to make it (in my opinion) more straightforward to understand what's going on. But happy to rename it back if people disagree.
Michael137
requested review from
adrian-prantl,
ldionne,
urnathan and
zygoloid
Michael137
added a commit
to Michael137/llvm-project
that referenced
this pull request
Nov 5, 2025
Depends on: * llvm#166577 The `isGtInsideTemplateArgs` wasn't the correct API to use for answering whether we are currently printing template arugments. One example is: ``` void func<(foo::Enum)1>() ``` Here `OutputBuffer::GtIsGt > 0` despite us being inside template arguments (because we incremented it when seeing '('). This patch now uses the correct API and adds the above as a test-case.
Member
|
@llvm/pr-subscribers-libcxxabi Author: Michael Buch (Michael137) ChangesCurrently LLDB's Here This patch adds a To make sure this API doesn't go untested in the actual libcxxabi test-suite, I changed the existing Full diff: https://github.com/llvm/llvm-project/pull/166577.diff 4 Files Affected:
diff --git a/libcxxabi/src/demangle/ItaniumDemangle.h b/libcxxabi/src/demangle/ItaniumDemangle.h
index 6f27da7b9cadf..b999438ff2ca8 100644
--- a/libcxxabi/src/demangle/ItaniumDemangle.h
+++ b/libcxxabi/src/demangle/ItaniumDemangle.h
@@ -1366,7 +1366,7 @@ class TemplateTemplateParamDecl final : public Node {
template <typename Fn> void match(Fn F) const { F(Name, Params, Requires); }
void printLeft(OutputBuffer &OB) const override {
- ScopedOverride<unsigned> LT(OB.GtIsGt, 0);
+ ScopedOverride<bool> LT(OB.TemplateTracker.InsideTemplate, true);
OB += "template<";
Params.printWithComma(OB);
OB += "> typename ";
@@ -1550,7 +1550,7 @@ class TemplateArgs final : public Node {
NodeArray getParams() { return Params; }
void printLeft(OutputBuffer &OB) const override {
- ScopedOverride<unsigned> LT(OB.GtIsGt, 0);
+ ScopedOverride<bool> LT(OB.TemplateTracker.InsideTemplate, true);
OB += "<";
Params.printWithComma(OB);
OB += ">";
@@ -1824,7 +1824,7 @@ class ClosureTypeName : public Node {
void printDeclarator(OutputBuffer &OB) const {
if (!TemplateParams.empty()) {
- ScopedOverride<unsigned> LT(OB.GtIsGt, 0);
+ ScopedOverride<bool> LT(OB.TemplateTracker.InsideTemplate, true);
OB += "<";
TemplateParams.printWithComma(OB);
OB += ">";
@@ -1885,7 +1885,9 @@ class BinaryExpr : public Node {
}
void printLeft(OutputBuffer &OB) const override {
- bool ParenAll = OB.isGtInsideTemplateArgs() &&
+ // If we're printing a '<' inside of a template argument, and we haven't
+ // yet parenthesized the expression, do so now.
+ bool ParenAll = !OB.isInParensInTemplateArgs() &&
(InfixOperator == ">" || InfixOperator == ">>");
if (ParenAll)
OB.printOpen();
@@ -2061,7 +2063,7 @@ class CastExpr : public Node {
void printLeft(OutputBuffer &OB) const override {
OB += CastKind;
{
- ScopedOverride<unsigned> LT(OB.GtIsGt, 0);
+ ScopedOverride<bool> LT(OB.TemplateTracker.InsideTemplate, true);
OB += "<";
OB.printLeft(*To);
OB += ">";
diff --git a/libcxxabi/src/demangle/Utility.h b/libcxxabi/src/demangle/Utility.h
index 76243f5d3280c..df5b54dca492d 100644
--- a/libcxxabi/src/demangle/Utility.h
+++ b/libcxxabi/src/demangle/Utility.h
@@ -104,18 +104,32 @@ class OutputBuffer {
unsigned CurrentPackIndex = std::numeric_limits<unsigned>::max();
unsigned CurrentPackMax = std::numeric_limits<unsigned>::max();
- /// When zero, we're printing template args and '>' needs to be parenthesized.
- /// Use a counter so we can simply increment inside parentheses.
- unsigned GtIsGt = 1;
+ struct {
+ /// The depth of '(' and ')' inside the currently printed template
+ /// arguments.
+ unsigned ParenDepth = 0;
- bool isGtInsideTemplateArgs() const { return GtIsGt == 0; }
+ /// True if we're currently printing a template argument.
+ bool InsideTemplate = false;
+ } TemplateTracker;
+
+ /// Returns true if we're currently between a '(' and ')' when printing
+ /// template args.
+ bool isInParensInTemplateArgs() const {
+ return TemplateTracker.ParenDepth > 0;
+ }
+
+ /// Returns true if we're printing template args.
+ bool isInsideTemplateArgs() const { return TemplateTracker.InsideTemplate; }
void printOpen(char Open = '(') {
- GtIsGt++;
+ if (isInsideTemplateArgs())
+ TemplateTracker.ParenDepth++;
*this += Open;
}
void printClose(char Close = ')') {
- GtIsGt--;
+ if (isInsideTemplateArgs())
+ TemplateTracker.ParenDepth--;
*this += Close;
}
diff --git a/llvm/include/llvm/Demangle/ItaniumDemangle.h b/llvm/include/llvm/Demangle/ItaniumDemangle.h
index 62d427c3966bb..67de123fdbad5 100644
--- a/llvm/include/llvm/Demangle/ItaniumDemangle.h
+++ b/llvm/include/llvm/Demangle/ItaniumDemangle.h
@@ -1366,7 +1366,7 @@ class TemplateTemplateParamDecl final : public Node {
template <typename Fn> void match(Fn F) const { F(Name, Params, Requires); }
void printLeft(OutputBuffer &OB) const override {
- ScopedOverride<unsigned> LT(OB.GtIsGt, 0);
+ ScopedOverride<bool> LT(OB.TemplateTracker.InsideTemplate, true);
OB += "template<";
Params.printWithComma(OB);
OB += "> typename ";
@@ -1550,7 +1550,7 @@ class TemplateArgs final : public Node {
NodeArray getParams() { return Params; }
void printLeft(OutputBuffer &OB) const override {
- ScopedOverride<unsigned> LT(OB.GtIsGt, 0);
+ ScopedOverride<bool> LT(OB.TemplateTracker.InsideTemplate, true);
OB += "<";
Params.printWithComma(OB);
OB += ">";
@@ -1824,7 +1824,7 @@ class ClosureTypeName : public Node {
void printDeclarator(OutputBuffer &OB) const {
if (!TemplateParams.empty()) {
- ScopedOverride<unsigned> LT(OB.GtIsGt, 0);
+ ScopedOverride<bool> LT(OB.TemplateTracker.InsideTemplate, true);
OB += "<";
TemplateParams.printWithComma(OB);
OB += ">";
@@ -1885,7 +1885,9 @@ class BinaryExpr : public Node {
}
void printLeft(OutputBuffer &OB) const override {
- bool ParenAll = OB.isGtInsideTemplateArgs() &&
+ // If we're printing a '<' inside of a template argument, and we haven't
+ // yet parenthesized the expression, do so now.
+ bool ParenAll = !OB.isInParensInTemplateArgs() &&
(InfixOperator == ">" || InfixOperator == ">>");
if (ParenAll)
OB.printOpen();
@@ -2061,7 +2063,7 @@ class CastExpr : public Node {
void printLeft(OutputBuffer &OB) const override {
OB += CastKind;
{
- ScopedOverride<unsigned> LT(OB.GtIsGt, 0);
+ ScopedOverride<bool> LT(OB.TemplateTracker.InsideTemplate, true);
OB += "<";
OB.printLeft(*To);
OB += ">";
diff --git a/llvm/include/llvm/Demangle/Utility.h b/llvm/include/llvm/Demangle/Utility.h
index 6e6203d716e7a..afdc1a397ca6f 100644
--- a/llvm/include/llvm/Demangle/Utility.h
+++ b/llvm/include/llvm/Demangle/Utility.h
@@ -104,18 +104,32 @@ class OutputBuffer {
unsigned CurrentPackIndex = std::numeric_limits<unsigned>::max();
unsigned CurrentPackMax = std::numeric_limits<unsigned>::max();
- /// When zero, we're printing template args and '>' needs to be parenthesized.
- /// Use a counter so we can simply increment inside parentheses.
- unsigned GtIsGt = 1;
+ struct {
+ /// The depth of '(' and ')' inside the currently printed template
+ /// arguments.
+ unsigned ParenDepth = 0;
- bool isGtInsideTemplateArgs() const { return GtIsGt == 0; }
+ /// True if we're currently printing a template argument.
+ bool InsideTemplate = false;
+ } TemplateTracker;
+
+ /// Returns true if we're currently between a '(' and ')' when printing
+ /// template args.
+ bool isInParensInTemplateArgs() const {
+ return TemplateTracker.ParenDepth > 0;
+ }
+
+ /// Returns true if we're printing template args.
+ bool isInsideTemplateArgs() const { return TemplateTracker.InsideTemplate; }
void printOpen(char Open = '(') {
- GtIsGt++;
+ if (isInsideTemplateArgs())
+ TemplateTracker.ParenDepth++;
*this += Open;
}
void printClose(char Close = ')') {
- GtIsGt--;
+ if (isInsideTemplateArgs())
+ TemplateTracker.ParenDepth--;
*this += Close;
}
|
Member
Author
|
Actually, abandoning in favour of the combined LLDB PR #166578. Otherwise CI won't pass since we're removing an API that LLDB used. |
Michael137
added a commit
to Michael137/llvm-project
that referenced
this pull request
Nov 6, 2025
Depends on: * llvm#166577 The `isGtInsideTemplateArgs` wasn't the correct API to use for answering whether we are currently printing template arugments. One example is: ``` void func<(foo::Enum)1>() ``` Here `OutputBuffer::GtIsGt > 0` despite us being inside template arguments (because we incremented it when seeing '('). This patch now uses the correct API and adds the above as a test-case.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently
OutputBuffer::GtIsGtis used to tell us if we're inside template arguments and have printed a '(' without a closing ')'. If so, we don't need to quote '<' when printing it as part of a binary expression inside a template argument. Otherwise we need to. E.g.,LLDB's
TrackingOutputBufferhas heuristics that rely on checking whether we are inside template arguments, regardless of the current parentheses depth. We've been usingisGtInsideTemplateArgsfor this, but that isn't correct. Resulting in us incorrectly tracking the basename of function like:Here
GtIsGt > 0despite us being inside template arguments (because we incremented it when seeing '(').This patch adds a
isInsideTemplateArgsAPI which LLDB will use to more accurately track parts of the demangled name.To make sure this API doesn't go untested in the actual libcxxabi test-suite, I changed the existing
GtIsGtlogic to use it. Also renamed the various variables/APIs involved to make it (in my opinion) more straightforward to understand what's going on. But happy to rename it back if people disagree.Example of how the new API helps us in LLDB: #166578