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 all uses of fail with die!, then remove fail and rename die! to fail! #4524

Closed
bstrie opened this issue Jan 17, 2013 · 14 comments
Closed
Labels
A-frontend Area: Compiler frontend (errors, parsing and HIR) A-syntaxext Area: Syntax extensions E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Milestone

Comments

@bstrie
Copy link
Contributor

bstrie commented Jan 17, 2013

fail is becoming a macro as per #3627, but first all uses of the fail keyword must be removed. There's a new macro named die! that can do the job of fail. Make the switch, then remove the fail keyword from the parser and mass-rename die! to fail!.

@nickdesaulniers
Copy link

Can I get dibs on this?

@bstrie
Copy link
Contributor Author

bstrie commented Jan 18, 2013

@nickdesaulniers you can!

@nickdesaulniers
Copy link

Just an update on the state of this bug:

Step 1: Replace instances of fail with die!(), DONE TESTS GREEN
Step 2: Remove fail keyword, DONE TESTS GREEN
Step 3: Mass replace die!() with fail!(), IN PROGRESS

This will be three separate commits, many changes, will probably take some time to review. I also missed a few instances of fail from step one but caught them in step 2. Tests are still green in both commits. Hope this is not an issue. Do all of these commits need to be rebased?

@bstrie
Copy link
Contributor Author

bstrie commented Jan 31, 2013

Actually, I may have overlooked one step. You may need to wait for a new snapshot before you can complete step 3, because the stage0 compiler currently has no idea what the die!() macro is so it'll just choke. For now, I think you'll need to back up to steps 1 and 2, and then insert a step 2.5 where you define the die!() macro as an identical alternative to fail!(), but don't actually use the new macro anywhere. Only once a new snapshot has been produced with knowledge of die!() can you then actually begin using it.

@nickdesaulniers
Copy link

@brson, so duplicate die macro as fail macro in libsyntax/ext/expand.rs? I'm closing pull request #4721 , will issue a new one.

@bstrie
Copy link
Contributor Author

bstrie commented Feb 1, 2013

Actually, until the fail keyword is entirely removed from the stage0 compiler it will probably yell at you even for having the fail! macro defined at all. So you really need to take these steps:

  1. Convert all fail to die!
  2. Remove fail as a keyword
  3. Wait for a snapshot
  4. Add fail! macro
  5. Wait for a snapshot
  6. Rename all die! to fail!

The joys of self-hosting!

@brson
Copy link
Contributor

brson commented Feb 1, 2013

I think it can be done in one snap

  1. Convert fail calls to die!
  2. Remove fail
  3. Duplicate die! as fail! (but don't use it)
  4. Snapshot
  5. Replace die! with fail!
  6. Remove die!

@nickdesaulniers
Copy link

Pull request #4744 was just merged. Waiting on a snapshot (@brson). In the meantime, I think all new code based on top of this is going to have to use die!() for a short time, then start using fail!(). Do all rust devs need to be aware of this...?

@Dretch
Copy link
Contributor

Dretch commented Feb 2, 2013

Could fail just be a regular function? What magic does it do that requires it to be a syntax-extension/macro/magical-!-thing?

Edit: Never mind I found some explanations here: #2232

@nickdesaulniers
Copy link

@brson , is pull request #4895 bit rotted? I can rebase my patch and update it.

@nickdesaulniers
Copy link

Completely forgot to remove the definition of die in expand.rs

@bstrie
Copy link
Contributor Author

bstrie commented Feb 15, 2013

Can this be closed?

@nickdesaulniers
Copy link

Yes, but I can't close it. I'm part of the Mozilla organization, but I don't have permissions for this repo.
@brson

@graydon graydon closed this as completed Feb 15, 2013
@brson
Copy link
Contributor

brson commented Feb 15, 2013

Thanks, @nickdesaulniers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-frontend Area: Compiler frontend (errors, parsing and HIR) A-syntaxext Area: Syntax extensions E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

No branches or pull requests

5 participants