Differential D23096
[Sema] Pass CombineWithOuterScope = true to constructor of LocalInstantiationScope Authored by ahatanak on Aug 2 2016, 4:39 PM.
Details 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).
Diff Detail Event Timeline
Comment Actions 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>;Comment Actions 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. Comment Actions 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>; } Comment Actions Fix Sema::getTemplateInstantiationArgs to return the template instantiation args when variable templates are being instantiated.
Comment Actions 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. Comment Actions 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. Comment Actions 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. Comment Actions
| ||||||||||||||||||
I would propose to use something like 'getVarTemplateArguments' to make the name more similar to other methods.