Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[New DIP] Multiple Template constraints #131

Open
wants to merge 41 commits into
base: master
Choose a base branch
from

Conversation

thewilsonator
Copy link
Contributor

@thewilsonator thewilsonator commented Aug 8, 2018

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.

@mdparker
Copy link
Member

mdparker commented Aug 9, 2018

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

@thewilsonator
Copy link
Contributor Author

Done

@thewilsonator
Copy link
Contributor Author

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

That is `if` is to static foreach and static assert what `in` is to
foreach and assert.
@thewilsonator
Copy link
Contributor Author

thewilsonator commented Aug 17, 2018

The spec diff assumes that dlang/dlang.org#2446 has been merged. It has.

@thewilsonator
Copy link
Contributor Author

thewilsonator commented Aug 28, 2018

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
Copy link
Contributor Author

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

@yshui
Copy link
Contributor

yshui commented Aug 28, 2018

@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.

@thewilsonator
Copy link
Contributor Author

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.

@yshui
Copy link
Contributor

yshui commented Aug 28, 2018

@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".

@thewilsonator
Copy link
Contributor Author

thewilsonator commented Aug 29, 2018

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

@thewilsonator
Copy link
Contributor Author

thewilsonator commented Aug 29, 2018

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

@thewilsonator
Copy link
Contributor Author

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.

@jmh530
Copy link

jmh530 commented Sep 11, 2018

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

@thewilsonator
Copy link
Contributor Author

thewilsonator commented Sep 12, 2018

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
Copy link
Contributor Author

@andralex

DIPs/DIP1xxx.md Outdated Show resolved Hide resolved
DIPs/DIP1xxx.md Outdated Show resolved Hide resolved
Copy link
Member

@schveiguy schveiguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love the whole idea. I thought that perhaps we could simply highlight the failing parts in the boolean expression, but this provides so much better power AND gives the compiler easy ability to show the line if that's all that's needed. Only downside compared to parsing the existing format out is that you have to change your constraints to take advantage.

DIPs/DIP1xxx.md Outdated Show resolved Hide resolved
DIPs/DIP1xxx.md Outdated Show resolved Hide resolved
DIPs/DIP1xxx.md Outdated Show resolved Hide resolved
DIPs/DIP1xxx.md Outdated Show resolved Hide resolved
DIPs/DIP1xxx.md Outdated Show resolved Hide resolved
@thewilsonator
Copy link
Contributor Author

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.

@mleise
Copy link

mleise commented Jan 15, 2019

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.

@thewilsonator
Copy link
Contributor Author

The new Dlangoliers tool

What?

@mleise
Copy link

mleise commented Jan 15, 2019

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.

DIPs/DIP1xxx.md Outdated Show resolved Hide resolved
DIPs/DIP1xxx.md Outdated
bool all(Range)(Range range)
if (isInputRange!Range)
if (is(typeof(unaryFun!pred(range.front))),
"`" ~ pred.stringof[1..$-1] ~ "` isn't a unary predicate function for range.front"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This variant isn't equivalent to the two below, right? Presenting three forms of the same equivalent contracts would help comparing them.

DIPs/DIP1xxx.md Outdated Show resolved Hide resolved
Co-Authored-By: thewilsonator <thewilsonator@users.noreply.github.com>
@mdparker
Copy link
Member

@thewilsonator Please email me!

@schveiguy
Copy link
Member

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?

@jmh530
Copy link

jmh530 commented Mar 31, 2020

@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.

@schveiguy
Copy link
Member

@jmh530 Thanks, that does look like it needs work. But if fixed, I still am wondering if this DIP is relevant.

At the least, it needs updating, as some of the rationale no longer applies.

@PetarKirov
Copy link
Member

@schveiguy One year later I still think we have a very long way to go in terms template error messages. See my previous comment for more details: #131 (comment)

@schveiguy
Copy link
Member

schveiguy commented Mar 31, 2020

@PetarKirov agreed. One of the most frustrating things is, if you fail something like isInputRange, but it's not obvious it should fail, there's no way to make the compiler spit out what's failing. Even if there was something like __traits(explainWhy, !isInputRange!T) that you could call with pragma(msg), it would go a long way.

At the same time, this DIP doesn't seem to address any of that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants