removal of fail keyword Issue #4524 #4721
Conversation
| fail fmt!("The .times method expects a nonnegative number, \ | ||
| but found %?", self); | ||
| die!(fmt!("The .times method expects a nonnegative number, \ | ||
| but found %?", self)); |
There was a problem hiding this comment.
Unrelated observation here, but I wonder how hard it would be to make die! act like debug! et al with respect to having an implicit fmt! inside of it.
|
Referencing Issue #4524 |
|
Heh, I've never before seen a patch so long that Github refuses to show its complete diff. Looks good, though I'm not qualified to comment on the part about ripping out the |
|
Awesome! Thanks for taking the time to review it for me @bstrie! I appreciate it! |
|
There were some big merge conflicts (there have been a lot of wide-reaching changes going in lately), so I've pushed just the first commit converting to The next step is to rebase your branch onto mozilla/incoming and fix all the merge conflicts ... and everything will be a conflict :-D. @nickdesaulniers would you mind trying to do that? It's not going to be pleasant, and I'm sorry about that, but it's the nature of these big syntax changes. |
|
That's ok, I need some experience with merge conflict resolution. So I think I've done it right. Either 100% correct or 100% wrong. Building now. Seeing warnings when building src/libcore/private/finally.rs L35 warning: argument 1 uses by-copy mode. Not sure that I caused that. Will have an incoming pull request shortly, hopefully! |
|
@nickdesaulniers The warning is unrelated. Before we do a snapshot you will need to create a duplicate definition of |
Removed fail keyword. 2 of the 3 steps here. The followup pull request will be replacing die! with fail! after a new snapshot is created. The first commit removes almost all invocations of fail with die!, the rest are caught in the second after fail is removed from the lexer & parser. Tests are green after applying the first or both patches. I'd like to separate these from the final commit that involves a new snapshot. Heads up @bstrie & @brson.