Added writer version of toString to Nullable

[JackStouffer]

Also deprecates the delegate versions because

1. they're slower most of the time due to the inability to inline the delegate
2. they were already undocumented
3. they're rarely directly called by the user, and instead are called by things like to!string, which now automatically call the new overload.

[dlang-bot]

Thanks for your pull request, @JackStouffer!

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.

[wilzbach]

AFAICT you only need isOutputRange as a module-level import?

[wilzbach]

Needs the @@@DEPRECATED_2.086@@@ comment, s.t. it can be found.

[wilzbach]

Trailing whitespace.

[JackStouffer]

@wilzbach Looks like some sort of c library failure with dscanner.

[JackStouffer]

Also added overload which gives a string because it can be useful for simple code.

[MetaLang]

This can at least be made pure and @safe. Is there a reason you made this overload a template instead of a regular function and marking it with those attributes? Inference doesn't really buy us anything in this case because there are no template arguments that the attributes could depend on.

[JackStouffer]

The toString of _value can be anything.

[quickfur]

@MetaLang It can depend on template parameters on the containing struct.

It's actually a bad idea to put explicit attributes on templates, and members of templated aggregates, because that constrains what types the template can be instantiated with. It should be rather left up to the compiler to infer them, and unittests to enforce mandatory attributes for specific instantiations of the template.

[MetaLang]

Fair enough, but I don't think making it a no-arg template is necessary as it's inside a templated struct.

import std.array : appender;
import std.format;
import std.range;

struct Test()
{
    string toString()
    {
        auto app = appender!string();
        auto spec = singleSpec("%s");
        toString(app, spec);
        return app.data;
    }

    /// ditto
    void toString(W)(ref W writer, const ref FormatSpec!char fmt)
        if (isOutputRange!(W, char))
    {
        import std.range.primitives : put;
        put(writer, "Nullable.null");
    }
}

void main()
{
	import std.stdio;
    import std.traits;
   	writeln(hasFunctionAttributes!(Test!().toString, "@safe", "pure")); //Prints "true"
    writeln(!hasFunctionAttributes!(Test!().toString, "nothrow", "@nogc", "@system")); //Prints "false"
}

As you can see, the inference is done even though toString is not an explicitly templated function. Put in a call to an @system function and this will print "false" and "true" instead.

[JackStouffer]

@MetaLang Fixed

[quickfur]

This PR seems to have introduced a deprecation warning:

void main() {
        import std.typecons : Nullable;
        Nullable!int x;
}

Compile with dmd -unittest test.d:

/usr/src/d/phobos/std/format.d(3723): Deprecation: function `std.typecons.Nullable!int.Nullable.toString!().toString` is deprecated - To be removed after 2.086. Please use the output range overload instead.

Not sure where exactly the problem is, but basically it causes the deprecation warning to appear whenever user code uses Nullable and compiles with -unittest. Would be nice to fix this.

[quickfur]

@JackStouffer

[JackStouffer]

If it's in a unittest, it's a simple manner of marking the block as deprecated unittest, but that line number makes no sense. It's in a unittest block inside of std.format.

[quickfur]

I know it makes no sense, and I don't have time to dig into it right now, that's why I'm asking. :-D Otherwise I'd fix it myself.