Page MenuHomePhabricator

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

Authored by erik.pilkington on Jul 19 2019, 2:46 PM.

Details

Reviewers
rsmith
rjmccall
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

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptJul 19 2019, 2:46 PM

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.

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

Rebase on top of r367367.

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?