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 fail invocations with ice_condition.raise in rustc #1928

Closed
brson opened this issue Mar 5, 2012 · 11 comments
Closed

Replace fail invocations with ice_condition.raise in rustc #1928

brson opened this issue Mar 5, 2012 · 11 comments
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. P-low Low priority

Comments

@brson
Copy link
Contributor

brson commented Mar 5, 2012

We should see if conditions can suit our purposes; we may not be able to get rid of every fail! invocation that rustc can reach, but it would not hurt to try.


Original title: Replace fail with session.bug in rustc

When possible rustc should be using session.bug or session.span_bug instead of fail because that prints out proper error messages.

@brson
Copy link
Contributor Author

brson commented Mar 5, 2012

Or, if it's a real error condition of course it should use fatal or error

@marijnh
Copy link
Contributor

marijnh commented Mar 6, 2012

I really think we should wire things up in a way that allows us to print out the error message from normal failure. Passing sessions to things that otherwise don't take sessions, just to be able to call session.bug, doesn't sound attractive. Not to mention that we're also calling library routines that can fail.

@brson
Copy link
Contributor Author

brson commented Mar 6, 2012

I'm inclined to agree. We need failure and logging to be more powerful.

@catamorphism
Copy link
Contributor

Is it reasonable to consider this bug "postponed until error/logging infrastructure improves"?

@pnkfelix
Copy link
Member

  1. The documentation in the comment above "pub fn monitor" in rustc.rc was quite helpful for me to get my head around our current setup.
  2. brson says we can probably close this, as "its probably not worth the effort to expunge all calls to fail! at this point"; however, marijnh's comment makes me pause before closing. Perhaps this represents some sort of weakness, if there really is not a way to make at least a best-effort attempt to extract the string passed to fail! (if any) from the parent task of the failing subtask...

@graydon
Copy link
Contributor

graydon commented Apr 24, 2013

This is an ideal candidate for conditions.

pnkfelix added a commit to pnkfelix/rust that referenced this issue May 6, 2013
@pnkfelix
Copy link
Member

pnkfelix commented May 6, 2013

I've got a sketch started over here: https://github.com/pnkfelix/rust/commits/issue1928-add-ice-condition

(Most of the redundancies that I identified while doing it come from known problems in our macro and condition system.)

@pnkfelix
Copy link
Member

pnkfelix commented Jun 3, 2013

Note that some issues with conditions may need to be resolved first; e.g. #5446.

@thestinger
Copy link
Contributor

This is still an open issue.

@catamorphism
Copy link
Contributor

Low, not 1.0. (Can be closed if we remove conditions.)

@alexcrichton
Copy link
Member

Conditions have been removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. P-low Low priority
Projects
None yet
Development

No branches or pull requests

7 participants