Skip to content

BetterC: make try-finally work without exceptions#7305

Merged
dlang-bot merged 1 commit intodlang:masterfrom
WalterBright:try-finally
Nov 15, 2017
Merged

BetterC: make try-finally work without exceptions#7305
dlang-bot merged 1 commit intodlang:masterfrom
WalterBright:try-finally

Conversation

@WalterBright
Copy link
Member

Instead of relying on the exception unwinding mechanism, this just calls the finally block. Since destructors and scope statements are rewritten to use try-finally, they'll work with this.

@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.

@ibuclaw
Copy link
Member

ibuclaw commented Nov 12, 2017

This seems to be a bit of an anti feature. Shouldn't you instead just detect if an eh region needs to be generated?

Also, libgcc supports handling of dwarf EH as a "pass-through" between C and interfacing languages.

@WalterBright
Copy link
Member Author

Shouldn't you instead just detect if an eh region needs to be generated?

It needs to be generated if there is RAII, and then the D personality routine is needed, which is in druntime.

Also, libgcc supports handling of dwarf EH as a "pass-through" between C and interfacing languages.

I know. And this should still work with that, it's just the destructors won't get called when unwinding is done.

@jpf91
Copy link
Contributor

jpf91 commented Nov 12, 2017

As this is a backend-only change I'm not too concerned about this, but I'd rather like to see an implementation which:

  • Detects in the frontend if everything in the try-block is nothrow.
  • Then removes the try block.
  • For BetterC, users then have to mark all functions as nothrow. (Could also add a compiler flag which marks all functions as nothrow)

This then also optimizes code like this:

void foo()
{
    scope(exit) checkErrors();
    //nothrow code goes here....
}

@jpf91
Copy link
Contributor

jpf91 commented Nov 12, 2017

Hmm, seems like #7304 proposes such statement level nothrow tracking. So wouldn't it make sense to use isNothrow on the try statement in this pull request as well?

@WalterBright
Copy link
Member Author

@jpf91 those sorts of optimizations will add a lot of value, and I intend to add them in the future. But they aren't trivial to implement, so one step at a time. Currently, not having RAII for BetterC is a major blocker, and this PR solves that issue.

Copy link
Member

@andralex andralex left a comment

Choose a reason for hiding this comment

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

macros suk

config.exe = EX_LINUX;
config.ehmethod = EH_DWARF;
config.ehmethod = betterC ? EH_NONE : EH_DWARF;
if (!exe)
Copy link
Member

Choose a reason for hiding this comment

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

Yellow in codecov? That's new. What does it mean?

#if SCPP
if (config.exe == EX_WIN32)
{ usednteh |= NTEH_try;
#endif
Copy link
Member

Choose a reason for hiding this comment

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

Oy this use of the preprocessor is quite nasty... far as I understand exactly one of MARS or SCPP must be defined. Then this is quite a bit easier on the eyes:

#if MARS
            if (config.ehmethod == EH_NONE)
            {
            }
            else
            if (config.ehmethod == EH_WIN32)
#else
            if (config.exe == EX_WIN32)
#endif

Copy link
Member Author

Choose a reason for hiding this comment

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

I avoid that style now because MARS isn't on-off. The back end supports multiple separate builds - SCPP, SPP, HTOD, and MARS.

Copy link
Member

Choose a reason for hiding this comment

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

All I'm saying is the code doesn't work unless exactly one of MARS and SCPP is defined, and that's difficult to infer from the 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.

I know, but in the rest of the code I removed the #else because of bugs and confusion about what code was for which build.

Copy link
Member Author

Choose a reason for hiding this comment

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

This particular piece of code is ugly, and I'm going to straighten it out in a follow-on, as it will require some refactoring. In the meantime, it gets the RAII working.

@dlang-bot dlang-bot merged commit 94cfa80 into dlang:master Nov 15, 2017
@WalterBright WalterBright deleted the try-finally branch November 15, 2017 05:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants