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

Extend unused_must_use to cover block exprs #112529

Merged
merged 1 commit into from
Jun 16, 2023
Merged
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
2 changes: 1 addition & 1 deletion compiler/rustc_arena/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ struct ArenaChunk<T = u8> {

unsafe impl<#[may_dangle] T> Drop for ArenaChunk<T> {
fn drop(&mut self) {
unsafe { Box::from_raw(self.storage.as_mut()) };
unsafe { drop(Box::from_raw(self.storage.as_mut())) }
}
}

Expand Down
53 changes: 42 additions & 11 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1534,8 +1534,29 @@ pub struct UnusedOp<'a> {
pub op: &'a str,
#[label]
pub label: Span,
#[suggestion(style = "verbose", code = "let _ = ", applicability = "maybe-incorrect")]
pub suggestion: Span,
#[subdiagnostic]
pub suggestion: UnusedOpSuggestion,
}

#[derive(Subdiagnostic)]
pub enum UnusedOpSuggestion {
#[suggestion(
lint_suggestion,
style = "verbose",
code = "let _ = ",
applicability = "maybe-incorrect"
)]
NormalExpr {
#[primary_span]
span: Span,
},
#[multipart_suggestion(lint_suggestion, style = "verbose", applicability = "maybe-incorrect")]
BlockTailExpr {
#[suggestion_part(code = "let _ = ")]
before_span: Span,
#[suggestion_part(code = ";")]
after_span: Span,
},
}

#[derive(LintDiagnostic)]
Expand Down Expand Up @@ -1578,15 +1599,25 @@ pub struct UnusedDef<'a, 'b> {
}

#[derive(Subdiagnostic)]
#[suggestion(
lint_suggestion,
style = "verbose",
code = "let _ = ",
applicability = "maybe-incorrect"
)]
pub struct UnusedDefSuggestion {
#[primary_span]
pub span: Span,

pub enum UnusedDefSuggestion {
#[suggestion(
lint_suggestion,
style = "verbose",
code = "let _ = ",
applicability = "maybe-incorrect"
)]
NormalExpr {
#[primary_span]
span: Span,
},
#[multipart_suggestion(lint_suggestion, style = "verbose", applicability = "maybe-incorrect")]
BlockTailExpr {
#[suggestion_part(code = "let _ = ")]
before_span: Span,
#[suggestion_part(code = ";")]
after_span: Span,
},
}

// Needed because of def_path_str
Expand Down
94 changes: 81 additions & 13 deletions compiler/rustc_lint/src/unused.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use crate::lints::{
PathStatementDrop, PathStatementDropSub, PathStatementNoEffect, UnusedAllocationDiag,
UnusedAllocationMutDiag, UnusedClosure, UnusedDef, UnusedDefSuggestion, UnusedDelim,
UnusedDelimSuggestion, UnusedGenerator, UnusedImportBracesDiag, UnusedOp, UnusedResult,
UnusedDelimSuggestion, UnusedGenerator, UnusedImportBracesDiag, UnusedOp, UnusedOpSuggestion,
UnusedResult,
};
use crate::Lint;
use crate::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext};
Expand Down Expand Up @@ -93,7 +94,15 @@ declare_lint_pass!(UnusedResults => [UNUSED_MUST_USE, UNUSED_RESULTS]);

impl<'tcx> LateLintPass<'tcx> for UnusedResults {
fn check_stmt(&mut self, cx: &LateContext<'_>, s: &hir::Stmt<'_>) {
let hir::StmtKind::Semi(expr) = s.kind else { return; };
let hir::StmtKind::Semi(mut expr) = s.kind else { return; };

let mut expr_is_from_block = false;
while let hir::ExprKind::Block(blk, ..) = expr.kind
&& let hir::Block { expr: Some(e), .. } = blk
{
expr = e;
expr_is_from_block = true;
}

if let hir::ExprKind::Ret(..) = expr.kind {
return;
Expand All @@ -113,6 +122,7 @@ impl<'tcx> LateLintPass<'tcx> for UnusedResults {
expr.span,
"output of future returned by ",
"",
expr_is_from_block,
)
{
// We have a bare `foo().await;` on an opaque type from an async function that was
Expand All @@ -125,13 +135,13 @@ impl<'tcx> LateLintPass<'tcx> for UnusedResults {
let must_use_result = is_ty_must_use(cx, ty, &expr, expr.span);
let type_lint_emitted_or_suppressed = match must_use_result {
Some(path) => {
emit_must_use_untranslated(cx, &path, "", "", 1, false);
emit_must_use_untranslated(cx, &path, "", "", 1, false, expr_is_from_block);
true
}
None => false,
};

let fn_warned = check_fn_must_use(cx, expr);
let fn_warned = check_fn_must_use(cx, expr, expr_is_from_block);

if !fn_warned && type_lint_emitted_or_suppressed {
// We don't warn about unused unit or uninhabited types.
Expand Down Expand Up @@ -176,7 +186,14 @@ impl<'tcx> LateLintPass<'tcx> for UnusedResults {
UnusedOp {
op: must_use_op,
label: expr.span,
suggestion: expr.span.shrink_to_lo(),
suggestion: if expr_is_from_block {
UnusedOpSuggestion::BlockTailExpr {
before_span: expr.span.shrink_to_lo(),
after_span: expr.span.shrink_to_hi(),
}
} else {
UnusedOpSuggestion::NormalExpr { span: expr.span.shrink_to_lo() }
},
},
);
op_warned = true;
Expand All @@ -186,7 +203,11 @@ impl<'tcx> LateLintPass<'tcx> for UnusedResults {
cx.emit_spanned_lint(UNUSED_RESULTS, s.span, UnusedResult { ty });
}

fn check_fn_must_use(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool {
fn check_fn_must_use(
cx: &LateContext<'_>,
expr: &hir::Expr<'_>,
expr_is_from_block: bool,
) -> bool {
let maybe_def_id = match expr.kind {
hir::ExprKind::Call(ref callee, _) => {
match callee.kind {
Expand All @@ -207,7 +228,14 @@ impl<'tcx> LateLintPass<'tcx> for UnusedResults {
_ => None,
};
if let Some(def_id) = maybe_def_id {
check_must_use_def(cx, def_id, expr.span, "return value of ", "")
check_must_use_def(
cx,
def_id,
expr.span,
"return value of ",
"",
expr_is_from_block,
)
} else {
false
}
Expand Down Expand Up @@ -350,6 +378,7 @@ impl<'tcx> LateLintPass<'tcx> for UnusedResults {
span: Span,
descr_pre_path: &str,
descr_post_path: &str,
expr_is_from_block: bool,
) -> bool {
is_def_must_use(cx, def_id, span)
.map(|must_use_path| {
Expand All @@ -360,6 +389,7 @@ impl<'tcx> LateLintPass<'tcx> for UnusedResults {
descr_post_path,
1,
false,
expr_is_from_block,
)
})
.is_some()
Expand All @@ -373,28 +403,59 @@ impl<'tcx> LateLintPass<'tcx> for UnusedResults {
descr_post: &str,
plural_len: usize,
is_inner: bool,
expr_is_from_block: bool,
) {
let plural_suffix = pluralize!(plural_len);

match path {
MustUsePath::Suppressed => {}
MustUsePath::Boxed(path) => {
let descr_pre = &format!("{}boxed ", descr_pre);
emit_must_use_untranslated(cx, path, descr_pre, descr_post, plural_len, true);
emit_must_use_untranslated(
cx,
path,
descr_pre,
descr_post,
plural_len,
true,
expr_is_from_block,
);
}
MustUsePath::Opaque(path) => {
let descr_pre = &format!("{}implementer{} of ", descr_pre, plural_suffix);
emit_must_use_untranslated(cx, path, descr_pre, descr_post, plural_len, true);
emit_must_use_untranslated(
cx,
path,
descr_pre,
descr_post,
plural_len,
true,
expr_is_from_block,
);
}
MustUsePath::TraitObject(path) => {
let descr_post = &format!(" trait object{}{}", plural_suffix, descr_post);
emit_must_use_untranslated(cx, path, descr_pre, descr_post, plural_len, true);
emit_must_use_untranslated(
cx,
path,
descr_pre,
descr_post,
plural_len,
true,
expr_is_from_block,
);
}
MustUsePath::TupleElement(elems) => {
for (index, path) in elems {
let descr_post = &format!(" in tuple element {}", index);
emit_must_use_untranslated(
cx, path, descr_pre, descr_post, plural_len, true,
cx,
path,
descr_pre,
descr_post,
plural_len,
true,
expr_is_from_block,
);
}
}
Expand All @@ -407,6 +468,7 @@ impl<'tcx> LateLintPass<'tcx> for UnusedResults {
descr_post,
plural_len.saturating_add(usize::try_from(*len).unwrap_or(usize::MAX)),
true,
expr_is_from_block,
);
}
MustUsePath::Closure(span) => {
Expand All @@ -433,8 +495,14 @@ impl<'tcx> LateLintPass<'tcx> for UnusedResults {
cx,
def_id: *def_id,
note: *reason,
suggestion: (!is_inner)
.then_some(UnusedDefSuggestion { span: span.shrink_to_lo() }),
suggestion: (!is_inner).then_some(if expr_is_from_block {
UnusedDefSuggestion::BlockTailExpr {
before_span: span.shrink_to_lo(),
after_span: span.shrink_to_hi(),
}
} else {
UnusedDefSuggestion::NormalExpr { span: span.shrink_to_lo() }
}),
},
);
}
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/primitive_docs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ mod prim_never {}
///
/// ```no_run
/// // Undefined behaviour
/// unsafe { char::from_u32_unchecked(0x110000) };
/// let _ = unsafe { char::from_u32_unchecked(0x110000) };
/// ```
///
/// USVs are also the exact set of values that may be encoded in UTF-8. Because
Expand Down
2 changes: 1 addition & 1 deletion library/core/tests/ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1001,7 +1001,7 @@ fn nonnull_tagged_pointer_with_provenance() {
assert_eq!(p.tag(), 3);
assert_eq!(unsafe { *p.pointer().as_ptr() }, 10);

unsafe { Box::from_raw(p.pointer().as_ptr()) };
unsafe { drop(Box::from_raw(p.pointer().as_ptr())) };

/// A non-null pointer type which carries several bits of metadata and maintains provenance.
#[repr(transparent)]
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/primitive_docs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ mod prim_never {}
///
/// ```no_run
/// // Undefined behaviour
/// unsafe { char::from_u32_unchecked(0x110000) };
/// let _ = unsafe { char::from_u32_unchecked(0x110000) };
/// ```
///
/// USVs are also the exact set of values that may be encoded in UTF-8. Because
Expand Down
3 changes: 2 additions & 1 deletion src/tools/clippy/tests/ui/transmute_ptr_to_ref.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#![warn(clippy::transmute_ptr_to_ref)]
#![allow(clippy::match_single_binding)]
#![allow(unused_must_use)]

unsafe fn _ptr_to_ref<T, U>(p: *const T, m: *mut T, o: *const U, om: *mut U) {
let _: &T = &*p;
Expand Down Expand Up @@ -38,7 +39,7 @@ fn _issue1231() {

type Bar<'a> = &'a u8;
let raw = 42 as *const i32;
unsafe { &*(raw as *const u8) };
let _ = unsafe { &*(raw as *const u8) };
}

unsafe fn _issue8924<'a, 'b, 'c>(x: *const &'a u32, y: *const &'b u32) -> &'c &'b u32 {
Expand Down
3 changes: 2 additions & 1 deletion src/tools/clippy/tests/ui/transmute_ptr_to_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#![warn(clippy::transmute_ptr_to_ref)]
#![allow(clippy::match_single_binding)]
#![allow(unused_must_use)]

unsafe fn _ptr_to_ref<T, U>(p: *const T, m: *mut T, o: *const U, om: *mut U) {
let _: &T = std::mem::transmute(p);
Expand Down Expand Up @@ -38,7 +39,7 @@ fn _issue1231() {

type Bar<'a> = &'a u8;
let raw = 42 as *const i32;
unsafe { std::mem::transmute::<_, Bar>(raw) };
let _ = unsafe { std::mem::transmute::<_, Bar>(raw) };
}

unsafe fn _issue8924<'a, 'b, 'c>(x: *const &'a u32, y: *const &'b u32) -> &'c &'b u32 {
Expand Down
Loading