From 54c9ddddd1da353b0303df1e86cf444c5363733d Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Fri, 7 Nov 2025 07:52:03 +0000 Subject: [PATCH] [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> // 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). --- libcxxabi/src/demangle/ItaniumDemangle.h | 12 +++++---- libcxxabi/src/demangle/Utility.h | 26 +++++++++++++++----- lldb/source/Core/DemangledNameInfo.cpp | 4 +-- lldb/unittests/Core/MangledTest.cpp | 10 ++++++++ llvm/include/llvm/Demangle/ItaniumDemangle.h | 12 +++++---- llvm/include/llvm/Demangle/Utility.h | 26 +++++++++++++++----- 6 files changed, 66 insertions(+), 24 deletions(-) diff --git a/libcxxabi/src/demangle/ItaniumDemangle.h b/libcxxabi/src/demangle/ItaniumDemangle.h index 6f27da7b9cad..b999438ff2ca 100644 --- a/libcxxabi/src/demangle/ItaniumDemangle.h +++ b/libcxxabi/src/demangle/ItaniumDemangle.h @@ -1366,7 +1366,7 @@ public: template void match(Fn F) const { F(Name, Params, Requires); } void printLeft(OutputBuffer &OB) const override { - ScopedOverride LT(OB.GtIsGt, 0); + ScopedOverride LT(OB.TemplateTracker.InsideTemplate, true); OB += "template<"; Params.printWithComma(OB); OB += "> typename "; @@ -1550,7 +1550,7 @@ public: NodeArray getParams() { return Params; } void printLeft(OutputBuffer &OB) const override { - ScopedOverride LT(OB.GtIsGt, 0); + ScopedOverride LT(OB.TemplateTracker.InsideTemplate, true); OB += "<"; Params.printWithComma(OB); OB += ">"; @@ -1824,7 +1824,7 @@ public: void printDeclarator(OutputBuffer &OB) const { if (!TemplateParams.empty()) { - ScopedOverride LT(OB.GtIsGt, 0); + ScopedOverride LT(OB.TemplateTracker.InsideTemplate, true); OB += "<"; TemplateParams.printWithComma(OB); OB += ">"; @@ -1885,7 +1885,9 @@ public: } 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 @@ public: void printLeft(OutputBuffer &OB) const override { OB += CastKind; { - ScopedOverride LT(OB.GtIsGt, 0); + ScopedOverride 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 76243f5d3280..df5b54dca492 100644 --- a/libcxxabi/src/demangle/Utility.h +++ b/libcxxabi/src/demangle/Utility.h @@ -104,18 +104,32 @@ public: unsigned CurrentPackIndex = std::numeric_limits::max(); unsigned CurrentPackMax = std::numeric_limits::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/lldb/source/Core/DemangledNameInfo.cpp b/lldb/source/Core/DemangledNameInfo.cpp index 76f8987c5149..16fbfda299b2 100644 --- a/lldb/source/Core/DemangledNameInfo.cpp +++ b/lldb/source/Core/DemangledNameInfo.cpp @@ -16,7 +16,7 @@ bool TrackingOutputBuffer::shouldTrack() const { if (!isPrintingTopLevelFunctionType()) return false; - if (isGtInsideTemplateArgs()) + if (isInsideTemplateArgs()) return false; if (NameInfo.ArgumentsRange.first > 0) @@ -29,7 +29,7 @@ bool TrackingOutputBuffer::canFinalize() const { if (!isPrintingTopLevelFunctionType()) return false; - if (isGtInsideTemplateArgs()) + if (isInsideTemplateArgs()) return false; if (NameInfo.ArgumentsRange.first == 0) diff --git a/lldb/unittests/Core/MangledTest.cpp b/lldb/unittests/Core/MangledTest.cpp index cbc0c5d951b9..706e67801e01 100644 --- a/lldb/unittests/Core/MangledTest.cpp +++ b/lldb/unittests/Core/MangledTest.cpp @@ -636,6 +636,16 @@ DemanglingPartsTestCase g_demangling_parts_test_cases[] = { /*.basename=*/"operator()", /*.scope=*/"dyld4::Loader::runInitializersBottomUpPlusUpwardLinks(dyld4::RuntimeState&) const::$_0::", /*.qualifiers=*/" const", + }, + {"_Z4funcILN3foo4EnumE1EEvv", + { + /*.BasenameRange=*/{5, 9}, /*.TemplateArgumentsRange=*/{9, 23}, /*.ScopeRange=*/{5, 5}, + /*.ArgumentsRange=*/{23, 25}, /*.QualifiersRange=*/{25, 25}, /*.NameQualifiersRange=*/{0, 0}, + /*.PrefixRange=*/{0, 0}, /*.SuffixRange=*/{0, 0} + }, + /*.basename=*/"func", + /*.scope=*/"", + /*.qualifiers=*/"", } // clang-format on }; diff --git a/llvm/include/llvm/Demangle/ItaniumDemangle.h b/llvm/include/llvm/Demangle/ItaniumDemangle.h index 62d427c3966b..67de123fdbad 100644 --- a/llvm/include/llvm/Demangle/ItaniumDemangle.h +++ b/llvm/include/llvm/Demangle/ItaniumDemangle.h @@ -1366,7 +1366,7 @@ public: template void match(Fn F) const { F(Name, Params, Requires); } void printLeft(OutputBuffer &OB) const override { - ScopedOverride LT(OB.GtIsGt, 0); + ScopedOverride LT(OB.TemplateTracker.InsideTemplate, true); OB += "template<"; Params.printWithComma(OB); OB += "> typename "; @@ -1550,7 +1550,7 @@ public: NodeArray getParams() { return Params; } void printLeft(OutputBuffer &OB) const override { - ScopedOverride LT(OB.GtIsGt, 0); + ScopedOverride LT(OB.TemplateTracker.InsideTemplate, true); OB += "<"; Params.printWithComma(OB); OB += ">"; @@ -1824,7 +1824,7 @@ public: void printDeclarator(OutputBuffer &OB) const { if (!TemplateParams.empty()) { - ScopedOverride LT(OB.GtIsGt, 0); + ScopedOverride LT(OB.TemplateTracker.InsideTemplate, true); OB += "<"; TemplateParams.printWithComma(OB); OB += ">"; @@ -1885,7 +1885,9 @@ public: } 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 @@ public: void printLeft(OutputBuffer &OB) const override { OB += CastKind; { - ScopedOverride LT(OB.GtIsGt, 0); + ScopedOverride 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 6e6203d716e7..afdc1a397ca6 100644 --- a/llvm/include/llvm/Demangle/Utility.h +++ b/llvm/include/llvm/Demangle/Utility.h @@ -104,18 +104,32 @@ public: unsigned CurrentPackIndex = std::numeric_limits::max(); unsigned CurrentPackMax = std::numeric_limits::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; }