Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.

Conversation

MartinNowak
Copy link
Member

Add toString(sink) overload to Throwable, so that it can be used without the GC.

Copy link
Contributor

Choose a reason for hiding this comment

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

I hope, we'll remove this file in #482 :)

@ghost
Copy link

ghost commented Oct 15, 2013

I thought the plan was to remove stuff from the Object class? Note that if you already had this toString in your class you're now forced to make it virtual. It may or may not be an issue.. Since Object is rarely (if ever) used in user/library code I don't see why we need to introduce virtuals.

@MartinNowak
Copy link
Member Author

It's Throwable not Object.
The point here is that Throwable can't use the default Object.toString method because that requires a working GC. So instead this pull switches to a sink method.
Old code that only overrides string toString in exception classes will continue to work as before but this pull makes it possible to use custom messages even for uncatched Throwables.

@ghost
Copy link

ghost commented Oct 16, 2013

Ah, I misread. Sorry.

src/object_.d Outdated
Copy link

Choose a reason for hiding this comment

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

buf is dead code.

Copy link
Member Author

Choose a reason for hiding this comment

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

thx, done

@ghost
Copy link

ghost commented Oct 25, 2013

Other than the small issues this LGTM.

- Add toString(sink) overload to Throwable, so that
  it can be used without the GC.

- Rewrite string toString() in terms of the sink overload.
  Thus toString(sink) becomes the preferred method to
  override in derived Throwables.
ghost pushed a commit that referenced this pull request Oct 25, 2013
Add Throwable.toString(sink) overload
@ghost ghost merged commit 887f170 into dlang:master Oct 25, 2013
@ghost
Copy link

ghost commented Oct 25, 2013

Thanks.

@jacob-carlborg
Copy link
Contributor

The commit db7dc40 causes problems with unit test failures.

extern (C) int printf (in char*, ...);

unittest {
    printf("Reached unittest.\n");
    assert(0);
    printf("After failed assertion.\n");
}

void main ()
{

}

The above code should print:

Reached unittest
core.exception.AssertError@main(7): unittest failure

And a stack trace. After the above mentioned commit it only prints "Reached unittest.". See http://forum.dlang.org/thread/lgxkcylkpugpktpstpgh@forum.dlang.org#post-lgxkcylkpugpktpstpgh:40forum.dlang.org.

@jacob-carlborg
Copy link
Contributor

@ghost
Copy link

ghost commented Oct 26, 2013

Argh, so we had no stack-trace tests before? Even some partial string matching could suffice.

Anyway @dawgfoto, any ideas what went wrong?

denis-sh added a commit to denis-sh/druntime that referenced this pull request Oct 28, 2013
@MartinNowak MartinNowak deleted the sinkToStringThrowable branch October 30, 2013 15:14
MartinNowak added a commit to MartinNowak/druntime that referenced this pull request Oct 30, 2013
- The function printThrowable relied on the Throwable.toString(sink)
  overload from dlang#636 which is not available in 2.064.

- Fixed by readding the old exception print code.
MartinNowak added a commit to MartinNowak/druntime that referenced this pull request Oct 30, 2013
- The function printThrowable relied on the Throwable.toString(sink)
  overload from dlang#636 which is not available in 2.064.

- Fixed by re-adding the old exception print code.
andralex pushed a commit to andralex/druntime that referenced this pull request Dec 16, 2013
@complexmath
Copy link
Contributor

It looks like no one updated core.exception, so all the standard exceptions may have lost their custom error messages. The exceptions defined there all override toString() only. Also, what about exceptions defined in Phobos? Have any of those been updated to work with this new output routine?

@MartinNowak
Copy link
Member Author

It looks like no one updated core.exception, so all the standard exceptions may have lost their custom error messages.

AFAIR this only changed how uncatched exceptions were printed, they didn't show custom messages before because it's not possible to use toString without a GC.
I agree, that more exceptions should be changed to use the new sink overload.

@MartinNowak
Copy link
Member Author

Can you please file a bugzilla ticket for this?

@complexmath
Copy link
Contributor

Uncaught exceptions are handled before static module dtors are run and before the GC is terminated though. Custom messages should have worked up until this change. I'll file a ticket.

This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants