Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions changelog/toString.dd
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
`toString` Can Now Use Output Ranges

The standard library has been modified to recognize and use `toString` overloads
that accept $(REF_ALTTEXT output ranges, isOutputRange, std, range, primitives)
when such overloads exist.

-------
import std.range.primitives;
import std.stdio;

struct MyType
{
void toString(W)(ref W writer) if (isOutputRange!(W, char))
{
put(writer, "Custom toString");
}
}

auto t = MyType();
writeln(t); // writes "Custom toString"
-------

This has several benefits for the user. First, this design is much friendlier
to inlining than the `toString(scope void delegate(const(char)[]) sink)` method of
`toString`. Second, this cuts down on memory usage, as characters are placed right
into the output buffers of functions like $(REF format, std, format). Third,
because `toString` is now a template, can be marked `@safe` via inference much more
often.

All previous forms of `toString` will continue to work.
142 changes: 131 additions & 11 deletions std/format.d
Original file line number Diff line number Diff line change
Expand Up @@ -1735,10 +1735,22 @@ FormatSpec!Char singleSpec(Char)(Char[] fmt)
* `toString` should have one of the following signatures:
*
* ---
* const void toString(scope void delegate(const(char)[]) sink, FormatSpec fmt);
* const void toString(scope void delegate(const(char)[]) sink, string fmt);
* const void toString(scope void delegate(const(char)[]) sink);
* const string toString();
* void toString(W)(ref W w, const ref FormatSpec fmt)
* void toString(W)(ref W w)
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't have a public unittest showing this to the user. I know it's trivial, but I still would add one to avoid guaranteed questions on the NG.

* string toString();
* ---
*
* Where `W` is an $(REF_ALTTEXT output range, isOutputRange, std,range,primitives)
* which accepts characters. The template type does not have to be called `W`.
*
* The following overloads are also accepted for legacy reasons or for use in virtual
* functions. It's recommended that any new code forgo these overloads if possible for
* speed and attribute acceptance reasons.
*
* ---
* void toString(scope void delegate(const(char)[]) sink, const ref FormatSpec fmt);
* void toString(scope void delegate(const(char)[]) sink, string fmt);
* void toString(scope void delegate(const(char)[]) sink);
* ---
*
* For the class objects which have input range interface,
Expand All @@ -1756,7 +1768,7 @@ FormatSpec!Char singleSpec(Char)(Char[] fmt)
* Otherwise, are formatted just as their type name.
*
* Params:
* w = The $(REF_ALTTEXT output _range, isOutputRange, std,_range,primitives) to write to.
* w = The $(REF_ALTTEXT output range, isOutputRange, std,_range,primitives) to write to.
* val = The value to write.
* f = The $(REF FormatSpec, std, format) defining how to write the value.
*/
Expand Down Expand Up @@ -1917,13 +1929,52 @@ void formatValue(Writer, T, Char)(auto ref Writer w, auto ref T val, const ref F
}

/**
* Formatting of a `struct` with a defined `toString`.
* Formatting a struct by defining a method `toString`, which takes an output
* range.
*
* It's recommended that any `toString` using $(REF_ALTTEXT output ranges, isOutputRange, std,range,primitives)
* use $(REF put, std,range,primitives) rather than use the `put` method of the range
* directly.
*/
@safe unittest
{
import std.array : appender;
import std.range.primitives;

static struct Point
{
int x, y;

void toString(W)(ref W writer, const ref FormatSpec!char f)
if (isOutputRange!(W, char))
{
// std.range.primitives.put
put(writer, "(");
formatValue(writer, x, f);
put(writer, ",");
formatValue(writer, y, f);
put(writer, ")");
}
}

auto w = appender!string();
auto spec = singleSpec("%s");
auto p = Point(16, 11);

formatValue(w, p, spec);
assert(w.data == "(16,11)");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This public tests is failing, run with:

make -f posix.mak std/format.publictests

Copy link
Contributor

Choose a reason for hiding this comment

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

The line number on the error message is the one from the generated file.
(it probably makes sense to insert empty lines in it, s.t. the line numbers match up)

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'm not really sure how to debug this. This is green on the autotester

Copy link
Contributor

@wilzbach wilzbach Jan 9, 2018

Choose a reason for hiding this comment

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

  1. Compile Phobos
  2. Run the extracted test with Phobos-dev:
unittest
{
    import std.array : appender;
    import std.format;

    static struct Point
    {
        int x, y;

        void toString(W)(ref W writer, const ref FormatSpec!char f) if (isOutputRange!(W, char))
        {
            // std.range.primitives.put
            put(writer, "(");
            formatValue(writer, x, f);
            put(writer, ",");
            formatValue(writer, y, f);
            put(writer, ")");
        }
    }

    auto w = appender!string();
    auto spec = singleSpec("%s");
    auto p = Point(16, 11);

    formatValue(w, p, spec);
    assert(w.data == "(16,11)");
}

Copy link
Contributor

Choose a reason for hiding this comment

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

The issue is that w.data is Point(16, 11) here.


/**
* Another example of formatting a `struct` with a defined `toString`,
* this time using the `scope delegate` method.
*
* `formatValue` also allows to reuse existing format specifiers:
* $(RED This method is now discouraged for non-virtual functions).
* If possible, please use the output range method instead.
*/
@safe unittest
{
struct Point
static struct Point
{
int x, y;

Expand Down Expand Up @@ -3455,13 +3506,32 @@ if (is(AssocArrayTypeOf!T) && !is(T == enum) && !hasToString!(T, Char))
formatTest(e2, "[A, B, C]");
}

template hasToString(T, Char)
private template hasToString(T, Char)
{
static if (isPointer!T && !isAggregateType!T)
{
// X* does not have toString, even if X is aggregate type has toString.
enum hasToString = 0;
}
else static if (is(typeof(
{T val = void; FormatSpec!Char f; static struct S {void put(Char s){}}
S s; val.toString(s, f);
// force toString to take output range by ref
static assert(ParameterStorageClassTuple!(T.toString!(S))[0] == ParameterStorageClass.ref_);
static assert(ParameterStorageClassTuple!(T.toString!(S))[1] == ParameterStorageClass.ref_);
static assert(is(Parameters!(T.toString!(S))[1] == const));}
)))
{
enum hasToString = 6;
}
else static if (is(typeof(
{T val = void; static struct S {void put(Char s){}}
S s; val.toString(s);
static assert(ParameterStorageClassTuple!(T.toString!(S))[0] == ParameterStorageClass.ref_);}
)))
{
enum hasToString = 5;
}
else static if (is(typeof({ T val = void; FormatSpec!Char f; val.toString((const(char)[] s){}, f); })))
{
enum hasToString = 4;
Expand All @@ -3484,11 +3554,59 @@ template hasToString(T, Char)
}
}

@safe unittest
{
static struct A
{
void toString(Writer)(ref Writer w) if (isOutputRange!(Writer, string)) {}
}
static struct B
{
void toString(scope void delegate(const(char)[]) sink, FormatSpec!char fmt) {}
}
static struct C
{
void toString(scope void delegate(const(char)[]) sink, string fmt) {}
}
static struct D
{
void toString(scope void delegate(const(char)[]) sink) {}
}
static struct E
{
string toString() {return "";}
}
static struct F
{
void toString(Writer)(ref Writer w, const ref FormatSpec!char fmt) if (isOutputRange!(Writer, string)) {}
}

static assert(hasToString!(A, char) == 5);
static assert(hasToString!(B, char) == 4);
static assert(hasToString!(C, char) == 3);
static assert(hasToString!(D, char) == 2);
static assert(hasToString!(E, char) == 1);
static assert(hasToString!(F, char) == 6);
}

// object formatting with toString
private void formatObject(Writer, T, Char)(ref Writer w, ref T val, const ref FormatSpec!Char f)
if (hasToString!(T, Char))
{
static if (is(typeof(val.toString((const(char)[] s){}, f))))
enum overload = hasToString!(T, Char);

static if (overload == 6)
{
val.toString(w, f);
}
else static if (overload == 5)
{
val.toString(w);
}
// not using the overload enum to not break badly defined toString overloads
// e.g. defining the FormatSpec as ref and not const ref led this function
// to ignore that toString overload
Copy link
Member

Choose a reason for hiding this comment

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

Just a thought: to prevent similar breakages in the future, what about if the new unittest added above is changed to have non-trivial return values for the toString overloads, and the unittest actually checks for these values? The idea being that if a particular toString overload is ignored or not called correctly for whatever reason, the unittest would catch it.

else static if (is(typeof(val.toString((const(char)[] s){}, f))))
{
val.toString((const(char)[] s) { put(w, s); }, f);
}
Expand All @@ -3505,12 +3623,14 @@ if (hasToString!(T, Char))
put(w, val.toString());
}
else
{
static assert(0);
}
}

void enforceValidFormatSpec(T, Char)(const ref FormatSpec!Char f)
{
static if (!isInputRange!T && hasToString!(T, Char) != 4)
static if (!isInputRange!T && hasToString!(T, Char) < 4)
{
enforceFmt(f.spec == 's',
"Expected '%s' format specifier for type '" ~ T.stringof ~ "'");
Expand Down