Patchwork PR c++/52343 - error with alias template as template template argument

login
register
mail settings
Submitter Dodji Seketeli
Date Dec. 21, 2012, 12:35 p.m.
Message ID <87a9t74m7m.fsf@redhat.com>
Download mbox | patch
Permalink /patch/207795/
State New
Headers show

Comments

Dodji Seketeli - Dec. 21, 2012, 12:35 p.m.
Hello,

In the example accompanying this patch, check_instantiated_arg tries
to ensure that a non-type template argument should be a constant if it
has integral or enumeration type.

The problem is that an alias template which type-id is, e.g, an
integer, looks like an argument that has integral/enumeration type:
its TREE_TYPE is an integer type.  So check_instantiated_arg
mistenkaly barks that this integral non-type argument is not a
constant.

I am proposing to tighten the test in check_instantiated_arg to allow
type template (and thus aliast template) arguments.

Bootstrapped and tested on x86_64-unknown-linux-gnu against trunk.

gcc/cp/

	PR c++/52343
	* pt.c (check_instantiated_arg): Allow type template arguments.

gcc/testsuite/

	PR c++/52343
	* g++.dg/cpp0x/alias-decl-29.C: New test.
---
 gcc/cp/pt.c                                |  4 +++-
 gcc/testsuite/g++.dg/cpp0x/alias-decl-29.C | 10 ++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/alias-decl-29.C
Gabriel Dos Reis - Dec. 21, 2012, 3:11 p.m.
The example is valid, but I am not sure I understand your explanation...

-- Gaby
Dodji Seketeli - Dec. 21, 2012, 4:25 p.m.
Gabriel Dos Reis <gdr@integrable-solutions.net> writes:

> The example is valid, but I am not sure I understand your
>  explanation...

Ah, sorry.  I realize just now that I haven't mentioned the initial
erratic behaviour.  Maybe that could have made my message easier to
understand.

So consider the test case of the message:

     1	template<typename>
     2	using A = int;
     3	
     4	template<template<class> class>
     5	struct B {};
     6	
     7	B<A> b;

test.cc:7:4: error: integral expression ‘A’ is not constant
 B<A> b;
    ^
Followed by some irrelevant other error messages.

As I was saying my earlier message, here, the TREE_TYPE of the
template_decl A is an integer; so check_instantiated_arg takes it as
if A is an integer value (a decl with integer type), and thus, should
be a constant.

The fix I am proposing is just to allow check_instantiated_arg to make
the difference between a classical integer decl, and an alias template
which type-id is an integer.
Gabriel Dos Reis - Dec. 21, 2012, 11:20 p.m.
On Fri, Dec 21, 2012 at 10:25 AM, Dodji Seketeli <dodji@redhat.com> wrote:
> Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
>
>> The example is valid, but I am not sure I understand your
>>  explanation...
>
> Ah, sorry.  I realize just now that I haven't mentioned the initial
> erratic behaviour.  Maybe that could have made my message easier to
> understand.
>
> So consider the test case of the message:
>
>      1  template<typename>
>      2  using A = int;
>      3
>      4  template<template<class> class>
>      5  struct B {};
>      6
>      7  B<A> b;
>
> test.cc:7:4: error: integral expression ‘A’ is not constant
>  B<A> b;
>     ^
> Followed by some irrelevant other error messages.
>
> As I was saying my earlier message, here, the TREE_TYPE of the
> template_decl A is an integer; so check_instantiated_arg takes it as
> if A is an integer value (a decl with integer type), and thus, should
> be a constant.
>
> The fix I am proposing is just to allow check_instantiated_arg to make
> the difference between a classical integer decl, and an alias template
> which type-id is an integer.
>
> --
>                 Dodji

Hi Dodji,

Thank you very much for the explanation; your previous message
makes sense to me now.  The question I have is why are we
using TREE_TYPE of a TEMPLATE_DECL to represent the
current instantiation of a template alias?  Should not we use
TEMPLATE_RESULT instead?

-- Gaby
Dodji Seketeli - Dec. 22, 2012, 3:53 p.m.
Gabriel Dos Reis <gdr@integrable-solutions.net> writes:

