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

workaround Issue 20906 - unnecessary divide-by-zero errors when const… #3130

Closed
wants to merge 1 commit into from

Conversation

WalterBright
Copy link
Member

…ant folding short circuits

Doesn't look like this bug will be resolved anytime soon, see dlang/dmd#11252

In the meantime, need to move forward with dlang/dmd#11236

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @WalterBright!

Bugzilla references

Auto-close Bugzilla Severity Description
20906 normal unnecessary divide-by-zero errors when constant folding short circuits

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 + druntime#3130"

@WalterBright
Copy link
Member Author

Baffling log file errors that have seemingly no connection with this PR.

@MoonlightSentinel
Copy link
Contributor

The azure failure is an unrelated (it's a phobos bug that rarely manifests inside of run.d).

Copy link
Member

@Geod24 Geod24 left a comment

Choose a reason for hiding this comment

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

This doesn't make sense to me. The only reason to pull this would be to accept the pending changes to inline, which would mean accepting introducing a regression, which is a big no-no in everyone's book.
And Buildkite shows us that it triggers in a few projects.

@WalterBright
Copy link
Member Author

The azure failure is an unrelated (it's a phobos bug that rarely manifests inside of run.d).

Is there a bugzilla for it? Any fix?

@WalterBright
Copy link
Member Author

which would mean accepting introducing a regression

How so?

@MoonlightSentinel
Copy link
Contributor

Is there a bugzilla for it? Any fix?

Working on it, gonna open a PR soon

@Geod24
Copy link
Member

Geod24 commented Jun 10, 2020

How so?

If you fix the bug, we can pull your DMD PR and this workaround becomes useless.
If you don't fix the bug, we shouldn't pull your DMD PR, and there's no point in pulling this workaround either.

@WalterBright
Copy link
Member Author

If you don't fix the bug, we shouldn't pull your DMD PR

Why?

If you fix the bug

I could use some help with that (finding the piece of code that causes it).

@kinke
Copy link
Contributor

kinke commented Jun 12, 2020

In any case, such an ugly workaround should be restricted to DMD (version (DigitalMars)); no reason to pessimize LDC etc. because of a DMD limitation.

@WalterBright
Copy link
Member Author

This can be closed when dlang/dmd#11252 passes.

@kinke this function has likely never been inlined, otherwise the failure would have showed up before.

@kinke
Copy link
Contributor

kinke commented Jun 14, 2020

this function has likely never been inlined

Just because it wasn't for DMD doesn't mean it wasn't/isn't with LDC - if it isn't clear by now, LDC does not use the DMD AST inliner at all.

@WalterBright
Copy link
Member Author

No longer needed.

@WalterBright WalterBright deleted the workaround20906 branch June 16, 2020 06:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
7 participants