Skip to content

Better static assertion on template contraints with 'models' #3677

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

Closed
wants to merge 2 commits into from

Conversation

atilaneves
Copy link
Contributor

This is similar to the work done on this PR, which is now dependent on the current one.

The idea is to have compiler error messages when a type that was intended to satisfy a template constraint but doesn't. Currently, when a template contraint isn't satisfied by accident, it's hard to know why. This PR attempts to make that process easier, with a similar intent as C++ concepts.

Also, I wanted to add a static assert if models is true but checkXXX doesn't compile; I didn't because I'd have to move the isFoo and checkFoo functions used in the accompanying unit tests to global scope.

* The template contraint predicate must start with the word $(D is)
* (e.g. $(D isInputRange)) and an associated template function
* with the "is" replaced by "check" should exist (e.g. $(D checkInputRange))
*/
Copy link
Member

Choose a reason for hiding this comment

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

Can you add the Params and Returns sections.

enum untilOpenParen = P.stringof[2 .. openParenIndex];
enum checkName = "check" ~ untilOpenParen;
enum mixinStr = checkName ~ "!(T, A);";
mixin("import " ~ moduleName!(P) ~ ";"); //make it visible first
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this entail massive code bloat as you're importing at least one module per check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only if it was already going to fail to compile, so no.

@atilaneves
Copy link
Contributor Author

@JackStouffer Will do
@MetaLang Code bloat how? First of all, that import only happens if the template contraint fails, so it won't compile anyway. Secondly, the import only makes the checkXXX function visible from std.traits; whoever is using models has to have imported the module where checkXXX is.

@DmitryOlshansky
Copy link
Member

The idea is to have compiler error messages when a type that was intended to satisfy a template constraint but doesn't. Currently, when a template contraint isn't satisfied by accident, it's hard to know why. This PR attempts to make that process easier, with a similar intent as C++ concepts.

Looks cute. Will review later ...

@schuetzm
Copy link
Contributor

It's ugly that in the UDA version you have to repeat the type's name...

@atilaneves
Copy link
Contributor Author

@schuetzm I agree, but there's nothing I can do about that.



///
unittest
Copy link
Member

Choose a reason for hiding this comment

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

Adding in another ddoc-ed unit test in front of this with range type check examples would be helpful as that's the most common use case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd have to change isInputRange first and I wanted to make the changes orthogonal.

@atilaneves
Copy link
Contributor Author

@JackStouffer That's not possible since isInputRange doesn't have a corresponding checkInputRange. That's done in a different PR.

{
bool models()
{
import std.algorithm;
Copy link
Member

Choose a reason for hiding this comment

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

import std.algorithm.searching : countUntil;

@JackStouffer
Copy link
Member

This sounds like something that would be good for a dub release. If it's popular and works then it can be reconsidered for Phobos.

@wilzbach
Copy link
Member

Looks cute. Will review later ... [more than 1 year ago]
It's ugly that in the UDA version you have to repeat the type's name...
This sounds like something that would be good for a dub release. If it's popular and works then it can be reconsidered for Phobos.

While I like the idea too, I am inclined to close this as there still seems to be no consensus after one year & going the DUB or DIP road make more sense than getting stalled here?

@ others: What do you think?

@atilaneves atilaneves closed this Jan 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants