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

Replace intrinsics::abort with process::abort #16680

Merged
merged 1 commit into from May 1, 2017
Merged

Conversation

@mbrubeck
Copy link
Contributor

mbrubeck commented May 1, 2017

This removes some unsafe/unstable code and replaces it with a new safe/stable alternative.

Note that process::abort is not identical to intrinsics::abort, since it runs global cleanups to do things like flush stderr (though neither function performs stack unwinding). I don't think the difference matters for our use cases.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • There are tests for these changes OR
  • These changes do not require tests because they should not change functionality

This change is Reviewable

@jdm
Copy link
Member

jdm commented May 1, 2017

@bors-servo
Copy link
Contributor

bors-servo commented May 1, 2017

📌 Commit a239419 has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented May 1, 2017

Testing commit a239419 with merge 52920ed...

bors-servo added a commit that referenced this pull request May 1, 2017
Replace intrinsics::abort with process::abort

This removes some unsafe/unstable code and replaces it with a new safe/stable alternative.

Note that `process::abort` is not identical to `intrinsics::abort`, since it runs global cleanups to do things like flush stderr (though neither function performs stack unwinding).  I don't *think* the difference matters for our use cases.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because they should not change functionality

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16680)
<!-- Reviewable:end -->
@mbrubeck mbrubeck mentioned this pull request May 1, 2017
90 of 99 tasks complete
@bors-servo
Copy link
Contributor

bors-servo commented May 1, 2017

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-msvc-dev
Approved by: jdm
Pushing 52920ed to master...

@bors-servo bors-servo merged commit a239419 into servo:master May 1, 2017
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
@mbrubeck mbrubeck deleted the mbrubeck:abort branch May 2, 2017
@jdm jdm mentioned this pull request May 16, 2017
3 of 3 tasks complete
bors-servo added a commit that referenced this pull request May 16, 2017
Revert "Replace intrinsics::abort with process::abort"

This reverts #16680. Calling `process::abort` from the crash handler causes the crash handler to be invoked again recursively.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #16898

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16899)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.