diff mbox

PR c++/55663 - constexpr function templ instantiation considered non-const as alias templ arg

Message ID 87k3rlrrkw.fsf@redhat.com
State New
Headers show

Commit Message

Dodji Seketeli Jan. 10, 2013, 3:22 p.m. UTC 
Gabriel Dos Reis <gdr@integrable-solutions.net> writes:

> I read your reply.  I am now even more puzzled
> than before.  The call to uses_template_parm indicates that
> we expect that code to work when are also when processing a template
> (e.g. for non-dependent cases inside a template.)
> That makes me wonder how it could possibly work for the
> cases at hand because for non-type template arguments we need
> full instantiation information to determine convertibility and
> "constant"ness.

I introduced the confusion, sorry.  I overlooked the fact that this is
actually happening while calling instantiate_alias_template on Alias
with the argument the_thruth<int> non folded.  In that code patch, we
didn't even go through convert_template_argument like what I was saying.
More on this below.

Jason Merrill <jason@redhat.com> writes:

> On 01/09/2013 10:02 AM, Dodji Seketeli wrote:
>> As the argument 'the_truth<T>()' we care about is type dependant,
>> uses_template_parms returns true and so convert_nontype_argument is
>> never called.
>
> Right, but we should call it for the instantiated argument, too.  We
> do that for class templates by calling lookup_template_class again,
> which calls coerce_template_parms.  We need to make sure we call
> coerce_template_parms when instantiating alias templates, too.

Indeed.

The problem is happening during the instantiation of test<int> using
instantiate_class_template.  In that case the argument {int} was
previously properly coerced and stored in CLASSTYPE_TI_ARGS by
lookup_class_template, as you are saying.  So instantiate_class_template
always uses coerced arguments.

But during the instantiation of the *members* of test<int>, we try to
instantiate Alias<the_truth<int>>, without coercing (and thus without
folding) the argument {the_truth<int>}.  We do this using
instantiate_alias_template, called from tsubst.

The patch below makes sure instantiate_alias_template coerces the
arguments it uses to instantiate the alias template, like what I
understood you are suggesting.  I have tested it without boostrap and a
full boostrap is currently running.

If this approach looks acceptable, could I replace (part of) the
template argument coercing code in lookup_class_template_1 with the new
coerce_template_parms_all_level I introduced in this patch?

Also, I am not sure if this patch would be appropriate for commit, now
that we are in 'regression-only' mode.  But seeing this alias-template
related thing not working hurts me.  :)  So at worst I'll schedule the
patch for when stage1 opens again.

Thanks.

gcc/cp/

	PR c++/55663
	* pt.c (coerce_template_parms_all_levels): New static function.
	(instantiate_alias_template):  Use it here.

gcc/testsuite/

	PR c++/55663
	* g++.dg/cpp0x/alias-decl-31.C: New test.
---
 gcc/cp/pt.c                                | 64 +++++++++++++++++++++++++++++-
 gcc/testsuite/g++.dg/cpp0x/alias-decl-31.C | 20 ++++++++++
 2 files changed, 83 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/alias-decl-31.C

Comments

Gabriel Dos Reis Jan. 10, 2013, 4:02 p.m. UTC | #1 
On Thu, Jan 10, 2013 at 9:22 AM, Dodji Seketeli <dodji@redhat.com> wrote:
>
> Also, I am not sure if this patch would be appropriate for commit, now
> that we are in 'regression-only' mode.  But seeing this alias-template
> related thing not working hurts me.  :)  So at worst I'll schedule the
> patch for when stage1 opens again.

It is certainly a blocker for some people here using both constexpr
and template alias.  (The code uses a technique documented in the
upcoming 4th edition of TC++PL which is due anytime now.  It would be
embarrassing if GCC-4.8 didn't accept it.)

-- Gaby
Gabriel Dos Reis Jan. 10, 2013, 4:11 p.m. UTC | #2 
On Thu, Jan 10, 2013 at 9:22 AM, Dodji Seketeli <dodji@redhat.com> wrote:

> But during the instantiation of the *members* of test<int>, we try to
> instantiate Alias<the_truth<int>>, without coercing (and thus without
> folding) the argument {the_truth<int>}.  We do this using
> instantiate_alias_template, called from tsubst.
>
> The patch below makes sure instantiate_alias_template coerces the
> arguments it uses to instantiate the alias template, like what I
> understood you are suggesting.  I have tested it without boostrap and a
> full boostrap is currently running.

Hmm, is it necessary to coerce all levels as opposed to just the
innermost arguments?

-- Gaby
Jason Merrill Jan. 10, 2013, 5 p.m. UTC | #3 
On 01/10/2013 11:11 AM, Gabriel Dos Reis wrote:
> Hmm, is it necessary to coerce all levels as opposed to just the
> innermost arguments?

No.  But if you read the code, it's really only coercing the innermost 
level.  Just the name is misleading.

> If this approach looks acceptable, could I replace (part of) the
> template argument coercing code in lookup_class_template_1 with the new
> coerce_template_parms_all_level I introduced in this patch?

Yes.

Jason
Dodji Seketeli Jan. 11, 2013, 10:41 a.m. UTC | #4 
Gabriel Dos Reis <gdr@integrable-solutions.net> writes:

> On Thu, Jan 10, 2013 at 9:22 AM, Dodji Seketeli <dodji@redhat.com> wrote:
>>
>> Also, I am not sure if this patch would be appropriate for commit, now
>> that we are in 'regression-only' mode.  But seeing this alias-template
>> related thing not working hurts me.  :)  So at worst I'll schedule the
>> patch for when stage1 opens again.
>
> It is certainly a blocker for some people here using both constexpr
> and template alias.  (The code uses a technique documented in the
> upcoming 4th edition of TC++PL which is due anytime now.  It would be
> embarrassing if GCC-4.8 didn't accept it.)

/me nods.

Hopefully when the review is done, we can ask our friendly release
managers to consider interceding for us in this matter.  :-)
Jakub Jelinek Jan. 11, 2013, 10:47 a.m. UTC | #5 
On Fri, Jan 11, 2013 at 11:41:34AM +0100, Dodji Seketeli wrote:
> Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
> 
> > On Thu, Jan 10, 2013 at 9:22 AM, Dodji Seketeli <dodji@redhat.com> wrote:
> >>
> >> Also, I am not sure if this patch would be appropriate for commit, now
> >> that we are in 'regression-only' mode.  But seeing this alias-template
> >> related thing not working hurts me.  :)  So at worst I'll schedule the
> >> patch for when stage1 opens again.
> >
> > It is certainly a blocker for some people here using both constexpr
> > and template alias.  (The code uses a technique documented in the
> > upcoming 4th edition of TC++PL which is due anytime now.  It would be
> > embarrassing if GCC-4.8 didn't accept it.)
> 
> /me nods.
> 
> Hopefully when the review is done, we can ask our friendly release
> managers to consider interceding for us in this matter.  :-)

This is ok for 4.8 from RM POV.

	Jakub
Dodji Seketeli Jan. 11, 2013, 11:31 a.m. UTC | #6 
Jakub Jelinek <jakub@redhat.com> writes:

> On Fri, Jan 11, 2013 at 11:41:34AM +0100, Dodji Seketeli wrote:
>> Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
>> 
>> > On Thu, Jan 10, 2013 at 9:22 AM, Dodji Seketeli <dodji@redhat.com> wrote:
>> >>
>> >> Also, I am not sure if this patch would be appropriate for commit, now
>> >> that we are in 'regression-only' mode.  But seeing this alias-template
>> >> related thing not working hurts me.  :)  So at worst I'll schedule the
>> >> patch for when stage1 opens again.
>> >
>> > It is certainly a blocker for some people here using both constexpr
>> > and template alias.  (The code uses a technique documented in the
>> > upcoming 4th edition of TC++PL which is due anytime now.  It would be
>> > embarrassing if GCC-4.8 didn't accept it.)
>> 
>> /me nods.
>> 
>> Hopefully when the review is done, we can ask our friendly release
>> managers to consider interceding for us in this matter.  :-)
>
> This is ok for 4.8 from RM POV.