> Thank you very much for the explanation; your previous message
> makes sense to me now.

You are welcome.

> The question I have is why are we using TREE_TYPE of a TEMPLATE_DECL
> to represent the current instantiation of a template alias?

My understanding is that in the instantiation at line 7 below

     1	template<typename>
     2	using A = int;
     3	
     4	template<template<class> class>
     5	struct B {};
     6	
     7	B<A> b;

check_instantiated_arg is not looking at the current instantiation of
the template alias A.  Rather, it is looking at the template-name A
(which resolved, IMHO rightfully, to the decl for A which happens to be
a TEMPLATE_DECL).  This seems consistent with the fact that the
parameter of B is a template itself so its argument ought to be
template-name, rather than a template instantiation.
Gabriel Dos Reis - Dec. 22, 2012, 4:42 p.m.
On Sat, Dec 22, 2012 at 9:53 AM, Dodji Seketeli <dodji@redhat.com> wrote:
> Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
>
>> Thank you very much for the explanation; your previous message
>> makes sense to me now.
>
> You are welcome.
>
>> The question I have is why are we using TREE_TYPE of a TEMPLATE_DECL
>> to represent the current instantiation of a template alias?
>
> My understanding is that in the instantiation at line 7 below
>
>      1  template<typename>
>      2  using A = int;
>      3
>      4  template<template<class> class>
>      5  struct B {};
>      6
>      7  B<A> b;
>
> check_instantiated_arg is not looking at the current instantiation of
> the template alias A.  Rather, it is looking at the template-name A
> (which resolved, IMHO rightfully, to the decl for A which happens to be
> a TEMPLATE_DECL).  This seems consistent with the fact that the
> parameter of B is a template itself so its argument ought to be
> template-name, rather than a template instantiation.

Sorry for the confusion, "current instantiation" was the wrong word.
What I meant to say was that the use and interpretation of TREE_TYPE
is very confusing in this context.

-- Gaby
Jason Merrill - Dec. 24, 2012, 5:04 a.m.
On 12/21/2012 07:35 AM, Dodji Seketeli wrote:
>     else if (TREE_TYPE (t)
>   	   && INTEGRAL_OR_ENUMERATION_TYPE_P (TREE_TYPE (t))
> -	   && !TREE_CONSTANT (t))
> +	   && !TREE_CONSTANT (t)
> +	   /* Class template and alias template arguments should be OK.  */
> +	   && !DECL_TYPE_TEMPLATE_P (t))

Instead, let's add a previous else if to catch template template 
arguments (and do nothing) so that when we hit this else if, we know 
we're dealing with a non-type argument.

Jason
Gabriel Dos Reis - Dec. 24, 2012, 5:38 a.m.
On Sun, Dec 23, 2012 at 11:04 PM, Jason Merrill <jason@redhat.com> wrote:
> On 12/21/2012 07:35 AM, Dodji Seketeli wrote:
>>
>>     else if (TREE_TYPE (t)
>>            && INTEGRAL_OR_ENUMERATION_TYPE_P (TREE_TYPE (t))
>> -          && !TREE_CONSTANT (t))
>> +          && !TREE_CONSTANT (t)
>> +          /* Class template and alias template arguments should be OK.
>> */
>> +          && !DECL_TYPE_TEMPLATE_P (t))
>
>
> Instead, let's add a previous else if to catch template template arguments
> (and do nothing) so that when we hit this else if, we know we're dealing
> with a non-type argument.

Thanks; that would make the logic clearer.  I would suggest that we
abstract this series of conjunction into a separate (static inline)
function, e.g. nontype_argument_p.

-- Gaby
Dodji Seketeli - Dec. 24, 2012, 1:57 p.m.
Gabriel Dos Reis <gdr@integrable-solutions.net> writes:

G> On Sun, Dec 23, 2012 at 11:04 PM, Jason Merrill <jason@redhat.com> wrote:
>> On 12/21/2012 07:35 AM, Dodji Seketeli wrote:
>>>
>>>     else if (TREE_TYPE (t)
>>>            && INTEGRAL_OR_ENUMERATION_TYPE_P (TREE_TYPE (t))
>>> -          && !TREE_CONSTANT (t))
>>> +          && !TREE_CONSTANT (t)
>>> +          /* Class template and alias template arguments should be OK.
>>> */
>>> +          && !DECL_TYPE_TEMPLATE_P (t))
>>
>>
>> Instead, let's add a previous else if to catch template template arguments
>> (and do nothing) so that when we hit this else if, we know we're dealing
>> with a non-type argument.
>
> Thanks; that would make the logic clearer.  I would suggest that we
> abstract this series of conjunction into a separate (static inline)
> function, e.g. nontype_argument_p.

These conjunctions represents a non-type argument only if they are
satisfied /and/ the previous 'if' branches are not taken.  So just
putting the conjunctions in e.g, nontype_argument_p could be seen as
confusion too, IMHO.

Could we just consider that the patch + comment of my earlier message and the
comment of these conjunctions

   /* A non-type argument of integral or enumerated type must be a
      constant.  */

should make this less confusing?
Gabriel Dos Reis - Dec. 24, 2012, 8:18 p.m.
On Mon, Dec 24, 2012 at 7:57 AM, Dodji Seketeli <dodji@redhat.com> wrote:
> Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
>
> G> On Sun, Dec 23, 2012 at 11:04 PM, Jason Merrill <jason@redhat.com> wrote:
>>> On 12/21/2012 07:35 AM, Dodji Seketeli wrote:
>>>>
>>>>     else if (TREE_TYPE (t)
>>>>            && INTEGRAL_OR_ENUMERATION_TYPE_P (TREE_TYPE (t))
>>>> -          && !TREE_CONSTANT (t))
>>>> +          && !TREE_CONSTANT (t)
>>>> +          /* Class template and alias template arguments should be OK.
>>>> */
>>>> +          && !DECL_TYPE_TEMPLATE_P (t))
>>>
>>>
>>> Instead, let's add a previous else if to catch template template arguments
>>> (and do nothing) so that when we hit this else if, we know we're dealing
>>> with a non-type argument.
>>
>> Thanks; that would make the logic clearer.  I would suggest that we
>> abstract this series of conjunction into a separate (static inline)
>> function, e.g. nontype_argument_p.
>
> These conjunctions represents a non-type argument only if they are
> satisfied /and/ the previous 'if' branches are not taken.  So just
> putting the conjunctions in e.g, nontype_argument_p could be seen as
> confusion too, IMHO.
>
> Could we just consider that the patch + comment of my earlier message and the
> comment of these conjunctions
>
>    /* A non-type argument of integral or enumerated type must be a
>       constant.  */
>
> should make this less confusing?

I am not sure the suggestion would be confusing, but I am
inclined to do as you suggest.

I do believe though that we should aim for reducing the
size of most of our functions; there are many places where
we replicate verbatim logics that should be abstracted
into separate functions, increasing reuse and coherence,
and decreasing likelihood of bugs

Thanks!

-- Gaby

Patch

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 1b3f039..e2e8311 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -14425,7 +14425,9 @@  check_instantiated_arg (tree tmpl, tree t, tsubst_flags_t complain)
      constant.  */
   else if (TREE_TYPE (t)
 	   && INTEGRAL_OR_ENUMERATION_TYPE_P (TREE_TYPE (t))
-	   && !TREE_CONSTANT (t))
+	   && !TREE_CONSTANT (t)
+	   /* Class template and alias template arguments should be OK.  */
+	   && !DECL_TYPE_TEMPLATE_P (t))
     {
       if (complain & tf_error)
 	error ("integral expression %qE is not constant", t);
diff --git a/gcc/testsuite/g++.dg/cpp0x/alias-decl-29.C b/gcc/testsuite/g++.dg/cpp0x/alias-decl-29.C
new file mode 100644
index 0000000..f6cc695
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/alias-decl-29.C
@@ -0,0 +1,10 @@ 
+// Origin: PR c++/52343
+// { dg-do compile { target c++11 } }
+
+template<typename>
+using A = int;
+
+template<template<class> class>
+struct B {};
+
+B<A> b;