Re: [PATCH] c++: Fix ICE in tsubst_default_argument [PR92010]
On Tue, 7 Apr 2020, Jason Merrill wrote: > On 4/7/20 1:40 PM, Patrick Palka wrote: > > On Mon, 6 Apr 2020, Jason Merrill wrote: > > > On 4/6/20 11:45 AM, Patrick Palka wrote: > > > > On Wed, 1 Apr 2020, Jason Merrill wrote: > > > > > > > > > On 4/1/20 6:29 PM, Jason Merrill wrote: > > > > > > On 3/31/20 3:50 PM, Patrick Palka wrote: > > > > > > > On Tue, 31 Mar 2020, Jason Merrill wrote: > > > > > > > > > > > > > > > On 3/30/20 6:46 PM, Patrick Palka wrote: > > > > > > > > > On Mon, 30 Mar 2020, Jason Merrill wrote: > > > > > > > > > > On 3/30/20 3:58 PM, Patrick Palka wrote: > > > > > > > > > > > On Thu, 26 Mar 2020, Jason Merrill wrote: > > > > > > > > > > > > > > > > > > > > > > > On 3/22/20 9:21 PM, Patrick Palka wrote: > > > > > > > > > > > > > This patch relaxes an assertion in > > > > > > > > > > > > > tsubst_default_argument > > > > > > > > > > > > > that > > > > > > > > > > > > > exposes > > > > > > > > > > > > > a > > > > > > > > > > > > > latent > > > > > > > > > > > > > bug in how we substitute an array type into a > > > > > > > > > > > > > cv-qualified > > > > > > > > > > > > > wildcard > > > > > > > > > > > > > function > > > > > > > > > > > > > parameter type. Concretely, the latent bug is that > > > > > > > > > > > > > given > > > > > > > > > > > > > the > > > > > > > > > > > > > function > > > > > > > > > > > > > template > > > > > > > > > > > > > > > > > > > > > > > > > > template void foo(const T t); > > > > > > > > > > > > > > > > > > > > > > > > > > one would expect the type of foo to be > > > > > > > > > > > > > void(const > > > > > > > > > > > > > int*), but > > > > > > > > > > > > > we > > > > > > > > > > > > > (seemingly prematurely) strip function parameter types > > > > > > > > > > > > > of > > > > > > > > > > > > > their > > > > > > > > > > > > > top-level > > > > > > > > > > > > > cv-qualifiers when building the function's > > > > > > > > > > > > > TYPE_ARG_TYPES, > > > > > > > > > > > > > and > > > > > > > > > > > > > instead > > > > > > > > > > > > > end > > > > > > > > > > > > > up > > > > > > > > > > > > > obtaining void(int*) as the type of foo after > > > > > > > > > > > > > substitution > > > > > > > > > > > > > and > > > > > > > > > > > > > decaying. > > > > > > > > > > > > > > > > > > > > > > > > > > We still however correctly substitute into and decay > > > > > > > > > > > > > the > > > > > > > > > > > > > formal > > > > > > > > > > > > > parameter > > > > > > > > > > > > > type, > > > > > > > > > > > > > obtaining const int* as the type of t after > > > > > > > > > > > > > substitution. > > > > > > > > > > > > > But > > > > > > > > > > > > > this > > > > > > > > > > > > > then > > > > > > > > > > > > > leads > > > > > > > > > > > > > to us tripping over the assert in > > > > > > > > > > > > > tsubst_default_argument > > > > > > > > > > > > > that > > > > > > > > > > > > > verifies > > > > > > > > > > > > > the > > > > > > > > > > > > > formal parameter type and the function type are > > > > > > > > > > > > > consistent. > > > > > > > > > > > > > > > > > > > > > > > > > > Assuming it's too late at this stage to fix the > > > > > > > > > > > > > substitution > > > > > > > > > > > > > bug, we > > > > > > > > > > > > > can > > > > > > > > > > > > > still > > > > > > > > > > > > > relax the assertion like so. Tested on > > > > > > > > > > > > > x86_64-pc-linux-gnu, > > > > > > > > > > > > > does > > > > > > > > > > > > > this > > > > > > > > > > > > > look > > > > > > > > > > > > > OK? > > > > > > > > > > > > > > > > > > > > > > > > This is core issues 1001/1322, which have not been > > > > > > > > > > > > resolved. > > > > > > > > > > > > Clang > > > > > > > > > > > > does > > > > > > > > > > > > the > > > > > > > > > > > > substitution the way you suggest; EDG rejects the > > > > > > > > > > > > testcase > > > > > > > > > > > > because the > > > > > > > > > > > > two > > > > > > > > > > > > substitutions produce different results. I think it > > > > > > > > > > > > would > > > > > > > > > > > > make > > > > > > > > > > > > sense > > > > > > > > > > > > to > > > > > > > > > > > > follow the EDG behavior until this issue is actually > > > > > > > > > > > > resolved. > > > > > > > > > > > > > > > > > > > > > > Here is what I have so far towards that end. When > > > > > > > > > > > substituting > > > > > > > > > > > into the > > > > > > > > > > > PARM_DECLs of a function decl, we now additionally check > > > > > > > > > > > if > > > > > > > > > > > the > > > > > > > > > > > aforementioned Core issues are relevant and issue a > > > > > > > > > > > (fatal) > > > > > > > > > > > diagnostic > > > > > > > > > > > if so. This patch checks this in tsubst_decl > > > > > > > > > > PARM_DECL> > > > > > > > > > > > rather > > > > > > > > > > > than in tsubst_function_decl for efficiency reasons, so > > > > > > > > > > > that > > > > > > > > > > > we > > > > > > > > > > > don't > > > > > > > > > > > have to perform another traversal over the DECL_ARGUMENTS > > >
Re: [PATCH] c++: Fix ICE in tsubst_default_argument [PR92010]
On 4/7/20 1:40 PM, Patrick Palka wrote:
On Mon, 6 Apr 2020, Jason Merrill wrote:
On 4/6/20 11:45 AM, Patrick Palka wrote:
On Wed, 1 Apr 2020, Jason Merrill wrote:
On 4/1/20 6:29 PM, Jason Merrill wrote:
On 3/31/20 3:50 PM, Patrick Palka wrote:
On Tue, 31 Mar 2020, Jason Merrill wrote:
On 3/30/20 6:46 PM, Patrick Palka wrote:
On Mon, 30 Mar 2020, Jason Merrill wrote:
On 3/30/20 3:58 PM, Patrick Palka wrote:
On Thu, 26 Mar 2020, Jason Merrill wrote:
On 3/22/20 9:21 PM, Patrick Palka wrote:
This patch relaxes an assertion in tsubst_default_argument
that
exposes
a
latent
bug in how we substitute an array type into a cv-qualified
wildcard
function
parameter type. Concretely, the latent bug is that given
the
function
template
template void foo(const T t);
one would expect the type of foo to be void(const
int*), but
we
(seemingly prematurely) strip function parameter types of
their
top-level
cv-qualifiers when building the function's TYPE_ARG_TYPES,
and
instead
end
up
obtaining void(int*) as the type of foo after
substitution
and
decaying.
We still however correctly substitute into and decay the
formal
parameter
type,
obtaining const int* as the type of t after substitution.
But
this
then
leads
to us tripping over the assert in tsubst_default_argument
that
verifies
the
formal parameter type and the function type are
consistent.
Assuming it's too late at this stage to fix the
substitution
bug, we
can
still
relax the assertion like so. Tested on
x86_64-pc-linux-gnu,
does
this
look
OK?
This is core issues 1001/1322, which have not been resolved.
Clang
does
the
substitution the way you suggest; EDG rejects the testcase
because the
two
substitutions produce different results. I think it would
make
sense
to
follow the EDG behavior until this issue is actually
resolved.
Here is what I have so far towards that end. When
substituting
into the
PARM_DECLs of a function decl, we now additionally check if
the
aforementioned Core issues are relevant and issue a (fatal)
diagnostic
if so. This patch checks this in tsubst_decl
rather
than in tsubst_function_decl for efficiency reasons, so that
we
don't
have to perform another traversal over the DECL_ARGUMENTS /
TYPE_ARG_TYPES just to implement this check.
Hmm, this seems like writing more complicated code for a very
marginal
optimization; how many function templates have so many
parameters
that
walking
over them once to compare types will have any effect on compile
time?
Good point... though I just tried implementing this check in
tsubst_function_decl, and it seems it might be just as complicated
to
implement it there instead, at least if we want to handle function
parameter packs correctly.
If we were to implement this check in tsubst_function_decl, then
since
we have access to the instantiated function, it would presumably
suffice
to compare its substituted DECL_ARGUMENTS with its substituted
TYPE_ARG_TYPES to see if they're consistent. Doing so would
certainly
catch the original testcase, i.e.
template
void foo(const T);
int main() { foo(0); }
because the DECL_ARGUMENTS of foo would be {const int*} and
its
TYPE_ARG_TYPES would be {int*}. But apparently it doesn't catch
the
corresponding testcase that uses a function parameter pack, i.e.
template
void foo(const Ts...);
int main() { foo(0); }
because it turns out we don't strip top-level cv-qualifiers from
function parameter packs from TYPE_ARG_TYPES at declaration time,
as
we
do with regular function parameters. So in this second testcase
both
DECL_ARGUMENTS and TYPE_ARG_TYPES of foo would be {const
int*},
and yet we would (presumably) want to reject this instantiation
too.
So it seems comparing TYPE_ARG_TYPES and DECL_ARGUMENTS from
tsubst_function_decl would not suffice, and we would still need to
do
a
variant of the trick that's done in this patch, i.e. substitute
into
each dependent parameter type stripped of its top-level
cv-qualifiers,
to see if these cv-qualifiers make a material difference in the
resulting function type. Or maybe there's yet another way to
detect
this?
I think let's go ahead with comparing TYPE_ARG_TYPES and
DECL_ARGUMENTS;
the
problem comes when they disagree. If we're handling pack expansions
wrong,
that's a separate issue.
Hm, comparing TYPE_ARG_TYPES and DECL_ARGUMENTS for compatibility
seems
to be exposing a latent bug with how we handle lambdas that appear in
function parameter types. Take g++.dg/cpp2a/lambda-uneval3.C for
example:
template void spam(decltype([]{}) (*s)[sizeof(T)]) {}
int main() { spam(nullptr); }
According to tsubst_function_decl in current trunk, the type of the
function paremeter 's' of spam according to its TYPE_ARG_TYPES
is
struct ._anon_4[1] *
and according to its DECL_ARGUMENTS the type of 's' is
struct ._anon_5[1] *
The disagreement happens because we call tsubst_lambda_expr twice
during
substitution and thereby generate two distinct lambda
Re: [PATCH] c++: Fix ICE in tsubst_default_argument [PR92010]
On Tue, 7 Apr 2020, Patrick Palka wrote: > On Mon, 6 Apr 2020, Jason Merrill wrote: > > On 4/6/20 11:45 AM, Patrick Palka wrote: > > > On Wed, 1 Apr 2020, Jason Merrill wrote: > > > > > > > On 4/1/20 6:29 PM, Jason Merrill wrote: > > > > > On 3/31/20 3:50 PM, Patrick Palka wrote: > > > > > > On Tue, 31 Mar 2020, Jason Merrill wrote: > > > > > > > > > > > > > On 3/30/20 6:46 PM, Patrick Palka wrote: > > > > > > > > On Mon, 30 Mar 2020, Jason Merrill wrote: > > > > > > > > > On 3/30/20 3:58 PM, Patrick Palka wrote: > > > > > > > > > > On Thu, 26 Mar 2020, Jason Merrill wrote: > > > > > > > > > > > > > > > > > > > > > On 3/22/20 9:21 PM, Patrick Palka wrote: > > > > > > > > > > > > This patch relaxes an assertion in > > > > > > > > > > > > tsubst_default_argument > > > > > > > > > > > > that > > > > > > > > > > > > exposes > > > > > > > > > > > > a > > > > > > > > > > > > latent > > > > > > > > > > > > bug in how we substitute an array type into a > > > > > > > > > > > > cv-qualified > > > > > > > > > > > > wildcard > > > > > > > > > > > > function > > > > > > > > > > > > parameter type. Concretely, the latent bug is that > > > > > > > > > > > > given > > > > > > > > > > > > the > > > > > > > > > > > > function > > > > > > > > > > > > template > > > > > > > > > > > > > > > > > > > > > > > > template void foo(const T t); > > > > > > > > > > > > > > > > > > > > > > > > one would expect the type of foo to be void(const > > > > > > > > > > > > int*), but > > > > > > > > > > > > we > > > > > > > > > > > > (seemingly prematurely) strip function parameter types > > > > > > > > > > > > of > > > > > > > > > > > > their > > > > > > > > > > > > top-level > > > > > > > > > > > > cv-qualifiers when building the function's > > > > > > > > > > > > TYPE_ARG_TYPES, > > > > > > > > > > > > and > > > > > > > > > > > > instead > > > > > > > > > > > > end > > > > > > > > > > > > up > > > > > > > > > > > > obtaining void(int*) as the type of foo after > > > > > > > > > > > > substitution > > > > > > > > > > > > and > > > > > > > > > > > > decaying. > > > > > > > > > > > > > > > > > > > > > > > > We still however correctly substitute into and decay the > > > > > > > > > > > > formal > > > > > > > > > > > > parameter > > > > > > > > > > > > type, > > > > > > > > > > > > obtaining const int* as the type of t after > > > > > > > > > > > > substitution. > > > > > > > > > > > > But > > > > > > > > > > > > this > > > > > > > > > > > > then > > > > > > > > > > > > leads > > > > > > > > > > > > to us tripping over the assert in > > > > > > > > > > > > tsubst_default_argument > > > > > > > > > > > > that > > > > > > > > > > > > verifies > > > > > > > > > > > > the > > > > > > > > > > > > formal parameter type and the function type are > > > > > > > > > > > > consistent. > > > > > > > > > > > > > > > > > > > > > > > > Assuming it's too late at this stage to fix the > > > > > > > > > > > > substitution > > > > > > > > > > > > bug, we > > > > > > > > > > > > can > > > > > > > > > > > > still > > > > > > > > > > > > relax the assertion like so. Tested on > > > > > > > > > > > > x86_64-pc-linux-gnu, > > > > > > > > > > > > does > > > > > > > > > > > > this > > > > > > > > > > > > look > > > > > > > > > > > > OK? > > > > > > > > > > > > > > > > > > > > > > This is core issues 1001/1322, which have not been > > > > > > > > > > > resolved. > > > > > > > > > > > Clang > > > > > > > > > > > does > > > > > > > > > > > the > > > > > > > > > > > substitution the way you suggest; EDG rejects the testcase > > > > > > > > > > > because the > > > > > > > > > > > two > > > > > > > > > > > substitutions produce different results. I think it would > > > > > > > > > > > make > > > > > > > > > > > sense > > > > > > > > > > > to > > > > > > > > > > > follow the EDG behavior until this issue is actually > > > > > > > > > > > resolved. > > > > > > > > > > > > > > > > > > > > Here is what I have so far towards that end. When > > > > > > > > > > substituting > > > > > > > > > > into the > > > > > > > > > > PARM_DECLs of a function decl, we now additionally check if > > > > > > > > > > the > > > > > > > > > > aforementioned Core issues are relevant and issue a (fatal) > > > > > > > > > > diagnostic > > > > > > > > > > if so. This patch checks this in tsubst_decl > > > > > > > > > PARM_DECL> > > > > > > > > > > rather > > > > > > > > > > than in tsubst_function_decl for efficiency reasons, so that > > > > > > > > > > we > > > > > > > > > > don't > > > > > > > > > > have to perform another traversal over the DECL_ARGUMENTS / > > > > > > > > > > TYPE_ARG_TYPES just to implement this check. > > > > > > > > > > > > > > > > > > Hmm, this seems like writing more complicated code for a very > > > > > > > > > marginal > > > > > > > > > optimization; how many function templates have so many > > > > > > > > > parameters > > > > > > > > > that > > > > > > > > > walking > > > > > > > > > over them once to compare types will have any effect on > >
Re: [PATCH] c++: Fix ICE in tsubst_default_argument [PR92010]
On Mon, 6 Apr 2020, Jason Merrill wrote: > On 4/6/20 11:45 AM, Patrick Palka wrote: > > On Wed, 1 Apr 2020, Jason Merrill wrote: > > > > > On 4/1/20 6:29 PM, Jason Merrill wrote: > > > > On 3/31/20 3:50 PM, Patrick Palka wrote: > > > > > On Tue, 31 Mar 2020, Jason Merrill wrote: > > > > > > > > > > > On 3/30/20 6:46 PM, Patrick Palka wrote: > > > > > > > On Mon, 30 Mar 2020, Jason Merrill wrote: > > > > > > > > On 3/30/20 3:58 PM, Patrick Palka wrote: > > > > > > > > > On Thu, 26 Mar 2020, Jason Merrill wrote: > > > > > > > > > > > > > > > > > > > On 3/22/20 9:21 PM, Patrick Palka wrote: > > > > > > > > > > > This patch relaxes an assertion in tsubst_default_argument > > > > > > > > > > > that > > > > > > > > > > > exposes > > > > > > > > > > > a > > > > > > > > > > > latent > > > > > > > > > > > bug in how we substitute an array type into a cv-qualified > > > > > > > > > > > wildcard > > > > > > > > > > > function > > > > > > > > > > > parameter type. Concretely, the latent bug is that given > > > > > > > > > > > the > > > > > > > > > > > function > > > > > > > > > > > template > > > > > > > > > > > > > > > > > > > > > > template void foo(const T t); > > > > > > > > > > > > > > > > > > > > > > one would expect the type of foo to be void(const > > > > > > > > > > > int*), but > > > > > > > > > > > we > > > > > > > > > > > (seemingly prematurely) strip function parameter types of > > > > > > > > > > > their > > > > > > > > > > > top-level > > > > > > > > > > > cv-qualifiers when building the function's TYPE_ARG_TYPES, > > > > > > > > > > > and > > > > > > > > > > > instead > > > > > > > > > > > end > > > > > > > > > > > up > > > > > > > > > > > obtaining void(int*) as the type of foo after > > > > > > > > > > > substitution > > > > > > > > > > > and > > > > > > > > > > > decaying. > > > > > > > > > > > > > > > > > > > > > > We still however correctly substitute into and decay the > > > > > > > > > > > formal > > > > > > > > > > > parameter > > > > > > > > > > > type, > > > > > > > > > > > obtaining const int* as the type of t after substitution. > > > > > > > > > > > But > > > > > > > > > > > this > > > > > > > > > > > then > > > > > > > > > > > leads > > > > > > > > > > > to us tripping over the assert in tsubst_default_argument > > > > > > > > > > > that > > > > > > > > > > > verifies > > > > > > > > > > > the > > > > > > > > > > > formal parameter type and the function type are > > > > > > > > > > > consistent. > > > > > > > > > > > > > > > > > > > > > > Assuming it's too late at this stage to fix the > > > > > > > > > > > substitution > > > > > > > > > > > bug, we > > > > > > > > > > > can > > > > > > > > > > > still > > > > > > > > > > > relax the assertion like so. Tested on > > > > > > > > > > > x86_64-pc-linux-gnu, > > > > > > > > > > > does > > > > > > > > > > > this > > > > > > > > > > > look > > > > > > > > > > > OK? > > > > > > > > > > > > > > > > > > > > This is core issues 1001/1322, which have not been resolved. > > > > > > > > > > Clang > > > > > > > > > > does > > > > > > > > > > the > > > > > > > > > > substitution the way you suggest; EDG rejects the testcase > > > > > > > > > > because the > > > > > > > > > > two > > > > > > > > > > substitutions produce different results. I think it would > > > > > > > > > > make > > > > > > > > > > sense > > > > > > > > > > to > > > > > > > > > > follow the EDG behavior until this issue is actually > > > > > > > > > > resolved. > > > > > > > > > > > > > > > > > > Here is what I have so far towards that end. When > > > > > > > > > substituting > > > > > > > > > into the > > > > > > > > > PARM_DECLs of a function decl, we now additionally check if > > > > > > > > > the > > > > > > > > > aforementioned Core issues are relevant and issue a (fatal) > > > > > > > > > diagnostic > > > > > > > > > if so. This patch checks this in tsubst_decl > > > > > > > > > rather > > > > > > > > > than in tsubst_function_decl for efficiency reasons, so that > > > > > > > > > we > > > > > > > > > don't > > > > > > > > > have to perform another traversal over the DECL_ARGUMENTS / > > > > > > > > > TYPE_ARG_TYPES just to implement this check. > > > > > > > > > > > > > > > > Hmm, this seems like writing more complicated code for a very > > > > > > > > marginal > > > > > > > > optimization; how many function templates have so many > > > > > > > > parameters > > > > > > > > that > > > > > > > > walking > > > > > > > > over them once to compare types will have any effect on compile > > > > > > > > time? > > > > > > > > > > > > > > Good point... though I just tried implementing this check in > > > > > > > tsubst_function_decl, and it seems it might be just as complicated > > > > > > > to > > > > > > > implement it there instead, at least if we want to handle function > > > > > > > parameter packs correctly. > > > > > > > > > > > > > > If we were to implement this check in tsubst_function_decl, then > > > > > > > since > > > > > > > we have access to the
Re: [PATCH] c++: Fix ICE in tsubst_default_argument [PR92010]
On 4/6/20 11:45 AM, Patrick Palka wrote:
On Wed, 1 Apr 2020, Jason Merrill wrote:
On 4/1/20 6:29 PM, Jason Merrill wrote:
On 3/31/20 3:50 PM, Patrick Palka wrote:
On Tue, 31 Mar 2020, Jason Merrill wrote:
On 3/30/20 6:46 PM, Patrick Palka wrote:
On Mon, 30 Mar 2020, Jason Merrill wrote:
On 3/30/20 3:58 PM, Patrick Palka wrote:
On Thu, 26 Mar 2020, Jason Merrill wrote:
On 3/22/20 9:21 PM, Patrick Palka wrote:
This patch relaxes an assertion in tsubst_default_argument
that
exposes
a
latent
bug in how we substitute an array type into a cv-qualified
wildcard
function
parameter type. Concretely, the latent bug is that given the
function
template
template void foo(const T t);
one would expect the type of foo to be void(const
int*), but
we
(seemingly prematurely) strip function parameter types of
their
top-level
cv-qualifiers when building the function's TYPE_ARG_TYPES, and
instead
end
up
obtaining void(int*) as the type of foo after
substitution
and
decaying.
We still however correctly substitute into and decay the
formal
parameter
type,
obtaining const int* as the type of t after substitution. But
this
then
leads
to us tripping over the assert in tsubst_default_argument that
verifies
the
formal parameter type and the function type are consistent.
Assuming it's too late at this stage to fix the substitution
bug, we
can
still
relax the assertion like so. Tested on x86_64-pc-linux-gnu,
does
this
look
OK?
This is core issues 1001/1322, which have not been resolved.
Clang
does
the
substitution the way you suggest; EDG rejects the testcase
because the
two
substitutions produce different results. I think it would make
sense
to
follow the EDG behavior until this issue is actually resolved.
Here is what I have so far towards that end. When substituting
into the
PARM_DECLs of a function decl, we now additionally check if the
aforementioned Core issues are relevant and issue a (fatal)
diagnostic
if so. This patch checks this in tsubst_decl
rather
than in tsubst_function_decl for efficiency reasons, so that we
don't
have to perform another traversal over the DECL_ARGUMENTS /
TYPE_ARG_TYPES just to implement this check.
Hmm, this seems like writing more complicated code for a very
marginal
optimization; how many function templates have so many parameters
that
walking
over them once to compare types will have any effect on compile
time?
Good point... though I just tried implementing this check in
tsubst_function_decl, and it seems it might be just as complicated to
implement it there instead, at least if we want to handle function
parameter packs correctly.
If we were to implement this check in tsubst_function_decl, then since
we have access to the instantiated function, it would presumably
suffice
to compare its substituted DECL_ARGUMENTS with its substituted
TYPE_ARG_TYPES to see if they're consistent. Doing so would certainly
catch the original testcase, i.e.
template
void foo(const T);
int main() { foo(0); }
because the DECL_ARGUMENTS of foo would be {const int*} and its
TYPE_ARG_TYPES would be {int*}. But apparently it doesn't catch the
corresponding testcase that uses a function parameter pack, i.e.
template
void foo(const Ts...);
int main() { foo(0); }
because it turns out we don't strip top-level cv-qualifiers from
function parameter packs from TYPE_ARG_TYPES at declaration time, as
we
do with regular function parameters. So in this second testcase both
DECL_ARGUMENTS and TYPE_ARG_TYPES of foo would be {const int*},
and yet we would (presumably) want to reject this instantiation too.
So it seems comparing TYPE_ARG_TYPES and DECL_ARGUMENTS from
tsubst_function_decl would not suffice, and we would still need to do
a
variant of the trick that's done in this patch, i.e. substitute into
each dependent parameter type stripped of its top-level cv-qualifiers,
to see if these cv-qualifiers make a material difference in the
resulting function type. Or maybe there's yet another way to detect
this?
I think let's go ahead with comparing TYPE_ARG_TYPES and DECL_ARGUMENTS;
the
problem comes when they disagree. If we're handling pack expansions
wrong,
that's a separate issue.
Hm, comparing TYPE_ARG_TYPES and DECL_ARGUMENTS for compatibility seems
to be exposing a latent bug with how we handle lambdas that appear in
function parameter types. Take g++.dg/cpp2a/lambda-uneval3.C for
example:
template void spam(decltype([]{}) (*s)[sizeof(T)]) {}
int main() { spam(nullptr); }
According to tsubst_function_decl in current trunk, the type of the
function paremeter 's' of spam according to its TYPE_ARG_TYPES is
struct ._anon_4[1] *
and according to its DECL_ARGUMENTS the type of 's' is
struct ._anon_5[1] *
The disagreement happens because we call tsubst_lambda_expr twice during
substitution and thereby generate two distinct lambda types, one when
substituting into the TYPE_ARG_TYPES and another when substituting into
the
Re: [PATCH] c++: Fix ICE in tsubst_default_argument [PR92010]
On Wed, 1 Apr 2020, Jason Merrill wrote:
> On 4/1/20 6:29 PM, Jason Merrill wrote:
> > On 3/31/20 3:50 PM, Patrick Palka wrote:
> > > On Tue, 31 Mar 2020, Jason Merrill wrote:
> > >
> > > > On 3/30/20 6:46 PM, Patrick Palka wrote:
> > > > > On Mon, 30 Mar 2020, Jason Merrill wrote:
> > > > > > On 3/30/20 3:58 PM, Patrick Palka wrote:
> > > > > > > On Thu, 26 Mar 2020, Jason Merrill wrote:
> > > > > > >
> > > > > > > > On 3/22/20 9:21 PM, Patrick Palka wrote:
> > > > > > > > > This patch relaxes an assertion in tsubst_default_argument
> > > > > > > > > that
> > > > > > > > > exposes
> > > > > > > > > a
> > > > > > > > > latent
> > > > > > > > > bug in how we substitute an array type into a cv-qualified
> > > > > > > > > wildcard
> > > > > > > > > function
> > > > > > > > > parameter type. Concretely, the latent bug is that given the
> > > > > > > > > function
> > > > > > > > > template
> > > > > > > > >
> > > > > > > > > template void foo(const T t);
> > > > > > > > >
> > > > > > > > > one would expect the type of foo to be void(const
> > > > > > > > > int*), but
> > > > > > > > > we
> > > > > > > > > (seemingly prematurely) strip function parameter types of
> > > > > > > > > their
> > > > > > > > > top-level
> > > > > > > > > cv-qualifiers when building the function's TYPE_ARG_TYPES, and
> > > > > > > > > instead
> > > > > > > > > end
> > > > > > > > > up
> > > > > > > > > obtaining void(int*) as the type of foo after
> > > > > > > > > substitution
> > > > > > > > > and
> > > > > > > > > decaying.
> > > > > > > > >
> > > > > > > > > We still however correctly substitute into and decay the
> > > > > > > > > formal
> > > > > > > > > parameter
> > > > > > > > > type,
> > > > > > > > > obtaining const int* as the type of t after substitution. But
> > > > > > > > > this
> > > > > > > > > then
> > > > > > > > > leads
> > > > > > > > > to us tripping over the assert in tsubst_default_argument that
> > > > > > > > > verifies
> > > > > > > > > the
> > > > > > > > > formal parameter type and the function type are consistent.
> > > > > > > > >
> > > > > > > > > Assuming it's too late at this stage to fix the substitution
> > > > > > > > > bug, we
> > > > > > > > > can
> > > > > > > > > still
> > > > > > > > > relax the assertion like so. Tested on x86_64-pc-linux-gnu,
> > > > > > > > > does
> > > > > > > > > this
> > > > > > > > > look
> > > > > > > > > OK?
> > > > > > > >
> > > > > > > > This is core issues 1001/1322, which have not been resolved.
> > > > > > > > Clang
> > > > > > > > does
> > > > > > > > the
> > > > > > > > substitution the way you suggest; EDG rejects the testcase
> > > > > > > > because the
> > > > > > > > two
> > > > > > > > substitutions produce different results. I think it would make
> > > > > > > > sense
> > > > > > > > to
> > > > > > > > follow the EDG behavior until this issue is actually resolved.
> > > > > > >
> > > > > > > Here is what I have so far towards that end. When substituting
> > > > > > > into the
> > > > > > > PARM_DECLs of a function decl, we now additionally check if the
> > > > > > > aforementioned Core issues are relevant and issue a (fatal)
> > > > > > > diagnostic
> > > > > > > if so. This patch checks this in tsubst_decl
> > > > > > > rather
> > > > > > > than in tsubst_function_decl for efficiency reasons, so that we
> > > > > > > don't
> > > > > > > have to perform another traversal over the DECL_ARGUMENTS /
> > > > > > > TYPE_ARG_TYPES just to implement this check.
> > > > > >
> > > > > > Hmm, this seems like writing more complicated code for a very
> > > > > > marginal
> > > > > > optimization; how many function templates have so many parameters
> > > > > > that
> > > > > > walking
> > > > > > over them once to compare types will have any effect on compile
> > > > > > time?
> > > > >
> > > > > Good point... though I just tried implementing this check in
> > > > > tsubst_function_decl, and it seems it might be just as complicated to
> > > > > implement it there instead, at least if we want to handle function
> > > > > parameter packs correctly.
> > > > >
> > > > > If we were to implement this check in tsubst_function_decl, then since
> > > > > we have access to the instantiated function, it would presumably
> > > > > suffice
> > > > > to compare its substituted DECL_ARGUMENTS with its substituted
> > > > > TYPE_ARG_TYPES to see if they're consistent. Doing so would certainly
> > > > > catch the original testcase, i.e.
> > > > >
> > > > > template
> > > > > void foo(const T);
> > > > > int main() { foo(0); }
> > > > >
> > > > > because the DECL_ARGUMENTS of foo would be {const int*} and its
> > > > > TYPE_ARG_TYPES would be {int*}. But apparently it doesn't catch the
> > > > > corresponding testcase that uses a function parameter pack, i.e.
> > > > >
> > > > > template
> > > > > void foo(const Ts...);
> > > > > int main() { foo(0); }
> > > > >
> > > > > because it turns out we don't strip top-level
Re: [PATCH] c++: Fix ICE in tsubst_default_argument [PR92010]
On 4/1/20 6:29 PM, Jason Merrill wrote:
On 3/31/20 3:50 PM, Patrick Palka wrote:
On Tue, 31 Mar 2020, Jason Merrill wrote:
On 3/30/20 6:46 PM, Patrick Palka wrote:
On Mon, 30 Mar 2020, Jason Merrill wrote:
On 3/30/20 3:58 PM, Patrick Palka wrote:
On Thu, 26 Mar 2020, Jason Merrill wrote:
On 3/22/20 9:21 PM, Patrick Palka wrote:
This patch relaxes an assertion in tsubst_default_argument that
exposes
a
latent
bug in how we substitute an array type into a cv-qualified wildcard
function
parameter type. Concretely, the latent bug is that given the
function
template
template void foo(const T t);
one would expect the type of foo to be void(const int*), but
we
(seemingly prematurely) strip function parameter types of their
top-level
cv-qualifiers when building the function's TYPE_ARG_TYPES, and
instead
end
up
obtaining void(int*) as the type of foo after substitution
and
decaying.
We still however correctly substitute into and decay the formal
parameter
type,
obtaining const int* as the type of t after substitution. But this
then
leads
to us tripping over the assert in tsubst_default_argument that
verifies
the
formal parameter type and the function type are consistent.
Assuming it's too late at this stage to fix the substitution
bug, we
can
still
relax the assertion like so. Tested on x86_64-pc-linux-gnu, does
this
look
OK?
This is core issues 1001/1322, which have not been resolved. Clang
does
the
substitution the way you suggest; EDG rejects the testcase
because the
two
substitutions produce different results. I think it would make
sense
to
follow the EDG behavior until this issue is actually resolved.
Here is what I have so far towards that end. When substituting
into the
PARM_DECLs of a function decl, we now additionally check if the
aforementioned Core issues are relevant and issue a (fatal)
diagnostic
if so. This patch checks this in tsubst_decl rather
than in tsubst_function_decl for efficiency reasons, so that we don't
have to perform another traversal over the DECL_ARGUMENTS /
TYPE_ARG_TYPES just to implement this check.
Hmm, this seems like writing more complicated code for a very marginal
optimization; how many function templates have so many parameters that
walking
over them once to compare types will have any effect on compile time?
Good point... though I just tried implementing this check in
tsubst_function_decl, and it seems it might be just as complicated to
implement it there instead, at least if we want to handle function
parameter packs correctly.
If we were to implement this check in tsubst_function_decl, then since
we have access to the instantiated function, it would presumably
suffice
to compare its substituted DECL_ARGUMENTS with its substituted
TYPE_ARG_TYPES to see if they're consistent. Doing so would certainly
catch the original testcase, i.e.
template
void foo(const T);
int main() { foo(0); }
because the DECL_ARGUMENTS of foo would be {const int*} and its
TYPE_ARG_TYPES would be {int*}. But apparently it doesn't catch the
corresponding testcase that uses a function parameter pack, i.e.
template
void foo(const Ts...);
int main() { foo(0); }
because it turns out we don't strip top-level cv-qualifiers from
function parameter packs from TYPE_ARG_TYPES at declaration time, as we
do with regular function parameters. So in this second testcase both
DECL_ARGUMENTS and TYPE_ARG_TYPES of foo would be {const int*},
and yet we would (presumably) want to reject this instantiation too.
So it seems comparing TYPE_ARG_TYPES and DECL_ARGUMENTS from
tsubst_function_decl would not suffice, and we would still need to do a
variant of the trick that's done in this patch, i.e. substitute into
each dependent parameter type stripped of its top-level cv-qualifiers,
to see if these cv-qualifiers make a material difference in the
resulting function type. Or maybe there's yet another way to detect
this?
I think let's go ahead with comparing TYPE_ARG_TYPES and
DECL_ARGUMENTS; the
problem comes when they disagree. If we're handling pack expansions
wrong,
that's a separate issue.
Hm, comparing TYPE_ARG_TYPES and DECL_ARGUMENTS for compatibility seems
to be exposing a latent bug with how we handle lambdas that appear in
function parameter types. Take g++.dg/cpp2a/lambda-uneval3.C for
example:
template void spam(decltype([]{}) (*s)[sizeof(T)]) {}
int main() { spam(nullptr); }
According to tsubst_function_decl in current trunk, the type of the
function paremeter 's' of spam according to its TYPE_ARG_TYPES is
struct ._anon_4[1] *
and according to its DECL_ARGUMENTS the type of 's' is
struct ._anon_5[1] *
The disagreement happens because we call tsubst_lambda_expr twice during
substitution and thereby generate two distinct lambda types, one when
substituting into the TYPE_ARG_TYPES and another when substituting into
the DECL_ARGUMENTS. I'm not sure how to work around this
bug/false-positive..
Oof.
I
Re: [PATCH] c++: Fix ICE in tsubst_default_argument [PR92010]
On 3/31/20 3:50 PM, Patrick Palka wrote:
On Tue, 31 Mar 2020, Jason Merrill wrote:
On 3/30/20 6:46 PM, Patrick Palka wrote:
On Mon, 30 Mar 2020, Jason Merrill wrote:
On 3/30/20 3:58 PM, Patrick Palka wrote:
On Thu, 26 Mar 2020, Jason Merrill wrote:
On 3/22/20 9:21 PM, Patrick Palka wrote:
This patch relaxes an assertion in tsubst_default_argument that
exposes
a
latent
bug in how we substitute an array type into a cv-qualified wildcard
function
parameter type. Concretely, the latent bug is that given the
function
template
template void foo(const T t);
one would expect the type of foo to be void(const int*), but
we
(seemingly prematurely) strip function parameter types of their
top-level
cv-qualifiers when building the function's TYPE_ARG_TYPES, and
instead
end
up
obtaining void(int*) as the type of foo after substitution
and
decaying.
We still however correctly substitute into and decay the formal
parameter
type,
obtaining const int* as the type of t after substitution. But this
then
leads
to us tripping over the assert in tsubst_default_argument that
verifies
the
formal parameter type and the function type are consistent.
Assuming it's too late at this stage to fix the substitution bug, we
can
still
relax the assertion like so. Tested on x86_64-pc-linux-gnu, does
this
look
OK?
This is core issues 1001/1322, which have not been resolved. Clang
does
the
substitution the way you suggest; EDG rejects the testcase because the
two
substitutions produce different results. I think it would make sense
to
follow the EDG behavior until this issue is actually resolved.
Here is what I have so far towards that end. When substituting into the
PARM_DECLs of a function decl, we now additionally check if the
aforementioned Core issues are relevant and issue a (fatal) diagnostic
if so. This patch checks this in tsubst_decl rather
than in tsubst_function_decl for efficiency reasons, so that we don't
have to perform another traversal over the DECL_ARGUMENTS /
TYPE_ARG_TYPES just to implement this check.
Hmm, this seems like writing more complicated code for a very marginal
optimization; how many function templates have so many parameters that
walking
over them once to compare types will have any effect on compile time?
Good point... though I just tried implementing this check in
tsubst_function_decl, and it seems it might be just as complicated to
implement it there instead, at least if we want to handle function
parameter packs correctly.
If we were to implement this check in tsubst_function_decl, then since
we have access to the instantiated function, it would presumably suffice
to compare its substituted DECL_ARGUMENTS with its substituted
TYPE_ARG_TYPES to see if they're consistent. Doing so would certainly
catch the original testcase, i.e.
template
void foo(const T);
int main() { foo(0); }
because the DECL_ARGUMENTS of foo would be {const int*} and its
TYPE_ARG_TYPES would be {int*}. But apparently it doesn't catch the
corresponding testcase that uses a function parameter pack, i.e.
template
void foo(const Ts...);
int main() { foo(0); }
because it turns out we don't strip top-level cv-qualifiers from
function parameter packs from TYPE_ARG_TYPES at declaration time, as we
do with regular function parameters. So in this second testcase both
DECL_ARGUMENTS and TYPE_ARG_TYPES of foo would be {const int*},
and yet we would (presumably) want to reject this instantiation too.
So it seems comparing TYPE_ARG_TYPES and DECL_ARGUMENTS from
tsubst_function_decl would not suffice, and we would still need to do a
variant of the trick that's done in this patch, i.e. substitute into
each dependent parameter type stripped of its top-level cv-qualifiers,
to see if these cv-qualifiers make a material difference in the
resulting function type. Or maybe there's yet another way to detect
this?
I think let's go ahead with comparing TYPE_ARG_TYPES and DECL_ARGUMENTS; the
problem comes when they disagree. If we're handling pack expansions wrong,
that's a separate issue.
Hm, comparing TYPE_ARG_TYPES and DECL_ARGUMENTS for compatibility seems
to be exposing a latent bug with how we handle lambdas that appear in
function parameter types. Take g++.dg/cpp2a/lambda-uneval3.C for
example:
template void spam(decltype([]{}) (*s)[sizeof(T)]) {}
int main() { spam(nullptr); }
According to tsubst_function_decl in current trunk, the type of the
function paremeter 's' of spam according to its TYPE_ARG_TYPES is
struct ._anon_4[1] *
and according to its DECL_ARGUMENTS the type of 's' is
struct ._anon_5[1] *
The disagreement happens because we call tsubst_lambda_expr twice during
substitution and thereby generate two distinct lambda types, one when
substituting into the TYPE_ARG_TYPES and another when substituting into
the DECL_ARGUMENTS. I'm not sure how to work around this
bug/false-positive..
Oof.
I think probably the right answer is to rebuild
Re: [PATCH] c++: Fix ICE in tsubst_default_argument [PR92010]
On Tue, 31 Mar 2020, Jason Merrill wrote:
> On 3/30/20 6:46 PM, Patrick Palka wrote:
> > On Mon, 30 Mar 2020, Jason Merrill wrote:
> > > On 3/30/20 3:58 PM, Patrick Palka wrote:
> > > > On Thu, 26 Mar 2020, Jason Merrill wrote:
> > > >
> > > > > On 3/22/20 9:21 PM, Patrick Palka wrote:
> > > > > > This patch relaxes an assertion in tsubst_default_argument that
> > > > > > exposes
> > > > > > a
> > > > > > latent
> > > > > > bug in how we substitute an array type into a cv-qualified wildcard
> > > > > > function
> > > > > > parameter type. Concretely, the latent bug is that given the
> > > > > > function
> > > > > > template
> > > > > >
> > > > > > template void foo(const T t);
> > > > > >
> > > > > > one would expect the type of foo to be void(const int*), but
> > > > > > we
> > > > > > (seemingly prematurely) strip function parameter types of their
> > > > > > top-level
> > > > > > cv-qualifiers when building the function's TYPE_ARG_TYPES, and
> > > > > > instead
> > > > > > end
> > > > > > up
> > > > > > obtaining void(int*) as the type of foo after substitution
> > > > > > and
> > > > > > decaying.
> > > > > >
> > > > > > We still however correctly substitute into and decay the formal
> > > > > > parameter
> > > > > > type,
> > > > > > obtaining const int* as the type of t after substitution. But this
> > > > > > then
> > > > > > leads
> > > > > > to us tripping over the assert in tsubst_default_argument that
> > > > > > verifies
> > > > > > the
> > > > > > formal parameter type and the function type are consistent.
> > > > > >
> > > > > > Assuming it's too late at this stage to fix the substitution bug, we
> > > > > > can
> > > > > > still
> > > > > > relax the assertion like so. Tested on x86_64-pc-linux-gnu, does
> > > > > > this
> > > > > > look
> > > > > > OK?
> > > > >
> > > > > This is core issues 1001/1322, which have not been resolved. Clang
> > > > > does
> > > > > the
> > > > > substitution the way you suggest; EDG rejects the testcase because the
> > > > > two
> > > > > substitutions produce different results. I think it would make sense
> > > > > to
> > > > > follow the EDG behavior until this issue is actually resolved.
> > > >
> > > > Here is what I have so far towards that end. When substituting into the
> > > > PARM_DECLs of a function decl, we now additionally check if the
> > > > aforementioned Core issues are relevant and issue a (fatal) diagnostic
> > > > if so. This patch checks this in tsubst_decl rather
> > > > than in tsubst_function_decl for efficiency reasons, so that we don't
> > > > have to perform another traversal over the DECL_ARGUMENTS /
> > > > TYPE_ARG_TYPES just to implement this check.
> > >
> > > Hmm, this seems like writing more complicated code for a very marginal
> > > optimization; how many function templates have so many parameters that
> > > walking
> > > over them once to compare types will have any effect on compile time?
> >
> > Good point... though I just tried implementing this check in
> > tsubst_function_decl, and it seems it might be just as complicated to
> > implement it there instead, at least if we want to handle function
> > parameter packs correctly.
> >
> > If we were to implement this check in tsubst_function_decl, then since
> > we have access to the instantiated function, it would presumably suffice
> > to compare its substituted DECL_ARGUMENTS with its substituted
> > TYPE_ARG_TYPES to see if they're consistent. Doing so would certainly
> > catch the original testcase, i.e.
> >
> >template
> > void foo(const T);
> >int main() { foo(0); }
> >
> > because the DECL_ARGUMENTS of foo would be {const int*} and its
> > TYPE_ARG_TYPES would be {int*}. But apparently it doesn't catch the
> > corresponding testcase that uses a function parameter pack, i.e.
> >
> >template
> > void foo(const Ts...);
> >int main() { foo(0); }
> >
> > because it turns out we don't strip top-level cv-qualifiers from
> > function parameter packs from TYPE_ARG_TYPES at declaration time, as we
> > do with regular function parameters. So in this second testcase both
> > DECL_ARGUMENTS and TYPE_ARG_TYPES of foo would be {const int*},
> > and yet we would (presumably) want to reject this instantiation too.
> >
> > So it seems comparing TYPE_ARG_TYPES and DECL_ARGUMENTS from
> > tsubst_function_decl would not suffice, and we would still need to do a
> > variant of the trick that's done in this patch, i.e. substitute into
> > each dependent parameter type stripped of its top-level cv-qualifiers,
> > to see if these cv-qualifiers make a material difference in the
> > resulting function type. Or maybe there's yet another way to detect
> > this?
>
> I think let's go ahead with comparing TYPE_ARG_TYPES and DECL_ARGUMENTS; the
> problem comes when they disagree. If we're handling pack expansions wrong,
> that's a separate issue.
Hm, comparing TYPE_ARG_TYPES and DECL_ARGUMENTS for compatibility seems
to be exposing a latent
Re: [PATCH] c++: Fix ICE in tsubst_default_argument [PR92010]
On 3/30/20 6:46 PM, Patrick Palka wrote:
On Mon, 30 Mar 2020, Jason Merrill wrote:
On 3/30/20 3:58 PM, Patrick Palka wrote:
On Thu, 26 Mar 2020, Jason Merrill wrote:
On 3/22/20 9:21 PM, Patrick Palka wrote:
This patch relaxes an assertion in tsubst_default_argument that exposes
a
latent
bug in how we substitute an array type into a cv-qualified wildcard
function
parameter type. Concretely, the latent bug is that given the function
template
template void foo(const T t);
one would expect the type of foo to be void(const int*), but we
(seemingly prematurely) strip function parameter types of their
top-level
cv-qualifiers when building the function's TYPE_ARG_TYPES, and instead
end
up
obtaining void(int*) as the type of foo after substitution and
decaying.
We still however correctly substitute into and decay the formal
parameter
type,
obtaining const int* as the type of t after substitution. But this then
leads
to us tripping over the assert in tsubst_default_argument that verifies
the
formal parameter type and the function type are consistent.
Assuming it's too late at this stage to fix the substitution bug, we can
still
relax the assertion like so. Tested on x86_64-pc-linux-gnu, does this
look
OK?
This is core issues 1001/1322, which have not been resolved. Clang does
the
substitution the way you suggest; EDG rejects the testcase because the two
substitutions produce different results. I think it would make sense to
follow the EDG behavior until this issue is actually resolved.
Here is what I have so far towards that end. When substituting into the
PARM_DECLs of a function decl, we now additionally check if the
aforementioned Core issues are relevant and issue a (fatal) diagnostic
if so. This patch checks this in tsubst_decl rather
than in tsubst_function_decl for efficiency reasons, so that we don't
have to perform another traversal over the DECL_ARGUMENTS /
TYPE_ARG_TYPES just to implement this check.
Hmm, this seems like writing more complicated code for a very marginal
optimization; how many function templates have so many parameters that walking
over them once to compare types will have any effect on compile time?
Good point... though I just tried implementing this check in
tsubst_function_decl, and it seems it might be just as complicated to
implement it there instead, at least if we want to handle function
parameter packs correctly.
If we were to implement this check in tsubst_function_decl, then since
we have access to the instantiated function, it would presumably suffice
to compare its substituted DECL_ARGUMENTS with its substituted
TYPE_ARG_TYPES to see if they're consistent. Doing so would certainly
catch the original testcase, i.e.
template
void foo(const T);
int main() { foo(0); }
because the DECL_ARGUMENTS of foo would be {const int*} and its
TYPE_ARG_TYPES would be {int*}. But apparently it doesn't catch the
corresponding testcase that uses a function parameter pack, i.e.
template
void foo(const Ts...);
int main() { foo(0); }
because it turns out we don't strip top-level cv-qualifiers from
function parameter packs from TYPE_ARG_TYPES at declaration time, as we
do with regular function parameters. So in this second testcase both
DECL_ARGUMENTS and TYPE_ARG_TYPES of foo would be {const int*},
and yet we would (presumably) want to reject this instantiation too.
So it seems comparing TYPE_ARG_TYPES and DECL_ARGUMENTS from
tsubst_function_decl would not suffice, and we would still need to do a
variant of the trick that's done in this patch, i.e. substitute into
each dependent parameter type stripped of its top-level cv-qualifiers,
to see if these cv-qualifiers make a material difference in the
resulting function type. Or maybe there's yet another way to detect
this?
I think let's go ahead with comparing TYPE_ARG_TYPES and DECL_ARGUMENTS;
the problem comes when they disagree. If we're handling pack expansions
wrong, that's a separate issue.
Jason
Re: [PATCH] c++: Fix ICE in tsubst_default_argument [PR92010]
On Mon, 30 Mar 2020, Jason Merrill wrote:
> On 3/30/20 3:58 PM, Patrick Palka wrote:
> > On Thu, 26 Mar 2020, Jason Merrill wrote:
> >
> > > On 3/22/20 9:21 PM, Patrick Palka wrote:
> > > > This patch relaxes an assertion in tsubst_default_argument that exposes
> > > > a
> > > > latent
> > > > bug in how we substitute an array type into a cv-qualified wildcard
> > > > function
> > > > parameter type. Concretely, the latent bug is that given the function
> > > > template
> > > >
> > > > template void foo(const T t);
> > > >
> > > > one would expect the type of foo to be void(const int*), but we
> > > > (seemingly prematurely) strip function parameter types of their
> > > > top-level
> > > > cv-qualifiers when building the function's TYPE_ARG_TYPES, and instead
> > > > end
> > > > up
> > > > obtaining void(int*) as the type of foo after substitution and
> > > > decaying.
> > > >
> > > > We still however correctly substitute into and decay the formal
> > > > parameter
> > > > type,
> > > > obtaining const int* as the type of t after substitution. But this then
> > > > leads
> > > > to us tripping over the assert in tsubst_default_argument that verifies
> > > > the
> > > > formal parameter type and the function type are consistent.
> > > >
> > > > Assuming it's too late at this stage to fix the substitution bug, we can
> > > > still
> > > > relax the assertion like so. Tested on x86_64-pc-linux-gnu, does this
> > > > look
> > > > OK?
> > >
> > > This is core issues 1001/1322, which have not been resolved. Clang does
> > > the
> > > substitution the way you suggest; EDG rejects the testcase because the two
> > > substitutions produce different results. I think it would make sense to
> > > follow the EDG behavior until this issue is actually resolved.
> >
> > Here is what I have so far towards that end. When substituting into the
> > PARM_DECLs of a function decl, we now additionally check if the
> > aforementioned Core issues are relevant and issue a (fatal) diagnostic
> > if so. This patch checks this in tsubst_decl rather
> > than in tsubst_function_decl for efficiency reasons, so that we don't
> > have to perform another traversal over the DECL_ARGUMENTS /
> > TYPE_ARG_TYPES just to implement this check.
>
> Hmm, this seems like writing more complicated code for a very marginal
> optimization; how many function templates have so many parameters that walking
> over them once to compare types will have any effect on compile time?
Good point... though I just tried implementing this check in
tsubst_function_decl, and it seems it might be just as complicated to
implement it there instead, at least if we want to handle function
parameter packs correctly.
If we were to implement this check in tsubst_function_decl, then since
we have access to the instantiated function, it would presumably suffice
to compare its substituted DECL_ARGUMENTS with its substituted
TYPE_ARG_TYPES to see if they're consistent. Doing so would certainly
catch the original testcase, i.e.
template
void foo(const T);
int main() { foo(0); }
because the DECL_ARGUMENTS of foo would be {const int*} and its
TYPE_ARG_TYPES would be {int*}. But apparently it doesn't catch the
corresponding testcase that uses a function parameter pack, i.e.
template
void foo(const Ts...);
int main() { foo(0); }
because it turns out we don't strip top-level cv-qualifiers from
function parameter packs from TYPE_ARG_TYPES at declaration time, as we
do with regular function parameters. So in this second testcase both
DECL_ARGUMENTS and TYPE_ARG_TYPES of foo would be {const int*},
and yet we would (presumably) want to reject this instantiation too.
So it seems comparing TYPE_ARG_TYPES and DECL_ARGUMENTS from
tsubst_function_decl would not suffice, and we would still need to do a
variant of the trick that's done in this patch, i.e. substitute into
each dependent parameter type stripped of its top-level cv-qualifiers,
to see if these cv-qualifiers make a material difference in the
resulting function type. Or maybe there's yet another way to detect
this?
>
> > Is something like this what you have in mind?
> >
> > -- >8 --
> >
> > Subject: [PATCH] c++: Reject some instantiations that depend on resolution
> > of
> > Core issues 1001/1322
> >
> > gcc/cp/ChangeLog:
> >
> > Core issues 1001 and 1322
> > PR c++/92010
> > * pt.c (tsubst_decl) : Detect and reject the case
> > where
> > the function type depends on whether we strip top-level qualifiers
> > from
> > the type of T before or after substitution.
> >
> > gcc/testsuite/ChangeLog:
> >
> > Core issues 1001 and 1322
> > PR c++/92010
> > * g++.dg/template/array33.C: New test.
> > * g++.dg/template/array34.C: New test.
> > ---
> > gcc/cp/pt.c | 70 -
> > gcc/testsuite/g++.dg/template/array33.C | 39 ++
> > gcc/testsuite/g++.dg/template/array34.C | 63
Re: [PATCH] c++: Fix ICE in tsubst_default_argument [PR92010]
On 3/30/20 3:58 PM, Patrick Palka wrote:
On Thu, 26 Mar 2020, Jason Merrill wrote:
On 3/22/20 9:21 PM, Patrick Palka wrote:
This patch relaxes an assertion in tsubst_default_argument that exposes a
latent
bug in how we substitute an array type into a cv-qualified wildcard function
parameter type. Concretely, the latent bug is that given the function
template
template void foo(const T t);
one would expect the type of foo to be void(const int*), but we
(seemingly prematurely) strip function parameter types of their top-level
cv-qualifiers when building the function's TYPE_ARG_TYPES, and instead end
up
obtaining void(int*) as the type of foo after substitution and
decaying.
We still however correctly substitute into and decay the formal parameter
type,
obtaining const int* as the type of t after substitution. But this then
leads
to us tripping over the assert in tsubst_default_argument that verifies the
formal parameter type and the function type are consistent.
Assuming it's too late at this stage to fix the substitution bug, we can
still
relax the assertion like so. Tested on x86_64-pc-linux-gnu, does this look
OK?
This is core issues 1001/1322, which have not been resolved. Clang does the
substitution the way you suggest; EDG rejects the testcase because the two
substitutions produce different results. I think it would make sense to
follow the EDG behavior until this issue is actually resolved.
Here is what I have so far towards that end. When substituting into the
PARM_DECLs of a function decl, we now additionally check if the
aforementioned Core issues are relevant and issue a (fatal) diagnostic
if so. This patch checks this in tsubst_decl rather
than in tsubst_function_decl for efficiency reasons, so that we don't
have to perform another traversal over the DECL_ARGUMENTS /
TYPE_ARG_TYPES just to implement this check.
Hmm, this seems like writing more complicated code for a very marginal
optimization; how many function templates have so many parameters that
walking over them once to compare types will have any effect on compile
time?
Is something like this what you have in mind?
-- >8 --
Subject: [PATCH] c++: Reject some instantiations that depend on resolution of
Core issues 1001/1322
gcc/cp/ChangeLog:
Core issues 1001 and 1322
PR c++/92010
* pt.c (tsubst_decl) : Detect and reject the case where
the function type depends on whether we strip top-level qualifiers from
the type of T before or after substitution.
gcc/testsuite/ChangeLog:
Core issues 1001 and 1322
PR c++/92010
* g++.dg/template/array33.C: New test.
* g++.dg/template/array34.C: New test.
---
gcc/cp/pt.c | 70 -
gcc/testsuite/g++.dg/template/array33.C | 39 ++
gcc/testsuite/g++.dg/template/array34.C | 63 ++
3 files changed, 171 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/template/array33.C
create mode 100644 gcc/testsuite/g++.dg/template/array34.C
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 8564eb11df4..fd99053df36 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -14072,9 +14072,77 @@ tsubst_decl (tree t, tree args, tsubst_flags_t
complain)
/* We're dealing with a normal parameter. */
type = tsubst (TREE_TYPE (t), args, complain, in_decl);
+ const int type_quals = cp_type_quals (type);
+
+ /* Determine whether the function type after substitution into the
+ type of the function parameter T depends on the resolution of
+ Core issues 1001/1322, which have not been resolved. In
+ particular, the following detects the case where the resulting
+ function type depends on whether we strip top-level qualifiers
+ from the type of T before or after substitution.
+
+ This can happen only when the dependent type of T is a
+ cv-qualified wildcard type, and substitution yields a
+ (cv-qualified) array type before array-to-pointer conversion. */
+ tree unqual_expanded_types = NULL_TREE;
+ if (TREE_CODE (type) == ARRAY_TYPE
+ && (type_quals & (TYPE_QUAL_CONST|TYPE_QUAL_VOLATILE))
+ && DECL_FUNCTION_SCOPE_P (t)
+ && !DECL_TEMPLATE_PARM_P (t))
+ {
+ tree type_pattern = (PACK_EXPANSION_P (TREE_TYPE (t))
+? PACK_EXPANSION_PATTERN (TREE_TYPE (t))
+: TREE_TYPE (t));
+ if (WILDCARD_TYPE_P (type_pattern)
+ && cv_qualified_p (type_pattern))
+ {
+ /* Substitute into the corresponding wildcard type that is
+ stripped of its own top-level cv-qualifiers. */
+ tree unqual_type;
+ if (PACK_EXPANSION_P (TREE_TYPE
Re: [PATCH] c++: Fix ICE in tsubst_default_argument [PR92010]
On Mon, 30 Mar 2020, Patrick Palka wrote:
> On Thu, 26 Mar 2020, Jason Merrill wrote:
>
> > On 3/22/20 9:21 PM, Patrick Palka wrote:
> > > This patch relaxes an assertion in tsubst_default_argument that exposes a
> > > latent
> > > bug in how we substitute an array type into a cv-qualified wildcard
> > > function
> > > parameter type. Concretely, the latent bug is that given the function
> > > template
> > >
> > >template void foo(const T t);
> > >
> > > one would expect the type of foo to be void(const int*), but we
> > > (seemingly prematurely) strip function parameter types of their top-level
> > > cv-qualifiers when building the function's TYPE_ARG_TYPES, and instead end
> > > up
> > > obtaining void(int*) as the type of foo after substitution and
> > > decaying.
> > >
> > > We still however correctly substitute into and decay the formal parameter
> > > type,
> > > obtaining const int* as the type of t after substitution. But this then
> > > leads
> > > to us tripping over the assert in tsubst_default_argument that verifies
> > > the
> > > formal parameter type and the function type are consistent.
> > >
> > > Assuming it's too late at this stage to fix the substitution bug, we can
> > > still
> > > relax the assertion like so. Tested on x86_64-pc-linux-gnu, does this
> > > look
> > > OK?
> >
> > This is core issues 1001/1322, which have not been resolved. Clang does the
> > substitution the way you suggest; EDG rejects the testcase because the two
> > substitutions produce different results. I think it would make sense to
> > follow the EDG behavior until this issue is actually resolved.
>
> Here is what I have so far towards that end. When substituting into the
> PARM_DECLs of a function decl, we now additionally check if the
> aforementioned Core issues are relevant and issue a (fatal) diagnostic
> if so. This patch checks this in tsubst_decl rather
> than in tsubst_function_decl for efficiency reasons, so that we don't
> have to perform another traversal over the DECL_ARGUMENTS /
> TYPE_ARG_TYPES just to implement this check.
>
> Is something like this what you have in mind?
Oops, noticed a few bugs in the patch as soon as I hit send. Please
consider this patch instead, which compared to the previous patch
declares unqual_expanded_types in the correct scope and refrains from
doing a potentially incorrect CSE of "cp_type_quals (type)".
-- >8 --
gcc/cp/ChangeLog:
Core issues 1001 and 1322
PR c++/92010
* pt.c (tsubst_decl) : Detect and reject the case where
the function type depends on whether we strip top-level qualifiers from
the type of T before or after substitution.
gcc/testsuite/ChangeLog:
Core issues 1001 and 1322
PR c++/92010
* g++.dg/template/array33.C: New test.
* g++.dg/template/array34.C: New test.
---
gcc/cp/pt.c | 68 +
gcc/testsuite/g++.dg/template/array33.C | 39 ++
gcc/testsuite/g++.dg/template/array34.C | 63 +++
3 files changed, 170 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/template/array33.C
create mode 100644 gcc/testsuite/g++.dg/template/array34.C
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 8564eb11df4..a1efaef2c1e 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -14046,6 +14046,8 @@ tsubst_decl (tree t, tree args, tsubst_flags_t complain)
}
}
+ tree unqual_expanded_types = NULL_TREE;
+
/* Loop through all of the parameters we'll build. When T is
a function parameter pack, LEN is the number of expanded
types in EXPANDED_TYPES; otherwise, LEN is 1. */
@@ -14072,6 +14074,72 @@ tsubst_decl (tree t, tree args, tsubst_flags_t
complain)
/* We're dealing with a normal parameter. */
type = tsubst (TREE_TYPE (t), args, complain, in_decl);
+ /* Determine whether the function type after substitution into the
+ type of the function parameter T depends on the resolution of
+ Core issues 1001/1322, which have not been resolved. In
+ particular, the following detects when the resulting function
+ type depends on whether we strip top-level qualifiers from the
+ type of T before or after substitution.
+
+ This can happen only when the dependent type of T is a
+ cv-qualified wildcard type, and substitution yields a
+ (cv-qualified) array type before array-to-pointer conversion. */
+ if (TREE_CODE (type) == ARRAY_TYPE
+ && cv_qualified_p (type)
+ && DECL_FUNCTION_SCOPE_P (t)
+ && !DECL_TEMPLATE_PARM_P (t))
+ {
+ tree type_pattern = (PACK_EXPANSION_P (TREE_TYPE (t))
+? PACK_EXPANSION_PATTERN (TREE_TYPE (t))
+: TREE_TYPE (t));
+
Re: [PATCH] c++: Fix ICE in tsubst_default_argument [PR92010]
On Thu, 26 Mar 2020, Jason Merrill wrote:
> On 3/22/20 9:21 PM, Patrick Palka wrote:
> > This patch relaxes an assertion in tsubst_default_argument that exposes a
> > latent
> > bug in how we substitute an array type into a cv-qualified wildcard function
> > parameter type. Concretely, the latent bug is that given the function
> > template
> >
> >template void foo(const T t);
> >
> > one would expect the type of foo to be void(const int*), but we
> > (seemingly prematurely) strip function parameter types of their top-level
> > cv-qualifiers when building the function's TYPE_ARG_TYPES, and instead end
> > up
> > obtaining void(int*) as the type of foo after substitution and
> > decaying.
> >
> > We still however correctly substitute into and decay the formal parameter
> > type,
> > obtaining const int* as the type of t after substitution. But this then
> > leads
> > to us tripping over the assert in tsubst_default_argument that verifies the
> > formal parameter type and the function type are consistent.
> >
> > Assuming it's too late at this stage to fix the substitution bug, we can
> > still
> > relax the assertion like so. Tested on x86_64-pc-linux-gnu, does this look
> > OK?
>
> This is core issues 1001/1322, which have not been resolved. Clang does the
> substitution the way you suggest; EDG rejects the testcase because the two
> substitutions produce different results. I think it would make sense to
> follow the EDG behavior until this issue is actually resolved.
Here is what I have so far towards that end. When substituting into the
PARM_DECLs of a function decl, we now additionally check if the
aforementioned Core issues are relevant and issue a (fatal) diagnostic
if so. This patch checks this in tsubst_decl rather
than in tsubst_function_decl for efficiency reasons, so that we don't
have to perform another traversal over the DECL_ARGUMENTS /
TYPE_ARG_TYPES just to implement this check.
Is something like this what you have in mind?
-- >8 --
Subject: [PATCH] c++: Reject some instantiations that depend on resolution of
Core issues 1001/1322
gcc/cp/ChangeLog:
Core issues 1001 and 1322
PR c++/92010
* pt.c (tsubst_decl) : Detect and reject the case where
the function type depends on whether we strip top-level qualifiers from
the type of T before or after substitution.
gcc/testsuite/ChangeLog:
Core issues 1001 and 1322
PR c++/92010
* g++.dg/template/array33.C: New test.
* g++.dg/template/array34.C: New test.
---
gcc/cp/pt.c | 70 -
gcc/testsuite/g++.dg/template/array33.C | 39 ++
gcc/testsuite/g++.dg/template/array34.C | 63 ++
3 files changed, 171 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/template/array33.C
create mode 100644 gcc/testsuite/g++.dg/template/array34.C
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 8564eb11df4..fd99053df36 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -14072,9 +14072,77 @@ tsubst_decl (tree t, tree args, tsubst_flags_t
complain)
/* We're dealing with a normal parameter. */
type = tsubst (TREE_TYPE (t), args, complain, in_decl);
+ const int type_quals = cp_type_quals (type);
+
+ /* Determine whether the function type after substitution into the
+ type of the function parameter T depends on the resolution of
+ Core issues 1001/1322, which have not been resolved. In
+ particular, the following detects the case where the resulting
+ function type depends on whether we strip top-level qualifiers
+ from the type of T before or after substitution.
+
+ This can happen only when the dependent type of T is a
+ cv-qualified wildcard type, and substitution yields a
+ (cv-qualified) array type before array-to-pointer conversion. */
+ tree unqual_expanded_types = NULL_TREE;
+ if (TREE_CODE (type) == ARRAY_TYPE
+ && (type_quals & (TYPE_QUAL_CONST|TYPE_QUAL_VOLATILE))
+ && DECL_FUNCTION_SCOPE_P (t)
+ && !DECL_TEMPLATE_PARM_P (t))
+ {
+ tree type_pattern = (PACK_EXPANSION_P (TREE_TYPE (t))
+? PACK_EXPANSION_PATTERN (TREE_TYPE (t))
+: TREE_TYPE (t));
+ if (WILDCARD_TYPE_P (type_pattern)
+ && cv_qualified_p (type_pattern))
+ {
+ /* Substitute into the corresponding wildcard type that is
+ stripped of its own top-level cv-qualifiers. */
+ tree unqual_type;
+ if (PACK_EXPANSION_P (TREE_TYPE (t)))
+ {
+ if (unqual_expanded_types == NULL_TREE)
+ {
+ tree
Re: [PATCH] c++: Fix ICE in tsubst_default_argument [PR92010]
On 3/22/20 9:21 PM, Patrick Palka wrote:
This patch relaxes an assertion in tsubst_default_argument that exposes a latent
bug in how we substitute an array type into a cv-qualified wildcard function
parameter type. Concretely, the latent bug is that given the function template
template void foo(const T t);
one would expect the type of foo to be void(const int*), but we
(seemingly prematurely) strip function parameter types of their top-level
cv-qualifiers when building the function's TYPE_ARG_TYPES, and instead end up
obtaining void(int*) as the type of foo after substitution and decaying.
We still however correctly substitute into and decay the formal parameter type,
obtaining const int* as the type of t after substitution. But this then leads
to us tripping over the assert in tsubst_default_argument that verifies the
formal parameter type and the function type are consistent.
Assuming it's too late at this stage to fix the substitution bug, we can still
relax the assertion like so. Tested on x86_64-pc-linux-gnu, does this look OK?
This is core issues 1001/1322, which have not been resolved. Clang does
the substitution the way you suggest; EDG rejects the testcase because
the two substitutions produce different results. I think it would make
sense to follow the EDG behavior until this issue is actually resolved.
gcc/cp/ChangeLog:
PR c++/92010
* pt.c (tsubst_default_argument): Relax assertion to permit
disagreements between the function type and the parameter type
about the cv-qualification of the pointed-to type.
gcc/testsuite/ChangeLog:
PR c++/92010
* g++.dg/template/defarg22.C: New test.
---
gcc/cp/pt.c | 17 -
gcc/testsuite/g++.dg/template/defarg22.C | 11 +++
2 files changed, 27 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/template/defarg22.C
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 9e1eb9416c9..923166276b8 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -13337,7 +13337,22 @@ tsubst_default_argument (tree fn, int parmnum, tree
type, tree arg,
if (parmtype == error_mark_node)
return error_mark_node;
- gcc_assert (same_type_ignoring_top_level_qualifiers_p (type, parmtype));
+ if (same_type_ignoring_top_level_qualifiers_p (type, parmtype))
+;
+ else if (POINTER_TYPE_P (type) && POINTER_TYPE_P (parmtype))
+{
+ /* The function type and the parameter type can disagree about the
+cv-qualification of the pointed-to type; see PR92010. */
+ gcc_assert (same_type_p (TREE_TYPE (type),
+ strip_top_quals (TREE_TYPE (parmtype;
+ /* Verify that this happens only when the dependent parameter type is a
+cv-qualified wildcard type. */
+ tree pattern_parm = get_pattern_parm (parm, DECL_TI_TEMPLATE (fn));
+ gcc_assert (WILDCARD_TYPE_P (TREE_TYPE (pattern_parm))
+ && cv_qualified_p (TREE_TYPE (pattern_parm)));
+}
+ else
+gcc_unreachable ();
tree *slot;
if (defarg_inst && (slot = defarg_inst->get (parm)))
diff --git a/gcc/testsuite/g++.dg/template/defarg22.C
b/gcc/testsuite/g++.dg/template/defarg22.C
new file mode 100644
index 000..cf6261916d8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/defarg22.C
@@ -0,0 +1,11 @@
+// PR c++/92010
+// { dg-do compile }
+
+template
+void foo(const T t = 0)
+{ }
+
+int main()
+{
+ foo();
+}
Re: [PATCH] c++: Fix ICE in tsubst_default_argument [PR92010]
On Sun, 22 Mar 2020, Patrick Palka wrote:
> This patch relaxes an assertion in tsubst_default_argument that exposes a
> latent
> bug in how we substitute an array type into a cv-qualified wildcard function
> parameter type. Concretely, the latent bug is that given the function
> template
>
> template void foo(const T t);
>
> one would expect the type of foo to be void(const int*), but we
> (seemingly prematurely) strip function parameter types of their top-level
> cv-qualifiers when building the function's TYPE_ARG_TYPES, and instead end up
> obtaining void(int*) as the type of foo after substitution and
> decaying.
>
> We still however correctly substitute into and decay the formal parameter
> type,
> obtaining const int* as the type of t after substitution. But this then leads
> to us tripping over the assert in tsubst_default_argument that verifies the
> formal parameter type and the function type are consistent.
>
> Assuming it's too late at this stage to fix the substitution bug, we can still
> relax the assertion like so. Tested on x86_64-pc-linux-gnu, does this look
> OK?
>
> gcc/cp/ChangeLog:
>
> PR c++/92010
> * pt.c (tsubst_default_argument): Relax assertion to permit
> disagreements between the function type and the parameter type
> about the cv-qualification of the pointed-to type.
>
> gcc/testsuite/ChangeLog:
>
> PR c++/92010
> * g++.dg/template/defarg22.C: New test.
> ---
Here's a slightly simpler version of the patch that moves the
POINTER_TYPE_P checks into the gcc_assert:
-- >8 --
---
gcc/cp/pt.c | 16 +++-
gcc/testsuite/g++.dg/template/defarg22.C | 11 +++
2 files changed, 26 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/template/defarg22.C
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 03a8dfbd37c..849628840d6 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -13337,7 +13337,21 @@ tsubst_default_argument (tree fn, int parmnum, tree
type, tree arg,
if (parmtype == error_mark_node)
return error_mark_node;
- gcc_assert (same_type_ignoring_top_level_qualifiers_p (type, parmtype));
+ if (same_type_ignoring_top_level_qualifiers_p (type, parmtype))
+;
+ else
+{
+ /* The function type and the parameter type can disagree about the
+cv-qualification of the pointed-to type; see PR92010. */
+ gcc_assert (POINTER_TYPE_P (type) && POINTER_TYPE_P (parmtype)
+ && same_type_p (TREE_TYPE (type),
+ strip_top_quals (TREE_TYPE (parmtype;
+ /* Verify that this happens only when the dependent parameter type is a
+cv-qualified wildcard type. */
+ tree pattern_parm = get_pattern_parm (parm, DECL_TI_TEMPLATE (fn));
+ gcc_assert (WILDCARD_TYPE_P (TREE_TYPE (pattern_parm))
+ && cv_qualified_p (TREE_TYPE (pattern_parm)));
+}
tree *slot;
if (defarg_inst && (slot = defarg_inst->get (parm)))
diff --git a/gcc/testsuite/g++.dg/template/defarg22.C
b/gcc/testsuite/g++.dg/template/defarg22.C
new file mode 100644
index 000..cf6261916d8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/defarg22.C
@@ -0,0 +1,11 @@
+// PR c++/92010
+// { dg-do compile }
+
+template
+void foo(const T t = 0)
+{ }
+
+int main()
+{
+ foo();
+}
--
2.26.0.rc1.11.g30e9940356
[PATCH] c++: Fix ICE in tsubst_default_argument [PR92010]
This patch relaxes an assertion in tsubst_default_argument that exposes a latent
bug in how we substitute an array type into a cv-qualified wildcard function
parameter type. Concretely, the latent bug is that given the function template
template void foo(const T t);
one would expect the type of foo to be void(const int*), but we
(seemingly prematurely) strip function parameter types of their top-level
cv-qualifiers when building the function's TYPE_ARG_TYPES, and instead end up
obtaining void(int*) as the type of foo after substitution and decaying.
We still however correctly substitute into and decay the formal parameter type,
obtaining const int* as the type of t after substitution. But this then leads
to us tripping over the assert in tsubst_default_argument that verifies the
formal parameter type and the function type are consistent.
Assuming it's too late at this stage to fix the substitution bug, we can still
relax the assertion like so. Tested on x86_64-pc-linux-gnu, does this look OK?
gcc/cp/ChangeLog:
PR c++/92010
* pt.c (tsubst_default_argument): Relax assertion to permit
disagreements between the function type and the parameter type
about the cv-qualification of the pointed-to type.
gcc/testsuite/ChangeLog:
PR c++/92010
* g++.dg/template/defarg22.C: New test.
---
gcc/cp/pt.c | 17 -
gcc/testsuite/g++.dg/template/defarg22.C | 11 +++
2 files changed, 27 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/template/defarg22.C
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 9e1eb9416c9..923166276b8 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -13337,7 +13337,22 @@ tsubst_default_argument (tree fn, int parmnum, tree
type, tree arg,
if (parmtype == error_mark_node)
return error_mark_node;
- gcc_assert (same_type_ignoring_top_level_qualifiers_p (type, parmtype));
+ if (same_type_ignoring_top_level_qualifiers_p (type, parmtype))
+;
+ else if (POINTER_TYPE_P (type) && POINTER_TYPE_P (parmtype))
+{
+ /* The function type and the parameter type can disagree about the
+cv-qualification of the pointed-to type; see PR92010. */
+ gcc_assert (same_type_p (TREE_TYPE (type),
+ strip_top_quals (TREE_TYPE (parmtype;
+ /* Verify that this happens only when the dependent parameter type is a
+cv-qualified wildcard type. */
+ tree pattern_parm = get_pattern_parm (parm, DECL_TI_TEMPLATE (fn));
+ gcc_assert (WILDCARD_TYPE_P (TREE_TYPE (pattern_parm))
+ && cv_qualified_p (TREE_TYPE (pattern_parm)));
+}
+ else
+gcc_unreachable ();
tree *slot;
if (defarg_inst && (slot = defarg_inst->get (parm)))
diff --git a/gcc/testsuite/g++.dg/template/defarg22.C
b/gcc/testsuite/g++.dg/template/defarg22.C
new file mode 100644
index 000..cf6261916d8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/defarg22.C
@@ -0,0 +1,11 @@
+// PR c++/92010
+// { dg-do compile }
+
+template
+void foo(const T t = 0)
+{ }
+
+int main()
+{
+ foo();
+}
--
2.26.0.rc1.11.g30e9940356
