[demangler] remove StdQualifiedName

[urnathan]

Summary

The StdQualifiedName node class is used for names exactly in the std namespace. It is not used for nested names that descend further --
those use a NestedName with NameType("std") as the scope. We can use the same structure for those exactly in std too, and reduce code size a bit.

This also makes a further simplification possible.

[ChuanqiXu]

Do you say: we would use StdQualifiedName for names in ::std originally and use NameType with value std for names in something like NamespaceA::std::? If I understand correctly, I feel that it might not be a good choice. I think the current design makes the semantics more clear and it have a higher level of abstraction. The description says the main intention is to make code shorter. But I feel that the shorter code is not always simpler to understand.

[urnathan]

No, sorry for being unclear.

std::name (St4Name)-> we would use StdQualifiedName
std::name1::name2 (NSt5Name15Name2E)-> we would use NestedName (std, ...)
std::name1<Inst> (NSt5Name1I4InstEE)-> we would also use NestedName (std, ...)

this patch moves to using NestedName in all these cases. (The other substitutions, such as 'Sa' remain unchanged)

[ChuanqiXu]

In D118249#3275622, @urnathan wrote:

No, sorry for being unclear.

std::name (St4Name)-> we would use StdQualifiedName
std::name1::name2 (NSt5Name15Name2E)-> we would use NestedName (std, ...)
std::name1<Inst> (NSt5Name1I4InstEE)-> we would also use NestedName (std, ...)

this patch moves to using NestedName in all these cases. (The other substitutions, such as 'Sa' remain unchanged)

Yeah, but it looks like StdQualifiedName is deleted completely...

[urnathan]

In D118249#3278121, @ChuanqiXu wrote:

In D118249#3275622, @urnathan wrote:

No, sorry for being unclear.

std::name (St4Name)-> we would use StdQualifiedName
std::name1::name2 (NSt5Name15Name2E)-> we would use NestedName (std, ...)
std::name1<Inst> (NSt5Name1I4InstEE)-> we would also use NestedName (std, ...)

this patch moves to using NestedName in all these cases. (The other substitutions, such as 'Sa' remain unchanged)

Yeah, but it looks like StdQualifiedName is deleted completely...

Yes. That is the intent.

[ChuanqiXu]

In D118249#3279254, @urnathan wrote:

In D118249#3278121, @ChuanqiXu wrote:

In D118249#3275622, @urnathan wrote:

No, sorry for being unclear.

std::name (St4Name)-> we would use StdQualifiedName
std::name1::name2 (NSt5Name15Name2E)-> we would use NestedName (std, ...)
std::name1<Inst> (NSt5Name1I4InstEE)-> we would also use NestedName (std, ...)

this patch moves to using NestedName in all these cases. (The other substitutions, such as 'Sa' remain unchanged)

Yeah, but it looks like StdQualifiedName is deleted completely...

Yes. That is the intent.

I got your point. hmmm I think such rewriting would make the code harder to read. Is it possible to use StdQualifiedName to replace NestedName (std, ...)?

[urnathan]

In D118249#3281281, @ChuanqiXu wrote:

In D118249#3279254, @urnathan wrote:

Yeah, but it looks like StdQualifiedName is deleted completely...

Yes. That is the intent.

I got your point. hmmm I think such rewriting would make the code harder to read. Is it possible to use StdQualifiedName to replace NestedName (std, ...)?

I'm sorry I disagree. I see no reason to special-case '::std' in this way, and doing so makes the code harder to read --we'd keep having to check if a name is ::std. Let's pick one way to represent nested names.

ETA: Let's expand this a bit more.
The substitution mechanism is a compression technique in the mangled symbols. The Node type hierarchy is a representation of the components of an entity's identity, and the subsequent Node graph is that entity. These latter two components need not and, I assert, should not contain representations of the compression -- that's mixing abstractions (The Nodes don't contain representations of the back-reference substitutions for instance.)

I hope that makes things clearer?

[ChuanqiXu]

In D118249#3283567, @urnathan wrote:

In D118249#3281281, @ChuanqiXu wrote:

In D118249#3279254, @urnathan wrote:

Yeah, but it looks like StdQualifiedName is deleted completely...

Yes. That is the intent.

I got your point. hmmm I think such rewriting would make the code harder to read. Is it possible to use StdQualifiedName to replace NestedName (std, ...)?

I'm sorry I disagree. I see no reason to special-case '::std' in this way, and doing so makes the code harder to read --we'd keep having to check if a name is ::std. Let's pick one way to represent nested names.

ETA: Let's expand this a bit more.
The substitution mechanism is a compression technique in the mangled symbols. The Node type hierarchy is a representation of the components of an entity's identity, and the subsequent Node graph is that entity. These latter two components need not and, I assert, should not contain representations of the compression -- that's mixing abstractions (The Nodes don't contain representations of the back-reference substitutions for instance.)

I hope that makes things clearer?

So the key point here is: Although ::std namespace is special in mangling, we could treat it homogeneously in the Node since we could handle it later. Do I understand right? If yes, I think this one should be good.

[urnathan]

In D118249#3299854, @ChuanqiXu wrote:

So the key point here is: Although ::std namespace is special in mangling, we could treat it homogeneously in the Node since we could handle it later. Do I understand right? If yes, I think this one should be good.

yes, that is correct. i hope you enjoyed your vacation.

[ChuanqiXu]

In D118249#3300683, @urnathan wrote:

In D118249#3299854, @ChuanqiXu wrote:

So the key point here is: Although ::std namespace is special in mangling, we could treat it homogeneously in the Node since we could handle it later. Do I understand right? If yes, I think this one should be good.

yes, that is correct. i hope you enjoyed your vacation.

Thanks : )

libcxxabi/src/demangle/ItaniumDemangle.h

#define FOR_EACH_NODE_KIND(X) \
     X(TemplateParamPackDecl) \
     X(ParameterPack) \
     X(TemplateArgumentPack) \
     X(ParameterPackExpansion) \
     X(TemplateArgs) \
     X(ForwardTemplateReference) \
     X(NameWithTemplateArgs) \
     X(GlobalQualifiedName) \
-    X(StdQualifiedName) \
     X(ExpandedSpecialSubstitution) \
     X(SpecialSubstitution) \
     X(CtorDtorName) \
     X(DtorName) \
     X(UnnamedTypeName) \
     X(ClosureTypeName) \
     X(StructuredBindingName) \
     X(BinaryExpr) \
public:
   StringView getBaseName() const override { return Child->getBaseName(); }
 
   void printLeft(OutputBuffer &OB) const override {
     OB += "::";
     Child->print(OB);
   }
 };
 
-struct StdQualifiedName : Node {
-  Node *Child;
-
-  StdQualifiedName(Node *Child_) : Node(KStdQualifiedName), Child(Child_) {}
-
-  template<typename Fn> void match(Fn F) const { F(Child); }
-
-  StringView getBaseName() const override { return Child->getBaseName(); }
-
-  void printLeft(OutputBuffer &OB) const override {
-    OB += "std::";
-    Child->print(OB);
-  }
-};
-
 enum class SpecialSubKind {
   allocator,
   basic_string,
   string,
   istream,
   ostream,
   iostream,
 };

 AbstractManglingParser<Derived, Alloc>::parseUnscopedName(NameState *State) {
   bool IsStd = consumeIf("St");
   if (IsStd)
     consumeIf('L');
 
   Node *Result = getDerived().parseUnqualifiedName(State);
   if (Result == nullptr)
     return nullptr;
-  if (IsStd)
-    Result = make<StdQualifiedName>(Result);
+  if (IsStd) {
+    if (auto *Std = make<NameType>("std"))
+      Result = make<NestedName>(Std, Result);
+    else
+      return nullptr;
+  }
 
   return Result;
 }
 
 // <unqualified-name> ::= <operator-name> [abi-tags]
 //                    ::= <ctor-dtor-name>
 //                    ::= <source-name>
 //                    ::= <unnamed-type-name>

llvm/include/llvm/Demangle/ItaniumDemangle.h

#define FOR_EACH_NODE_KIND(X) \
     X(TemplateParamPackDecl) \
     X(ParameterPack) \
     X(TemplateArgumentPack) \
     X(ParameterPackExpansion) \
     X(TemplateArgs) \
     X(ForwardTemplateReference) \
     X(NameWithTemplateArgs) \
     X(GlobalQualifiedName) \
-    X(StdQualifiedName) \
     X(ExpandedSpecialSubstitution) \
     X(SpecialSubstitution) \
     X(CtorDtorName) \
     X(DtorName) \
     X(UnnamedTypeName) \
     X(ClosureTypeName) \
     X(StructuredBindingName) \
     X(BinaryExpr) \
public:
   StringView getBaseName() const override { return Child->getBaseName(); }
 
   void printLeft(OutputBuffer &OB) const override {
     OB += "::";
     Child->print(OB);
   }
 };
 
-struct StdQualifiedName : Node {
-  Node *Child;
-
-  StdQualifiedName(Node *Child_) : Node(KStdQualifiedName), Child(Child_) {}
-
-  template<typename Fn> void match(Fn F) const { F(Child); }
-
-  StringView getBaseName() const override { return Child->getBaseName(); }
-
-  void printLeft(OutputBuffer &OB) const override {
-    OB += "std::";
-    Child->print(OB);
-  }
-};
-
 enum class SpecialSubKind {
   allocator,
   basic_string,
   string,
   istream,
   ostream,
   iostream,
 };

 AbstractManglingParser<Derived, Alloc>::parseUnscopedName(NameState *State) {
   bool IsStd = consumeIf("St");
   if (IsStd)
     consumeIf('L');
 
   Node *Result = getDerived().parseUnqualifiedName(State);
   if (Result == nullptr)
     return nullptr;
-  if (IsStd)
-    Result = make<StdQualifiedName>(Result);
+  if (IsStd) {
+    if (auto *Std = make<NameType>("std"))
+      Result = make<NestedName>(Std, Result);
+    else
+      return nullptr;
+  }
 
   return Result;
 }
 
 // <unqualified-name> ::= <operator-name> [abi-tags]
 //                    ::= <ctor-dtor-name>
 //                    ::= <source-name>
 //                    ::= <unnamed-type-name>

llvm/lib/Demangle/ItaniumDemangle.cpp

char *ItaniumPartialDemangler::getFunctionBaseName(char *Buf, size_t *N) const {
 
   const Node *Name = static_cast<const FunctionEncoding *>(RootNode)->getName();
 
   while (true) {
     switch (Name->getKind()) {
     case Node::KAbiTagAttr:
       Name = static_cast<const AbiTagAttr *>(Name)->Base;
       continue;
-    case Node::KStdQualifiedName:
-      Name = static_cast<const StdQualifiedName *>(Name)->Child;
-      continue;
     case Node::KNestedName:
       Name = static_cast<const NestedName *>(Name)->Name;
       continue;
     case Node::KLocalName:
       Name = static_cast<const LocalName *>(Name)->Entity;
       continue;
     case Node::KNameWithTemplateArgs:
       Name = static_cast<const NameWithTemplateArgs *>(Name)->Name;
while (true) {
     if (Name->getKind() == Node::KNameWithTemplateArgs) {
       Name = static_cast<const NameWithTemplateArgs *>(Name)->Name;
       continue;
     }
     break;
   }
 
   switch (Name->getKind()) {
-  case Node::KStdQualifiedName:
-    OB += "std";
-    break;
   case Node::KNestedName:
     static_cast<const NestedName *>(Name)->Qual->print(OB);
     break;
   case Node::KLocalName: {
     auto *LN = static_cast<const LocalName *>(Name);
     LN->Encoding->print(OB);
     OB += "::";
     Name = LN->Entity;
case Node::KLocalName:
       N = static_cast<const LocalName *>(N)->Entity;
       break;
     case Node::KNameWithTemplateArgs:
       N = static_cast<const NameWithTemplateArgs *>(N)->Name;
       break;
     case Node::KNestedName:
       N = static_cast<const NestedName *>(N)->Name;
       break;
-    case Node::KStdQualifiedName:
-      N = static_cast<const StdQualifiedName *>(N)->Child;
-      break;
     }
   }
   return false;
 }
 
 bool ItaniumPartialDemangler::isFunction() const {
   assert(RootNode != nullptr && "must call partialDemangle()");
   return static_cast<const Node *>(RootNode)->getKind() ==

llvm/lib/Support/ItaniumManglingCanonicalizer.cpp

public:
 
   void trackUsesOf(Node *N) {
     TrackedNode = N;
     TrackedNodeIsUsed = false;
   }
   bool trackedNodeIsUsed() const { return TrackedNodeIsUsed; }
 };
 
-/// Convert St3foo to NSt3fooE so that equivalences naming one also affect the
-/// other.
-template<>
-struct CanonicalizerAllocator::MakeNodeImpl<
-           itanium_demangle::StdQualifiedName> {
-  CanonicalizerAllocator &Self;
-  Node *make(Node *Child) {
-    Node *StdNamespace = Self.makeNode<itanium_demangle::NameType>("std");
-    if (!StdNamespace)
-      return nullptr;
-    return Self.makeNode<itanium_demangle::NestedName>(StdNamespace, Child);
-  }
-};
-
 // FIXME: Also expand built-in substitutions?
 
 using CanonicalizingDemangler =
     itanium_demangle::ManglingParser<CanonicalizerAllocator>;
 } // namespace
 
 struct ItaniumManglingCanonicalizer::Impl {
   CanonicalizingDemangler Demangler = {nullptr, nullptr};