[Sema] Always instantiate the initializer of a variable template with undeduced type (8.0 regression)

[erik.pilkington]

Summary

This patch fixes a regression introduced in r359947. Here:

template <class T2> struct Outer {
  template <class T1> static constexpr auto x = 1;
};

int main() {
  Outer<int> x;
  int i = x.x<int>;
}

We'd defer the instantiation of the initializer of the variable template when instantiating Outer<int> (leaving the variable template with an undeduced type). We do eventually instantiate the initializer and deduce the type of the variable when we're marking Outer<int>::x<int> referenced, but not before forming a MemberExpr that refers to the undeduced type. I think we could avoid instantiating the initializer of the variable template here, but that doesn't appear to be the intent of r359947, so this patch just falls back to the 8.0 behaviour.

Fixes rdar://52619644

Thanks for taking a look!
Erik

[rsmith]

Well, this restores an incorrect behaviour: we're not permitted to substitute into the initializer of the variable template here.

Can you look into fixing this properly, by instantiating the type when forming the MemberExpr? If not, taking this to fix the regression seems OK, but please file a bug for the introduced misbehaviour.

[erik.pilkington]

Fix the type of the MemberExpr/DeclRefExpr after instantiating the variable initializer.

[erik.pilkington]

Rebase on top of r367367.

[erik.pilkington]

Hmm, looks like this patch fails to fix the case where we're referencing the variable in an non-odr use context, since we don't even bother to instantiate the initializer here:

template <class T>
struct S {
  template <class T2>
  static constexpr auto x = 43;
};

int main() {
  sizeof(S<int>::x<char>);
}

I think we should instantiate the initializer before forming a referencing expression and marking it used. @rsmith: do you mind if I just land https://reviews.llvm.org/D65022?id=210905 and file a bug to fix the regression we're seeing here?

clang/lib/Sema/SemaExpr.cpp

 
     case Decl::CXXConversion:
     case Decl::CXXDestructor:
     case Decl::CXXConstructor:
       valueKind = VK_RValue;
       break;
     }
 
-    return BuildDeclRefExpr(VD, type, valueKind, NameInfo, &SS, FoundD,
-                            /*FIXME: TemplateKWLoc*/ SourceLocation(),
-                            TemplateArgs);
+    DeclRefExpr *DRE = BuildDeclRefExpr(
+        VD, type, valueKind, NameInfo, &SS, FoundD,
+        /*FIXME: TemplateKWLoc*/ SourceLocation(), TemplateArgs);
+
+    // BuildDeclRefExpr may have deduced an auto variable's type. Patch back
+    // that type to avoid forming an expression with undeduced type.
+    if (isa<VarTemplateSpecializationDecl>(VD))
+      DRE->setType(VD->getType().getNonReferenceType());
+    return DRE;
   }
 }
 
 static void ConvertUTF8ToWideString(unsigned CharByteWidth, StringRef Source,
                                     SmallString<32> &Target) {
   Target.resize(CharByteWidth * (Source.size() + 1));
   char *ResultPtr = &Target[0];
   const llvm::UTF8 *ErrorPtr;

clang/lib/Sema/SemaExprMember.cpp

   if (EnumConstantDecl *Enum = dyn_cast<EnumConstantDecl>(MemberDecl)) {
     return BuildMemberExpr(BaseExpr, IsArrow, OpLoc, &SS, TemplateKWLoc, Enum,
                            FoundDecl, /*HadMultipleCandidates=*/false,
                            MemberNameInfo, Enum->getType(), VK_RValue,
                            OK_Ordinary);
   }
   if (VarTemplateDecl *VarTempl = dyn_cast<VarTemplateDecl>(MemberDecl)) {
     if (VarDecl *Var = getVarTemplateSpecialization(
-            *this, VarTempl, TemplateArgs, MemberNameInfo, TemplateKWLoc))
-      return BuildMemberExpr(
+            *this, VarTempl, TemplateArgs, MemberNameInfo, TemplateKWLoc)) {
+      MemberExpr *ME = BuildMemberExpr(
           BaseExpr, IsArrow, OpLoc, &SS, TemplateKWLoc, Var, FoundDecl,
           /*HadMultipleCandidates=*/false, MemberNameInfo,
           Var->getType().getNonReferenceType(), VK_LValue, OK_Ordinary);
+
+      // BuildMemberExpr may have deduced an auto variable's type. Patch back
+      // that type to avoid forming an expression with undeduced type.
+      ME->setType(Var->getType().getNonReferenceType());
+      return ME;
+    }
     return ExprError();
   }
 
   // We found something that we didn't expect. Complain.
   if (isa<TypeDecl>(MemberDecl))
     Diag(MemberLoc, diag::err_typecheck_member_reference_type)
       << MemberName << BaseType << int(IsArrow);
   else

clang/test/SemaCXX/cxx1y-variable-templates_in_class.cpp

namespace dependent_static_var_template {
   int &f() { return T::template n; } // expected-error {{use of variable template 'n' requires template arguments}}
   int &s = f<A>(); // expected-note {{instantiation of}}
 
   namespace B {
     template<int = 0> static int n; // expected-note {{here}}
   }
   int &t = B::template n; // expected-error {{use of variable template 'n' requires template arguments}}
 }
+
+#ifndef PRECXX11
+namespace auto_variable_instantiate_initializer {
+template <class T2> struct S {
+  template <class T> static constexpr auto v = 1;
+  template <class T> static constexpr auto v2 = T{};
+
+  template <class T, int i = v<T>>
+  void implicit() {
+    double d = v2<T>;
+    int i2 = v<short>;
+  }
+
+  void implicit_nondep() {
+    int i = v<unsigned char>;
+    double d = v2<signed char>;
+  }
+};
+
+void useit() {
+  S<int> x;
+  // We used to fail to instantiate the initializer, leading to an auto type
+  // leaking through here.
+  int i = x.v<float>;
+  float j = x.v2<float>;
+
+  x.implicit<double>();
+  x.implicit_nondep();
+}
+}
+#endif