Skip to content

Conversation

weiznich
Copy link
Contributor

This commit improves warnings emitted for malformed on unimplemented attributes by:

  • Improving the span of the warnings
  • Adding a label message to them
  • Separating the messages for missing and unexpected options
  • Adding a help message that says which options are supported

r? @compiler-errors

I'm happy to work on further improvements, so feel free to make suggestions.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 19, 2023
Copy link
Member

@Urgau Urgau left a comment

Choose a reason for hiding this comment

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

Diagnostics should not start with a Upper case.

There is some guidelines on the rustc-dev-guide https://rustc-dev-guide.rust-lang.org/diagnostics.html#suggestion-style-guide but unfortunatly not enough.

This commit improves warnings emitted for malformed on unimplemented
attributes by:

* Improving the span of the warnings
* Adding a label message to them
* Separating the messages for missing and unexpected options
* Adding a help message that says which options are supported
@weiznich weiznich force-pushed the improve_diagnostic_on_unimplemented_warnings branch from 6dfbcee to 9017b97 Compare October 19, 2023 13:01
LL | | note = "not used yet"
LL | | )]
| |__^
| |__^ invalid option found here
Copy link
Member

Choose a reason for hiding this comment

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

Should be able to point at the if(Self = ()) here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've pushed a "fix" to address this somewhat. The problem there is that if(Self = ()) doesn't parse as meta_item_list(). I'm not sure why that's the case or whether that's the right behavior. The pushed "fix" just turns the attribute into something that can be parsed (if(Self = "()") in that case).

Copy link
Member

Choose a reason for hiding this comment

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

oh, ok

@compiler-errors
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Oct 25, 2023

📌 Commit 3d03a8a has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors 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 Oct 25, 2023
@bors bors merged commit d30fe8b into rust-lang:master Oct 26, 2023
@rustbot rustbot added this to the 1.75.0 milestone Oct 26, 2023
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. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants