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

Add context to E0084, E0517, E0518 #45984

Merged
merged 3 commits into from
Nov 16, 2017

Conversation

ExpHP
Copy link
Contributor

@ExpHP ExpHP commented Nov 14, 2017

A small diagnostic enhancement to get my feet wet. Please scrutinize!

This modifies errors E0084, E0517, and E0518 to include both the annotation and the annotated item. All of these errors already had labels; I moved the label to the other span, and rephrased it as necessary.

Fixes #45886

Show both the attribute and the item

#[repr(C)]
enum EExtern { A, B }

#[repr(align(8))] //~ ERROR: attribute should be applied to struct
enum EAlign { A, B }
enum EAlign { A, B } // not a struct

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, just noticed these are missing tildes (but the test still passes?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

surveying nearby examples it seems I simply shouldn't include the "note" messages in this test anyways? (we just want to know that the error is triggered here?)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ExpHP if you run build/<ENV>/stage1/bin/rustc src/test/compile-fail/attr-usage-repr.rs you will see the actual output. The test as it was previously was incorrect, as it wasn't asserting the label in that line. It should have been a //~ NOTE: not a struct there. Please change it here and elsewhere to avoid regressions and actually test this change.

@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 14, 2017
@estebank
Copy link
Contributor

r=me after replacing all the // with //~ NOTE: in the tests. Also, could yo paste in a comment the visual output (before and after preferably) before we merge?

@ExpHP
Copy link
Contributor Author

ExpHP commented Nov 14, 2017

Before

error[E0518]: attribute should be applied to function
  --> examples/E0518.rs:14:1
   |
14 | #[inline(never)] //~ ERROR E0518
   | ^^^^^^^^^^^^^^^^ requires a function

error[E0517]: attribute should be applied to struct or union
  --> examples/E0517.rs:14:1
   |
14 | #[repr(packed)] //~ requires a struct
   | ^^^^^^^^^^^^^^^ requires a struct or union

error[E0084]: unsupported representation for zero-variant enum
  --> examples/E0084.rs:12:1
   |
12 | enum Foo {}  //~ ERROR E0084
   | ^^^^^^^^^^^ unsupported enum representation

After

error[E0518]: attribute should be applied to function
  --> examples/E0518.rs:14:1
   |
14 |   #[inline(never)] //~ ERROR E0518
   |   ^^^^^^^^^^^^^^^^
15 | / impl Foo {       //~ not a function
16 | | }
   | |_- not a function

error[E0517]: attribute should be applied to struct or union
  --> examples/E0517.rs:14:1
   |
14 | #[repr(packed)] //~ requires a struct
   | ^^^^^^^^^^^^^^^
15 | enum Foo2 {Bar, Baz} //~ ERROR E0517
   | -------------------- not a struct or union

error[E0084]: unsupported representation for zero-variant enum
  --> examples/E0084.rs:11:1
   |
11 | #[repr(i32)] //~ unsupported enum representation
   | ^^^^^^^^^^^^
12 | enum Foo {}  //~ ERROR E0084
   | ----------- zero-variant enum

(In color)

Saved terminal output with ansi escape sequences:

$ # before
$ curl -s https://gist.githubusercontent.com/ExpHP/31fda7073c66462f633219a87e14c99a/raw/8cb74221ca2e7f0f138e85931541f1c754d81ffd/before-ansi

$ # after
$ curl -s https://gist.githubusercontent.com/ExpHP/88beefb1d1dfe8cbd21ad8883358b438/raw/95143504316b7dfe0c3c17f50684352546f99cf1/examples-ansi

@estebank
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Nov 15, 2017

📌 Commit ead9ac3 has been approved by estebank

@kennytm kennytm added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 15, 2017
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Nov 16, 2017
Add context to E0084, E0517, E0518

A small diagnostic enhancement to get my feet wet.  Please scrutinize!

This modifies errors E0084, E0517, and E0518 to include both the annotation and the annotated item.  All of these errors already had labels; I moved the label to the other span, and rephrased it as necessary.

Fixes rust-lang#45886
bors added a commit that referenced this pull request Nov 16, 2017
Rollup of 6 pull requests

- Successful merges: #45951, #45973, #45984, #45993, #46005, #46010
- Failed merges:
@bors bors merged commit ead9ac3 into rust-lang:master Nov 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants