[clang][NFC] Standard substitution checking cleanup

[urnathan]

Summary

In preparing for module mangling changes I noticed some issues with the way we check for std::basic_string instantiations and friends.

*) there's a single routine for std::basic_{i,o,io}stream but it is templatized on the length of the name. Really? just use a StringRef, rather than clone the entire routine just for 'basic_iostream'.

*) We have a helper routine to check for char type, and call it from several places. But given all the instantiations are of the form TPL<char, Other<char> ...> we could just check the first arg is char and the later templated args are instantiating that same type. A simpler type comparison.

*) Because basic_string has a third allocator parameter, it is open coded, which I found a little confusing. But otherwise it's exactly the same pattern as the iostream ones. Just tell that checker about whether there's an expected allocator argument.[*]

*) We may as well return in each block of mangleStandardSubstitution once we determine it is not one of the entities of interest -- it certainly cannot be one of the other kinds of entities.

FWIW this shaves about 500 bytes off the compiler executable.

• I suppose we could also have this routine a tri-value, with one to indicat 'it is this name, but it's not the one you're looking for', to avoid later calls trying different names?

[ChuanqiXu]

LGTM with nit suggestions.

clang/lib/AST/ItaniumMangle.cpp

5972–5975

Returns whether S is a template specialization of std::Name with a single

6016–6017

This would be simpler: if (!A && !B)

6066

The code style of clang would require to add param name for boolean value:

isStdCharSpecialization(SD, "basic_string", /*HasAllocator=*/true)

Same for following.

[urnathan]

clang/lib/AST/ItaniumMangle.cpp

6016–6017

Yeah, I um'ed an ah'd over that too :)

clang/lib/AST/ItaniumMangle.cpp

bool CXXNameMangler::mangleSubstitution(uintptr_t Ptr) {
 
   unsigned SeqID = I->second;
   Out << 'S';
   mangleSeqID(SeqID);
 
   return true;
 }
 
