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

Update E0010 to use the new format #35439

Merged
merged 1 commit into from
Aug 7, 2016

Conversation

pcn
Copy link
Contributor

@pcn pcn commented Aug 6, 2016

For #35194

@rust-highfive
Copy link
Collaborator

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

@sophiajt
Copy link
Contributor

sophiajt commented Aug 6, 2016

Great! Next make sure you also update the unit test so that you're testing to make sure that new label is there.

@pcn
Copy link
Contributor Author

pcn commented Aug 6, 2016

Ah, yes. Thanks for reminding me about that. I'll also revert the message to the recommended message from #35194. It seems like I riff'd here.

@pcn
Copy link
Contributor Author

pcn commented Aug 6, 2016

It looks like like the existing test is only matching on the string E0010 so no change needs to be made there:

spacey@masonjar:~/dvcs/github/rust-lang/rust$ python src/bootstrap/bootstrap.py --step check-cfail E0010 --stage 1
Building stage0 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
Building stage0 test artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
Building stage0 compiler artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
Building stage1 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
Building stage1 test artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
Building stage1 tool compiletest (x86_64-unknown-linux-gnu)
Check compiletest compile-fail (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)

running 1 test
test [compile-fail] compile-fail/E0010.rs ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured

Build completed in 0:00:01

And I've changed the inline message to match the top line message, which is what the request actually asked for:

spacey@masonjar:~/dvcs/github/rust-lang/rust$ RUST_NEW_ERROR_FORMAT=true ./build/x86_64-unknown-linux-gnu/stage1/bin/rustc src/test/compile-fail/E0010.rs
error[E0010]: allocations are not allowed in constants
  --> src/test/compile-fail/E0010.rs:13:24
   |
13 | const CON : Box<i32> = box 0; //~ ERROR E0010
   |                        ^^^^^ allocation not allowed in constants

error: aborting due to previous error

@sophiajt
Copy link
Contributor

sophiajt commented Aug 7, 2016

There are some quirks in the test runner. I talk about them in the "Extra credit" section of the blog post. You'll need to add at least one NOTE to the test for it to start checking that the label is there.

Once you add that NOTE, you can know that it's at least being tested. With that, I think we'll be good to go.

Btw, thanks for fixing my grammar :)

@pcn
Copy link
Contributor Author

pcn commented Aug 7, 2016

OK, looks like I skimmed that part when I focused on the tips section below. I'm looking into adding a NOTE now.

@pcn
Copy link
Contributor Author

pcn commented Aug 7, 2016

Tested a bogus NOTE and then this NOTE and it looks as though it's now working as expected.

@sophiajt
Copy link
Contributor

sophiajt commented Aug 7, 2016

Great. Can you squash your three 3 commits down to 1 commit? With that I think we'll be ready.

@pcn pcn force-pushed the update-E0010-error-message branch from 0a4bf92 to dfb66c3 Compare August 7, 2016 14:28
@pcn
Copy link
Contributor Author

pcn commented Aug 7, 2016

Rebased.

@sophiajt
Copy link
Contributor

sophiajt commented Aug 7, 2016

Great!

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Aug 7, 2016

📌 Commit dfb66c3 has been approved by jonathandturner

sophiajt pushed a commit to sophiajt/rust that referenced this pull request Aug 7, 2016
…onathandturner

Update E0010 to use the new format

For rust-lang#35194
bors added a commit that referenced this pull request Aug 7, 2016
@bors bors merged commit dfb66c3 into rust-lang:master Aug 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants