[ItaniumMangle] Undeduced auto type doesn't belong in the substitution table

[erik.pilkington]

Summary

Since "Da" and "Dc" (auto and decltype(auto)) are under <builtin-type> in the mangling grammer, they shouldn't have a substitution (https://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangling-compression). I believe that a deduced AutoType shouldn't be able to reach mangleType(AutoType) because it's deduced type would have been canonicalized in mangleType(QualType) before, so I removed that branch and added an assert. FWIW, with this patch applied Clang and GCC agree on the manglings in the affected file.

Thanks for taking a look!
Erik

[rjmccall]

clang/lib/AST/ItaniumMangle.cpp

2342 ↗

(On Diff #141700)

I agree with your analysis that this shouldn't be a substitution candidate. However, I think this probably needs an ABI-compatibility guard.

[erik.pilkington]

Add an ABI compatibility guard. Thanks!

[rjmccall]

clang/lib/AST/ItaniumMangle.cpp

2342 ↗

(On Diff #141700)

You should probably add a comment like "Prior to LLVM v6, Clang accidentally treated deduced auto types as substitution candidates."

2342 ↗

(On Diff #141700)

Er, not "prior to", I guess. "through", maybe, or "up to and including".

[erik.pilkington]

Add a comment.

clang/lib/AST/ItaniumMangle.cpp

2342 ↗

(On Diff #141700)

Sure, I added that comment into the new patch. You meant to say "undeduced auto types", right? Deduced auto types should have been desugared to their underlying type before this function get called in mangleType().

[rjmccall]

clang/lib/AST/ItaniumMangle.cpp

2342 ↗

(On Diff #141700)

Yes, right, thanks. LGTM.

cfe/trunk/lib/AST/ItaniumMangle.cpp

case RQ_RValue:
     break;
   }
 }
 
 void CXXNameMangler::mangleObjCMethodName(const ObjCMethodDecl *MD) {
   Context.mangleObjCMethodName(MD, Out);
 }
 
-static bool isTypeSubstitutable(Qualifiers Quals, const Type *Ty) {
+static bool isTypeSubstitutable(Qualifiers Quals, const Type *Ty,
+                                ASTContext &Ctx) {
   if (Quals)
     return true;
   if (Ty->isSpecificBuiltinType(BuiltinType::ObjCSel))
     return true;
   if (Ty->isOpenCLSpecificType())
     return true;
   if (Ty->isBuiltinType())
     return false;
-
+  // Through to Clang 6.0, we accidentally treated undeduced auto types as
+  // substitution candidates.
+  if (Ctx.getLangOpts().getClangABICompat() > LangOptions::ClangABI::Ver6 &&
+      isa<AutoType>(Ty))
+    return false;
   return true;
 }
 
 void CXXNameMangler::mangleType(QualType T) {
   // If our type is instantiation-dependent but not dependent, we mangle
   // it as it was written in the source, removing any top-level sugar. 
   // Otherwise, use the canonical type.
   //
do {
       
       T = Desugared;
     } while (true);
   }
   SplitQualType split = T.split();
   Qualifiers quals = split.Quals;
   const Type *ty = split.Ty;
 
-  bool isSubstitutable = isTypeSubstitutable(quals, ty);
+  bool isSubstitutable =
+    isTypeSubstitutable(quals, ty, Context.getASTContext());
   if (isSubstitutable && mangleSubstitution(T))
     return;
 
   // If we're mangling a qualified array type, push the qualifiers to
   // the element type.
   if (quals && isa<ArrayType>(T)) {
     ty = Context.getASTContext().getAsArrayType(T);
     quals = Qualifiers();
switch (T->getUTTKind()) {
         break;
     }
   }
 
   mangleType(T->getBaseType());
 }
 
 void CXXNameMangler::mangleType(const AutoType *T) {
-  QualType D = T->getDeducedType();
-  // <builtin-type> ::= Da  # dependent auto
-  if (D.isNull()) {
-    assert(T->getKeyword() != AutoTypeKeyword::GNUAutoType &&
-           "shouldn't need to mangle __auto_type!");
-    Out << (T->isDecltypeAuto() ? "Dc" : "Da");
-  } else
-    mangleType(D);
+  assert(T->getDeducedType().isNull() &&
+         "Deduced AutoType shouldn't be handled here!");
+  // <builtin-type> ::= Da # auto
+  //                ::= Dc # decltype(auto)
 }
 
 void CXXNameMangler::mangleType(const DeducedTemplateSpecializationType *T) {
   // FIXME: This is not the right mangling. We also need to include a scope
   // here in some cases.
   QualType D = T->getDeducedType();
   if (D.isNull())
     mangleUnscopedTemplateName(T->getTemplateName(), nullptr);

cfe/trunk/test/CodeGenCXX/lambda-expressions-inside-auto-functions.cpp

-// RUN: %clang_cc1 -triple x86_64-apple-darwin10.0.0 -fblocks -emit-llvm -o - %s -fexceptions -std=c++1y | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10.0.0 -fblocks -emit-llvm -o - %s -fexceptions -std=c++1y | FileCheck --check-prefix CHECK_ABI_LATEST %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10.0.0 -fblocks -emit-llvm -o - %s -fexceptions -std=c++1y -fclang-abi-compat=6.0 | FileCheck --check-prefix CHECK_ABIV6 %s
 
 // CHECK-LABEL: define void @_ZN19non_inline_function3fooEv
 // CHECK-LABEL: define internal void @"_ZZN19non_inline_function3fooEvENK3$_0clEi"(%class.anon
 // CHECK-LABEL: define internal signext i8 @"_ZZZN19non_inline_function3fooEvENK3$_0clEiENKUlcE_clEc"(%class.anon
 // CHECK-LABEL: define linkonce_odr void @_ZN19non_inline_function4foo2IiEEDav()
 namespace non_inline_function {
 auto foo() {
   auto L = [](int a) {
template<typename T> static auto default_lambda() {
     return [](const T&) { return 42; };
   }
 
   template<class U = decltype(default_lambda<S>())>
     U func(U u = default_lambda<S>()) { return u; }
   
   template<class T> auto foo() { return [](const T&) { return 42; }; }
 };
-//CHECK-LABEL: define linkonce_odr i32 @_ZZN22inline_member_function1AIdE14default_lambdaIdEEDavENKUlRKdE_clES5_(%class.anon
+//CHECK_ABIV6: define linkonce_odr i32 @_ZZN22inline_member_function1AIdE14default_lambdaIdEEDavENKUlRKdE_clES5_(%class.anon
+//CHECK_ABI_LATEST: define linkonce_odr i32 @_ZZN22inline_member_function1AIdE14default_lambdaIdEEDavENKUlRKdE_clES4_(%class.anon
 int run2 = A<double>{}.func()(3.14);
 
-//CHECK-LABEL: define linkonce_odr i32 @_ZZN22inline_member_function1AIcE14default_lambdaIcEEDavENKUlRKcE_clES5_(%class.anon
+//CHECK_ABIV6: define linkonce_odr i32 @_ZZN22inline_member_function1AIcE14default_lambdaIcEEDavENKUlRKcE_clES5_(%class.anon
+//CHECK_ABI_LATEST: define linkonce_odr i32 @_ZZN22inline_member_function1AIcE14default_lambdaIcEEDavENKUlRKcE_clES4_(%class.anon
 int run3 = A<char>{}.func()('a');
 } // end inline_member_function
 
 
 // CHECK-LABEL: define linkonce_odr void @_ZN15inline_function3fooEv()
 // CHECK: define linkonce_odr void @_ZZN15inline_function3fooEvENKUliE_clEi(%class.anon
 // CHECK: define linkonce_odr signext i8 @_ZZZN15inline_function3fooEvENKUliE_clEiENKUlcE_clEc(%class.anon
 namespace inline_function {