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

Do not apply #[do_not_recommend] if the feature flag is not set #128674

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2855,6 +2855,7 @@ pub struct Attribute {
/// or the construct this attribute is contained within (inner).
pub style: AttrStyle,
pub span: Span,
pub allowed_diagnostic_attribute: bool,
}

#[derive(Clone, Encodable, Decodable, Debug)]
Expand Down
9 changes: 8 additions & 1 deletion compiler/rustc_ast/src/attr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,13 @@ pub fn mk_doc_comment(
data: Symbol,
span: Span,
) -> Attribute {
Attribute { kind: AttrKind::DocComment(comment_kind, data), id: g.mk_attr_id(), style, span }
Attribute {
kind: AttrKind::DocComment(comment_kind, data),
id: g.mk_attr_id(),
style,
span,
allowed_diagnostic_attribute: false,
}
}

pub fn mk_attr(
Expand All @@ -592,6 +598,7 @@ pub fn mk_attr_from_item(
id: g.mk_attr_id(),
style,
span,
allowed_diagnostic_attribute: false,
}
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast/src/mut_visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,7 @@ fn walk_local<T: MutVisitor>(vis: &mut T, local: &mut P<Local>) {
}

fn walk_attribute<T: MutVisitor>(vis: &mut T, attr: &mut Attribute) {
let Attribute { kind, id: _, style: _, span } = attr;
let Attribute { kind, id: _, style: _, span, allowed_diagnostic_attribute: _ } = attr;
match kind {
AttrKind::Normal(normal) => {
let NormalAttr {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast/src/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1214,7 +1214,7 @@ pub fn walk_vis<'a, V: Visitor<'a>>(visitor: &mut V, vis: &'a Visibility) -> V::
}

pub fn walk_attribute<'a, V: Visitor<'a>>(visitor: &mut V, attr: &'a Attribute) -> V::Result {
let Attribute { kind, id: _, style: _, span: _ } = attr;
let Attribute { kind, id: _, style: _, span: _, allowed_diagnostic_attribute: _ } = attr;
match kind {
AttrKind::Normal(normal) => {
let NormalAttr { item, tokens: _ } = &**normal;
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
id: self.tcx.sess.psess.attr_id_generator.mk_attr_id(),
style: AttrStyle::Outer,
span: unstable_span,
allowed_diagnostic_attribute: false,
}],
);
}
Expand Down
8 changes: 7 additions & 1 deletion compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -945,7 +945,13 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
AttrKind::DocComment(comment_kind, data) => AttrKind::DocComment(comment_kind, data),
};

Attribute { kind, id: attr.id, style: attr.style, span: self.lower_span(attr.span) }
Attribute {
kind,
id: attr.id,
style: attr.style,
span: self.lower_span(attr.span),
allowed_diagnostic_attribute: attr.allowed_diagnostic_attribute,
}
}

fn alias_attrs(&mut self, id: HirId, target_id: HirId) {
Expand Down
30 changes: 30 additions & 0 deletions compiler/rustc_ast_passes/src/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use std::mem;
use std::ops::{Deref, DerefMut};

use itertools::{Either, Itertools};
use mut_visit::MutVisitor;
use rustc_ast::ptr::P;
use rustc_ast::visit::{walk_list, AssocCtxt, BoundKind, FnCtxt, FnKind, Visitor};
use rustc_ast::*;
Expand Down Expand Up @@ -1781,3 +1782,32 @@ pub fn check_crate(

validator.has_proc_macro_decls
}

struct MarkDiagnosticAttributesAsUnstable<'a> {
features: &'a Features,
}

impl<'a> MutVisitor for MarkDiagnosticAttributesAsUnstable<'a> {
fn visit_attribute(&mut self, at: &mut Attribute) {
if at.is_doc_comment() {
return;
}
let diagnostic_item = match at.path().as_slice() {
[sym::diagnostic, item] => *item,
_ => return,
};
match diagnostic_item {
sym::on_unimplemented => at.allowed_diagnostic_attribute = true,
sym::do_not_recommend => {
at.allowed_diagnostic_attribute = self.features.do_not_recommend
}
_ => {}
}
}
}

pub fn apply_diagnostic_attribute_stablilty(features: &Features, krate: &mut Crate) {
let mut visitor = MarkDiagnosticAttributesAsUnstable { features };

mut_visit::walk_crate(&mut visitor, krate);
}
2 changes: 2 additions & 0 deletions compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,8 @@ fn configure_and_expand(
rustc_builtin_macros::test_harness::inject(&mut krate, sess, features, resolver)
});

rustc_ast_passes::ast_validation::apply_diagnostic_attribute_stablilty(features, &mut krate);

let has_proc_macro_decls = sess.time("AST_validation", || {
rustc_ast_passes::ast_validation::check_crate(
sess,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_query_system/src/ich/impls_syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ impl<'ctx> rustc_ast::HashStableContext for StableHashingContext<'ctx> {
debug_assert!(!attr.ident().is_some_and(|ident| self.is_ignored_attr(ident.name)));
debug_assert!(!attr.is_doc_comment());

let ast::Attribute { kind, id: _, style, span } = attr;
let ast::Attribute { kind, id: _, style, span, allowed_diagnostic_attribute: _ } = attr;
if let ast::AttrKind::Normal(normal) = kind {
normal.item.hash_stable(self, hasher);
style.hash_stable(self, hasher);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1630,11 +1630,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
.tcx
.all_impls(def_id)
// ignore `do_not_recommend` items
.filter(|def_id| {
!self
.tcx
.has_attrs_with_path(*def_id, &[sym::diagnostic, sym::do_not_recommend])
})
.filter(|def_id| self.do_not_recommend_filter(*def_id))
// Ignore automatically derived impls and `!Trait` impls.
.filter_map(|def_id| self.tcx.impl_trait_header(def_id))
.filter_map(|header| {
Expand Down Expand Up @@ -1904,12 +1900,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
let impl_candidates = impl_candidates
.into_iter()
.cloned()
.filter(|cand| {
!self.tcx.has_attrs_with_path(
cand.impl_def_id,
&[sym::diagnostic, sym::do_not_recommend],
)
})
.filter(|cand| self.do_not_recommend_filter(cand.impl_def_id))
.collect::<Vec<_>>();

let def_id = trait_ref.def_id();
Expand Down Expand Up @@ -1953,6 +1944,12 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
report(impl_candidates, err)
}

fn do_not_recommend_filter(&self, def_id: DefId) -> bool {
let do_not_recommend =
self.tcx.get_attrs_by_path(def_id, &[sym::diagnostic, sym::do_not_recommend]).next();
!matches!(do_not_recommend, Some(a) if a.allowed_diagnostic_attribute)
}

fn report_similar_impl_candidates_for_root_obligation(
&self,
obligation: &PredicateObligation<'tcx>,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#![allow(unknown_or_malformed_diagnostic_attributes)]

trait Foo {}

#[diagnostic::do_not_recommend]
impl<A> Foo for (A,) {}

#[diagnostic::do_not_recommend]
impl<A, B> Foo for (A, B) {}

#[diagnostic::do_not_recommend]
impl<A, B, C> Foo for (A, B, C) {}

impl Foo for i32 {}

fn check(a: impl Foo) {}

fn main() {
check(());
//~^ ERROR the trait bound `(): Foo` is not satisfied
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
error[E0277]: the trait bound `(): Foo` is not satisfied
--> $DIR/do_not_apply_attribute_without_feature_flag.rs:19:11
|
LL | check(());
| ----- ^^ the trait `Foo` is not implemented for `()`
| |
| required by a bound introduced by this call
|
= help: the following other types implement trait `Foo`:
(A, B)
(A, B, C)
(A,)
note: required by a bound in `check`
--> $DIR/do_not_apply_attribute_without_feature_flag.rs:16:18
|
LL | fn check(a: impl Foo) {}
| ^^^ required by this bound in `check`

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0277`.
Loading