>> 2. I left the <cstdlib> include in system.h, because removing it will
>> result in errors due to an inclusion of <algorithm>. It's probable
>> that both can be removed, but I didn't test it with this patch.
>
>
> You mean you don't need <algorithm> anymore in logic.cc? I think we want
> the <cstdlib> include in general if we're going to support people using the
> C++ standard library.
I don't. Those decisions are above my pay grade, so I'm doing my best
not to make them.
>> + reason = template_unification_error_rejection ();
>
>
> Can't you do
>
> template_constraint_failure (DECL_TI_TEMPLATE (fn), DECL_TI_ARGS (fn))
I can indeed. Fixed.
>> if (fn == error_mark_node)
>> {
>> + if (!check_template_constraints (tmpl, targs))
>> + reason = template_constraint_failure (tmpl, targs);
>> + else if (errorcount+sorrycount == errs)
>> /* Don't repeat unification later if it already resulted in errors.
>> */
>
>
> A constraint failure is a form of unification failure, like SFINAE; let's
> handle it in that context rather than separately here, and reserve
> rr_constraint_failure for the case of a non-template member function.
And emit the diagnostics in fn_type_unification when explain_p is set?
Seems reasonable.
> The uses of "actual template" in the comment and function name seem to mean
> different things. Maybe call the function template_decl_for_candidate?
Fixed.
> Can friend temploids be constrained?
I have not thought deeply about constrained friend declarations. What
would a friend temploid look like?
> Seems like the comment is no longer accurate; the function now only returns
> NULL_TREE if both a and b are NULL_TREE.
It's not. And I removed disjoin_requirements. I don't think it will
ever be used.
>> +static tree
>> +get_constraint_check (tree call)
>
> The comment needs updating. And the function isn't used anywhere; it seems
> redundant with resolve_constraint_check.
I removed the function.
>> +// and other template nodes from generating new expressions
>> +// during instantiation.
>
> I'm not clear on the issue. Perhaps leaving processing_template_decl alone
> and using fold_non_dependent_expr would accomplish what you want?
I don't think that will work. The problem comes from the instantiation
of traits (and some other expressions) during constraint checking.
When processing_template_decl is non-zero, finish_trait_expr will
create TRAIT_EXPR nodes, which aren't handled by the constexpr
evaluation engine.
I could updated constexpr to fix that, or changed finish_trait_expr
(and others? noexcept maybe) to evaluate for non-dependent arguments,
but I was trying to limit the scope of my changes.
I'd really like for this structure to go away. It's very fragile.
>> +// Check the template's constraints for the specified arguments,
>> +// returning true if the constraints are satisfied and false
>> +// otherwise.
>> +bool
>> +check_template_constraints (tree t, tree args)
>> +{
>> + return check_constraints (get_constraints (t), args);
>> +}
>
>
> Seems like the last function would also work for types and non-template
> decls.
It should. Updated the comment to reflect the broader applicability of
the function.
>> + error_at (loc, "constraints not satisfied");
>
>
> I assume you're planning to make this more verbose? It could use a TODO to
> that effect.
Very much so. Added comments.
>> + else if (are_constrained_overloads (newdecl, olddecl))
>> + {
>> + // If newdecl and olddecl are function templates with the
>> + // same type but different constraints, then they cannot be
>> + // duplicate declarations. However, if the olddecl is declared
>> + // as a concept, then the program is ill-formed.
>> + if (DECL_DECLARED_CONCEPT_P (DECL_TEMPLATE_RESULT (olddecl)))
>> + {
>> + error ("cannot specialize concept %q#D", olddecl);
>> + return error_mark_node;
>> + }
>> + return NULL;
>> + }
>
>
> We should check for differing constraints in decls_match, and then this code
> should be in the !types_match section of duplicate_decls.
I'll get back to you on this. I need to work my way through that code.
>
>> error ("concept %q#D result must be convertible to bool", fn);
>
>
> Why don't we require the result to actually be bool? It seems difficult to
> decompose dependent requirements if the return type can be something else.
I'm probably sitting somewhere between two ideas. Requiring the result
to be exactly bool is fine.
>> + cp_lexer_next_token_is_keyword (parser->lexer,
>> RID_REQUIRES))
>> {
>> tree reqs = release (current_template_reqs);
>> - if (tree r = cp_parser_requires_clause_opt (parser))
>> + if (tree r = cp_parser_requires_clause (parser))
>
>
> I was thinking to change the name of cp_requires_clause to
> cp_parser_requires_clause_opt and change the assert to a test for the
> keyword and return NULL_TREE if it isn't found.
I see. That is cleaner.
>> + // FIXME: we should be checking the constraints, not just
>> + // instantiating them.
>
>
> Let's check them in determine_specialization, along with the SFINAE check
> after the comment
> /* Make sure that the deduced arguments actually work. */
Okay. I'm not sure where the diagnostics for those failures fit in
yet. I've left a TODO note.
> Can explicit specializations have constraints, to indicate which template
> they are specializing?
Good question. I don't think so. I believe that it would be a
specialization of the most specialized function template whose
constraints were satisfied. So:
template<Arithmetic T> void f(T x); // #1
template<Integral T> void f(T x); // #2
template<> void f(double); // Specializes #1
template<> void f(int); // Specializes #2
I haven't tested this yet, but it seems like it would follow from the
usual overload resolution rules.
>> +// Argument deduction ios applied to all template arguments, and
>
>
> Passing 'true' for require_all_args means no deduction will be done; rather,
> all arguments must either be specified or have default arguments.
I see. It seems like I should be passing false here, since I want to
ensure that the resulting argument list can be used to instantiate the
template.
>> + // Check class template requirements. Note that constraints will
>> + // already have been checked when trying to find a most specialized
>> + // class among multiple specializations.
>
>
> I don't see where that is; more_specialized_class doesn't seem to compare
> constraints yet.
It doesn't. I have to rethink some aspects of how partial
specialization work. I'm also waiting until specializations get
TEMPLATE_DECs.
Checking specializations will probably happen in get_class_bindings.
That doesn't seem to get called when there are no specializations.
>> + // FIXME: Do we need to attach constraints?
>> + // TODO: Is this actually necessary?
>
>
> I don't think you need to fill in TI_CONSTRAINT for anything but the main
> template patterns, here or other places.
Agreed.
>> +substitute_requirements (tree reqs, tree args)
>
> This seems redundant with instantiate_requirements.
It is. I'm using it for constraint diagnosis in a separate version,
but I may not need it. I've removed the function for now.
I'll look over the decls_match code and try to get a revised patch out tomorrow.
Andrew
