allow custom error message in template constraints

[WebFreak001]

Constraints like isInputRange can now return a custom struct that casts to bool and defines a toString method. When the expression evaluates to false, (returns false in the opCast) the toString method is evaluated and printed with the template error message when a constraint does not match.

The future syntax / semantics is backwards compatible with current dmd.

Just an idea, thought of this because of this forum thread: https://forum.dlang.org/thread/[email protected]

Example with input ranges using the range_primitives_helper library:

import std.stdio;
import std.string;
import range_primitives_helper;

import std.range :
	origIsInputRange = isInputRange,
	origIsRandomAccessRange = isRandomAccessRange
;

struct ConstraintInfo
{
	bool matches;
	string errorMessage;

	bool opCast() const { return matches; }
	string toString() const { return errorMessage; }
}

enum isInputRange(T) = ConstraintInfo(origIsInputRange!T, isInputRangeErrorFormatter!T.replace("\n", "\n\t"));
enum isRandomAccessRange(T) = ConstraintInfo(origIsRandomAccessRange!T, isRandomAccessRangeErrorFormatter!T.replace("\n", "\n\t"));

void fun(T)(T t) if(isInputRange!T && !isRandomAccessRange!T) {}

void fun(T)(T t) if(isRandomAccessRange!T) {}

struct Sample1
{
	auto front() @property { return null; }
	bool empty = true;
}

struct Sample2
{
	auto front() @property { return null; }
	bool empty = true;
	void popFront() {}
}

void test()
{
	Sample1 s1;
	Sample2 s2;
	fun(s1);
	fun(s2);
}

outputs:

Performing "debug" build using ../dmd/generated/linux/release/64/dmd for x86_64.
range_primitives_helper 1.0.0: target for configuration "library" is up to date.
test_inputrange ~master: building configuration "application"...
source/app.d(43,5): Error: template `app.fun` cannot deduce function from argument types `!()(Sample1)`
source/app.d(22,6):        Candidates are: `fun(T)(T t)`
  with `T = Sample1`
  must satisfy the following constraint:
`       isInputRange!T: Sample1 is not an InputRange because:
                the function 'popFront' does not exist`
source/app.d(24,6):                        `fun(T)(T t)`
  with `T = Sample1`
  must satisfy the following constraint:
`       isRandomAccessRange!T: Sample1 is not an RandomAccessRange because
                the function 'popFront' does not exist
                and the property 'save' does not exist
                and must allow for array indexing, aka. [] access`
../dmd/generated/linux/release/64/dmd failed with exit code 1.

What could be improved: the indentation should probably be made on the dmd side instead of in this example in the code, but I would like to suggest the change as-is first.

[dlang-bot]

Thanks for your pull request, @WebFreak001!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#13511"

[pbackus]

1. This requires a DIP.
2. Any user-defined type that can be cast to bool should be allowed, not just a struct.
3. There are a number of possible "interfaces" the compiler could use to convert the result to a string:
   • Access a property with a specific name (such as toString).
   • Cast the value to a string (which would invoke opCast!string).
   • Access a property with a specific UDA (like core.attribute.errorMessage).

I was actually planning to write a DIP for this once I was finished with DIP 1038 (@mustUse), so I'd be happy to discuss possible designs and/or help with the work of writing up a proposal.

[WebFreak001]

does this really require a DIP? The change is backwards compatible (new code still works the same with old compilers, as opCast!bool was already used) and the changes don't change the behavior of the resulting program, it's just optional improvements for the error messages.

[maxhaton]

There are many things the compiler could use, but given the intended use case toString or maybe a defined cast seems sufficient, no?

[pbackus]

Of course, it's ultimately up to @WalterBright and @atilaneves whether a DIP is required, but plenty of DIPs involve only backwards-compatible changes, and this feature will require a change to the language spec to document which method/property/operator the compiler uses to create the error message. So it's not obvious that a DIP isn't required.

I also think that going through the DIP review process with this will likely result in a better design, although that's just my own opinion.

[ljmf00]

Thanks! I look forward to seeing this implemented on the compiler, especially to improve complex library templated structures. I think we should discuss further how we use the existing protocols to implement this. Is toString the best choice? As pointed by @pbackus, there are some other choices we have to put on the table. I think a DIP can trigger much more community discussion and end up following a better path here.

[pbackus]

There are many things the compiler could use, but given the intended use case toString or maybe a defined cast seems sufficient, no?

The main argument against requiring a specific method name (be it toString or opCast!string) is that it's possible the user will want to use that method name for something else (e.g., they might want toString to produce output like ConstraintInfo(false, "It didn't work") which is better for writeln-debugging). The fact that toString is already recognized by Phobos exacerbates this issue, since it means there is always a potential "something else" competing for usage of that method name.

The main argument in favor of requiring a specific method name (instead of using a UDA) is that it saves you from writing an import.

Compare:

struct FirstVersion {
    bool result;
    string toString;
    bool opCast(T)() if (T == bool) { return result; }
}

import core.attribute: errorMessage;
struct SecondVersion {
    bool result;
    @errorMessage string message;
    bool opCast(T)() if (T == bool) { return result; }
}

It's not a huge deal either way, but there is a tradeoff here.

[burner]

hm, I'm torn. I like this PR, but I do not like template constraints, so I think we shouldn't add something that makes using them easier.

[ljmf00]

hm, I'm torn. I like this PR, but I do not like template constraints, so I think we shouldn't add something that makes using them easier.

What do you mean? Can you elaborate on your opinion about template constraints? Sometimes they are needed to distinguish overloads and there are some structs in Phobos that heavily use such features. This will radically improve those. The example I see from the top of my head is SumType, almost every templated range and the containers. For example, everyone that tinkers enough with SumType knows that, due to its complexity, the error messages are far from great and this PR can improve that substantially.

[burner]

@ljmf00 sure, have a read here https://github.com/burner/range_primitives_helper/blob/master/README.md

[ljmf00]

@ljmf00 sure, have a read here burner/range_primitives_helper@master/README.md

I can't see the benefit of your approach. Using static if to simulate constraint overloads is just a workaround to a language feature. Plus, that makes the end-user write more unclean code. The reason for those constraints is to make overloading possible in a clean way. It is also worth mentioning that the presented approach in the compiler using some sort of protocols won't need additional changes in the user code, just on the standard library side.

[pbackus]

@ljmf00 I should point out that this proposal will not actually help with SumType—the errors that are hidden there are ones that occur inside __traits(compiles), not template constraints.

To improve SumType's errors, we would need to enhance __traits(compiles) to return an "info" struct similar to the one described here.

[ljmf00]

@ljmf00 I should point out that this proposal will not actually help with SumType—the errors that are hidden there are ones that occur inside __traits(compiles), not template constraints.

To improve SumType's errors, we would need to enhance __traits(compiles) to return an "info" struct similar to the one described here.

Ok, I thought that most of the constraints were defined using the overload constraint syntax. Well, we can design something that can also accommodate __traits(compiles) too structure-wise with some defined protocol, the same as we have with range protocols.

[adamdruppe]

I'm kinda of the opinion that the error message should be string[] but I suppose you can .join something. Bu my thought is if you eventually did make __Traits(compileS) return one there might be multiple errors.

[radcapricorn]

I tried to raise some attention to this five years ago: https://forum.dlang.org/thread/[email protected]

So, don't show this PR to Steven :D But it's sure nice to see this being thought of, at least.