On 9/14/21 00:31, Barrett Adair wrote:
I reworked the fix today based on feedback from Jason and Jakub (thank
you), and the subject line is now outdated. I added another test for a
closely related bug that's also fixed here (dependent-expr11.C -- this
one would even fail without the second declaration). All the new tests
in the patch succeed with the change (only two of them succeed with
trunk). On my box, the bootstrap succeeds, the g++ test suite passes
(matching today's posted results anyway), and the libstdc++ test suite
is looking good but is still running after a long time. I'll leave the
full "make check" running overnight.
Some potentially controversial changes here:
1. Adding new bool member to cp_parser. I'd like to avoid this, any tips?
2. Relaxing an assert in tsubst_copy. This change feels correct to me,
but maybe I'm missing something.
3. Pushing a function scope in PARM_DECL case in tsubst_copy_and_build
to make process_outer_var_ref happy for trailing return types. I don't
yet fully appreciate the consequences of these changes, so this needs
some eyes.
These all are to support dependent-expr11.C, right? This seems like a
separate issue, that should be a separate patch.
And I don't think there's anything special about a trailing return type.
I am surprised to discover that I don't see anything prohibiting that
use, but I similarly don't see anything prohibiting
template<class T> auto bar(T t, bool_c<t()>) -> bool_c<t()>;
or even
template <class T> using FP = void (*)(T t, int (*)[t()]);
So I guess the "use of parameter outside function body" code in
finish_id_expression_1 is obsolete with constexpr; removing that should
address #1.
One way to approach #2 might be to
begin_scope (sk_function_parms, NULL_TREE);
in tsubst_function_type, so that parsing_function_declarator (which I'm
about to check in) is true, and change the assert to also check that.
Maybe that will also help with #3. Really, outer_var_p should be false
for t, so we shouldn't ever get to process_outer_var_ref.
-----
OK, now for the part of the patch that corresponds to the subject line:
4. Traversing each template arg's tree in
any_template_arguments_need_structural_equality_p to identify dependent
expressions in trailing return types. This could probably be done
better. I check current_function_decl here as an optimization (since
it's NULL in the only place that "needs" this), but that seems brittle.
I think that optimization makes sense; within a function we shouldn't
need structural comparison, only for comparing two template declarations.
Also, the new find_dependent_parm_decl_r callback implementation may
have the unintended consequence of forcing structural comparison on
member function trailing return types that depend on class template
parameters. I think I really only want to force structural comparison
for "arg tree has a dependent parm decl and we're in a trailing return
type" -- is there a better way to do this?
I don't think whether the parm is dependent is important: the case we
want to catch is if the argument as a whole is dependent, and contains a
mention of a parameter.
Also note that I found another related bug which I have not yet solved:
template<int i>
struct foo {
constexpr operator int() { return i; }
};
void bar() {
[](auto i) -> foo<i> {
return {};
}(foo<1>{});
}
With the attached patch, failure occurs at invocation, while trunk fails
to parse the return type. This seems like a step in the right direction,
but we should consider whether such an incomplete fix introduces more
issues than it solves (e.g. unfriendlier error messages, or perhaps
something more sinister).
This would also be related to the separate change under 1-3 above.
Jason
