Fix float bug in help suggestion for incorrect tuple indexing #28247
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
|
@nikomatsakis By the way, some changes from an earlier pull request of mine found their way into this PR branch, so I had to change them back. |
|
You should squash those commits to simplify the history. |
|
Yes, defiantly! |
|
Can you add a testcase? I'm always torn about tests that have specific error msgs, but it seems appropriate in this instance. If you could include a comment identifying the precise thing you are testing (the formatting of the float in the suggestion) that'd be great. |
|
Sure. |
|
How do the tests work? |
|
I'm changing the help message in this PR, so it seems like I need to test the help message, but I'm not sure how using the tests that I see already there. Heres my start on the test (in // Copyright 2012 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// compile-flags: -Z parse-only
fn main () {
(1, (2, 3)).1.1;
//~ERROR unexpected token: `1.1`
} |
I think you can just add a line like |
|
Do you have any idea why this test is failing? // Copyright 2012 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// compile-flags: -Z parse-only
fn main () {
(1, (2, 3)).1.1; //~HELP try parenthesizing the first index; e.g., `(foo.1).1`
} |
|
"Unexpected compiler error or warning". Looks like you have to tell it about the actual error. Try this: The |
|
@jonas-schievink Thanks. I think that should do the trick! |
|
☔ The latest upstream changes (presumably #28350) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@nikomatsakis Can you make bors retry? |
|
@ChristopherDumas I don't see any approvals on this PR. The @bors message above was just a friendly reminder that the PR needed rebasing. |
|
Why is this a merge conflict? I tried to rebase to rust-lang/rust master, and llvm has a merge conflict. I tried all kinds of things, but I'm stuck. |
|
@ChristopherDumas You already fixed it, but the comment doesn't just go away. |
|
@ChristopherDumas As for LLVM, use some tool like |
|
@nikomatsakis This is ready to be merged.... |
|
|
||
| fn main () { | ||
| (1, (2, 3)).1.1; //~ ERROR unexpected token | ||
| //~^ HELP try parenthesizing the first index |
There was a problem hiding this comment.
the actual fix would help (e.g. try parenthesizing the first index; e.g., (foo.1).1``)
There was a problem hiding this comment.
Totally! Thanks.
|
Could you squash all commits together? r+ modulo that |
|
Yes! thanks. |
|
I squashed the commits. |
|
@ChristopherDumas You might want to amend the commit to fix the message (I don't think the issue there is even related to this PR). |
|
Whoops. sorry i must have gotten confused. |
|
I amended the commit message. |
|
Thanks! @bors r+ |
|
📌 Commit afa905f has been approved by |
|
⌛ Testing commit afa905f with merge 7b2fdc4... |
|
💔 Test failed - auto-win-msvc-64-opt |
|
@bors: retry On Mon, Sep 14, 2015 at 10:10 AM, bors [email protected] wrote:
|
as per #28243.