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

Comments

Projects
None yet
7 participants
@brson
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

This comment has been minimized.

Show comment
Hide comment
@brson

brson Mar 5, 2012

Contributor

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

Contributor

brson commented Mar 5, 2012

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

@marijnh

This comment has been minimized.

Show comment
Hide comment
@marijnh

marijnh Mar 6, 2012

Contributor

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.

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

This comment has been minimized.

Show comment
Hide comment
@brson

brson Mar 6, 2012

Contributor

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

Contributor

brson commented Mar 6, 2012

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

@catamorphism

This comment has been minimized.

Show comment
Hide comment
@catamorphism

catamorphism Mar 8, 2012

Contributor

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

Contributor

catamorphism commented Mar 8, 2012

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

@pnkfelix

This comment has been minimized.

Show comment
Hide comment
@pnkfelix

pnkfelix Apr 23, 2013

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...
Member

pnkfelix commented Apr 23, 2013

  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

This comment has been minimized.

Show comment
Hide comment
@graydon

graydon Apr 24, 2013

Contributor

This is an ideal candidate for conditions.

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

This comment has been minimized.

Show comment
Hide comment
@pnkfelix

pnkfelix May 6, 2013

Member

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.)

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

This comment has been minimized.

Show comment
Hide comment
@pnkfelix

pnkfelix Jun 3, 2013

Member

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

Member

pnkfelix commented Jun 3, 2013

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

@thestinger

This comment has been minimized.

Show comment
Hide comment
@thestinger

thestinger Aug 5, 2013

Contributor

This is still an open issue.

Contributor

thestinger commented Aug 5, 2013

This is still an open issue.

@catamorphism

This comment has been minimized.

Show comment
Hide comment
@catamorphism

catamorphism Oct 24, 2013

Contributor

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

Contributor

catamorphism commented Oct 24, 2013

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

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton Mar 6, 2014

Member

Conditions have been removed.

Member

alexcrichton commented Mar 6, 2014

Conditions have been removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment