Skip to content

Commit

Permalink
Fix #9811 Remove const from Tuple.toString (#10646)
Browse files Browse the repository at this point in the history
Now, member structs won't become const due to transitiveness and the
code in `std.format` can choose (erroneously) non-cost toString()
functions of member structs.

An expert has to decide whether this is worth the breaking change. Now,
code that uses `const` tuples and calls toString directly on them will
break. Alternatively, we could use
[Template this parameters](https://dlang.org/spec/template.html#template_this_parameter).

That way, we get a template parameter with the real type of this. Then we can
cast away constness of `this` when we now for certain that it isn't,
since `is(T != typeof(this))` (where T is the template this parameter).
Of course, this implies making toString `@trusted` too.

This might also lead to unforeseen bugs when `const` is cast away but
the member objects are actually const. I'm not sure how this works.

Fwiw, currently std.format and `std.conv: to` already intercepts const tuples,
thus it at least won't call toString directly. The breaking change will
only effect code that calls toString on const tuples directly.

That's why I have added non-prescritive tests. If something in
std.format changes, they'll be alerted and can then decide whether to
change the tests or whether this module also needs work, in case this
would lead a bigger breaking change.

Followup to #10645.
  • Loading branch information
Inkrementator authored Feb 22, 2025
1 parent 310bd9b commit 3a8f31a
Showing 1 changed file with 38 additions and 4 deletions.
42 changes: 38 additions & 4 deletions std/typecons.d
Original file line number Diff line number Diff line change
Expand Up @@ -1294,11 +1294,11 @@ if (distinctFieldNames!(Specs))
* Returns:
* The string representation of this `Tuple`.
*/
string toString()() const
string toString()()
{
import std.array : appender;
auto app = appender!string();
this.toString((const(char)[] chunk) => app ~= chunk);
toString((const(char)[] chunk) => app ~= chunk);
return app.data;
}

Expand All @@ -1320,14 +1320,14 @@ if (distinctFieldNames!(Specs))
* sink = A `char` accepting delegate
* fmt = A $(REF FormatSpec, std,format)
*/
void toString(DG)(scope DG sink) const
void toString(DG)(scope DG sink)
{
auto f = FormatSpec!char();
toString(sink, f);
}

/// ditto
void toString(DG, Char)(scope DG sink, scope const ref FormatSpec!Char fmt) const
void toString(DG, Char)(scope DG sink, scope const ref FormatSpec!Char fmt)
{
import std.format : format, FormatException;
import std.format.write : formattedWrite;
Expand Down Expand Up @@ -1822,6 +1822,40 @@ private template ReverseTupleSpecs(T...)
assert(t[0] == 10 && t[1] == "str");
assert(t.to!string == `Tuple!(int, string)(10, "str")`, t.to!string);
}
/* https://github.com/dlang/phobos/issues/9811
* Note: This is just documenting current behaviour, dependent on `std.format` implementation
* details. None of this is defined in a spec or should be regarded as rigid.
*/
{
static struct X
{
/** Usually, toString() should be const where possible.
* But as long as the tuple is also non-const, this will work
*/
string toString()
{
return "toString non-const";
}
}
assert(tuple(X()).to!string == "Tuple!(X)(toString non-const)");
const t = tuple(X());
// This is an implementation detail of `format`
// if the tuple is const, than non-const toString will not be called
assert(t.to!string == "const(Tuple!(X))(const(X)())");

static struct X2
{
string toString() const /* const toString will work in more cases */
{
return "toString const";
}
}
assert(tuple(X2()).to!string == "Tuple!(X2)(toString const)");
const t2 = tuple(X2());
// This is an implementation detail of `format`
// if the tuple is const, than non-const toString will not be called
assert(t2.to!string == "const(Tuple!(X2))(toString const)");
}
{
Tuple!(int, "a", double, "b") x;
static assert(x.a.offsetof == x[0].offsetof);
Expand Down

0 comments on commit 3a8f31a

Please sign in to comment.