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.

Sign up for GitHub

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

Jump to bottom

make asm diagnostic instruction optional #59242

Merged
merged 1 commit into from
Mar 25, 2019
Merged

make asm diagnostic instruction optional #59242

merged 1 commit into from
Mar 25, 2019

Conversation

euclio
Copy link
Contributor

@euclio euclio commented Mar 16, 2019

DiagnosticInfoInlineAsm::getInstruction may return a null pointer, so
the instruction shouldn't be blindly unwrapped.

Reopening from #55193. I was unable to trigger the assertion on Windows after rebasing.

Fixes #23458.
Fixes #55216.

@rust-highfive
Copy link
Collaborator

r? @cramertj

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 16, 2019
@cramertj
Copy link
Member

r? @nagisa

@rust-highfive rust-highfive assigned nagisa and unassigned cramertj Mar 18, 2019
@nagisa
Copy link
Member

nagisa commented Mar 20, 2019

@bors r+

@bors
Copy link
Contributor

bors commented Mar 20, 2019

📌 Commit 8d64f9c52257f328e2c5986f55a2f86157886dac has been approved by nagisa

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 20, 2019
@kennytm kennytm mentioned this pull request Mar 24, 2019
Merged
@kennytm
Copy link
Member

kennytm commented Mar 24, 2019

@bors r-

This PR caused failure in #59397 (comment). Please restrict the new test on x86 and x86_64 only.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 24, 2019
@euclio
make asm diagnostic instruction optional
4728433 
`DiagnosticInfoInlineAsm::getInstruction` may return a null pointer, so
the instruction shouldn't be blindly unwrapped.
@euclio
Copy link
Contributor Author

euclio commented Mar 24, 2019

Added an only-x86_64 directive. The test seemed to always be ignored if I added a only-x86 directive as well.

@kennytm
Copy link
Member

kennytm commented Mar 24, 2019

@bors r=nagisa

@bors
Copy link
Contributor

bors commented Mar 24, 2019

📌 Commit 4728433 has been approved by nagisa

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 24, 2019
@bors
Copy link
Contributor

bors commented Mar 25, 2019

⌛ Testing commit 4728433 with merge 3f36ac4...

bors added a commit that referenced this pull request Mar 25, 2019
@bors
Auto merge of #59242 - euclio:asm-ice, r=nagisa
3f36ac4 
make asm diagnostic instruction optional

`DiagnosticInfoInlineAsm::getInstruction` may return a null pointer, so
the instruction shouldn't be blindly unwrapped.

Reopening from #55193. I was unable to trigger the assertion on Windows after rebasing.

Fixes #23458.
Fixes #55216.
@bors
Copy link
Contributor

bors commented Mar 25, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: nagisa
Pushing 3f36ac4 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 25, 2019
@bors bors merged commit 4728433 into rust-lang:master Mar 25, 2019
@euclio euclio deleted the asm-ice branch March 25, 2019 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@nagisa nagisa

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@euclio @rust-highfive @cramertj @nagisa @bors @kennytm