-
Notifications
You must be signed in to change notification settings - Fork 14k
Add a diagnostic attribute for special casing const bound errors for non-const impls #148641
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
base: master
Are you sure you want to change the base?
Conversation
|
Some changes occurred in compiler/rustc_passes/src/check_attr.rs |
This comment has been minimized.
This comment has been minimized.
c0e67f0 to
e501aad
Compare
This comment has been minimized.
This comment has been minimized.
e501aad to
d04c00d
Compare
|
The job Click to see the possible cause of the failure (guessed by this bot) |
| /// Checks if `#[diagnostic::on_const]` is applied to a trait impl | ||
| fn check_diagnostic_on_const(&self, attr_span: Span, hir_id: HirId, target: Target) { | ||
| if !matches!(target, Target::Impl { of_trait: true }) { | ||
| self.tcx.emit_node_span_lint( | ||
| MISPLACED_DIAGNOSTIC_ATTRIBUTES, | ||
| hir_id, | ||
| attr_span, | ||
| DiagnosticOnConstOnlyForTraitImpls, | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also check that the impl is not const? Otherwise the annotation is fully inert in that case, right?
| //! This is an unusual feature gate test, as it doesn't test the feature | ||
| //! gate, but the fact that not adding the feature gate to | ||
| //! the aux crate will cause the diagnostic to not emit the | ||
| //! custom diagnostic message |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that strictly necessary? I feel like at least for std, if we annotate with diagnostic::on_const we'd want that message to appear even in stable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
std has the feature enabled. but in this case the aux crate doesn't have it. So if a user crate doesn't have the feature gate, their uses of diagnostic::on_const get ignored
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah! I see. So the feature must be enabled where the annotation is. Gotcha.
Somewhat of a follow-up to #144194
My plan is to resolve
https://github.com/rust-lang/rust/blob/f4e19c68786211f3c3cf2593442629599678800a/compiler/rustc_hir_typeck/src/callee.rs#L907-913
but doing so without being able to mark impls the way I do in this PR wrould cause all nice diagnostics about for loops and pointer comparisons to just be a
*const u32 does not implement [const] PartialEqerrors.