Skip to content

Commit 54c9ddd

Browse files
Michael137Michael137
authored
[libcxxabi][ItaniumDemangle] Separate GtIsGt counter into more states (#166578)
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. Also adjusted LLDB to use the newly introduced API (and added a unit-test that would previously fail).
1 parent fc5e0c0 commit 54c9ddd

File tree

6 files changed

+66
-24
lines changed

6 files changed

+66
-24
lines changed

‎libcxxabi/src/demangle/ItaniumDemangle.h‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1366,7 +1366,7 @@ class TemplateTemplateParamDecl final : public Node {
13661366
template <typename Fn> void match(Fn F) const { F(Name, Params, Requires); }
13671367

13681368
void printLeft(OutputBuffer &OB) const override {
1369-
ScopedOverride<unsigned> LT(OB.GtIsGt, 0);
1369+
ScopedOverride<bool> LT(OB.TemplateTracker.InsideTemplate, true);
13701370
OB += "template<";
13711371
Params.printWithComma(OB);
13721372
OB += "> typename ";
@@ -1550,7 +1550,7 @@ class TemplateArgs final : public Node {
15501550
NodeArray getParams() { return Params; }
15511551

15521552
void printLeft(OutputBuffer &OB) const override {
1553-
ScopedOverride<unsigned> LT(OB.GtIsGt, 0);
1553+
ScopedOverride<bool> LT(OB.TemplateTracker.InsideTemplate, true);
15541554
OB += "<";
15551555
Params.printWithComma(OB);
15561556
OB += ">";
@@ -1824,7 +1824,7 @@ class ClosureTypeName : public Node {
18241824

18251825
void printDeclarator(OutputBuffer &OB) const {
18261826
if (!TemplateParams.empty()) {
1827-
ScopedOverride<unsigned> LT(OB.GtIsGt, 0);
1827+
ScopedOverride<bool> LT(OB.TemplateTracker.InsideTemplate, true);
18281828
OB += "<";
18291829
TemplateParams.printWithComma(OB);
18301830
OB += ">";
@@ -1885,7 +1885,9 @@ class BinaryExpr : public Node {
18851885
}
18861886

18871887
void printLeft(OutputBuffer &OB) const override {
1888-
bool ParenAll = OB.isGtInsideTemplateArgs() &&
1888+
// If we're printing a '<' inside of a template argument, and we haven't
1889+
// yet parenthesized the expression, do so now.
1890+
bool ParenAll = !OB.isInParensInTemplateArgs() &&
18891891
(InfixOperator == ">" || InfixOperator == ">>");
18901892
if (ParenAll)
18911893
OB.printOpen();
@@ -2061,7 +2063,7 @@ class CastExpr : public Node {
20612063
void printLeft(OutputBuffer &OB) const override {
20622064
OB += CastKind;
20632065
{
2064-
ScopedOverride<unsigned> LT(OB.GtIsGt, 0);
2066+
ScopedOverride<bool> LT(OB.TemplateTracker.InsideTemplate, true);
20652067
OB += "<";
20662068
OB.printLeft(*To);
20672069
OB += ">";

‎libcxxabi/src/demangle/Utility.h‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -104,18 +104,32 @@ class OutputBuffer {
104104
unsigned CurrentPackIndex = std::numeric_limits<unsigned>::max();
105105
unsigned CurrentPackMax = std::numeric_limits<unsigned>::max();
106106

107-
/// When zero, we're printing template args and '>' needs to be parenthesized.
108-
/// Use a counter so we can simply increment inside parentheses.
109-
unsigned GtIsGt = 1;
107+
struct {
108+
/// The depth of '(' and ')' inside the currently printed template
109+
/// arguments.
110+
unsigned ParenDepth = 0;
110111

111-
bool isGtInsideTemplateArgs() const { return GtIsGt == 0; }
112+
/// True if we're currently printing a template argument.
113+
bool InsideTemplate = false;
114+
} TemplateTracker;
115+
116+
/// Returns true if we're currently between a '(' and ')' when printing
117+
/// template args.
118+
bool isInParensInTemplateArgs() const {
119+
return TemplateTracker.ParenDepth > 0;
120+
}
121+
122+
/// Returns true if we're printing template args.
123+
bool isInsideTemplateArgs() const { return TemplateTracker.InsideTemplate; }
112124

113125
void printOpen(char Open = '(') {
114-
GtIsGt++;
126+
if (isInsideTemplateArgs())
127+
TemplateTracker.ParenDepth++;
115128
*this += Open;
116129
}
117130
void printClose(char Close = ')') {
118-
GtIsGt--;
131+
if (isInsideTemplateArgs())
132+
TemplateTracker.ParenDepth--;
119133
*this += Close;
120134
}
121135

‎lldb/source/Core/DemangledNameInfo.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ bool TrackingOutputBuffer::shouldTrack() const {
1616
if (!isPrintingTopLevelFunctionType())
1717
return false;
1818

19-
if (isGtInsideTemplateArgs())
19+
if (isInsideTemplateArgs())
2020
return false;
2121

2222
if (NameInfo.ArgumentsRange.first > 0)
@@ -29,7 +29,7 @@ bool TrackingOutputBuffer::canFinalize() const {
2929
if (!isPrintingTopLevelFunctionType())
3030
return false;
3131

32-
if (isGtInsideTemplateArgs())
32+
if (isInsideTemplateArgs())
3333
return false;
3434

3535
if (NameInfo.ArgumentsRange.first == 0)

‎lldb/unittests/Core/MangledTest.cpp‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -636,6 +636,16 @@ DemanglingPartsTestCase g_demangling_parts_test_cases[] = {
636636
/*.basename=*/"operator()",
637637
/*.scope=*/"dyld4::Loader::runInitializersBottomUpPlusUpwardLinks(dyld4::RuntimeState&) const::$_0::",
638638
/*.qualifiers=*/" const",
639+
},
640+
{"_Z4funcILN3foo4EnumE1EEvv",
641+
{
642+
/*.BasenameRange=*/{5, 9}, /*.TemplateArgumentsRange=*/{9, 23}, /*.ScopeRange=*/{5, 5},
643+
/*.ArgumentsRange=*/{23, 25}, /*.QualifiersRange=*/{25, 25}, /*.NameQualifiersRange=*/{0, 0},
644+
/*.PrefixRange=*/{0, 0}, /*.SuffixRange=*/{0, 0}
645+
},
646+
/*.basename=*/"func",
647+
/*.scope=*/"",
648+
/*.qualifiers=*/"",
639649
}
640650
// clang-format on
641651
};

‎llvm/include/llvm/Demangle/ItaniumDemangle.h‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1366,7 +1366,7 @@ class TemplateTemplateParamDecl final : public Node {
13661366
template <typename Fn> void match(Fn F) const { F(Name, Params, Requires); }
13671367

13681368
void printLeft(OutputBuffer &OB) const override {
1369-
ScopedOverride<unsigned> LT(OB.GtIsGt, 0);
1369+
ScopedOverride<bool> LT(OB.TemplateTracker.InsideTemplate, true);
13701370
OB += "template<";
13711371
Params.printWithComma(OB);
13721372
OB += "> typename ";
@@ -1550,7 +1550,7 @@ class TemplateArgs final : public Node {
15501550
NodeArray getParams() { return Params; }
15511551

15521552
void printLeft(OutputBuffer &OB) const override {
1553-
ScopedOverride<unsigned> LT(OB.GtIsGt, 0);
1553+
ScopedOverride<bool> LT(OB.TemplateTracker.InsideTemplate, true);
15541554
OB += "<";
15551555
Params.printWithComma(OB);
15561556
OB += ">";
@@ -1824,7 +1824,7 @@ class ClosureTypeName : public Node {
18241824

18251825
void printDeclarator(OutputBuffer &OB) const {
18261826
if (!TemplateParams.empty()) {
1827-
ScopedOverride<unsigned> LT(OB.GtIsGt, 0);
1827+
ScopedOverride<bool> LT(OB.TemplateTracker.InsideTemplate, true);
18281828
OB += "<";
18291829
TemplateParams.printWithComma(OB);
18301830
OB += ">";
@@ -1885,7 +1885,9 @@ class BinaryExpr : public Node {
18851885
}
18861886

18871887
void printLeft(OutputBuffer &OB) const override {
1888-
bool ParenAll = OB.isGtInsideTemplateArgs() &&
1888+
// If we're printing a '<' inside of a template argument, and we haven't
1889+
// yet parenthesized the expression, do so now.
1890+
bool ParenAll = !OB.isInParensInTemplateArgs() &&
18891891
(InfixOperator == ">" || InfixOperator == ">>");
18901892
if (ParenAll)
18911893
OB.printOpen();
@@ -2061,7 +2063,7 @@ class CastExpr : public Node {
20612063
void printLeft(OutputBuffer &OB) const override {
20622064
OB += CastKind;
20632065
{
2064-
ScopedOverride<unsigned> LT(OB.GtIsGt, 0);
2066+
ScopedOverride<bool> LT(OB.TemplateTracker.InsideTemplate, true);
20652067
OB += "<";
20662068
OB.printLeft(*To);
20672069
OB += ">";

‎llvm/include/llvm/Demangle/Utility.h‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -104,18 +104,32 @@ class OutputBuffer {
104104
unsigned CurrentPackIndex = std::numeric_limits<unsigned>::max();
105105
unsigned CurrentPackMax = std::numeric_limits<unsigned>::max();
106106

107-
/// When zero, we're printing template args and '>' needs to be parenthesized.
108-
/// Use a counter so we can simply increment inside parentheses.
109-
unsigned GtIsGt = 1;
107+
struct {
108+
/// The depth of '(' and ')' inside the currently printed template
109+
/// arguments.
110+
unsigned ParenDepth = 0;
110111

111-
bool isGtInsideTemplateArgs() const { return GtIsGt == 0; }
112+
/// True if we're currently printing a template argument.
113+
bool InsideTemplate = false;
114+
} TemplateTracker;
115+
116+
/// Returns true if we're currently between a '(' and ')' when printing
117+
/// template args.
118+
bool isInParensInTemplateArgs() const {
119+
return TemplateTracker.ParenDepth > 0;
120+
}
121+
122+
/// Returns true if we're printing template args.
123+
bool isInsideTemplateArgs() const { return TemplateTracker.InsideTemplate; }
112124

113125
void printOpen(char Open = '(') {
114-
GtIsGt++;
126+
if (isInsideTemplateArgs())
127+
TemplateTracker.ParenDepth++;
115128
*this += Open;
116129
}
117130
void printClose(char Close = ')') {
118-
GtIsGt--;
131+
if (isInsideTemplateArgs())
132+
TemplateTracker.ParenDepth--;
119133
*this += Close;
120134
}
121135

0 commit comments

Comments
 (0)