| Message ID | fb884cb0-6c94-862f-c80a-b2a2ad574282@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | [C++] PR 84536 ("[7/8/9 Regression] ICE with non-type template parameter") | expand |
On 2/17/19 6:58 AM, Paolo Carlini wrote: > Hi, > > here, when we don't see an initializer we believe we are surely dealing > with a case of C++17 template argument deduction for class templates, > but, in fact, it's just an ill-formed C++14 template variable > specialization. Conveniently, we can use here too the predicate > variable_template_specialization_p. Not 100% sure about the exact > wording of the error message, I added '#' to %qD to explicitly print the > auto-using type too. I guess we should change the assert to a test, so that we give the error if we aren't dealing with a class template placeholder. Variable templates don't seem to be important to test for. This error is also pretty poor for this testcase, where there is an initializer. Jason
Hi Jason, On 18/02/19 10:20, Jason Merrill wrote: > On 2/17/19 6:58 AM, Paolo Carlini wrote: >> Hi, >> >> here, when we don't see an initializer we believe we are surely >> dealing with a case of C++17 template argument deduction for class >> templates, but, in fact, it's just an ill-formed C++14 template >> variable specialization. Conveniently, we can use here too the >> predicate variable_template_specialization_p. Not 100% sure about the >> exact wording of the error message, I added '#' to %qD to explicitly >> print the auto-using type too. > > I guess we should change the assert to a test, so that we give the > error if we aren't dealing with a class template placeholder. Variable > templates don't seem to be important to test for. Thanks, simpler patch. > This error is also pretty poor for this testcase, where there is an > initializer. Well, implementation-wise, certainly init == NULL_TREE and only when we have an empty pack this specific issue occurs. In practice, clang simply talks about an empty initializer (during instantiation, etc, like we do), whereas EDG explicitly says that pack expansion produces an empty list of expressions. I don't think that in cp_finish_decl it would be easy for us to do exactly the same, we simply see a NULL_TREE as second argument. Or we could just *assume* that we are dealing with the outcome of a pack expansion, say something like EDG even if we don't have details beyond the fact that init == NULL_TREE. I believe that without a variadic template the problem cannot occur, because we catch the empty initializer much earlier, in grokdeclarator - indeed using a !CLASS_PLACEHOLDER_TEMPLATE (auto_node) check. What do you think? Again "instantiated for an empty pack" or something similar? Thanks, Paolo. ////////////////////// Index: cp/decl.c =================================================================== --- cp/decl.c (revision 268968) +++ cp/decl.c (working copy) @@ -6945,8 +6945,13 @@ cp_finish_decl (tree decl, tree init, bool init_co && (auto_node = type_uses_auto (type))) { tree d_init; - if (init == NULL_TREE) - gcc_assert (CLASS_PLACEHOLDER_TEMPLATE (auto_node)); + if (init == NULL_TREE + && !CLASS_PLACEHOLDER_TEMPLATE (auto_node)) + { + error ("initializer for %q#D is empty", decl); + TREE_TYPE (decl) = error_mark_node; + return; + } d_init = init; if (d_init) { Index: testsuite/g++.dg/cpp1y/var-templ60.C =================================================================== --- testsuite/g++.dg/cpp1y/var-templ60.C (nonexistent) +++ testsuite/g++.dg/cpp1y/var-templ60.C (working copy) @@ -0,0 +1,9 @@ +// PR c++/84536 +// { dg-do compile { target c++14 } } + +template<int... N> auto foo(N...); // { dg-error "initializer" } + +void bar() +{ + foo<>(); +}
On 2/18/19 5:31 AM, Paolo Carlini wrote: > Hi Jason, > > On 18/02/19 10:20, Jason Merrill wrote: >> On 2/17/19 6:58 AM, Paolo Carlini wrote: >>> Hi, >>> >>> here, when we don't see an initializer we believe we are surely >>> dealing with a case of C++17 template argument deduction for class >>> templates, but, in fact, it's just an ill-formed C++14 template >>> variable specialization. Conveniently, we can use here too the >>> predicate variable_template_specialization_p. Not 100% sure about the >>> exact wording of the error message, I added '#' to %qD to explicitly >>> print the auto-using type too. >> >> I guess we should change the assert to a test, so that we give the >> error if we aren't dealing with a class template placeholder. Variable >> templates don't seem to be important to test for. > Thanks, simpler patch. >> This error is also pretty poor for this testcase, where there is an >> initializer. > > Well, implementation-wise, certainly init == NULL_TREE and only when we > have an empty pack this specific issue occurs. > > In practice, clang simply talks about an empty initializer (during > instantiation, etc, like we do), whereas EDG explicitly says that pack > expansion produces an empty list of expressions. I don't think that in > cp_finish_decl it would be easy for us to do exactly the same, we simply > see a NULL_TREE as second argument. Or we could just *assume* that we > are dealing with the outcome of a pack expansion, say something like EDG > even if we don't have details beyond the fact that init == NULL_TREE. I > believe that without a variadic template the problem cannot occur, > because we catch the empty initializer much earlier, in grokdeclarator - > indeed using a !CLASS_PLACEHOLDER_TEMPLATE (auto_node) check. What do > you think? Again "instantiated for an empty pack" or something similar? Perhaps we could complain in the code for empty pack expansion handling in tsubst_init? Jason
Hi Jason, On 18/02/19 19:28, Jason Merrill wrote: > On 2/18/19 5:31 AM, Paolo Carlini wrote: >> Hi Jason, >> >> On 18/02/19 10:20, Jason Merrill wrote: >>> On 2/17/19 6:58 AM, Paolo Carlini wrote: >>>> Hi, >>>> >>>> here, when we don't see an initializer we believe we are surely >>>> dealing with a case of C++17 template argument deduction for class >>>> templates, but, in fact, it's just an ill-formed C++14 template >>>> variable specialization. Conveniently, we can use here too the >>>> predicate variable_template_specialization_p. Not 100% sure about >>>> the exact wording of the error message, I added '#' to %qD to >>>> explicitly print the auto-using type too. >>> >>> I guess we should change the assert to a test, so that we give the >>> error if we aren't dealing with a class template placeholder. >>> Variable templates don't seem to be important to test for. >> Thanks, simpler patch. >>> This error is also pretty poor for this testcase, where there is an >>> initializer. >> >> Well, implementation-wise, certainly init == NULL_TREE and only when >> we have an empty pack this specific issue occurs. >> >> In practice, clang simply talks about an empty initializer (during >> instantiation, etc, like we do), whereas EDG explicitly says that >> pack expansion produces an empty list of expressions. I don't think >> that in cp_finish_decl it would be easy for us to do exactly the >> same, we simply see a NULL_TREE as second argument. Or we could just >> *assume* that we are dealing with the outcome of a pack expansion, >> say something like EDG even if we don't have details beyond the fact >> that init == NULL_TREE. I believe that without a variadic template >> the problem cannot occur, because we catch the empty initializer much >> earlier, in grokdeclarator - indeed using a >> !CLASS_PLACEHOLDER_TEMPLATE (auto_node) check. What do you think? >> Again "instantiated for an empty pack" or something similar? > > Perhaps we could complain in the code for empty pack expansion > handling in tsubst_init? Ah, thanks Jason. In fact, however, tsubst_init isn't currently involved at all, because, at the end of regenerate_decl_from_template we call by hand tsubst_expr and assign the result to DECL_INITIAL. Simply changing that avoids the ICE. However, the error we issue - likewise for the existing cpp0x/auto31.C - is the rather user-unfriendly "value-initialization of incomplete type ‘auto’", as produced by build_value_init. Thus a simple additional test along the lines already discussed, which now becomes much more simple to implement in a precise way. Again, wording only tentative. I'm also a little puzzled that, otherwise, we could get away with tubst_expr instead of tsubst_init... Thanks, Paolo. ////////////////////// Index: cp/pt.c =================================================================== --- cp/pt.c (revision 268995) +++ cp/pt.c (working copy) @@ -15424,6 +15424,14 @@ tsubst_init (tree init, tree decl, tree args, if (!init && TREE_TYPE (decl) != error_mark_node) { + if (type_uses_auto (TREE_TYPE (decl))) + { + if (complain & tf_error) + error ("initializer for %q#D expands to an empty list " + "of expressions", decl); + return error_mark_node; + } + /* If we had an initializer but it instantiated to nothing, value-initialize the object. This will @@ -24053,9 +24061,8 @@ regenerate_decl_from_template (tree decl, tree tmp { start_lambda_scope (decl); DECL_INITIAL (decl) = - tsubst_expr (DECL_INITIAL (code_pattern), args, - tf_error, DECL_TI_TEMPLATE (decl), - /*integral_constant_expression_p=*/false); + tsubst_init (DECL_INITIAL (code_pattern), decl, args, + tf_error, DECL_TI_TEMPLATE (decl)); finish_lambda_scope (); if (VAR_HAD_UNKNOWN_BOUND (decl)) TREE_TYPE (decl) = tsubst (TREE_TYPE (code_pattern), args, Index: testsuite/g++.dg/cpp1y/var-templ60.C =================================================================== --- testsuite/g++.dg/cpp1y/var-templ60.C (nonexistent) +++ testsuite/g++.dg/cpp1y/var-templ60.C (working copy) @@ -0,0 +1,9 @@ +// PR c++/84536 +// { dg-do compile { target c++14 } } + +template<int... N> auto foo(N...); // { dg-error "initializer" } + +void bar() +{ + foo<>(); +}
On 2/18/19 12:14 PM, Paolo Carlini wrote: > Hi Jason, > > On 18/02/19 19:28, Jason Merrill wrote: >> On 2/18/19 5:31 AM, Paolo Carlini wrote: >>> Hi Jason, >>> >>> On 18/02/19 10:20, Jason Merrill wrote: >>>> On 2/17/19 6:58 AM, Paolo Carlini wrote: >>>>> Hi, >>>>> >>>>> here, when we don't see an initializer we believe we are surely >>>>> dealing with a case of C++17 template argument deduction for class >>>>> templates, but, in fact, it's just an ill-formed C++14 template >>>>> variable specialization. Conveniently, we can use here too the >>>>> predicate variable_template_specialization_p. Not 100% sure about >>>>> the exact wording of the error message, I added '#' to %qD to >>>>> explicitly print the auto-using type too. >>>> >>>> I guess we should change the assert to a test, so that we give the >>>> error if we aren't dealing with a class template placeholder. >>>> Variable templates don't seem to be important to test for. >>> Thanks, simpler patch. >>>> This error is also pretty poor for this testcase, where there is an >>>> initializer. >>> >>> Well, implementation-wise, certainly init == NULL_TREE and only when >>> we have an empty pack this specific issue occurs. >>> >>> In practice, clang simply talks about an empty initializer (during >>> instantiation, etc, like we do), whereas EDG explicitly says that >>> pack expansion produces an empty list of expressions. I don't think >>> that in cp_finish_decl it would be easy for us to do exactly the >>> same, we simply see a NULL_TREE as second argument. Or we could just >>> *assume* that we are dealing with the outcome of a pack expansion, >>> say something like EDG even if we don't have details beyond the fact >>> that init == NULL_TREE. I believe that without a variadic template >>> the problem cannot occur, because we catch the empty initializer much >>> earlier, in grokdeclarator - indeed using a >>> !CLASS_PLACEHOLDER_TEMPLATE (auto_node) check. What do you think? >>> Again "instantiated for an empty pack" or something similar? >> >> Perhaps we could complain in the code for empty pack expansion >> handling in tsubst_init? > > Ah, thanks Jason. In fact, however, tsubst_init isn't currently involved > at all, because, at the end of regenerate_decl_from_template we call by > hand tsubst_expr and assign the result to DECL_INITIAL. Simply changing > that avoids the ICE. However, the error we issue - likewise for the > existing cpp0x/auto31.C - is the rather user-unfriendly > "value-initialization of incomplete type ‘auto’", as produced by > build_value_init. Thus a simple additional test along the lines already > discussed, which now becomes much more simple to implement in a precise > way. Again, wording only tentative. I'm also a little puzzled that, > otherwise, we could get away with tubst_expr instead of tsubst_init... > + if (type_uses_auto (TREE_TYPE (decl))) > + { > + if (complain & tf_error) > + error ("initializer for %q#D expands to an empty list " > + "of expressions", decl); > + return error_mark_node; > + } This needs to allow the CLASS_PLACEHOLDER_TEMPLATE case. And yes, we mustn't call build_value_init for a dependent type; if the type is dependent, we should just return the NULL_TREE. Jason
Hi, On 19/02/19 00:52, Jason Merrill wrote: > On 2/18/19 12:14 PM, Paolo Carlini wrote: >> Hi Jason, >> >> On 18/02/19 19:28, Jason Merrill wrote: >>> On 2/18/19 5:31 AM, Paolo Carlini wrote: >>>> Hi Jason, >>>> >>>> On 18/02/19 10:20, Jason Merrill wrote: >>>>> On 2/17/19 6:58 AM, Paolo Carlini wrote: >>>>>> Hi, >>>>>> >>>>>> here, when we don't see an initializer we believe we are surely >>>>>> dealing with a case of C++17 template argument deduction for >>>>>> class templates, but, in fact, it's just an ill-formed C++14 >>>>>> template variable specialization. Conveniently, we can use here >>>>>> too the predicate variable_template_specialization_p. Not 100% >>>>>> sure about the exact wording of the error message, I added '#' to >>>>>> %qD to explicitly print the auto-using type too. >>>>> >>>>> I guess we should change the assert to a test, so that we give the >>>>> error if we aren't dealing with a class template placeholder. >>>>> Variable templates don't seem to be important to test for. >>>> Thanks, simpler patch. >>>>> This error is also pretty poor for this testcase, where there is >>>>> an initializer. >>>> >>>> Well, implementation-wise, certainly init == NULL_TREE and only >>>> when we have an empty pack this specific issue occurs. >>>> >>>> In practice, clang simply talks about an empty initializer (during >>>> instantiation, etc, like we do), whereas EDG explicitly says that >>>> pack expansion produces an empty list of expressions. I don't think >>>> that in cp_finish_decl it would be easy for us to do exactly the >>>> same, we simply see a NULL_TREE as second argument. Or we could >>>> just *assume* that we are dealing with the outcome of a pack >>>> expansion, say something like EDG even if we don't have details >>>> beyond the fact that init == NULL_TREE. I believe that without a >>>> variadic template the problem cannot occur, because we catch the >>>> empty initializer much earlier, in grokdeclarator - indeed using a >>>> !CLASS_PLACEHOLDER_TEMPLATE (auto_node) check. What do you think? >>>> Again "instantiated for an empty pack" or something similar? >>> >>> Perhaps we could complain in the code for empty pack expansion >>> handling in tsubst_init? >> >> Ah, thanks Jason. In fact, however, tsubst_init isn't currently >> involved at all, because, at the end of regenerate_decl_from_template >> we call by hand tsubst_expr and assign the result to DECL_INITIAL. >> Simply changing that avoids the ICE. However, the error we issue - >> likewise for the existing cpp0x/auto31.C - is the rather >> user-unfriendly "value-initialization of incomplete type ‘auto’", as >> produced by build_value_init. Thus a simple additional test along the >> lines already discussed, which now becomes much more simple to >> implement in a precise way. Again, wording only tentative. I'm also a >> little puzzled that, otherwise, we could get away with tubst_expr >> instead of tsubst_init... > >> + if (type_uses_auto (TREE_TYPE (decl))) >> + { >> + if (complain & tf_error) >> + error ("initializer for %q#D expands to an empty list " >> + "of expressions", decl); >> + return error_mark_node; >> + } > > This needs to allow the CLASS_PLACEHOLDER_TEMPLATE case. > > And yes, we mustn't call build_value_init for a dependent type; if the > type is dependent, we should just return the NULL_TREE. Good. Then I'm finishing testing the below (currently in libstdc++). Thanks, Paolo. ////////////////// Index: cp/pt.c =================================================================== --- cp/pt.c (revision 268997) +++ cp/pt.c (working copy) @@ -15422,21 +15422,34 @@ tsubst_init (tree init, tree decl, tree args, init = tsubst_expr (init, args, complain, in_decl, false); - if (!init && TREE_TYPE (decl) != error_mark_node) + tree type = TREE_TYPE (decl); + + if (!init && type != error_mark_node) { - /* If we had an initializer but it - instantiated to nothing, - value-initialize the object. This will - only occur when the initializer was a - pack expansion where the parameter packs - used in that expansion were of length - zero. */ - init = build_value_init (TREE_TYPE (decl), - complain); - if (TREE_CODE (init) == AGGR_INIT_EXPR) - init = get_target_expr_sfinae (init, complain); - if (TREE_CODE (init) == TARGET_EXPR) - TARGET_EXPR_DIRECT_INIT_P (init) = true; + if (tree auto_node = type_uses_auto (type)) + if (!CLASS_PLACEHOLDER_TEMPLATE (auto_node)) + { + if (complain & tf_error) + error ("initializer for %q#D expands to an empty list " + "of expressions", decl); + return error_mark_node; + } + + if (!dependent_type_p (type)) + { + /* If we had an initializer but it + instantiated to nothing, + value-initialize the object. This will + only occur when the initializer was a + pack expansion where the parameter packs + used in that expansion were of length + zero. */ + init = build_value_init (type, complain); + if (TREE_CODE (init) == AGGR_INIT_EXPR) + init = get_target_expr_sfinae (init, complain); + if (TREE_CODE (init) == TARGET_EXPR) + TARGET_EXPR_DIRECT_INIT_P (init) = true; + } } return init; @@ -24053,9 +24066,8 @@ regenerate_decl_from_template (tree decl, tree tmp { start_lambda_scope (decl); DECL_INITIAL (decl) = - tsubst_expr (DECL_INITIAL (code_pattern), args, - tf_error, DECL_TI_TEMPLATE (decl), - /*integral_constant_expression_p=*/false); + tsubst_init (DECL_INITIAL (code_pattern), decl, args, + tf_error, DECL_TI_TEMPLATE (decl)); finish_lambda_scope (); if (VAR_HAD_UNKNOWN_BOUND (decl)) TREE_TYPE (decl) = tsubst (TREE_TYPE (code_pattern), args, Index: testsuite/g++.dg/cpp1y/var-templ60.C =================================================================== --- testsuite/g++.dg/cpp1y/var-templ60.C (nonexistent) +++ testsuite/g++.dg/cpp1y/var-templ60.C (working copy) @@ -0,0 +1,9 @@ +// PR c++/84536 +// { dg-do compile { target c++14 } } + +template<int... N> auto foo(N...); // { dg-error "initializer" } + +void bar() +{ + foo<>(); +}
On 2/18/19 3:15 PM, Paolo Carlini wrote: > Hi, > > On 19/02/19 00:52, Jason Merrill wrote: >> On 2/18/19 12:14 PM, Paolo Carlini wrote: >>> Hi Jason, >>> >>> On 18/02/19 19:28, Jason Merrill wrote: >>>> On 2/18/19 5:31 AM, Paolo Carlini wrote: >>>>> Hi Jason, >>>>> >>>>> On 18/02/19 10:20, Jason Merrill wrote: >>>>>> On 2/17/19 6:58 AM, Paolo Carlini wrote: >>>>>>> Hi, >>>>>>> >>>>>>> here, when we don't see an initializer we believe we are surely >>>>>>> dealing with a case of C++17 template argument deduction for >>>>>>> class templates, but, in fact, it's just an ill-formed C++14 >>>>>>> template variable specialization. Conveniently, we can use here >>>>>>> too the predicate variable_template_specialization_p. Not 100% >>>>>>> sure about the exact wording of the error message, I added '#' to >>>>>>> %qD to explicitly print the auto-using type too. >>>>>> >>>>>> I guess we should change the assert to a test, so that we give the >>>>>> error if we aren't dealing with a class template placeholder. >>>>>> Variable templates don't seem to be important to test for. >>>>> Thanks, simpler patch. >>>>>> This error is also pretty poor for this testcase, where there is >>>>>> an initializer. >>>>> >>>>> Well, implementation-wise, certainly init == NULL_TREE and only >>>>> when we have an empty pack this specific issue occurs. >>>>> >>>>> In practice, clang simply talks about an empty initializer (during >>>>> instantiation, etc, like we do), whereas EDG explicitly says that >>>>> pack expansion produces an empty list of expressions. I don't think >>>>> that in cp_finish_decl it would be easy for us to do exactly the >>>>> same, we simply see a NULL_TREE as second argument. Or we could >>>>> just *assume* that we are dealing with the outcome of a pack >>>>> expansion, say something like EDG even if we don't have details >>>>> beyond the fact that init == NULL_TREE. I believe that without a >>>>> variadic template the problem cannot occur, because we catch the >>>>> empty initializer much earlier, in grokdeclarator - indeed using a >>>>> !CLASS_PLACEHOLDER_TEMPLATE (auto_node) check. What do you think? >>>>> Again "instantiated for an empty pack" or something similar? >>>> >>>> Perhaps we could complain in the code for empty pack expansion >>>> handling in tsubst_init? >>> >>> Ah, thanks Jason. In fact, however, tsubst_init isn't currently >>> involved at all, because, at the end of regenerate_decl_from_template >>> we call by hand tsubst_expr and assign the result to DECL_INITIAL. >>> Simply changing that avoids the ICE. However, the error we issue - >>> likewise for the existing cpp0x/auto31.C - is the rather >>> user-unfriendly "value-initialization of incomplete type ‘auto’", as >>> produced by build_value_init. Thus a simple additional test along the >>> lines already discussed, which now becomes much more simple to >>> implement in a precise way. Again, wording only tentative. I'm also a >>> little puzzled that, otherwise, we could get away with tubst_expr >>> instead of tsubst_init... >> >>> + if (type_uses_auto (TREE_TYPE (decl))) >>> + { >>> + if (complain & tf_error) >>> + error ("initializer for %q#D expands to an empty list " >>> + "of expressions", decl); >>> + return error_mark_node; >>> + } >> >> This needs to allow the CLASS_PLACEHOLDER_TEMPLATE case. >> >> And yes, we mustn't call build_value_init for a dependent type; if the >> type is dependent, we should just return the NULL_TREE. > > Good. Then I'm finishing testing the below (currently in libstdc++). > + if (tree auto_node = type_uses_auto (type)) > + if (!CLASS_PLACEHOLDER_TEMPLATE (auto_node)) > + { > + if (complain & tf_error) > + error ("initializer for %q#D expands to an empty list " > + "of expressions", decl); > + return error_mark_node; > + } > + > + if (!dependent_type_p (type)) This should probably be 'else if', since we can have auto outside of a template and dependent_type_p will always return false outside of a template. Jason
Hi, On 19/02/19 02:39, Jason Merrill wrote: > On 2/18/19 3:15 PM, Paolo Carlini wrote: >> Hi, >> >> On 19/02/19 00:52, Jason Merrill wrote: >>> On 2/18/19 12:14 PM, Paolo Carlini wrote: >>>> Hi Jason, >>>> >>>> On 18/02/19 19:28, Jason Merrill wrote: >>>>> On 2/18/19 5:31 AM, Paolo Carlini wrote: >>>>>> Hi Jason, >>>>>> >>>>>> On 18/02/19 10:20, Jason Merrill wrote: >>>>>>> On 2/17/19 6:58 AM, Paolo Carlini wrote: >>>>>>>> Hi, >>>>>>>> >>>>>>>> here, when we don't see an initializer we believe we are surely >>>>>>>> dealing with a case of C++17 template argument deduction for >>>>>>>> class templates, but, in fact, it's just an ill-formed C++14 >>>>>>>> template variable specialization. Conveniently, we can use here >>>>>>>> too the predicate variable_template_specialization_p. Not 100% >>>>>>>> sure about the exact wording of the error message, I added '#' >>>>>>>> to %qD to explicitly print the auto-using type too. >>>>>>> >>>>>>> I guess we should change the assert to a test, so that we give >>>>>>> the error if we aren't dealing with a class template >>>>>>> placeholder. Variable templates don't seem to be important to >>>>>>> test for. >>>>>> Thanks, simpler patch. >>>>>>> This error is also pretty poor for this testcase, where there is >>>>>>> an initializer. >>>>>> >>>>>> Well, implementation-wise, certainly init == NULL_TREE and only >>>>>> when we have an empty pack this specific issue occurs. >>>>>> >>>>>> In practice, clang simply talks about an empty initializer >>>>>> (during instantiation, etc, like we do), whereas EDG explicitly >>>>>> says that pack expansion produces an empty list of expressions. I >>>>>> don't think that in cp_finish_decl it would be easy for us to do >>>>>> exactly the same, we simply see a NULL_TREE as second argument. >>>>>> Or we could just *assume* that we are dealing with the outcome of >>>>>> a pack expansion, say something like EDG even if we don't have >>>>>> details beyond the fact that init == NULL_TREE. I believe that >>>>>> without a variadic template the problem cannot occur, because we >>>>>> catch the empty initializer much earlier, in grokdeclarator - >>>>>> indeed using a !CLASS_PLACEHOLDER_TEMPLATE (auto_node) check. >>>>>> What do you think? Again "instantiated for an empty pack" or >>>>>> something similar? >>>>> >>>>> Perhaps we could complain in the code for empty pack expansion >>>>> handling in tsubst_init? >>>> >>>> Ah, thanks Jason. In fact, however, tsubst_init isn't currently >>>> involved at all, because, at the end of >>>> regenerate_decl_from_template we call by hand tsubst_expr and >>>> assign the result to DECL_INITIAL. Simply changing that avoids the >>>> ICE. However, the error we issue - likewise for the existing >>>> cpp0x/auto31.C - is the rather user-unfriendly >>>> "value-initialization of incomplete type ‘auto’", as produced by >>>> build_value_init. Thus a simple additional test along the lines >>>> already discussed, which now becomes much more simple to implement >>>> in a precise way. Again, wording only tentative. I'm also a little >>>> puzzled that, otherwise, we could get away with tubst_expr instead >>>> of tsubst_init... >>> >>>> + if (type_uses_auto (TREE_TYPE (decl))) >>>> + { >>>> + if (complain & tf_error) >>>> + error ("initializer for %q#D expands to an empty list " >>>> + "of expressions", decl); >>>> + return error_mark_node; >>>> + } >>> >>> This needs to allow the CLASS_PLACEHOLDER_TEMPLATE case. >>> >>> And yes, we mustn't call build_value_init for a dependent type; if >>> the type is dependent, we should just return the NULL_TREE. >> >> Good. Then I'm finishing testing the below (currently in libstdc++). > >> + if (tree auto_node = type_uses_auto (type)) >> + if (!CLASS_PLACEHOLDER_TEMPLATE (auto_node)) >> + { >> + if (complain & tf_error) >> + error ("initializer for %q#D expands to an empty list " >> + "of expressions", decl); >> + return error_mark_node; >> + } >> + >> + if (!dependent_type_p (type)) > > This should probably be 'else if', since we can have auto outside of a > template and dependent_type_p will always return false outside of a > template. Ok... Note that the CLASS_PLACEHOLDER_TEMPLATE (auto_node) check in tsubst_init doesn't really make a difference for our testsuite and I have yet to find a testcase where it does. In cp_finish_decl it did, for sure. That said, if I understand correctly, you mean that in principle when we have an 'auto' outside of a template and CLASS_PLACEHOLDER_TEMPLATE is true, we don't want to do the build_value_init bits - which we would certainly do because dependent_type_p is false. If we are on the same page on this, the below regtested successfully. Thanks! Paolo. ////////////////////// Index: cp/pt.c =================================================================== --- cp/pt.c (revision 268997) +++ cp/pt.c (working copy) @@ -15422,21 +15422,35 @@ tsubst_init (tree init, tree decl, tree args, init = tsubst_expr (init, args, complain, in_decl, false); - if (!init && TREE_TYPE (decl) != error_mark_node) + tree type = TREE_TYPE (decl); + + if (!init && type != error_mark_node) { - /* If we had an initializer but it - instantiated to nothing, - value-initialize the object. This will - only occur when the initializer was a - pack expansion where the parameter packs - used in that expansion were of length - zero. */ - init = build_value_init (TREE_TYPE (decl), - complain); - if (TREE_CODE (init) == AGGR_INIT_EXPR) - init = get_target_expr_sfinae (init, complain); - if (TREE_CODE (init) == TARGET_EXPR) - TARGET_EXPR_DIRECT_INIT_P (init) = true; + if (tree auto_node = type_uses_auto (type)) + { + if (!CLASS_PLACEHOLDER_TEMPLATE (auto_node)) + { + if (complain & tf_error) + error ("initializer for %q#D expands to an empty list " + "of expressions", decl); + return error_mark_node; + } + } + else if (!dependent_type_p (type)) + { + /* If we had an initializer but it + instantiated to nothing, + value-initialize the object. This will + only occur when the initializer was a + pack expansion where the parameter packs + used in that expansion were of length + zero. */ + init = build_value_init (type, complain); + if (TREE_CODE (init) == AGGR_INIT_EXPR) + init = get_target_expr_sfinae (init, complain); + if (TREE_CODE (init) == TARGET_EXPR) + TARGET_EXPR_DIRECT_INIT_P (init) = true; + } } return init; @@ -24053,9 +24067,8 @@ regenerate_decl_from_template (tree decl, tree tmp { start_lambda_scope (decl); DECL_INITIAL (decl) = - tsubst_expr (DECL_INITIAL (code_pattern), args, - tf_error, DECL_TI_TEMPLATE (decl), - /*integral_constant_expression_p=*/false); + tsubst_init (DECL_INITIAL (code_pattern), decl, args, + tf_error, DECL_TI_TEMPLATE (decl)); finish_lambda_scope (); if (VAR_HAD_UNKNOWN_BOUND (decl)) TREE_TYPE (decl) = tsubst (TREE_TYPE (code_pattern), args, Index: testsuite/g++.dg/cpp1y/var-templ60.C =================================================================== --- testsuite/g++.dg/cpp1y/var-templ60.C (nonexistent) +++ testsuite/g++.dg/cpp1y/var-templ60.C (working copy) @@ -0,0 +1,9 @@ +// PR c++/84536 +// { dg-do compile { target c++14 } } + +template<int... N> auto foo(N...); // { dg-error "initializer" } + +void bar() +{ + foo<>(); +}
OK. On Tue, Feb 19, 2019 at 1:16 AM Paolo Carlini <paolo.carlini@oracle.com> wrote: > > Hi, > > On 19/02/19 02:39, Jason Merrill wrote: > > On 2/18/19 3:15 PM, Paolo Carlini wrote: > >> Hi, > >> > >> On 19/02/19 00:52, Jason Merrill wrote: > >>> On 2/18/19 12:14 PM, Paolo Carlini wrote: > >>>> Hi Jason, > >>>> > >>>> On 18/02/19 19:28, Jason Merrill wrote: > >>>>> On 2/18/19 5:31 AM, Paolo Carlini wrote: > >>>>>> Hi Jason, > >>>>>> > >>>>>> On 18/02/19 10:20, Jason Merrill wrote: > >>>>>>> On 2/17/19 6:58 AM, Paolo Carlini wrote: > >>>>>>>> Hi, > >>>>>>>> > >>>>>>>> here, when we don't see an initializer we believe we are surely > >>>>>>>> dealing with a case of C++17 template argument deduction for > >>>>>>>> class templates, but, in fact, it's just an ill-formed C++14 > >>>>>>>> template variable specialization. Conveniently, we can use here > >>>>>>>> too the predicate variable_template_specialization_p. Not 100% > >>>>>>>> sure about the exact wording of the error message, I added '#' > >>>>>>>> to %qD to explicitly print the auto-using type too. > >>>>>>> > >>>>>>> I guess we should change the assert to a test, so that we give > >>>>>>> the error if we aren't dealing with a class template > >>>>>>> placeholder. Variable templates don't seem to be important to > >>>>>>> test for. > >>>>>> Thanks, simpler patch. > >>>>>>> This error is also pretty poor for this testcase, where there is > >>>>>>> an initializer. > >>>>>> > >>>>>> Well, implementation-wise, certainly init == NULL_TREE and only > >>>>>> when we have an empty pack this specific issue occurs. > >>>>>> > >>>>>> In practice, clang simply talks about an empty initializer > >>>>>> (during instantiation, etc, like we do), whereas EDG explicitly > >>>>>> says that pack expansion produces an empty list of expressions. I > >>>>>> don't think that in cp_finish_decl it would be easy for us to do > >>>>>> exactly the same, we simply see a NULL_TREE as second argument. > >>>>>> Or we could just *assume* that we are dealing with the outcome of > >>>>>> a pack expansion, say something like EDG even if we don't have > >>>>>> details beyond the fact that init == NULL_TREE. I believe that > >>>>>> without a variadic template the problem cannot occur, because we > >>>>>> catch the empty initializer much earlier, in grokdeclarator - > >>>>>> indeed using a !CLASS_PLACEHOLDER_TEMPLATE (auto_node) check. > >>>>>> What do you think? Again "instantiated for an empty pack" or > >>>>>> something similar? > >>>>> > >>>>> Perhaps we could complain in the code for empty pack expansion > >>>>> handling in tsubst_init? > >>>> > >>>> Ah, thanks Jason. In fact, however, tsubst_init isn't currently > >>>> involved at all, because, at the end of > >>>> regenerate_decl_from_template we call by hand tsubst_expr and > >>>> assign the result to DECL_INITIAL. Simply changing that avoids the > >>>> ICE. However, the error we issue - likewise for the existing > >>>> cpp0x/auto31.C - is the rather user-unfriendly > >>>> "value-initialization of incomplete type ‘auto’", as produced by > >>>> build_value_init. Thus a simple additional test along the lines > >>>> already discussed, which now becomes much more simple to implement > >>>> in a precise way. Again, wording only tentative. I'm also a little > >>>> puzzled that, otherwise, we could get away with tubst_expr instead > >>>> of tsubst_init... > >>> > >>>> + if (type_uses_auto (TREE_TYPE (decl))) > >>>> + { > >>>> + if (complain & tf_error) > >>>> + error ("initializer for %q#D expands to an empty list " > >>>> + "of expressions", decl); > >>>> + return error_mark_node; > >>>> + } > >>> > >>> This needs to allow the CLASS_PLACEHOLDER_TEMPLATE case. > >>> > >>> And yes, we mustn't call build_value_init for a dependent type; if > >>> the type is dependent, we should just return the NULL_TREE. > >> > >> Good. Then I'm finishing testing the below (currently in libstdc++). > > > >> + if (tree auto_node = type_uses_auto (type)) > >> + if (!CLASS_PLACEHOLDER_TEMPLATE (auto_node)) > >> + { > >> + if (complain & tf_error) > >> + error ("initializer for %q#D expands to an empty list " > >> + "of expressions", decl); > >> + return error_mark_node; > >> + } > >> + > >> + if (!dependent_type_p (type)) > > > > This should probably be 'else if', since we can have auto outside of a > > template and dependent_type_p will always return false outside of a > > template. > > Ok... Note that the CLASS_PLACEHOLDER_TEMPLATE (auto_node) check in > tsubst_init doesn't really make a difference for our testsuite and I > have yet to find a testcase where it does. In cp_finish_decl it did, for > sure. > > That said, if I understand correctly, you mean that in principle when we > have an 'auto' outside of a template and CLASS_PLACEHOLDER_TEMPLATE is > true, we don't want to do the build_value_init bits - which we would > certainly do because dependent_type_p is false. If we are on the same > page on this, the below regtested successfully. > > Thanks! Paolo. > > ////////////////////// >
Index: cp/decl.c =================================================================== --- cp/decl.c (revision 268968) +++ cp/decl.c (working copy) @@ -6946,7 +6946,15 @@ cp_finish_decl (tree decl, tree init, bool init_co { tree d_init; if (init == NULL_TREE) - gcc_assert (CLASS_PLACEHOLDER_TEMPLATE (auto_node)); + { + if (variable_template_specialization_p (decl)) + { + error ("initializer for %q#D is empty", decl); + TREE_TYPE (decl) = error_mark_node; + return; + } + gcc_assert (CLASS_PLACEHOLDER_TEMPLATE (auto_node)); + } d_init = init; if (d_init) { Index: testsuite/g++.dg/cpp1y/var-templ60.C =================================================================== --- testsuite/g++.dg/cpp1y/var-templ60.C (nonexistent) +++ testsuite/g++.dg/cpp1y/var-templ60.C (working copy) @@ -0,0 +1,9 @@ +// PR c++/84536 +// { dg-do compile { target c++14 } } + +template<int... N> auto foo(N...); // { dg-error "initializer" } + +void bar() +{ + foo<>(); +}