-static bool isCharType(QualType T) {
-  if (T.isNull())
+/// Returns whether S is a template specialization of std::Name with a single
+/// argument of type A.
+static bool isSpecializedAs(QualType S, llvm::StringRef Name, QualType A) {
+  if (S.isNull())
     return false;
 
-  return T->isSpecificBuiltinType(BuiltinType::Char_S) ||
-    T->isSpecificBuiltinType(BuiltinType::Char_U);
-}
-
-/// Returns whether a given type is a template specialization of a given name
-/// with a single argument of type char.
-static bool isCharSpecialization(QualType T, const char *Name) {
-  if (T.isNull())
-    return false;
-
-  const RecordType *RT = T->getAs<RecordType>();
+  const RecordType *RT = S->getAs<RecordType>();
   if (!RT)
     return false;
 
   const ClassTemplateSpecializationDecl *SD =
     dyn_cast<ClassTemplateSpecializationDecl>(RT->getDecl());
-  if (!SD)
+  if (!SD || !SD->getIdentifier()->isStr(Name))
     return false;
 
   if (!isStdNamespace(getEffectiveDeclContext(SD)))
     return false;
 
   const TemplateArgumentList &TemplateArgs = SD->getTemplateArgs();
   if (TemplateArgs.size() != 1)
     return false;
 
-  if (!isCharType(TemplateArgs[0].getAsType()))
+  if (TemplateArgs[0].getAsType() != A)
     return false;
 
-  return SD->getIdentifier()->getName() == Name;
+  return true;
 }
 
-template <std::size_t StrLen>
-static bool isStreamCharSpecialization(const ClassTemplateSpecializationDecl*SD,
-                                       const char (&Str)[StrLen]) {
-  if (!SD->getIdentifier()->isStr(Str))
+/// Returns whether SD is a template specialization std::Name<char,
+/// std::char_traits<char> [, std::allocator<char>]>
+/// HasAllocator controls whether the 3rd template argument is needed.
+static bool isStdCharSpecialization(const ClassTemplateSpecializationDecl *SD,
+                                    llvm::StringRef Name, bool HasAllocator) {
+  if (!SD->getIdentifier()->isStr(Name))
     return false;
 
   const TemplateArgumentList &TemplateArgs = SD->getTemplateArgs();
-  if (TemplateArgs.size() != 2)
+  if (TemplateArgs.size() != (HasAllocator ? 3 : 2))
     return false;
 
-  if (!isCharType(TemplateArgs[0].getAsType()))
+  QualType A = TemplateArgs[0].getAsType();
+  if (A.isNull())
+    return false;
+  // Plain 'char' is named Char_S or Char_U depending on the target ABI.
+  if (!A->isSpecificBuiltinType(BuiltinType::Char_S) &&
+      !A->isSpecificBuiltinType(BuiltinType::Char_U))
     return false;
 
-  if (!isCharSpecialization(TemplateArgs[1].getAsType(), "char_traits"))
+  if (!isSpecializedAs(TemplateArgs[1].getAsType(), "char_traits", A))
+    return false;
+
+  if (HasAllocator &&
+      !isSpecializedAs(TemplateArgs[2].getAsType(), "allocator", A))
     return false;
 
   return true;
 }
 
 bool CXXNameMangler::mangleStandardSubstitution(const NamedDecl *ND) {
   // <substitution> ::= St # ::std::
   if (const NamespaceDecl *NS = dyn_cast<NamespaceDecl>(ND)) {
     if (isStd(NS)) {
       Out << "St";
       return true;
     }
+    return false;
   }
 
   if (const ClassTemplateDecl *TD = dyn_cast<ClassTemplateDecl>(ND)) {
     if (!isStdNamespace(getEffectiveDeclContext(TD)))
       return false;
 
     // <substitution> ::= Sa # ::std::allocator
     if (TD->getIdentifier()->isStr("allocator")) {
       Out << "Sa";
       return true;
     }
 
     // <<substitution> ::= Sb # ::std::basic_string
     if (TD->getIdentifier()->isStr("basic_string")) {
       Out << "Sb";
       return true;
     }
+    return false;
   }
 
   if (const ClassTemplateSpecializationDecl *SD =
         dyn_cast<ClassTemplateSpecializationDecl>(ND)) {
     if (!isStdNamespace(getEffectiveDeclContext(SD)))
       return false;
 
     //    <substitution> ::= Ss # ::std::basic_string<char,
     //                            ::std::char_traits<char>,
     //                            ::std::allocator<char> >
-    if (SD->getIdentifier()->isStr("basic_string")) {
-      const TemplateArgumentList &TemplateArgs = SD->getTemplateArgs();
-
-      if (TemplateArgs.size() != 3)
-        return false;
-
-      if (!isCharType(TemplateArgs[0].getAsType()))
-        return false;
-
-      if (!isCharSpecialization(TemplateArgs[1].getAsType(), "char_traits"))
-        return false;
-
-      if (!isCharSpecialization(TemplateArgs[2].getAsType(), "allocator"))
-        return false;
-
+    if (isStdCharSpecialization(SD, "basic_string", /*HasAllocator=*/true)) {
       Out << "Ss";
       return true;
     }
 
     //    <substitution> ::= Si # ::std::basic_istream<char,
     //                            ::std::char_traits<char> >
-    if (isStreamCharSpecialization(SD, "basic_istream")) {
+    if (isStdCharSpecialization(SD, "basic_istream", /*HasAllocator=*/false)) {
       Out << "Si";
       return true;
     }
 
     //    <substitution> ::= So # ::std::basic_ostream<char,
     //                            ::std::char_traits<char> >
-    if (isStreamCharSpecialization(SD, "basic_ostream")) {
+    if (isStdCharSpecialization(SD, "basic_ostream", /*HasAllocator=*/false)) {
       Out << "So";
       return true;
     }
 
     //    <substitution> ::= Sd # ::std::basic_iostream<char,
     //                            ::std::char_traits<char> >
-    if (isStreamCharSpecialization(SD, "basic_iostream")) {
+    if (isStdCharSpecialization(SD, "basic_iostream", /*HasAllocator=*/false)) {
       Out << "Sd";
       return true;
     }
+    return false;
   }
+
   return false;
 }
 
 void CXXNameMangler::addSubstitution(QualType T) {
   if (!hasMangledSubstitutionQualifiers(T)) {
     if (const RecordType *RT = T->getAs<RecordType>()) {
       addSubstitution(RT->getDecl());
       return;