Changed error message E0408 to new format #36307

faebser · 2016-09-06T21:04:04Z

Followed your text and was able to change the ouput to the new format.
I did not encounter any broken test therefore this is a really small commit.

Thanks for letting me hack on the compiler :)

r? @jonathandturner

rust-highfive · 2016-09-06T21:04:27Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @jonathandturner (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. Due to 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.

faebser · 2016-09-06T21:06:13Z

Part of #35233
Fixes #35698

GuillaumeGomez · 2016-09-07T10:07:09Z

Thanks for your PR!

You also need to update the corresponding test(s) to reflect your changes. They are located in src/test/compile_fail/.

sophiajt · 2016-09-08T15:44:05Z

@faebser - as @GuillaumeGomez mentions, the test infrastructure has some quirks. One is that the labels aren't tested until you add your first NOTE. You can read about this in the Extra Credit part of the blog post

faebser · 2016-09-09T15:06:44Z

@jonathandturner thanks for the heads up. I will work on the test as soon I have time.

GuillaumeGomez · 2016-09-09T18:02:28Z

@faebser: Don't worry, we're not in a hurry. Just comment here once done.

faebser · 2016-09-22T22:27:17Z

@jonathandturner I don't really get where I have to add the note?
If i try to add a note to https://github.com/rust-lang/rust/blob/master/src/test/compile-fail/E0408.rs (which I think is the original test case that you refer to in your blog post) I just get an unexpected note error if i run the test.

sophiajt · 2016-09-23T14:36:25Z

@faebser - can you add your changes to this PR so we can take a look?

faebser · 2016-09-25T21:25:10Z

@jonathandturner I added my try as a commit to the PR.
Running python src/bootstrap/bootstrap.py --step check-cfail --stage 1 gives the following errors:

failures:

---- [compile-fail] compile-fail/E0408.rs stdout ----

error: /home/faebser/workspace/rust/src/test/compile-fail/E0408.rs:15: unexpected "error": '15:19: 15:23: variable `y` from pattern #1 is not bound in pattern #2 [E0408]'

error: /home/faebser/workspace/rust/src/test/compile-fail/E0408.rs:15: unexpected "note": 'pattern doesn't bind `y`'

error: /home/faebser/workspace/rust/src/test/compile-fail/E0408.rs:14: expected error not found: variable `y` from pattern #2 is not bound in pattern #1

error: /home/faebser/workspace/rust/src/test/compile-fail/E0408.rs:14: expected note not found: pattern doesn't bind `y`

error: 2 unexpected errors found, 2 expected errors not found
status: exit code: 101

GuillaumeGomez · 2016-09-25T22:00:21Z

Then fix them? I don't know what to add actually. :-/

faebser · 2016-09-26T10:35:55Z

@GuillaumeGomez: I am sorry but I am a bit lost and I don't really understand how to fix this error. I added them to this PR because @jonathandturner asked me to.

GuillaumeGomez · 2016-09-26T15:56:24Z

The "errors" you see come from src/test/compile-fail/E0408.rs. So, you added annotations (like `//~ ERROR E0408') but some of them aren't matched with the current rustc output (that you updated).

So let's see the first one:

E0408.rs:15: unexpected "error": '15:19: 15:23: variable `y` from pattern #1 is not bound in pattern #2 [E0408]'

It says that an annotation was not found. So let's look to your file:

//~^ ERROR variable `y` from pattern #2 is not bound in pattern #1

It means you expect an error to be thrown by the line before this one (the "^" says it). Better like this?

faebser · 2016-09-26T22:15:35Z

@GuillaumeGomez: thanks for your comment. I also realised that #1 and #2 were the wrong way around thus generating the error.
I added one more test and will try to find the remaining ones tomorrow.

GuillaumeGomez

Once these fixed, please squash your commits. If you don't know how to do it, take a look here.

GuillaumeGomez · 2016-09-27T08:59:23Z

src/test/compile-fail/E0408.rs

-        Some(y) | None => {} //~ ERROR E0408
-        _ => ()
+        Some(y) | None => {} //~ ERROR variable `y` from pattern #1 is not bound in pattern #2
+        _ => () //~^ NOTE pattern doesn't bind `y`


Generally, we use "//~~|" and not "//~~^" when it's on the same line. I mean that the note is thrown by the same line than the error, so it's the "same" issue. With this way to write it, we have the feeling that it's two different errors.

Also, can you align "//~~^ NOTE pattern doesn't bind y" with "//~~ ERROR variable y from pattern #1 is not bound in pattern #2" please?

GuillaumeGomez · 2016-09-27T08:59:36Z

src/test/compile-fail/issue-2848.rs

@@ -20,6 +20,6 @@ fn main() {
    use bar::foo::{alpha, charlie};
    match alpha {
      alpha | beta => {} //~ ERROR variable `beta` from pattern #2 is not bound in pattern #1
-      charlie => {}
+      charlie => {} //~^ NOTE pattern doesn't bind `beta`


Same as above.

faebser · 2016-09-27T22:18:41Z

@GuillaumeGomez can you please review the third test too? thanks

GuillaumeGomez · 2016-09-28T12:02:57Z

All good for me. Please squash your commits now.

KiChjang · 2016-10-12T17:33:28Z

This looks like it only requires squashing, what's blocking this for 2 weeks?

GuillaumeGomez · 2016-10-12T20:10:34Z

No clue. :-/

faebser · 2016-10-12T22:15:21Z

Sorry for the delay, I thought that the squash was okay but after having another look today I saw that it did not work. I am working on fixing it right now.

r? @jonathandturner

faebser · 2016-10-12T22:19:47Z

@GuillaumeGomez: squash is okay now

GuillaumeGomez · 2016-10-13T13:11:51Z

Thanks!

@bors: r+ rollup

bors · 2016-10-13T13:11:52Z

📌 Commit 595b754 has been approved by GuillaumeGomez

bors · 2016-10-13T13:11:54Z

⌛ Testing commit 595b754 with merge c80d21d...

bors · 2016-10-13T13:27:53Z

💔 Test failed - auto-linux-cross-opt

arielb1 · 2016-10-13T16:39:15Z

@bors retry

alexcrichton · 2016-10-13T20:23:22Z

@bors: retry

…uillaumeGomez Changed error message E0408 to new format Followed your text and was able to change the ouput to the new format. I did not encounter any broken test therefore this is a really small commit. Thanks for letting me hack on the compiler :) r? @jonathandturner

Rollup of 10 pull requests - Successful merges: #36307, #36755, #36961, #37102, #37115, #37119, #37122, #37123, #37141, #37159 - Failed merges:

rust-highfive assigned sophiajt Sep 6, 2016

faebser mentioned this pull request Sep 25, 2016

E0408 needs to be updated to new format #35698

Closed

GuillaumeGomez requested changes Sep 27, 2016

View reviewed changes

faebser force-pushed the E0408_new_error_format branch from caa2204 to 4dde9a5 Compare October 12, 2016 22:14

Changed error message E0408 to new format

595b754

r? @jonathandturner

faebser force-pushed the E0408_new_error_format branch from 4dde9a5 to 595b754 Compare October 12, 2016 22:18

sophiajt mentioned this pull request Oct 14, 2016

Rollup of 10 pull requests #37170

Merged

bors added a commit that referenced this pull request Oct 14, 2016

Auto merge of #37170 - jonathandturner:rollup, r=jonathandturner

a8d189a

Rollup of 10 pull requests - Successful merges: #36307, #36755, #36961, #37102, #37115, #37119, #37122, #37123, #37141, #37159 - Failed merges:

bors merged commit 595b754 into rust-lang:master Oct 15, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Changed error message E0408 to new format #36307

Changed error message E0408 to new format #36307

faebser commented Sep 6, 2016

rust-highfive commented Sep 6, 2016

faebser commented Sep 6, 2016

GuillaumeGomez commented Sep 7, 2016

sophiajt commented Sep 8, 2016

faebser commented Sep 9, 2016

GuillaumeGomez commented Sep 9, 2016

faebser commented Sep 22, 2016

sophiajt commented Sep 23, 2016

faebser commented Sep 25, 2016

GuillaumeGomez commented Sep 25, 2016

faebser commented Sep 26, 2016

GuillaumeGomez commented Sep 26, 2016 •

edited

Loading

faebser commented Sep 26, 2016

GuillaumeGomez left a comment

GuillaumeGomez Sep 27, 2016

GuillaumeGomez Sep 27, 2016

faebser commented Sep 27, 2016

GuillaumeGomez commented Sep 28, 2016

KiChjang commented Oct 12, 2016

GuillaumeGomez commented Oct 12, 2016

faebser commented Oct 12, 2016

faebser commented Oct 12, 2016

GuillaumeGomez commented Oct 13, 2016

bors commented Oct 13, 2016

bors commented Oct 13, 2016

bors commented Oct 13, 2016

arielb1 commented Oct 13, 2016

alexcrichton commented Oct 13, 2016

Changed error message E0408 to new format #36307

Changed error message E0408 to new format #36307

Conversation

faebser commented Sep 6, 2016

rust-highfive commented Sep 6, 2016

faebser commented Sep 6, 2016

GuillaumeGomez commented Sep 7, 2016

sophiajt commented Sep 8, 2016

faebser commented Sep 9, 2016

GuillaumeGomez commented Sep 9, 2016

faebser commented Sep 22, 2016

sophiajt commented Sep 23, 2016

faebser commented Sep 25, 2016

GuillaumeGomez commented Sep 25, 2016

faebser commented Sep 26, 2016

GuillaumeGomez commented Sep 26, 2016 • edited Loading

faebser commented Sep 26, 2016

GuillaumeGomez left a comment

Choose a reason for hiding this comment

GuillaumeGomez Sep 27, 2016

Choose a reason for hiding this comment

GuillaumeGomez Sep 27, 2016

Choose a reason for hiding this comment

faebser commented Sep 27, 2016

GuillaumeGomez commented Sep 28, 2016

KiChjang commented Oct 12, 2016

GuillaumeGomez commented Oct 12, 2016

faebser commented Oct 12, 2016

faebser commented Oct 12, 2016

GuillaumeGomez commented Oct 13, 2016

bors commented Oct 13, 2016

bors commented Oct 13, 2016

bors commented Oct 13, 2016

arielb1 commented Oct 13, 2016

alexcrichton commented Oct 13, 2016

GuillaumeGomez commented Sep 26, 2016 •

edited

Loading