Re: [PATCH] PR c++/55663 - constexpr function templ instantiation considered non-const as alias templ arg
OK. Jason
Re: [PATCH] PR c++/55663 - constexpr function templ instantiation considered non-const as alias templ arg
Gabriel Dos Reis g...@integrable-solutions.net writes:
On Thu, Jan 10, 2013 at 9:22 AM, Dodji Seketeli do...@redhat.com wrote:
But during the instantiation of the *members* of testint, we try to
instantiate Aliasthe_truthint, without coercing (and thus without
folding) the argument {the_truthint}. 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 ja...@redhat.com writes:
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.
Correct. I even documented that in the descriptive comment of the
function. But ...
Just the name is misleading.
... as the name seems to be confusing, I have changed it to
coerce_innermost_template_parms. I hope that is less confusing.
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.
OK. I have done that in the patch below that passed bootstrap and
regression testing on x86_64-unknown-linux-gnu against trunk.
If nothing else, I'd need your opinion on this; in the template argument
coercing code in lookup_template_1 does this:
/* We temporarily reduce the length of the ARGLIST so
that coerce_template_parms will see only the arguments
corresponding to the template parameters it is
examining. */
TREE_VEC_LENGTH (arglist)--;
but when I read the code, it looks like this is not necessary. Am I
missing something? In any case, I haven't put that code in the new
coerce_innermost_template_parms. Is that OK?
Thanks.
gcc/cp/
PR c++/55663
* pt.c (coerce_innermost_template_parms): New static function.
(instantiate_alias_template): Use it here.
(lookup_template_class_1): Use it here too, for ease of
maintenance's sake.
gcc/testsuite/
PR c++/55663
* g++.dg/cpp0x/alias-decl-31.C: New test.
---
gcc/cp/pt.c| 130 -
gcc/testsuite/g++.dg/cpp0x/alias-decl-31.C | 20 +
2 files changed, 93 insertions(+), 57 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/alias-decl-31.C
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 30bafa0..67e6c97 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_innermost_template_parms (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,61 @@ 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) vector of
+ parameters, this function returns a vector containing just the
+ innermost resulting arguments. */
+
+static tree
+coerce_innermost_template_parms (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,
+
Re: [PATCH] PR c++/55663 - constexpr function templ instantiation considered non-const as alias templ arg
Gabriel Dos Reis g...@integrable-solutions.net writes: On Thu, Jan 10, 2013 at 9:22 AM, Dodji Seketeli do...@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. :-) -- Dodji
Re: [PATCH] PR c++/55663 - constexpr function templ instantiation considered non-const as alias templ arg
On Fri, Jan 11, 2013 at 11:41:34AM +0100, Dodji Seketeli wrote: Gabriel Dos Reis g...@integrable-solutions.net writes: On Thu, Jan 10, 2013 at 9:22 AM, Dodji Seketeli do...@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
Re: [PATCH] PR c++/55663 - constexpr function templ instantiation considered non-const as alias templ arg
Jakub Jelinek ja...@redhat.com writes: On Fri, Jan 11, 2013 at 11:41:34AM +0100, Dodji Seketeli wrote: Gabriel Dos Reis g...@integrable-solutions.net writes: On Thu, Jan 10, 2013 at 9:22 AM, Dodji Seketeli do...@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! -- Dodji
Re: [PATCH] PR c++/55663 - constexpr function templ instantiation considered non-const as alias templ arg
Jason Merrill ja...@redhat.com writes:
On 01/11/2013 05:38 AM, Dodji Seketeli wrote:
but when I read the code, it looks like this is not necessary. Am I
missing something? In any case, I haven't put that code in the new
coerce_innermost_template_parms. Is that OK?
I agree that it seems unnecessary. But to be safe, let's leave
lookup_template_class_1 alone until after 4.8 branches.
OK, here are the patches I am proposing then. The first one is the one
I'd like to commit to 4.8 and the second one would be scheduled for 4.9
when its branch opens.
Is that OK?
gcc/cp/
PR c++/55663
* pt.c (coerce_innermost_template_parms): 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, 84 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/alias-decl-31.C
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 6d78dd2..62fa2d9 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_innermost_template_parms (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,61 @@ 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) vector of
+ parameters, this function returns a vector containing just the
+ innermost resulting arguments. */
+
+static tree
+coerce_innermost_template_parms (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);
+
+ if (l == error_mark_node)
+ return error_mark_node;
+
+ 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
@@ -14642,6 +14699,13 @@ instantiate_alias_template (tree tmpl, tree args,
tsubst_flags_t complain)
ggc_free (tinst);
return error_mark_node;
}
+
+ args =
+coerce_innermost_template_parms (DECL_TEMPLATE_PARMS (tmpl),
+args, tmpl, complain,
+/*require_all_args=*/true,
+/*use_default_args=*/true);
+
tree r = instantiate_template (tmpl, args, complain);
pop_tinst_level ();
/* We can't free this if a pending_template entry or last_error_tinst_level
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 000..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 { };
+
+templatebool B
+ using Alias = Takes_boolB;
+
+templatetypename T
+ struct test {
Re: [PATCH] PR c++/55663 - constexpr function templ instantiation considered non-const as alias templ arg
Gabriel Dos Reis g...@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
constantness.
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_thruthint 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 ja...@redhat.com writes:
On 01/09/2013 10:02 AM, Dodji Seketeli wrote:
As the argument 'the_truthT()' 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 testint 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 testint, we try to
instantiate Aliasthe_truthint, without coercing (and thus without
folding) the argument {the_truthint}. 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
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)
+ {
+
Re: [PATCH] PR c++/55663 - constexpr function templ instantiation considered non-const as alias templ arg
On Thu, Jan 10, 2013 at 9:22 AM, Dodji Seketeli do...@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
Re: [PATCH] PR c++/55663 - constexpr function templ instantiation considered non-const as alias templ arg
On Thu, Jan 10, 2013 at 9:22 AM, Dodji Seketeli do...@redhat.com wrote:
But during the instantiation of the *members* of testint, we try to
instantiate Aliasthe_truthint, without coercing (and thus without
folding) the argument {the_truthint}. 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
Re: [PATCH] PR c++/55663 - constexpr function templ instantiation considered non-const as alias templ arg
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
Re: [PATCH] PR c++/55663 - constexpr function templ instantiation considered non-const as alias templ arg
Jason Merrill ja...@redhat.com writes: On 01/08/2013 08:58 AM, Dodji Seketeli wrote: There, when we check the argument 'the_truthint()' to see if it actually is a constant expression, in check_instantiated_arg, we fail to recognize its constexpr-ness b/c we just look at its TREE_CONSTANT. The problem is that by the time we get to check_instantiated_arg, we should have folded the expression into something TREE_CONSTANT. convert_template_argument should have done that; don't we ever call that function for this template argument? Presumably, you mean that convert_template_argument should call convert_nontype_argument to do that folding, right? I guess the reason why it's not doing it is that the call to convert_nontype_argument is conditional on else if (!uses_template_parms (orig_arg) !uses_template_parms (t)) /* We used to call digest_init here. However, digest_init will report errors, which we don't want when complain is zero. More importantly, digest_init will try too hard to convert things: for example, `0' should not be converted to pointer type at this point according to the standard. Accepting this is not merely an extension, since deciding whether or not these conversions can occur is part of determining which function template to call, or whether a given explicit argument specification is valid. */ val = convert_nontype_argument (t, orig_arg, complain); As the argument 'the_truthT()' we care about is type dependant, uses_template_parms returns true and so convert_nontype_argument is never called. What is your preferred way want to handle this? -- Dodji
Re: [PATCH] PR c++/55663 - constexpr function templ instantiation considered non-const as alias templ arg
Gabriel Dos Reis g...@integrable-solutions.net writes: We already have various predicates to test for constant expressions so I am uneasy to add yet another one. I understand. I got lost in the number of existing predicates to test for constant expressions, to the point that I thought (wrongly) the one I wanted wasn't present. :-) I think reduced_constant_expression_p is what you want. Thanks. I didn't realize this would work because the comment of initializer_constant_valid_p (that it uses) says: We assume that VALUE has been folded as much as possible On a side node, as Jason said in the thread, we might not even need to touch anything here, as check_instantiated_arg also assumes that its arg has been fully folded. I guess I'll propose to update the comment of that function to reflect that assumption. Thanks. -- Dodji
Re: [PATCH] PR c++/55663 - constexpr function templ instantiation considered non-const as alias templ arg
On Wed, Jan 9, 2013 at 9:30 AM, Dodji Seketeli do...@redhat.com wrote: Gabriel Dos Reis g...@integrable-solutions.net writes: We already have various predicates to test for constant expressions so I am uneasy to add yet another one. I understand. I got lost in the number of existing predicates to test for constant expressions, so am I :-) to the point that I thought (wrongly) the one I wanted wasn't present. :-) I think reduced_constant_expression_p is what you want. Thanks. I didn't realize this would work because the comment of initializer_constant_valid_p (that it uses) says: We assume that VALUE has been folded as much as possible On a side node, as Jason said in the thread, we might not even need to touch anything here, as check_instantiated_arg also assumes that its arg has been fully folded. I guess I'll propose to update the comment of that function to reflect that assumption. 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 constantness. -- Gaby
Re: [PATCH] PR c++/55663 - constexpr function templ instantiation considered non-const as alias templ arg
On 01/09/2013 10:02 AM, Dodji Seketeli wrote: As the argument 'the_truthT()' 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. Jason
Re: [PATCH] PR c++/55663 - constexpr function templ instantiation considered non-const as alias templ arg
On Tue, Jan 8, 2013 at 7:58 AM, Dodji Seketeli do...@redhat.com wrote:
Hello,
Consider the example of the problem report
1 template typename
2 constexpr bool the_truth () { return true; }
3
4 template bool
5struct Takes_bool { };
6
7 templatebool B
8using Alias = Takes_boolB;
9
10 templatetypename T
11struct test { using type = Aliasthe_truthT(); };
12
13 int main () {
14testint a;
15
16return 0;
17 }
that yields the error:
test.cc: In substitution of ‘templatebool B using Alias = Takes_boolB
[with bool B = the_truthint()]’:
test.cc:11:51: required from ‘struct testint’
test.cc:14:13: required from here
test.cc:11:51: error: integral expression ‘the_truthint()’ is not
constant
struct test { using type = Aliasthe_truthT(); };
I think the issue happens in the course of instantiating testint at
line 14, when we look into instantiating Aliasthe_truthT() (at
line 11), with T = int.
There, when we check the argument 'the_truthint()' to see if it
actually is a constant expression, in check_instantiated_arg, we fail
to recognize its constexpr-ness b/c we just look at its TREE_CONSTANT.
Thanks for the detective work!
We already have various predicates to test for constant
expressions so I am uneasy to add yet another one.
What we do no need -- which I already suggested -- is a
predicate to test valid non-type template arguments.
For example, we already have the predicate verify_constant
and reduced_constant_expression_p, require_potential_constant_expression.
I think reduced_constant_expression_p is what you want.
-- Gaby
Re: [PATCH] PR c++/55663 - constexpr function templ instantiation considered non-const as alias templ arg
On 01/08/2013 08:58 AM, Dodji Seketeli wrote: There, when we check the argument 'the_truthint()' to see if it actually is a constant expression, in check_instantiated_arg, we fail to recognize its constexpr-ness b/c we just look at its TREE_CONSTANT. The problem is that by the time we get to check_instantiated_arg, we should have folded the expression into something TREE_CONSTANT. convert_template_argument should have done that; don't we ever call that function for this template argument? Jason
