Comment #0 by joseph.wakeling — 2020-09-05T19:13:13Z
The following minimal example shows the problematic behaviour. A struct is defined with a custom toString that follows the recommendations and examples in std.format. However, there is a bug in the code: the `isOutputRange` template check is used but never imported.
```
struct S
{
int n;
void toString (Output) (ref Output output)
if (isOutputRange!(Output, char))
{
import std.format : formattedWrite;
output.formattedWrite!"%s"(2 * this.n);
}
}
void main ()
{
import std.stdio : writeln;
auto s = S(2);
// should output `4` if output-range toString is used,
// but will output `S(2)` if not
writeln(s);
}
```
This code compiles and runs fine, and produces the default struct formatting output:
S(2)
... instead of the custom toString expected output:
4
... so, the custom toString is being silently ignored. I assume the reason is that the `if (isOutputRange!(Output, char))` check evaluates to false because the `isOutputRange` symbol is not defined, so the toString implementation is never resolved. This is very unintuitive, as one would expect compilation to fail because of the undefined `isOutputRange` symbol.
Given that this templated `toString` form is the standard recommendation for how to write custom string formatting, it seems important that user implementation bugs like the one above should be clearly exposed -- not silently elided as described here.
Comment #1 by moonlightsentinel — 2020-09-05T21:50:21Z
Nope, this is an issue with the toString detection in phobos. dmd reports an error if the template constraint fails to compile (see https://run.dlang.io/is/9g2V1G).
The problem is that phobos uses speculative compilation to detect if a struct defines toString - which swallows the aforementioned error message.
Comment #2 by simen.kjaras — 2020-09-05T22:18:18Z
All of the below cases are instances of this same issue:
struct S1 {
string toString()() {
nonExistent();
}
}
struct S2 {
void toString(R)(ref R r) if (isOutputRange!(R, char)) {}
}
struct S3 {
import std.range : isOutputRange;
void toString(R)(ref R r) if (isOutputRange!(R, char)) {
nonExistent();
}
}
unittest {
import std.conv : to;
assert(S1().to!string == "S1()");
assert(S2().to!string == "S2()");
assert(S3().to!string == "S3()");
}
formatValueImpl in std.format already tests for @disabled toString - it should also test for instantiability.
Comment #3 by dlang-bot — 2020-09-05T22:19:17Z
@Biotronic created dlang/phobos pull request #7617 "Fix issue 21228 - Templated toString silently ignored when it cant be…" fixing this issue:
- Fix issue 21228 - Templated toString silently ignored when it cant be instantiated
https://github.com/dlang/phobos/pull/7617
Comment #4 by joseph.wakeling — 2020-09-06T12:32:38Z
One thing I don't understand here: why is the missing symbol not detected _anyway_ regardless of whether any attempt is made to instantiate the template or not? This feels like a more general problem where bugs in templates remain unflagged by the compiler if the templated code is not used.
It strikes me that if it's possible to address that, it might address the observed problem more robustly than just addressing how phobos consumes the errors.
Comment #5 by simen.kjaras — 2020-09-06T14:41:03Z
A missing toString is not an error - struct S {} should print.
One issue is there's umpteen different possible signatures for toString, and we don't know which was intended, so we could just be calling it wrong. Once we've figured out none of the possible ways is correct, we can say for sure that something's wrong, but not exactly what.
We could introduce something like __traits(getErrors, somecode) that would return the resulting error messages from an invocation, but 1) we don't have that, and 2) even if we did, inspecting error messages to figure out what's wrong is error-prone, and the exact text of the error messages may change in the future (at least until issue 10335 is implemented).
Perhaps a better option would be something like __traits(errorLocation, someInstantiation), that would return "body", "arguments", "template arguments", or "template constraints", depending on where the instantiation fails. I'm still not sold on that, though.
Comment #6 by bugzilla — 2020-12-05T09:43:08Z
(In reply to Joseph Rushton Wakeling from comment #4)
> The problem is that phobos uses speculative compilation to detect if a struct defines toString - which swallows the aforementioned error message.
What writeln() could do is first speculatively check to see that the symbol toString exists, and if it's there then compile it for keeps, rather than speculatively checking to see if calling it compiles.
That way it won't be silently swallowing the errors in compiling the toString call.
Re-categorizing as a problem in the std.stdio implementation of writeln().
A way to see if a symbol exists is to do something like:
__traits(compiles, alias xxx = toString);
Comment #7 by robert.schadek — 2024-12-01T16:37:35Z