make formatThrowable() @safe

[WalterBright]

Moving towards making Object.toString() @safe

[JackStouffer]

Why are you assuming toString is going to be safe?

Won't this change allow this to be run?

class ThrowMe : Throwable
{
    char* seq;
    ubyte len;

    override string toString() @system
    {
        string ret;
        int i;
        auto start = seq;

        for (i = 0; i <= len; i++) // oops, used <=, buffer overflow
        {
            ret ~= start;
            ++start;
        }
        return ret;
    }
}

main() @safe
{
    auto c = new ThrowMe();
    formatThrowable(c, someSink);
}

[WalterBright]

Why are you assuming toString is going to be safe?

Assuming, no, driving it that direction, yes. All of Object's methods need to be safe, otherwise @safe programming is a bit of a joke.

[dnadlinger]

All of Object's methods need to be safe, otherwise @safe programming is a bit of a joke.

But they currently aren't – can you provide another justification for your assumption of Throwable.toString being safe to call? If not, the joke's on you.

[WalterBright]

But they currently aren't

They aren't marked @safe, correct. But I haven't found one yet that is not actually @safe or could be easily made so.

can you provide another justification for your assumption of Throwable.toString being safe to call? If not, the joke's on you.

It needs to be safe to call - if you agree that there's any point to @safe at all.

It's a bit like const-correctness. You have to start somewhere.

[JackStouffer]

Why not make this a template that can infer @safe-ty

[dnadlinger]

But I haven't found one yet that is not actually @safe or could be easily made so.

Then the first thing to do would be to mark Throwable.toString @safe (or rather, figuring out an acceptable transition process to do so). There is no reason why every single piece of potentially crappy code out there used in implementations out there should be @trusted all of a sudden.

"It surely needs to be safe to call, and I haven't seen otherwise" is exactly that kind of faith-based programming @safe D has been designed to supplant.

[dnadlinger]

Apart from the above, please also note that formatThrowable seems to be only used from internal backtrace printing code (rt_init/unittest runner/…). Thus, the change adds a safety violation to the code (and thus potential future hazard, etc.) for no benefit.

[jmdavis]

Well, the issues with forcing particular attributes like @safe or const are why it was decided some time ago that we should remove functions like toString from Object: https://issues.dlang.org/show_bug.cgi?id=9772

However, there hasn't been much done towards that. The only work that I'm aware of in that direction is the work that Martin has done towards replacing the AA implementation (without which, we can't do it), and the attempts to templatize opEquals that kept getting blocked by compiler bugs, and the current PR for it has just lingered: #1439. So, while a decision was made, it hasn't had much practical effect.

And unfortunately, even if we solve that problem for Object, we're still stuck with it with Throwable, since the nature of Throwable is such that we need toString (or something like it) on Throwable and thus have to then either force it to be @safe/@trusted or allow it to be @system and then have anything which prints exception would have to be @trusted.

Now, fortunately, I think that forcing @safe in this situation is probably far more tenable that forcing const tends to be. It at least seems like it would be unreasonable to have a Throwable with an @system toString that can't be @trusted, but it's also not all that uncommon that someone comes up with a valid use case that seems to defy what seems obvious.

Regardless, while I would expect that moving towards @safe is the right way to go, I'm not sure how we do it without @trusting code that is potentially very much not @safe - which is what this is doing. And as David points out, this is just internal stuff that we can likely afford to let be @system, even if that's not ideal.

There was talk at one point though of replacing toString to allow exceptions more freedom in how they generate the text to be printed (e.g. not having to allocate a string to print a message), which would allow us the opportunity to change how @safe is involved with Throwable without breaking code or @trusting @system code which potentially already exists - though depending on how that new mechanism worked, it might not be guaranteed to be @safe, since it would certainly allow for more freedom in how it was implemented, since that would be the whole point. But it does seem pretty bad if we can't have a way to guarantee that printing an exception's message is @safe.

[andralex]

Looks like a catch-22, and this doesn't seem like the best first step.

[MartinNowak]

Yes, and we'll keep getting stuck on those. There is a bigger general problem with attribute transitions, in particular on interfaces and non-final classes.
Think we need to come up with some deprecation mechanism for those.
Maybe the @future proposal (dlang/DIPs#55) might be a fit for that.

[WalterBright]

We'll get nowhere if we continue like this. One has to start making progress somewhere.

[dnadlinger]

Yes – except that this isn't progress. If necessary, we can ignore safety violations (which is what this change does) at the point where the function is called. You still haven't justified why that should be necessary, or how formatThrowable blocks making the Object function @safe.

[WalterBright]

It's progress because nobody that calls formatThrowable directly or via other functions can be made @safe. In trying to make Phobos @safe, I often run into A->B->C->D->A again. At some point some part in this circle has to be made @safe, because trying to make the entire circle @safe in one PR is impractical.

As I said before, it's just like the viral nature of making things const-correct.

If you have a better way, please let me know.

[dnadlinger]

So where is the circle broken by this PR? (Hint: I don't think there is one, as I explained back in #1758 (comment).)

[yebblies]

@WalterBright I think the ability to override Throwable.toString as @safe will make this redundant, right? Either that or making Throwable derive from SafeObject.

[dlang-bot]

Thanks for your pull request, @WalterBright!

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.

[RazvanN7]

This PR doesn't seem to be going anywhere, so I'm going to close. @WalterBright please reopen if you still think this should be pursued.