Thank you!
diff mbox

Patch

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 30bafa0..aadc173 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -130,6 +130,8 @@  static tree tsubst_initializer_list (tree, tree);
 static tree get_class_bindings (tree, tree, tree, tree);
 static tree coerce_template_parms (tree, tree, tree, tsubst_flags_t,
 				   bool, bool);
+static tree coerce_template_parms_all_levels (tree, tree, tree, tsubst_flags_t,
+					      bool, bool);
 static void tsubst_enum	(tree, tree, tree);
 static tree add_to_template_args (tree, tree);
 static tree add_outermost_template_args (tree, tree);
@@ -6742,6 +6744,58 @@  coerce_template_parms (tree parms,
   return new_inner_args;
 }
 
+/* Like coerce_template_parms.  If PARMS represents all template
+   parameters levels, this function returns a vector of vectors
+   representing all the resulting argument levels.  Note that in this
+   case, only the innermost arguments are coerced because the
+   outermost ones are supposed to have been coerced already.
+
+   Otherwise, if PARMS represents only (the innermost) vectors of
+   parameters, returns a vector containing just the innermost
+   resulting arguments.  */
+
+static tree
+coerce_template_parms_all_levels (tree parms,
+				  tree args,
+				  tree in_decl,
+				  tsubst_flags_t complain,
+				  bool require_all_args,
+				  bool use_default_args)
+{
+  int parms_depth = TMPL_PARMS_DEPTH (parms);
+  int args_depth = TMPL_ARGS_DEPTH (args);
+  tree coerced_args;
+
+  if (parms_depth > 1)
+    {
+      coerced_args = make_tree_vec (parms_depth);
+      tree level;
+      int cur_depth;
+
+      for (level = parms, cur_depth = parms_depth;
+	   parms_depth > 0 && level != NULL_TREE;
+	   level = TREE_CHAIN (level), --cur_depth)
+	{
+	  tree l;
+	  if (cur_depth == args_depth)
+	    l = coerce_template_parms (TREE_VALUE (level),
+				       args, in_decl, complain,
+				       require_all_args,
+				       use_default_args);
+	  else
+	    l = TMPL_ARGS_LEVEL (args, cur_depth);
+
+	  SET_TMPL_ARGS_LEVEL (coerced_args, cur_depth, l);
+	}
+    }
+  else
+    coerced_args = coerce_template_parms (INNERMOST_TEMPLATE_PARMS (parms),
+					  args, in_decl, complain,
+					  require_all_args,
+					  use_default_args);
+  return coerced_args;
+}
+
 /* Returns 1 if template args OT and NT are equivalent.  */
 
 static int
@@ -14637,7 +14691,15 @@  instantiate_alias_template (tree tmpl, tree args, tsubst_flags_t complain)
       ggc_free (tinst);
       return error_mark_node;
     }
-  tree r = instantiate_template (tmpl, args, complain);
+
+  tree coerced_args =
+    coerce_template_parms_all_levels (DECL_TEMPLATE_PARMS (tmpl),
+				      args, tmpl, complain,
+				      /*require_all_args=*/true,
+				      /*use_default_args=*/true);
+
+  tree r = instantiate_template (tmpl, coerced_args, complain);
+
   pop_tinst_level ();
   /* We can't free this if a pending_template entry or last_error_tinst_level
      is pointing at it.  */
diff --git a/gcc/testsuite/g++.dg/cpp0x/alias-decl-31.C b/gcc/testsuite/g++.dg/cpp0x/alias-decl-31.C
new file mode 100644
index 0000000..83eea47
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/alias-decl-31.C
@@ -0,0 +1,20 @@ 
+// Origin: PR c++/55663
+// { dg-do compile { target c++11 } }
+
+template <typename>
+constexpr bool the_truth () { return true; }
+
+template <bool>
+  struct Takes_bool { };
+
+template<bool B>
+  using Alias = Takes_bool<B>;
+
+template<typename T>
+  struct test { using type = Alias<the_truth<T>()>; };
+
+int main () {
+  test<int> a;
+
+  return 0;
+}