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

fix Issue 10966 - Optimizer generates wrong code with try-catch #11541

Merged
merged 1 commit into from
Aug 10, 2020

Conversation

WalterBright
Copy link
Member

2 line fix.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @WalterBright!

Bugzilla references

Auto-close Bugzilla Severity Description
10966 blocker Optimizer generates wrong code with try-catch

⚠️⚠️⚠️ Warnings ⚠️⚠️⚠️

To target stable perform these two steps:

  1. Rebase your branch to upstream/stable:
git rebase --onto upstream/stable upstream/master
  1. Change the base branch of your PR to stable

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#11541"

@thewilsonator
Copy link
Contributor

Stable.

@WalterBright
Copy link
Member Author

It's a 7 year old bug.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

There is a need for official rules on when it is required to target stable or master. In the meantime let's take this as-is

@ghost ghost added the Merge:auto-merge label Aug 10, 2020
@dlang-bot dlang-bot merged commit 1da8a5e into dlang:master Aug 10, 2020
@MoonlightSentinel
Copy link
Contributor

MoonlightSentinel commented Aug 10, 2020

There are official rules ... but they get ignored again and again like this.

@Geod24
Copy link
Member

Geod24 commented Aug 10, 2020

Regression or critical bug fixes should always target the stable branch. Learn more about rebasing to stable or the D release process.

I mean the bug even write it. EVERY. TIME.

@ghost
Copy link

ghost commented Aug 10, 2020

"should" VS "must" or "have to"

@ghost
Copy link

ghost commented Aug 10, 2020

"it's mandatory for a .... to target the stable branch".

@wilzbach
Copy link
Member

wilzbach commented Aug 10, 2020

There is a need for official rules on when it is required to target stable or master. In the meantime let's take this as-is

There are official rules - https://wiki.dlang.org/DIP75#Branching_strategy
It's right in there:

TL;DR Safe fixes should always go into stable, all the time.

That's why there are so many ⚠️⚠️⚠️ in the automated bot message.

We stopped merging Walter's PRs in the past until they are targeting stable and that actually worked for a while.
It's literally just a one-line command to rebase to stable and the bot even tells one how to.

@ghost
Copy link

ghost commented Aug 10, 2020

as we are collaborating on a compiler people have to take care of the semantics. "should" denotes something optional. It does not constitute a rule that a community has to follow. "should" is a gentle and polite invitation but leaves the door opened.

@ghost
Copy link

ghost commented Aug 10, 2020

let's discuss and put it in the contribution guidlines.

@WalterBright
Copy link
Member Author

Changes always come with a risk, meaning "stable" can become destabilized. With ancient bugs, there isn't a critical need to put the fix into stable, making me reluctant to risk the instability. Especially things like changing the optimizer, which always has risk to it.

@WalterBright WalterBright deleted the fix10966 branch August 11, 2020 02:11
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.

6 participants