DIPs icon indicating copy to clipboard operation
DIPs copied to clipboard

[New DIP] Multiple Template constraints

Open thewilsonator opened this issue 5 years ago • 33 comments

This is a DIP to enable the compiler to emit better error messages for constrained template overloads and better document the constraints when reading the source. Essentially if constraints become the static equivalent of in contracts (assert and foreach become static assert and static foreach). Expression (á la DIP1009 but for constraints) and block statement forms are allowed.

~The static foreach section is to deal with constraints expressed over template variable arguments, as these are currently expressed as recursive templates which do not give nice error messages with the base DIP. The source is still able to convey the meaning with the constrains with messages.~ This is superseded by block constraints.

thewilsonator avatar Aug 08 '18 04:08 thewilsonator

Thanks, @thewilsonator. Please set the status to Draft.

mdparker avatar Aug 09 '18 03:08 mdparker

Done

thewilsonator avatar Aug 09 '18 03:08 thewilsonator

Sorry for the dev in queue, but I am going to change this somewhat because I've had a better idea.

thewilsonator avatar Aug 16 '18 13:08 thewilsonator

~The spec diff assumes that https://github.com/dlang/dlang.org/pull/2446 has been merged.~ It has.

thewilsonator avatar Aug 17 '18 03:08 thewilsonator

@mdparker this should be ready to go.

thewilsonator avatar Aug 26 '18 07:08 thewilsonator

@thewilsonator I don't quite like this solution. For one thing, this is going to cause tons of error message duplication.

yshui avatar Aug 27 '18 21:08 yshui

Please give an example.

If what you say is what I think you mean then this shouldn't be a problem because the messages can be factored out into an enum

enum msg(T)  = "`"~T.stringof~"`: Must be a range";

auto rangeAlgorithm1(R)(R r)
if (isInputRange!R, msg!R)
{
    ...
}

auto rangeAlgorithm2(R)(R r)
if (isInputRange!R, msg!R)
{
    ...
}

Note also that the message is optional, so for self explanatory things like isInputRange!R, I expect the message will be omitted.

thewilsonator avatar Aug 27 '18 23:08 thewilsonator

@thewilsonator I think that in order to have good error messages, we need to have an interface for traits like isInputRange(R) to output information to the user in case they return false. Yes, splitting the a long constraint expression (either automatically in the compiler, or by allowing the user to do it) is good, but even when you reach a logic experession atom (e.g. instantiation of a trait) it can be quite confusing both for the user and author of the template. Say we need an array-like type constraint. For the purpose of the example, let's assume that this would be a random access range with length and slicing and that we have an enum bool trait in phobos that tests if a type satisfies all those requirements. When the author of a function template uses this constraint, he would not have what else to say in the custom error message, except for "R needs to be a random access range with length and slicing". However if the user of that function template passes a range, whose length property is something else than size_t (say cent or BigInt) he will have a hard time figuring out what the problem is:

void foo(R)(R array)
if (isArrayLike!R, "`R` needs to be a random access range with length and slicing")
if (is(ElementType!R : const(char)[]), "`R` must be a range of `char` arrays")
{ /* ... */ } 

What I would like is to have a way for the fictional isArrayLike trait report the exact reason why R doesn't pass the constraint - e.g. R may be a forward range with indexing, but for some reason without a popBack implementation.

Of course the author of foo could manually split the isArrayLike trait (e.g. by using individual traits), so he could use a custom error message for each part of isArrayLike that may fail, but that would be a very suboptimal. What I would like is for isArrayLike to output an error message which would be concatenation of all error messages, that the traits it's internally made of would output. That way the author of foo won't even have to bother with writing an error message for isArrayLike, while it would still be preferable for him to write a custom message for the element type one:

void foo(R)(R array)
if (isArrayLike!R) // use the err msg phobos devs wrote
if (is(ElementType!R : const(char)[]), "`R` must be a range of `char` arrays")
{ /* ... */ } 

PetarKirov avatar Aug 28 '18 07:08 PetarKirov

~All we need do is, as we re-evaluate the constraint to print not satisfied: ..., if the constraint failed, reevaluate it again ungaged with speculative errors on with a flushed template cache.~ Scratch that, -verrors=spec has horrid SNR.

thewilsonator avatar Aug 28 '18 08:08 thewilsonator

If there are multiple inviable candidates which one do you print the diagnostics for? That is a hard problem. This DIP is a large step in the direction of narrowing down the relevant information because it increases the granularity of the constraint.

thewilsonator avatar Aug 28 '18 08:08 thewilsonator

@thewilsonator

because the messages can be factored out into an enum

Then presumably each constraint template will have its own error message enum paired with it. If that is the case, why don't we put error messages in the constraint template itself? And as @ZombineDev said, that will have added benefit of more relevant error messages.

yshui avatar Aug 28 '18 09:08 yshui

BTW, a better name for this DIP would probably be something like "Template constraint diagnostics", because current title doesn't state the real motivation of this DIP.

yshui avatar Aug 28 '18 09:08 yshui

Then presumably each constraint template will have its own error message enum paired with it. You missed the part about the message being optional.

why don't we put error messages in the constraint template itself?

What good does that do? You already know that the template is false. What Petar was talking about was reporting the exact reason why the template returned false. The best that the compiler can do is run any CNF parts of the template, which this DIP says nothing about and does not preclude from happening.

thewilsonator avatar Aug 28 '18 09:08 thewilsonator

You will note that the DIP is called "Expression and Block Statement Template Constraints".

thewilsonator avatar Aug 28 '18 09:08 thewilsonator

@thewilsonator I'm not Peter, but I think that's not what he was talking about.

if (isArrayLike!R) // use the err msg phobos devs wrote

So off the top of my head, I came up with a way to show what I meant. The following is just an example, and probably not a very good idea:

Assume isArrayLike returns something like this:

struct BoolWithErr {
    bool b;
    string error;
    alias b this;
}

And in isArrayLike, there can be something like this:

template isArrayLike(T) {
    ...
    else static if (!is(typeof(T.init.popBack)))
        enum isArrayLike = BoolWithErr(false, T.stringof~" does not have popBack");
    else
    ...
}

Then, when isArrayLike returns falsy, the compiler can report the attached error message, thus give the user a better idea why it fails.

You will note that the DIP is called "Expression and Block Statement Template Constraints".

The main motivation and benefit of this DIP is to have clearer and more understandable error messages, not having a template constraint block. IMO this title still does not convey that.

yshui avatar Aug 28 '18 10:08 yshui

That works fine for single template constraints, its even a good idea, but it is orthogonal to this DIP even if it is made feasible by it. However, in the presence of any logicals with an alias this <bool>; the message will get discarded.

The main motivation and benefit of this DIP is to have clearer and more understandable error messages, not having a template constraint block. IMO this title still does not convey that.

It is consistent with the rest of the DIPs whose title describes what, and the rationale describes why.

thewilsonator avatar Aug 28 '18 11:08 thewilsonator

@thewilsonator

but it is orthogonal to this DIP even if it is made feasible by it.

Yes. Having multiple if's and if blocks itself is a good idea. I should've said "this solution is missing parts that would make it more practical", rather than "this is not a good solution".

yshui avatar Aug 28 '18 12:08 yshui

~~[OT] Hahaha, https://issues.dlang.org/show_bug.cgi?id=19203 ~~ This got fixed.

thewilsonator avatar Aug 29 '18 04:08 thewilsonator

@yshui @ZombineDev Ahh, you mean like https://run.dlang.io/is/yzotl4 (will work as a static assert argument once https://github.com/dlang/dmd/pull/8634 goes in).

thewilsonator avatar Aug 29 '18 05:08 thewilsonator

Hmm actually it might be better to do something like

template isInputRange(T)
{
     @("is(typeof(R.init) == R)") enum canInit = is(typeof(R.init) == R);
     @("is(ReturnType!((R r) => r.empty) == bool") enum hasEmpty = is(ReturnType!((R r) => r.empty) == bool;
    ...
     enum bool isInputRange = canInit && hasEmpty && ...;
}

to save on compile time.

Anyway, point being, this is all this is all independent of the DIP.

thewilsonator avatar Aug 29 '18 07:08 thewilsonator

@thewilsonator For better error messages, what about Atila's concepts library?

jmh530 avatar Sep 11 '18 15:09 jmh530

Thanks, I had completely forgotten about that.

Constraints of the form

void checkFoo(T)()
{
    T t = T.init;
    t.foo();
}
enum isFoo(T) = is(typeof(checkFoo!T));

Would be easy to give nice errors for (now that I've seen how concepts does it), but I'm wary of proscribing in this DIP what must happen beyond the grammar since concepts and this DIP solve the same problem (better error messages) but in a different problem space: Concepts are for range/container writers validating that their code is e.g. an input range. Whereas this DIP is for algorithm writers (and their users) to validate the constraints of the algorithm, this includes non-modelling-type constraints like making sure predicates are valid.

Put another way if/when this goes in there are a number of possible solutions to the problem of more information about unsatisfied constraints (which is now much more granular than before) and I don't want to make that decision up front because I don't know what the other solutions would look like. I do like the concepts solution though.

thewilsonator avatar Sep 12 '18 02:09 thewilsonator

@andralex

thewilsonator avatar Jan 13 '19 23:01 thewilsonator

Only downside compared to parsing the existing format out is that you have to change your constraints to take advantage.

Such is the price of progress.

thewilsonator avatar Jan 14 '19 05:01 thewilsonator

Only downside compared to parsing the existing format out is that you have to change your constraints to take advantage.

The new Dlangoliers tool will take care of it and eat up your past constraints.

mleise avatar Jan 15 '19 12:01 mleise

The new Dlangoliers tool

What?

thewilsonator avatar Jan 15 '19 12:01 thewilsonator

I thought after Voldemort types, we'd recreate Dfix with a Stephen King reference. Breaking changes or just new methods to do old things mean that at some point someone (or a tool) will go over the code and remove traces of the old ways. Langoliers are creatures that erase the past by literally eating it up.

mleise avatar Jan 15 '19 12:01 mleise

@thewilsonator Please email me!

mdparker avatar Mar 31 '20 12:03 mdparker

It's been a while. In the meantime the constraint error reporting has gotten much better:

void foo(T, U)(T t, U u) if (is(T == int) && is(U == int)) {}

void main()
{
    foo("hello", 4);
}
onlineapp.d(5): Error: template onlineapp.foo cannot deduce function from argument types !()(string, int), candidates are:
onlineapp.d(1):        foo(T, U)(T t, U u)
  with T = string,
       U = int
  must satisfy the following constraint:
       is(T == int)

Is this DIP still necessary?

schveiguy avatar Mar 31 '20 13:03 schveiguy

@schveiguy As an aside, I am still finding some cases where the template deduction error messages could use some improvement. I just filed this for the case of template parameter default values.

jmh530 avatar Mar 31 '20 15:03 jmh530