[Sema] Pass CombineWithOuterScope = true to constructor of LocalInstantiationScope

[ahatanak]

Summary

This fixes PR28795.

https://llvm.org/bugs/show_bug.cgi?id=28795

Sema wasn't replacing DependentScopeDeclRefExpr with DeclRefExpr during template instantiation of the default argument of the lambda function because it failed to find the instantiated enum "foo" in LocalInstantiationScope. This patch fixes the bug by allowing it to search the outer scope (function func).

[sepavloff]

test/SemaTemplate/default-expr-arguments-3.cpp

20 ↗

(On Diff #66591)

Please add the following test to the patch:

template<typename T> void f1() {
  enum class foo { a, b };
  struct S {
    int g1(foo n = foo::a);
  };
}

template void f1<int>();

It must also compile correctly.

[ahatanak]

OK, I'll add the test.

When I apply this patch and compile your test, I see DependentScopeDeclRefExpr gets converted to DeclRefExpr when the function is instantiated. This doesn't happen without this patch, but somehow clang still manages to emit the same code.

Also, Erik Pilkington pointed out that clang still crashes when it compiles the following code. I'm trying to figure out why that happens.

// Template variable case:
enum class foo { a, b };
template <class T> auto a = [](foo f = foo::a) { return f; }();

auto foo1() {
  return a<int>;
}

// Template struct case:
template <class T> struct func {
  void bar() {
    enum class foo { a, b };
    [](foo f = foo::a) { return f; }();
  }
};

template struct func<int>;

[ahatanak]

In each of these cases, "foo::a" is a non-dependent type, so it seems that clang shouldn't use DependentScopeDeclRefExpr or CXXDependentScopeMemberExpr in the AST when the template definitions are parsed.

[ahatanak]

Handle the case in which the enum is declared inside a member function and add test cases.

The variable template test case still crashes. It looks like it crashes whenever the lambda expression takes a default argument:

template <class T> auto a = [](int f = 7) { return f; }();

auto foo1() {

return a<int>;

}

[ahatanak]

Fix Sema::getTemplateInstantiationArgs to return the template instantiation args when variable templates are being instantiated.

[sepavloff]

include/clang/Sema/Sema.h

6619 ↗

(On Diff #68078)

I would propose to use something like 'getVarTemplateArguments' to make the name more similar to other methods.

lib/Sema/SemaTemplateInstantiate.cpp

193–205 ↗

(On Diff #68078)

The code does not seem to be the best solution. It assumes that if instantiation of a variable is requested, its arguments define all parameters needed to instantiate code. It is not so for the code:

template<typename T2>
struct C1 {
  template<typename T>
  static int v1;
};

template<typename T2>
template<typename T>
int C1<T2>::v1 = [](int a = T2(1)) { return a; }();

int main(int argc, char *argv[]) {
  (void) C1<long*>::v1<int>;
  return 0;
}

This problem is caused by different nature of variable templates. When instantiating a function or a class all instantiated entities go to the contexts defined by the class or the function respectively. A variable does not represent a declaration context and its initializer and all declarations created during the instantiation go to enclosing context. So logic in getTemplateInstantiationArgs does not work.
May be we should extend the logic of getTemplateInstantiationArgs so that it supports variable template and still remains recursive?

[ahatanak]

I agree that extending the logic of getTemplateInstantiationArgs seems like a better approach. I changed Sema::getTemplateInstantiationArgs to search for the template arguments twice, first for the initializer's template arguments and then for the VarTemplateSpecializationDecl. This should enable handling cases where a variable template is declared inside a class/struct.

I wonder whether VarTemplateSpecializationDecl can be made a subclass of DeclContext and have it contain the lambda class of the initializer. I think that will simplify the logic of getTemplateInstantiationArgs.

[sepavloff]

IIUC, the problem is observed because Sema::getTemplateInstantiationArgs does not handle the case of variable templates properly. Classes and functions are declaration contexts and implementation of the aforementioned function (and probably others) relies on this fact. Variable does not represents a context, and this causes errors like this. We cannot make VarTemplateSpecializationDecl a subclass of DeclContext because the latter not only serves as a host for other declarations but also supports name lookup. None is pertinent to the case of variable specialization.

I think, logic of getTemplateInstantiationArgs should be changed. The new implementation could inspect current instantiation (Sema::ActiveTemplateInstantiations) to check if it is an instantiation of a variable template. This could eliminate need of VarTemplateSpec and VarTemplateSpecializationRAII. Such solution looks more flexible as variable initializer may contain references to other variable instantiations, so single value of VarTemplateSpec is not sufficient to track instantiations.

[ahatanak]

Do you think we can transform the Exprs for the uninstantiated default arguments in TreeTransform<Derived>::TransformLambdaExpr? I guess it would be much simpler than trying to make getTemplateInstantiationArgs return the correct template arguments.

[ahatanak]

• Transform uninstantiated default arguments Exprs in TreeTransform<Derived>::TransformLambdaExpr.
• In default-expr-arguments-3.cpp, check that there are no crashes or diagnostics emitted instead of printing the AST.

[ahatanak]

Rebase and ping.

[sepavloff]

Looks good. Thank you!

cfe/trunk/lib/Sema/TreeTransform.h

TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
   CXXMethodDecl *NewCallOperator = getSema().startLambdaDefinition(
       Class, E->getIntroducerRange(), NewCallOpTSI,
       E->getCallOperator()->getLocEnd(),
       NewCallOpTSI->getTypeLoc().castAs<FunctionProtoTypeLoc>().getParams(),
       E->getCallOperator()->isConstexpr());
 
   LSI->CallOperator = NewCallOperator;
 
+  for (unsigned I = 0, NumParams = NewCallOperator->getNumParams();
+       I != NumParams; ++I) {
+    auto *P = NewCallOperator->getParamDecl(I);
+    if (P->hasUninstantiatedDefaultArg()) {
+      EnterExpressionEvaluationContext Eval(
+          getSema(), Sema::PotentiallyEvaluatedIfUsed, P);
+      ExprResult R = getDerived().TransformExpr(
+          E->getCallOperator()->getParamDecl(I)->getDefaultArg());
+      P->setDefaultArg(R.get());
+    }
+  }
+
   getDerived().transformAttrs(E->getCallOperator(), NewCallOperator);
   getDerived().transformedLocalDecl(E->getCallOperator(), NewCallOperator);
 
   // Introduce the context of the call operator.
   Sema::ContextRAII SavedContext(getSema(), NewCallOperator,
                                  /*NewThisContext*/false);
 
   // Enter the scope of the lambda.

cfe/trunk/test/SemaCXX/vartemplate-lambda.cpp

 // RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify %s
-// expected-no-diagnostics
 
 template <class> auto fn0 = [] {};
 template <typename> void foo0() { fn0<char>(); }
 
 template<typename T> auto fn1 = [](auto a) { return a + T(1); };
+template<typename T> auto v1 = [](int a = T(1)) { return a; }();
+
+struct S {
+  template<class T>
+  static constexpr T t = [](int f = T(7)){return f;}(); // expected-error{{constexpr variable 't<int>' must be initialized by a constant expression}} expected-error{{a lambda expression may not appear inside of a constant expression}} expected-note{{cannot be used in a constant expression}}
+};
 
 template <typename X>
 int foo2() {
   X a = 0x61;
   fn1<char>(a);
+  (void)v1<int>;
+  (void)S::t<int>; // expected-note{{in instantiation of static data member 'S::t<int>' requested here}}
   return 0;
 }
 
+template<class C>
+int foo3() {
+  C::m1(); // expected-error{{type 'long long' cannot be used prior to '::' because it has no members}}
+  return 1;
+}
+
+template<class C>
+auto v2 = [](int a = foo3<C>()){};  // expected-note{{in instantiation of function template specialization 'foo3<long long>' requested here}}
+
 int main() {
+  v2<long long>();  // This line causes foo3<long long> to be instantiated.
+  v2<long long>(2); // This line does not.
   foo2<int>();
 }