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<typename T> void foo(const T t);
one would expect the type of foo<int[]> 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<int[]> 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 <case 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 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<typename T>
void foo(const T);
int main() { foo<int[]>(0); }
because the DECL_ARGUMENTS of foo<int[]> 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<typename... Ts>
void foo(const Ts...);
int main() { foo<int[]>(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<int[]> 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 <class T> void spam(decltype([]{}) (*s)[sizeof(T)]) {}
int main() { spam<char>(nullptr); }
According to tsubst_function_decl in current trunk, the type of the
function paremeter 's' of spam<char> 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 TYPE_ARG_TYPES from
DECL_ARGUMENTS if they don't match.
Jason
