Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implement DIP1008 enabling @nogc for 'throw new Throwable;' #6681

Merged
merged 1 commit into from
Dec 10, 2017

Conversation

WalterBright
Copy link
Member

It adds the -dip1006 to enable it. Of course, throwing that switch won't work until the druntime end is done.

The idea is to recognize throw new Throwable and call _d_newThrowable() instead of _d_newclass(). When caught, add a call to _d_delThrowable() at the end of the catch block.

The druntime will use malloc and free for these. Reference counting is used to handle rethrowing. It should mix naturally with Throwables allocated via other means, like static and the GC.

@thewilsonator
Copy link
Contributor

Please update ddmd/declaration.h as well

@WalterBright
Copy link
Member Author

Please update ddmd/declaration.h as well

Already there.

@ibuclaw
Copy link
Member

ibuclaw commented Apr 10, 2017

Already there.

Maybe they meant expression.h

@WalterBright
Copy link
Member Author

done

@mathias-lang-sociomantic
Copy link
Contributor

DIP1006 ? Thats not DIP1006.

@mdparker
Copy link
Member

Given the state of the DIP PR queue, this is likely going to be DIP 1008.

@WalterBright
Copy link
Member Author

Yes, I mistakenly thought 1006 was unassigned. I'll update the PR when assigned a proper number.

@WalterBright WalterBright changed the title implement DIP1006 enabling @nogc for 'throw new Throwable;' implement DIP1008 enabling @nogc for 'throw new Throwable;' Apr 23, 2017
@WalterBright WalterBright force-pushed the dip1006 branch 2 times, most recently from d1535cd to 869f27e Compare April 24, 2017 03:41
@wilzbach
Copy link
Member

wilzbach commented May 4, 2017

For reference: the link to DIP 1008 is https://github.com/dlang/DIPs/blob/master/DIPs/DIP1008.md

@wilzbach
Copy link
Member

wilzbach commented May 4, 2017

@WalterBright how about adding an actual example that ensures that one can throw in @nogc? Once #6742 is merged, CodeCov should start to complain again ;-)

@WalterBright
Copy link
Member Author

Needs druntime PR to be pulled first.

@JackStouffer
Copy link
Member

Just manually checked. Looks like the cycle is real, but it only exists in version(unittest).

@WalterBright
Copy link
Member Author

Looks like the cycle is real,

Sorry, posted my comment in the wrong place. Moved it here: dlang/phobos#4712

@dlang-bot
Copy link
Contributor

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.

@andralex
Copy link
Member

OK, time to move this along. Rebase first of all?

@WalterBright
Copy link
Member Author

rebased

ibuclaw
ibuclaw previously requested changes Nov 5, 2017
@@ -0,0 +1,20 @@
// PERMUTE_ARGS:
Copy link
Member

Choose a reason for hiding this comment

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

Missing argument to actually test the dip.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

Copy link
Member Author

Choose a reason for hiding this comment

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

but it'll never work until the druntime changes are pulled, so I disabled it

Copy link
Contributor

Choose a reason for hiding this comment

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

That's actually a good thing, it will prevent this P.R. from being pulled before it's ready.

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Which came first, the chicken or the egg?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants