Skip to content

Allow #[must_use] on associated types to warn on unused values in generic contexts #142590

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
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/mut_visit.rs
Original file line number Diff line number Diff line change
@@ -37,6 +37,7 @@ mod sealed {

/// This is for compatibility with the regular `Visitor`.
pub trait MutVisitorResult {
#[cfg_attr(not(bootstrap), must_use)]
type Result: VisitorResult;
}

7 changes: 4 additions & 3 deletions compiler/rustc_ast/src/visit.rs
Original file line number Diff line number Diff line change
@@ -130,6 +130,7 @@ pub enum LifetimeCtxt {
pub trait Visitor<'ast>: Sized {
/// The result type of the `visit_*` methods. Can be either `()`,
/// or `ControlFlow<T>`.
#[cfg_attr(not(bootstrap), must_use)]
type Result: VisitorResult = ();

fn visit_ident(&mut self, _ident: &'ast Ident) -> Self::Result {
@@ -884,7 +885,7 @@ macro_rules! common_visitor_and_walkers {
TyKind::BareFn(function_declaration) => {
let BareFnTy { safety, ext: _, generic_params, decl, decl_span } =
&$($mut)? **function_declaration;
visit_safety(vis, safety);
try_visit!(visit_safety(vis, safety));
try_visit!(visit_generic_params(vis, generic_params));
try_visit!(vis.visit_fn_decl(decl));
try_visit!(visit_span(vis, decl_span));
@@ -1235,7 +1236,7 @@ macro_rules! common_visitor_and_walkers {
bounds,
bound_generic_params,
}) => {
visit_generic_params(vis, bound_generic_params);
try_visit!(visit_generic_params(vis, bound_generic_params));
try_visit!(vis.visit_ty(bounded_ty));
walk_list!(vis, visit_param_bound, bounds, BoundKind::Bound);
}
@@ -1420,7 +1421,7 @@ macro_rules! common_visitor_and_walkers {
let StructExpr { qself, path, fields, rest } = &$($mut)?**se;
try_visit!(vis.visit_qself(qself));
try_visit!(vis.visit_path(path));
visit_expr_fields(vis, fields);
try_visit!(visit_expr_fields(vis, fields));
match rest {
StructRest::Base(expr) => try_visit!(vis.visit_expr(expr)),
StructRest::Rest(_span) => {}
1 change: 1 addition & 0 deletions compiler/rustc_hir/src/intravisit.rs
Original file line number Diff line number Diff line change
@@ -224,6 +224,7 @@ pub trait Visitor<'v>: Sized {

/// The result type of the `visit_*` methods. Can be either `()`,
/// or `ControlFlow<T>`.
#[cfg_attr(not(bootstrap), must_use)]
type Result: VisitorResult = ();

/// If `type NestedFilter` is set to visit nested items, this method
594 changes: 293 additions & 301 deletions compiler/rustc_lint/src/unused.rs

Large diffs are not rendered by default.

10 changes: 10 additions & 0 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
@@ -1560,6 +1560,16 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
return;
}

// `#[must_use]` can be applied to an associated type in a trait, so that it will
// warn on unused associated types in generic contexts.
if let Target::AssocTy = target
&& let parent_def_id = self.tcx.hir_get_parent_item(hir_id).def_id
&& let containing_item = self.tcx.hir_expect_item(parent_def_id)
&& let hir::ItemKind::Trait(..) = containing_item.kind
{
return;
}

let article = match target {
Target::ExternCrate
| Target::Enum
3 changes: 3 additions & 0 deletions compiler/rustc_privacy/src/lib.rs
Original file line number Diff line number Diff line change
@@ -67,8 +67,11 @@ impl<'tcx> fmt::Display for LazyDefPathStr<'tcx> {
/// manually. Second, it doesn't visit some type components like signatures of fn types, or traits
/// in `impl Trait`, see individual comments in `DefIdVisitorSkeleton::visit_ty`.
pub trait DefIdVisitor<'tcx> {
#[cfg_attr(not(bootstrap), must_use)]
type Result: VisitorResult = ();

const SHALLOW: bool = false;

fn skip_assoc_tys(&self) -> bool {
false
}
Original file line number Diff line number Diff line change
@@ -461,6 +461,7 @@ impl<'a, 'tcx> InspectGoal<'a, 'tcx> {

/// The public API to interact with proof trees.
pub trait ProofTreeVisitor<'tcx> {
#[cfg_attr(not(bootstrap), must_use)]
type Result: VisitorResult = ();

fn span(&self) -> Span;
1 change: 1 addition & 0 deletions compiler/rustc_ty_utils/src/sig_types.rs
Original file line number Diff line number Diff line change
@@ -9,6 +9,7 @@ use rustc_span::Span;
use tracing::{instrument, trace};

pub trait SpannedTypeVisitor<'tcx> {
#[cfg_attr(not(bootstrap), must_use)]
type Result: VisitorResult = ();
fn visit(&mut self, span: Span, value: impl TypeVisitable<TyCtxt<'tcx>>) -> Self::Result;
}
1 change: 1 addition & 0 deletions compiler/rustc_type_ir/src/visit.rs
Original file line number Diff line number Diff line change
@@ -89,6 +89,7 @@ pub trait TypeSuperVisitable<I: Interner>: TypeVisitable<I> {
/// that recurses into the type's fields in a non-custom fashion.
pub trait TypeVisitor<I: Interner>: Sized {
#[cfg(feature = "nightly")]
#[cfg_attr(not(bootstrap), must_use)]
type Result: VisitorResult = ();

#[cfg(not(feature = "nightly"))]
27 changes: 27 additions & 0 deletions tests/ui/lint/unused/unused-assoc-ty.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#![deny(unused_must_use)]

trait Foo {
#[must_use]
type Result;

fn process(arg: i32) -> Self::Result;
}

fn generic<T: Foo>() {
T::process(10);
//~^ ERROR unused `Foo::Result` that must be used
}

struct NoOp;
impl Foo for NoOp {
type Result = ();

fn process(_arg: i32) { println!("did nothing"); }
}

fn noop() {
// Should not lint.
<NoOp as Foo>::process(10);
}

fn main() {}
18 changes: 18 additions & 0 deletions tests/ui/lint/unused/unused-assoc-ty.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
error: unused `Foo::Result` that must be used
--> $DIR/unused-assoc-ty.rs:11:5
|
LL | T::process(10);
| ^^^^^^^^^^^^^^
|
note: the lint level is defined here
--> $DIR/unused-assoc-ty.rs:1:9
|
LL | #![deny(unused_must_use)]
| ^^^^^^^^^^^^^^^
help: use `let _ = ...` to ignore the resulting value
|
LL | let _ = T::process(10);
| +++++++

error: aborting due to 1 previous error

2 changes: 1 addition & 1 deletion tests/ui/lint/unused/unused_attributes-must_use.rs
Original file line number Diff line number Diff line change
@@ -67,7 +67,7 @@ fn qux<#[must_use] T>(_: T) {} //~ ERROR `#[must_use]` has no effect
trait Use {
#[must_use] //~ ERROR `#[must_use]` has no effect
const ASSOC_CONST: usize = 4;
#[must_use] //~ ERROR `#[must_use]` has no effect
#[must_use]
type AssocTy;

#[must_use]
8 changes: 1 addition & 7 deletions tests/ui/lint/unused/unused_attributes-must_use.stderr
Original file line number Diff line number Diff line change
@@ -123,12 +123,6 @@ error: `#[must_use]` has no effect when applied to an associated const
LL | #[must_use]
| ^^^^^^^^^^^

error: `#[must_use]` has no effect when applied to an associated type
--> $DIR/unused_attributes-must_use.rs:70:5
|
LL | #[must_use]
| ^^^^^^^^^^^

error: `#[must_use]` has no effect when applied to a provided trait method
--> $DIR/unused_attributes-must_use.rs:83:5
|
@@ -223,5 +217,5 @@ help: use `let _ = ...` to ignore the resulting value
LL | let _ = ().get_four();
| +++++++

error: aborting due to 29 previous errors
error: aborting due to 28 previous errors