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 19672 - Function uses the stack to read a static array para… #9357

Merged
merged 1 commit into from Feb 13, 2019

Conversation

Projects
None yet
7 participants
@WalterBright
Copy link
Member

commented Feb 13, 2019

…meter but it is passed in the registers

@@ -4678,7 +4685,7 @@ void pushParams(ref CodeBuilder cdb, elem* e, uint stackalign, tym_t tyf)

default:
break;
}
}

This comment has been minimized.

Copy link
@thewilsonator

This comment has been minimized.

Copy link
@thewilsonator

thewilsonator Feb 13, 2019

Contributor

Oh thats aligned now. nvm

This comment has been minimized.

Copy link
@WalterBright

WalterBright Feb 13, 2019

Author Member

A formatting error that creeped in on another PR. Not worth it's own PR.

@WalterBright WalterBright force-pushed the WalterBright:fix19672 branch from 30f504c to ada5abb Feb 13, 2019

@dlang-bot

This comment has been minimized.

Copy link
Contributor

commented Feb 13, 2019

Thanks for your pull request, @WalterBright!

Bugzilla references

Auto-close Bugzilla Severity Description
19672 regression Function uses the stack to read a static array parameter but it is passed in the registers

Testing this PR locally

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

dub fetch digger
dub run digger -- build "stable + dmd#9357"

@dlang-bot dlang-bot added the Bug Fix label Feb 13, 2019

@thewilsonator

This comment has been minimized.

Copy link
Contributor

commented Feb 13, 2019

Please target stable. otherwise looks good to go.

@WalterBright

This comment has been minimized.

Copy link
Member Author

commented Feb 13, 2019

I don't think it is an actual regression. Those checks have always been missing.

fix Issue 19672 - Function uses the stack to read a static array para…
…meter but it is passed in the registers

@WalterBright WalterBright force-pushed the WalterBright:fix19672 branch from ada5abb to 40836d4 Feb 13, 2019

@WalterBright WalterBright changed the base branch from master to stable Feb 13, 2019

@ZombineDev

This comment has been minimized.

Copy link
Member

commented Feb 13, 2019

@WalterBright all bug fixes should preferably go to stable, not only regressions.

@WalterBright

This comment has been minimized.

Copy link
Member Author

commented Feb 13, 2019

@WalterBright all bug fixes should preferably go to stable

This is not practical. Bug fixes can be destabilizing.

@ZombineDev

This comment has been minimized.

Copy link
Member

commented Feb 13, 2019

That's what I meant by "preferably" - if they're not destabilizing.

@WalterBright

This comment has been minimized.

Copy link
Member Author

commented Feb 13, 2019

AppVeyor says:

Error: unrecognized switch '-preview=fieldwise'

but this PR does not use that switch.

@ZombineDev

This comment has been minimized.

Copy link
Member

commented Feb 13, 2019

I could agree that this particular bug fix is a bit more risky, so staying on master is ok.

@wilzbach

This comment has been minimized.

Copy link
Member

commented Feb 13, 2019

Looks like both Appveyor and buildkite fail to detect that the target branch is stable and incorrectly checkout phobos from master.

@wilzbach wilzbach merged commit c2c6b4b into dlang:stable Feb 13, 2019

6 of 8 checks passed

buildkite/dmd Build #4568 failed (2 minutes, 46 seconds)
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
CyberShadow/DAutoTest Documentation OK (no changes)
Details
auto-tester Pass: 10
Details
ci/circleci: no_pic Your tests passed on CircleCI!
Details
ci/circleci: pic Your tests passed on CircleCI!
Details
codecov/patch Coverage not affected when comparing 73fcf82...40836d4
Details
semaphoreci The build passed on Semaphore.
Details
@wilzbach

This comment has been minimized.

Copy link
Member

commented Feb 13, 2019

Merged as (1) there won't be a 2.084.2, but only 2.085 and (2) AppVeyor and Buildkite failure are unrelated (and everything else passed).

@WalterBright WalterBright deleted the WalterBright:fix19672 branch Feb 14, 2019

@WalterBright

This comment has been minimized.

Copy link
Member Author

commented Feb 14, 2019

Thank you @wilzbach

@nordlow

This comment has been minimized.

Copy link
Contributor

commented Feb 14, 2019

Why wasn't this merged into master aswell?

@wilzbach

This comment has been minimized.

Copy link
Member

commented Feb 14, 2019

Because it's a bug fix and ALL BUGFIXES SHOULD GO TO STABLE.

Also note that master will be merged into stable tomorrow anyhow (2.085 master/stable branch off is scheduled for 15.2).

@JinShil

This comment has been minimized.

Copy link
Contributor

commented Apr 16, 2019

This PR may have introduced a regression: https://issues.dlang.org/show_bug.cgi?id=19804

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.