diff options
author | Daniel Baumann <daniel.baumann@progress-linux.org> | 2024-05-30 03:57:19 +0000 |
---|---|---|
committer | Daniel Baumann <daniel.baumann@progress-linux.org> | 2024-05-30 03:57:19 +0000 |
commit | a0b8f38ab54ac451646aa00cd5e91b6c76f22a84 (patch) | |
tree | fc451898ccaf445814e26b46664d78702178101d /compiler/rustc_lint | |
parent | Adding debian version 1.71.1+dfsg1-2. (diff) | |
download | rustc-a0b8f38ab54ac451646aa00cd5e91b6c76f22a84.tar.xz rustc-a0b8f38ab54ac451646aa00cd5e91b6c76f22a84.zip |
Merging upstream version 1.72.1+dfsg1.
Signed-off-by: Daniel Baumann <daniel.baumann@progress-linux.org>
Diffstat (limited to 'compiler/rustc_lint')
-rw-r--r-- | compiler/rustc_lint/messages.ftl | 22 | ||||
-rw-r--r-- | compiler/rustc_lint/src/builtin.rs | 104 | ||||
-rw-r--r-- | compiler/rustc_lint/src/context.rs | 6 | ||||
-rw-r--r-- | compiler/rustc_lint/src/drop_forget_useless.rs | 44 | ||||
-rw-r--r-- | compiler/rustc_lint/src/errors.rs | 2 | ||||
-rw-r--r-- | compiler/rustc_lint/src/invalid_from_utf8.rs | 118 | ||||
-rw-r--r-- | compiler/rustc_lint/src/late.rs | 11 | ||||
-rw-r--r-- | compiler/rustc_lint/src/lib.rs | 6 | ||||
-rw-r--r-- | compiler/rustc_lint/src/lints.rs | 151 | ||||
-rw-r--r-- | compiler/rustc_lint/src/multiple_supertrait_upcastable.rs | 2 | ||||
-rw-r--r-- | compiler/rustc_lint/src/nonstandard_style.rs | 4 | ||||
-rw-r--r-- | compiler/rustc_lint/src/opaque_hidden_inferred_bound.rs | 9 | ||||
-rw-r--r-- | compiler/rustc_lint/src/reference_casting.rs | 73 | ||||
-rw-r--r-- | compiler/rustc_lint/src/traits.rs | 5 | ||||
-rw-r--r-- | compiler/rustc_lint/src/types.rs | 407 | ||||
-rw-r--r-- | compiler/rustc_lint/src/unused.rs | 158 |
16 files changed, 929 insertions, 193 deletions
diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 0fa67cdb3..22e22c833 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -262,8 +262,6 @@ lint_improper_ctypes_char_help = consider using `u32` or `libc::wchar_t` instead lint_improper_ctypes_char_reason = the `char` type has no C equivalent lint_improper_ctypes_dyn = trait objects have no C equivalent -lint_improper_ctypes_enum_phantomdata = this enum contains a PhantomData field - lint_improper_ctypes_enum_repr_help = consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum @@ -304,6 +302,21 @@ lint_improper_ctypes_union_layout_help = consider adding a `#[repr(C)]` or `#[re lint_improper_ctypes_union_layout_reason = this union has unspecified layout lint_improper_ctypes_union_non_exhaustive = this union is non-exhaustive +# FIXME: we should ordinalize $valid_up_to when we add support for doing so +lint_invalid_from_utf8_checked = calls to `{$method}` with a invalid literal always return an error + .label = the literal was valid UTF-8 up to the {$valid_up_to} bytes + +# FIXME: we should ordinalize $valid_up_to when we add support for doing so +lint_invalid_from_utf8_unchecked = calls to `{$method}` with a invalid literal are undefined behavior + .label = the literal was valid UTF-8 up to the {$valid_up_to} bytes + +lint_invalid_nan_comparisons_eq_ne = incorrect NaN comparison, NaN cannot be directly compared to itself + .suggestion = use `f32::is_nan()` or `f64::is_nan()` instead + +lint_invalid_nan_comparisons_lt_le_gt_ge = incorrect NaN comparison, NaN is not orderable + +lint_invalid_reference_casting = casting `&T` to `&mut T` is undefined behavior, even if the reference is unused, consider instead using an `UnsafeCell` + lint_lintpass_by_hand = implementing `LintPass` by hand .help = try using `declare_lint_pass!` or `impl_lint_pass!` instead @@ -410,6 +423,7 @@ lint_overflowing_bin_hex = literal out of range for `{$ty}` .negative_becomes_note = and the value `-{$lit}` will become `{$actually}{$ty}` .positive_note = the literal `{$lit}` (decimal `{$dec}`) does not fit into the type `{$ty}` and will become `{$actually}{$ty}` .suggestion = consider using the type `{$suggestion_ty}` instead + .sign_bit_suggestion = to use as a negative number (decimal `{$negative_val}`), consider using the type `{$uint_ty}` for the literal and cast it to `{$int_ty}` .help = consider using the type `{$suggestion_ty}` instead lint_overflowing_int = literal out of range for `{$ty}` @@ -480,6 +494,10 @@ lint_tykind = usage of `ty::TyKind` lint_tykind_kind = usage of `ty::TyKind::<kind>` .suggestion = try using `ty::<kind>` directly +lint_undropped_manually_drops = calls to `std::mem::drop` with `std::mem::ManuallyDrop` instead of the inner value does nothing + .label = argument has type `{$arg_ty}` + .suggestion = use `std::mem::ManuallyDrop::into_inner` to get the inner value + lint_ungated_async_fn_track_caller = `#[track_caller]` on async functions is a no-op .label = this function will not propagate the caller location diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index 85141836e..b821933e9 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -286,7 +286,9 @@ impl<'tcx> LateLintPass<'tcx> for NonShorthandFieldPatterns { } declare_lint! { - /// The `unsafe_code` lint catches usage of `unsafe` code. + /// The `unsafe_code` lint catches usage of `unsafe` code and other + /// potentially unsound constructs like `no_mangle`, `export_name`, + /// and `link_section`. /// /// ### Example /// @@ -297,17 +299,29 @@ declare_lint! { /// /// } /// } + /// + /// #[no_mangle] + /// fn func_0() { } + /// + /// #[export_name = "exported_symbol_name"] + /// pub fn name_in_rust() { } + /// + /// #[no_mangle] + /// #[link_section = ".example_section"] + /// pub static VAR1: u32 = 1; /// ``` /// /// {{produces}} /// /// ### Explanation /// - /// This lint is intended to restrict the usage of `unsafe`, which can be - /// difficult to use correctly. + /// This lint is intended to restrict the usage of `unsafe` blocks and other + /// constructs (including, but not limited to `no_mangle`, `link_section` + /// and `export_name` attributes) wrong usage of which causes undefined + /// behavior. UNSAFE_CODE, Allow, - "usage of `unsafe` code" + "usage of `unsafe` code and other potentially unsound constructs" } declare_lint_pass!(UnsafeCode => [UNSAFE_CODE]); @@ -651,9 +665,7 @@ declare_lint_pass!(MissingCopyImplementations => [MISSING_COPY_IMPLEMENTATIONS]) impl<'tcx> LateLintPass<'tcx> for MissingCopyImplementations { fn check_item(&mut self, cx: &LateContext<'_>, item: &hir::Item<'_>) { - if !(cx.effective_visibilities.is_reachable(item.owner_id.def_id) - && cx.tcx.local_visibility(item.owner_id.def_id).is_public()) - { + if !cx.effective_visibilities.is_reachable(item.owner_id.def_id) { return; } let (def, ty) = match item.kind { @@ -662,21 +674,21 @@ impl<'tcx> LateLintPass<'tcx> for MissingCopyImplementations { return; } let def = cx.tcx.adt_def(item.owner_id); - (def, cx.tcx.mk_adt(def, ty::List::empty())) + (def, Ty::new_adt(cx.tcx, def, ty::List::empty())) } hir::ItemKind::Union(_, ref ast_generics) => { if !ast_generics.params.is_empty() { return; } let def = cx.tcx.adt_def(item.owner_id); - (def, cx.tcx.mk_adt(def, ty::List::empty())) + (def, Ty::new_adt(cx.tcx, def, ty::List::empty())) } hir::ItemKind::Enum(_, ref ast_generics) => { if !ast_generics.params.is_empty() { return; } let def = cx.tcx.adt_def(item.owner_id); - (def, cx.tcx.mk_adt(def, ty::List::empty())) + (def, Ty::new_adt(cx.tcx, def, ty::List::empty())) } _ => return, }; @@ -772,9 +784,7 @@ impl_lint_pass!(MissingDebugImplementations => [MISSING_DEBUG_IMPLEMENTATIONS]); impl<'tcx> LateLintPass<'tcx> for MissingDebugImplementations { fn check_item(&mut self, cx: &LateContext<'_>, item: &hir::Item<'_>) { - if !(cx.effective_visibilities.is_reachable(item.owner_id.def_id) - && cx.tcx.local_visibility(item.owner_id.def_id).is_public()) - { + if !cx.effective_visibilities.is_reachable(item.owner_id.def_id) { return; } @@ -1321,10 +1331,14 @@ declare_lint! { /// /// ### Explanation /// - /// A bare `pub` visibility may be misleading if the item is not actually - /// publicly exported from the crate. The `pub(crate)` visibility is - /// recommended to be used instead, which more clearly expresses the intent - /// that the item is only visible within its own crate. + /// The `pub` keyword both expresses an intent for an item to be publicly available, and also + /// signals to the compiler to make the item publicly accessible. The intent can only be + /// satisfied, however, if all items which contain this item are *also* publicly accessible. + /// Thus, this lint serves to identify situations where the intent does not match the reality. + /// + /// If you wish the item to be accessible elsewhere within the crate, but not outside it, the + /// `pub(crate)` visibility is recommended to be used instead. This more clearly expresses the + /// intent that the item is only visible within its own crate. /// /// This lint is "allow" by default because it will trigger for a large /// amount existing Rust code, and has some false-positives. Eventually it @@ -1447,8 +1461,8 @@ impl<'tcx> LateLintPass<'tcx> for TypeAliasBounds { let hir::ItemKind::TyAlias(ty, type_alias_generics) = &item.kind else { return }; - if let hir::TyKind::OpaqueDef(..) = ty.kind { - // Bounds are respected for `type X = impl Trait` + if cx.tcx.type_of(item.owner_id.def_id).skip_binder().has_opaque_types() { + // Bounds are respected for `type X = impl Trait` and `type X = (impl Trait, Y);` return; } if cx.tcx.type_of(item.owner_id).skip_binder().has_inherent_projections() { @@ -1574,34 +1588,25 @@ declare_lint_pass!( impl<'tcx> LateLintPass<'tcx> for TrivialConstraints { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) { - use rustc_middle::ty::Clause; - use rustc_middle::ty::PredicateKind::*; + use rustc_middle::ty::ClauseKind; if cx.tcx.features().trivial_bounds { let predicates = cx.tcx.predicates_of(item.owner_id); for &(predicate, span) in predicates.predicates { let predicate_kind_name = match predicate.kind().skip_binder() { - Clause(Clause::Trait(..)) => "trait", - Clause(Clause::TypeOutlives(..)) | - Clause(Clause::RegionOutlives(..)) => "lifetime", + ClauseKind::Trait(..) => "trait", + ClauseKind::TypeOutlives(..) | + ClauseKind::RegionOutlives(..) => "lifetime", // `ConstArgHasType` is never global as `ct` is always a param - Clause(Clause::ConstArgHasType(..)) | + ClauseKind::ConstArgHasType(..) // Ignore projections, as they can only be global // if the trait bound is global - Clause(Clause::Projection(..)) | - AliasRelate(..) | + | ClauseKind::Projection(..) // Ignore bounds that a user can't type - WellFormed(..) | - ObjectSafe(..) | - ClosureKind(..) | - Subtype(..) | - Coerce(..) | + | ClauseKind::WellFormed(..) // FIXME(generic_const_exprs): `ConstEvaluatable` can be written - ConstEvaluatable(..) | - ConstEquate(..) | - Ambiguous | - TypeWellFormedFromEnv(..) => continue, + | ClauseKind::ConstEvaluatable(..) => continue, }; if predicate.is_global() { cx.emit_spanned_lint( @@ -1971,8 +1976,8 @@ impl ExplicitOutlivesRequirements { ) -> Vec<ty::Region<'tcx>> { inferred_outlives .iter() - .filter_map(|(clause, _)| match *clause { - ty::Clause::RegionOutlives(ty::OutlivesPredicate(a, b)) => match *a { + .filter_map(|(clause, _)| match clause.kind().skip_binder() { + ty::ClauseKind::RegionOutlives(ty::OutlivesPredicate(a, b)) => match *a { ty::ReEarlyBound(ebr) if ebr.def_id == def_id => Some(b), _ => None, }, @@ -1987,8 +1992,8 @@ impl ExplicitOutlivesRequirements { ) -> Vec<ty::Region<'tcx>> { inferred_outlives .iter() - .filter_map(|(clause, _)| match *clause { - ty::Clause::TypeOutlives(ty::OutlivesPredicate(a, b)) => { + .filter_map(|(clause, _)| match clause.kind().skip_binder() { + ty::ClauseKind::TypeOutlives(ty::OutlivesPredicate(a, b)) => { a.is_param(index).then_some(b) } _ => None, @@ -2106,12 +2111,16 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitOutlivesRequirements { } let ty_generics = cx.tcx.generics_of(def_id); + let num_where_predicates = hir_generics + .predicates + .iter() + .filter(|predicate| predicate.in_where_clause()) + .count(); let mut bound_count = 0; let mut lint_spans = Vec::new(); let mut where_lint_spans = Vec::new(); - let mut dropped_predicate_count = 0; - let num_predicates = hir_generics.predicates.len(); + let mut dropped_where_predicate_count = 0; for (i, where_predicate) in hir_generics.predicates.iter().enumerate() { let (relevant_lifetimes, bounds, predicate_span, in_where_clause) = match where_predicate { @@ -2168,8 +2177,8 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitOutlivesRequirements { bound_count += bound_spans.len(); let drop_predicate = bound_spans.len() == bounds.len(); - if drop_predicate { - dropped_predicate_count += 1; + if drop_predicate && in_where_clause { + dropped_where_predicate_count += 1; } if drop_predicate { @@ -2178,7 +2187,7 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitOutlivesRequirements { } else if predicate_span.from_expansion() { // Don't try to extend the span if it comes from a macro expansion. where_lint_spans.push(predicate_span); - } else if i + 1 < num_predicates { + } else if i + 1 < num_where_predicates { // If all the bounds on a predicate were inferable and there are // further predicates, we want to eat the trailing comma. let next_predicate_span = hir_generics.predicates[i + 1].span(); @@ -2206,9 +2215,10 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitOutlivesRequirements { } } - // If all predicates are inferable, drop the entire clause + // If all predicates in where clause are inferable, drop the entire clause // (including the `where`) - if hir_generics.has_where_clause_predicates && dropped_predicate_count == num_predicates + if hir_generics.has_where_clause_predicates + && dropped_where_predicate_count == num_where_predicates { let where_span = hir_generics.where_clause_span; // Extend the where clause back to the closing `>` of the diff --git a/compiler/rustc_lint/src/context.rs b/compiler/rustc_lint/src/context.rs index 1d0c43e95..3761754f3 100644 --- a/compiler/rustc_lint/src/context.rs +++ b/compiler/rustc_lint/src/context.rs @@ -952,6 +952,10 @@ pub trait LintContext: Sized { db.span_label(first_reexport_span, format!("the name `{}` in the {} namespace is first re-exported here", name, namespace)); db.span_label(duplicate_reexport_span, format!("but the name `{}` in the {} namespace is also re-exported here", name, namespace)); } + BuiltinLintDiagnostics::HiddenGlobReexports { name, namespace, glob_reexport_span, private_item_span } => { + db.span_note(glob_reexport_span, format!("the name `{}` in the {} namespace is supposed to be publicly re-exported here", name, namespace)); + db.span_note(private_item_span, "but the private item here shadows it".to_owned()); + } } // Rewrap `db`, and pass control to the user. decorate(db) @@ -1341,7 +1345,7 @@ impl<'tcx> LateContext<'tcx> { tcx.associated_items(trait_id) .find_by_name_and_kind(tcx, Ident::from_str(name), ty::AssocKind::Type, trait_id) .and_then(|assoc| { - let proj = tcx.mk_projection(assoc.def_id, [self_ty]); + let proj = Ty::new_projection(tcx, assoc.def_id, [self_ty]); tcx.try_normalize_erasing_regions(self.param_env, proj).ok() }) } diff --git a/compiler/rustc_lint/src/drop_forget_useless.rs b/compiler/rustc_lint/src/drop_forget_useless.rs index 77e4a7669..467f53d44 100644 --- a/compiler/rustc_lint/src/drop_forget_useless.rs +++ b/compiler/rustc_lint/src/drop_forget_useless.rs @@ -1,8 +1,12 @@ use rustc_hir::{Arm, Expr, ExprKind, Node}; +use rustc_middle::ty; use rustc_span::sym; use crate::{ - lints::{DropCopyDiag, DropRefDiag, ForgetCopyDiag, ForgetRefDiag}, + lints::{ + DropCopyDiag, DropRefDiag, ForgetCopyDiag, ForgetRefDiag, UndroppedManuallyDropsDiag, + UndroppedManuallyDropsSuggestion, + }, LateContext, LateLintPass, LintContext, }; @@ -109,7 +113,29 @@ declare_lint! { "calls to `std::mem::forget` with a value that implements Copy" } -declare_lint_pass!(DropForgetUseless => [DROPPING_REFERENCES, FORGETTING_REFERENCES, DROPPING_COPY_TYPES, FORGETTING_COPY_TYPES]); +declare_lint! { + /// The `undropped_manually_drops` lint check for calls to `std::mem::drop` with + /// a value of `std::mem::ManuallyDrop` which doesn't drop. + /// + /// ### Example + /// + /// ```rust,compile_fail + /// struct S; + /// drop(std::mem::ManuallyDrop::new(S)); + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// `ManuallyDrop` does not drop it's inner value so calling `std::mem::drop` will + /// not drop the inner value of the `ManuallyDrop` either. + pub UNDROPPED_MANUALLY_DROPS, + Deny, + "calls to `std::mem::drop` with `std::mem::ManuallyDrop` instead of it's inner value" +} + +declare_lint_pass!(DropForgetUseless => [DROPPING_REFERENCES, FORGETTING_REFERENCES, DROPPING_COPY_TYPES, FORGETTING_COPY_TYPES, UNDROPPED_MANUALLY_DROPS]); impl<'tcx> LateLintPass<'tcx> for DropForgetUseless { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { @@ -134,6 +160,20 @@ impl<'tcx> LateLintPass<'tcx> for DropForgetUseless { sym::mem_forget if is_copy => { cx.emit_spanned_lint(FORGETTING_COPY_TYPES, expr.span, ForgetCopyDiag { arg_ty, label: arg.span }); } + sym::mem_drop if let ty::Adt(adt, _) = arg_ty.kind() && adt.is_manually_drop() => { + cx.emit_spanned_lint( + UNDROPPED_MANUALLY_DROPS, + expr.span, + UndroppedManuallyDropsDiag { + arg_ty, + label: arg.span, + suggestion: UndroppedManuallyDropsSuggestion { + start_span: arg.span.shrink_to_lo(), + end_span: arg.span.shrink_to_hi() + } + } + ); + } _ => return, }; } diff --git a/compiler/rustc_lint/src/errors.rs b/compiler/rustc_lint/src/errors.rs index bbae3d368..68167487a 100644 --- a/compiler/rustc_lint/src/errors.rs +++ b/compiler/rustc_lint/src/errors.rs @@ -39,7 +39,7 @@ impl AddToDiagnostic for OverruledAttributeSub { diag.span_label(span, fluent::lint_node_source); if let Some(rationale) = reason { #[allow(rustc::untranslatable_diagnostic)] - diag.note(rationale.as_str()); + diag.note(rationale.to_string()); } } OverruledAttributeSub::CommandLineSource => { diff --git a/compiler/rustc_lint/src/invalid_from_utf8.rs b/compiler/rustc_lint/src/invalid_from_utf8.rs new file mode 100644 index 000000000..3291286ad --- /dev/null +++ b/compiler/rustc_lint/src/invalid_from_utf8.rs @@ -0,0 +1,118 @@ +use std::str::Utf8Error; + +use rustc_ast::{BorrowKind, LitKind}; +use rustc_hir::{Expr, ExprKind}; +use rustc_span::source_map::Spanned; +use rustc_span::sym; + +use crate::lints::InvalidFromUtf8Diag; +use crate::{LateContext, LateLintPass, LintContext}; + +declare_lint! { + /// The `invalid_from_utf8_unchecked` lint checks for calls to + /// `std::str::from_utf8_unchecked` and `std::str::from_utf8_unchecked_mut` + /// with an invalid UTF-8 literal. + /// + /// ### Example + /// + /// ```rust,compile_fail + /// # #[allow(unused)] + /// unsafe { + /// std::str::from_utf8_unchecked(b"Ru\x82st"); + /// } + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Creating such a `str` would result in undefined behavior as per documentation + /// for `std::str::from_utf8_unchecked` and `std::str::from_utf8_unchecked_mut`. + pub INVALID_FROM_UTF8_UNCHECKED, + Deny, + "using a non UTF-8 literal in `std::str::from_utf8_unchecked`" +} + +declare_lint! { + /// The `invalid_from_utf8` lint checks for calls to + /// `std::str::from_utf8` and `std::str::from_utf8_mut` + /// with an invalid UTF-8 literal. + /// + /// ### Example + /// + /// ```rust + /// # #[allow(unused)] + /// std::str::from_utf8(b"Ru\x82st"); + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Trying to create such a `str` would always return an error as per documentation + /// for `std::str::from_utf8` and `std::str::from_utf8_mut`. + pub INVALID_FROM_UTF8, + Warn, + "using a non UTF-8 literal in `std::str::from_utf8`" +} + +declare_lint_pass!(InvalidFromUtf8 => [INVALID_FROM_UTF8_UNCHECKED, INVALID_FROM_UTF8]); + +impl<'tcx> LateLintPass<'tcx> for InvalidFromUtf8 { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + if let ExprKind::Call(path, [arg]) = expr.kind + && let ExprKind::Path(ref qpath) = path.kind + && let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id() + && let Some(diag_item) = cx.tcx.get_diagnostic_name(def_id) + && [sym::str_from_utf8, sym::str_from_utf8_mut, + sym::str_from_utf8_unchecked, sym::str_from_utf8_unchecked_mut].contains(&diag_item) + { + let lint = |utf8_error: Utf8Error| { + let label = arg.span; + let method = diag_item.as_str().strip_prefix("str_").unwrap(); + let method = format!("std::str::{method}"); + let valid_up_to = utf8_error.valid_up_to(); + let is_unchecked_variant = diag_item.as_str().contains("unchecked"); + + cx.emit_spanned_lint( + if is_unchecked_variant { INVALID_FROM_UTF8_UNCHECKED } else { INVALID_FROM_UTF8 }, + expr.span, + if is_unchecked_variant { + InvalidFromUtf8Diag::Unchecked { method, valid_up_to, label } + } else { + InvalidFromUtf8Diag::Checked { method, valid_up_to, label } + } + ) + }; + + match &arg.kind { + ExprKind::Lit(Spanned { node: lit, .. }) => { + if let LitKind::ByteStr(bytes, _) = &lit + && let Err(utf8_error) = std::str::from_utf8(bytes) + { + lint(utf8_error); + } + }, + ExprKind::AddrOf(BorrowKind::Ref, _, Expr { kind: ExprKind::Array(args), .. }) => { + let elements = args.iter().map(|e|{ + match &e.kind { + ExprKind::Lit(Spanned { node: lit, .. }) => match lit { + LitKind::Byte(b) => Some(*b), + LitKind::Int(b, _) => Some(*b as u8), + _ => None + } + _ => None + } + }).collect::<Option<Vec<_>>>(); + + if let Some(elements) = elements + && let Err(utf8_error) = std::str::from_utf8(&elements) + { + lint(utf8_error); + } + } + _ => {} + } + } + } +} diff --git a/compiler/rustc_lint/src/late.rs b/compiler/rustc_lint/src/late.rs index 8a4a451f8..fb12ded71 100644 --- a/compiler/rustc_lint/src/late.rs +++ b/compiler/rustc_lint/src/late.rs @@ -16,6 +16,7 @@ use crate::{passes::LateLintPassObject, LateContext, LateLintPass, LintStore}; use rustc_ast as ast; +use rustc_data_structures::stack::ensure_sufficient_stack; use rustc_data_structures::sync::{join, DynSend}; use rustc_hir as hir; use rustc_hir::def_id::LocalDefId; @@ -157,10 +158,12 @@ impl<'tcx, T: LateLintPass<'tcx>> hir_visit::Visitor<'tcx> for LateContextAndPas } fn visit_expr(&mut self, e: &'tcx hir::Expr<'tcx>) { - self.with_lint_attrs(e.hir_id, |cx| { - lint_callback!(cx, check_expr, e); - hir_visit::walk_expr(cx, e); - lint_callback!(cx, check_expr_post, e); + ensure_sufficient_stack(|| { + self.with_lint_attrs(e.hir_id, |cx| { + lint_callback!(cx, check_expr, e); + hir_visit::walk_expr(cx, e); + lint_callback!(cx, check_expr_post, e); + }) }) } diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index dfddfe09a..602071a55 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -60,6 +60,7 @@ mod expect; mod for_loops_over_fallibles; pub mod hidden_unicode_codepoints; mod internal; +mod invalid_from_utf8; mod late; mod let_underscore; mod levels; @@ -75,6 +76,7 @@ mod opaque_hidden_inferred_bound; mod pass_by_value; mod passes; mod redundant_semicolon; +mod reference_casting; mod traits; mod types; mod unused; @@ -102,6 +104,7 @@ use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums; use for_loops_over_fallibles::*; use hidden_unicode_codepoints::*; use internal::*; +use invalid_from_utf8::*; use let_underscore::*; use map_unit_fn::*; use methods::*; @@ -113,6 +116,7 @@ use noop_method_call::*; use opaque_hidden_inferred_bound::*; use pass_by_value::*; use redundant_semicolon::*; +use reference_casting::*; use traits::*; use types::*; use unused::*; @@ -207,10 +211,12 @@ late_lint_methods!( HardwiredLints: HardwiredLints, ImproperCTypesDeclarations: ImproperCTypesDeclarations, ImproperCTypesDefinitions: ImproperCTypesDefinitions, + InvalidFromUtf8: InvalidFromUtf8, VariantSizeDifferences: VariantSizeDifferences, BoxPointers: BoxPointers, PathStatements: PathStatements, LetUnderscore: LetUnderscore, + InvalidReferenceCasting: InvalidReferenceCasting, // Depends on referenced function signatures in expressions UnusedResults: UnusedResults, NonUpperCaseGlobals: NonUpperCaseGlobals, diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index d96723a68..9260237fb 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -10,7 +10,7 @@ use rustc_errors::{ use rustc_hir::def_id::DefId; use rustc_macros::{LintDiagnostic, Subdiagnostic}; use rustc_middle::ty::{ - inhabitedness::InhabitedPredicate, PolyExistentialTraitRef, Predicate, Ty, TyCtxt, + inhabitedness::InhabitedPredicate, Clause, PolyExistentialTraitRef, Ty, TyCtxt, }; use rustc_session::parse::ParseSess; use rustc_span::{edition::Edition, sym, symbol::Ident, Span, Symbol}; @@ -352,7 +352,7 @@ impl AddToDiagnostic for BuiltinTypeAliasGenericBoundsSuggestion { #[diag(lint_builtin_trivial_bounds)] pub struct BuiltinTrivialBounds<'a> { pub predicate_kind_name: &'a str, - pub predicate: Predicate<'a>, + pub predicate: Clause<'a>, } #[derive(LintDiagnostic)] @@ -699,6 +699,49 @@ pub struct ForgetCopyDiag<'a> { pub label: Span, } +#[derive(LintDiagnostic)] +#[diag(lint_undropped_manually_drops)] +pub struct UndroppedManuallyDropsDiag<'a> { + pub arg_ty: Ty<'a>, + #[label] + pub label: Span, + #[subdiagnostic] + pub suggestion: UndroppedManuallyDropsSuggestion, +} + +#[derive(Subdiagnostic)] +#[multipart_suggestion(lint_suggestion, applicability = "machine-applicable")] +pub struct UndroppedManuallyDropsSuggestion { + #[suggestion_part(code = "std::mem::ManuallyDrop::into_inner(")] + pub start_span: Span, + #[suggestion_part(code = ")")] + pub end_span: Span, +} + +// invalid_from_utf8.rs +#[derive(LintDiagnostic)] +pub enum InvalidFromUtf8Diag { + #[diag(lint_invalid_from_utf8_unchecked)] + Unchecked { + method: String, + valid_up_to: usize, + #[label] + label: Span, + }, + #[diag(lint_invalid_from_utf8_checked)] + Checked { + method: String, + valid_up_to: usize, + #[label] + label: Span, + }, +} + +// reference_casting.rs +#[derive(LintDiagnostic)] +#[diag(lint_invalid_reference_casting)] +pub struct InvalidReferenceCastingDiag; + // hidden_unicode_codepoints.rs #[derive(LintDiagnostic)] #[diag(lint_hidden_unicode_codepoints)] @@ -1219,7 +1262,7 @@ pub struct RedundantSemicolonsDiag { // traits.rs pub struct DropTraitConstraintsDiag<'a> { - pub predicate: Predicate<'a>, + pub predicate: Clause<'a>, pub tcx: TyCtxt<'a>, pub def_id: DefId, } @@ -1303,6 +1346,8 @@ pub struct OverflowingBinHex<'a> { pub sign: OverflowingBinHexSign, #[subdiagnostic] pub sub: Option<OverflowingBinHexSub<'a>>, + #[subdiagnostic] + pub sign_bit_sub: Option<OverflowingBinHexSignBitSub<'a>>, } pub enum OverflowingBinHexSign { @@ -1347,6 +1392,21 @@ pub enum OverflowingBinHexSub<'a> { Help { suggestion_ty: &'a str }, } +#[derive(Subdiagnostic)] +#[suggestion( + lint_sign_bit_suggestion, + code = "{lit_no_suffix}{uint_ty} as {int_ty}", + applicability = "maybe-incorrect" +)] +pub struct OverflowingBinHexSignBitSub<'a> { + #[primary_span] + pub span: Span, + pub lit_no_suffix: &'a str, + pub negative_val: String, + pub uint_ty: &'a str, + pub int_ty: &'a str, +} + #[derive(LintDiagnostic)] #[diag(lint_overflowing_int)] #[note] @@ -1395,6 +1455,36 @@ pub struct OverflowingLiteral<'a> { #[diag(lint_unused_comparisons)] pub struct UnusedComparisons; +#[derive(LintDiagnostic)] +pub enum InvalidNanComparisons { + #[diag(lint_invalid_nan_comparisons_eq_ne)] + EqNe { + #[subdiagnostic] + suggestion: InvalidNanComparisonsSuggestion, + }, + #[diag(lint_invalid_nan_comparisons_lt_le_gt_ge)] + LtLeGtGe, +} + +#[derive(Subdiagnostic)] +pub enum InvalidNanComparisonsSuggestion { + #[multipart_suggestion( + lint_suggestion, + style = "verbose", + applicability = "machine-applicable" + )] + Spanful { + #[suggestion_part(code = "!")] + neg: Option<Span>, + #[suggestion_part(code = ".is_nan()")] + float: Span, + #[suggestion_part(code = "")] + nan_plus_binop: Span, + }, + #[help(lint_suggestion)] + Spanless, +} + pub struct ImproperCTypes<'a> { pub ty: Ty<'a>, pub desc: &'a str, @@ -1465,8 +1555,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)] @@ -1509,15 +1620,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 @@ -1531,7 +1652,7 @@ impl<'a> DecorateLint<'a, ()> for UnusedDef<'_, '_> { diag.set_arg("def", self.cx.tcx.def_path_str(self.def_id)); // check for #[must_use = "..."] if let Some(note) = self.note { - diag.note(note.as_str()); + diag.note(note.to_string()); } if let Some(sugg) = self.suggestion { diag.subdiagnostic(sugg); diff --git a/compiler/rustc_lint/src/multiple_supertrait_upcastable.rs b/compiler/rustc_lint/src/multiple_supertrait_upcastable.rs index c2ed0e19f..53fe0ceb2 100644 --- a/compiler/rustc_lint/src/multiple_supertrait_upcastable.rs +++ b/compiler/rustc_lint/src/multiple_supertrait_upcastable.rs @@ -45,7 +45,7 @@ impl<'tcx> LateLintPass<'tcx> for MultipleSupertraitUpcastable { .super_predicates_of(def_id) .predicates .into_iter() - .filter_map(|(pred, _)| pred.to_opt_poly_trait_pred()); + .filter_map(|(pred, _)| pred.as_trait_clause()); if direct_super_traits_iter.count() > 1 { cx.emit_spanned_lint( MULTIPLE_SUPERTRAIT_UPCASTABLE, diff --git a/compiler/rustc_lint/src/nonstandard_style.rs b/compiler/rustc_lint/src/nonstandard_style.rs index 79253cbc8..145de4948 100644 --- a/compiler/rustc_lint/src/nonstandard_style.rs +++ b/compiler/rustc_lint/src/nonstandard_style.rs @@ -533,6 +533,10 @@ impl<'tcx> LateLintPass<'tcx> for NonUpperCaseGlobals { fn check_generic_param(&mut self, cx: &LateContext<'_>, param: &hir::GenericParam<'_>) { if let GenericParamKind::Const { .. } = param.kind { + // `rustc_host` params are explicitly allowed to be lowercase. + if cx.tcx.has_attr(param.def_id, sym::rustc_host) { + return; + } NonUpperCaseGlobals::check_upper_case(cx, "const parameter", ¶m.name.ident()); } } diff --git a/compiler/rustc_lint/src/opaque_hidden_inferred_bound.rs b/compiler/rustc_lint/src/opaque_hidden_inferred_bound.rs index 15715c8fc..09a1651c2 100644 --- a/compiler/rustc_lint/src/opaque_hidden_inferred_bound.rs +++ b/compiler/rustc_lint/src/opaque_hidden_inferred_bound.rs @@ -78,7 +78,7 @@ impl<'tcx> LateLintPass<'tcx> for OpaqueHiddenInferredBound { // Liberate bound regions in the predicate since we // don't actually care about lifetimes in this check. let predicate = cx.tcx.liberate_late_bound_regions(def_id, pred.kind()); - let ty::PredicateKind::Clause(ty::Clause::Projection(proj)) = predicate else { + let ty::ClauseKind::Projection(proj) = predicate else { continue; }; // Only check types, since those are the only things that may @@ -97,7 +97,7 @@ impl<'tcx> LateLintPass<'tcx> for OpaqueHiddenInferredBound { } let proj_ty = - cx.tcx.mk_projection(proj.projection_ty.def_id, proj.projection_ty.substs); + Ty::new_projection(cx.tcx, proj.projection_ty.def_id, proj.projection_ty.substs); // For every instance of the projection type in the bounds, // replace them with the term we're assigning to the associated // type in our opaque type. @@ -133,7 +133,7 @@ impl<'tcx> LateLintPass<'tcx> for OpaqueHiddenInferredBound { let add_bound = match (proj_term.kind(), assoc_pred.kind().skip_binder()) { ( ty::Alias(ty::Opaque, ty::AliasTy { def_id, .. }), - ty::PredicateKind::Clause(ty::Clause::Trait(trait_pred)), + ty::ClauseKind::Trait(trait_pred), ) => Some(AddBound { suggest_span: cx.tcx.def_span(*def_id).shrink_to_hi(), trait_ref: trait_pred.print_modifiers_and_trait_path(), @@ -144,7 +144,8 @@ impl<'tcx> LateLintPass<'tcx> for OpaqueHiddenInferredBound { OPAQUE_HIDDEN_INFERRED_BOUND, pred_span, OpaqueHiddenInferredBoundLint { - ty: cx.tcx.mk_opaque( + ty: Ty::new_opaque( + cx.tcx, def_id, ty::InternalSubsts::identity_for_item(cx.tcx, def_id), ), diff --git a/compiler/rustc_lint/src/reference_casting.rs b/compiler/rustc_lint/src/reference_casting.rs new file mode 100644 index 000000000..db8d7302a --- /dev/null +++ b/compiler/rustc_lint/src/reference_casting.rs @@ -0,0 +1,73 @@ +use rustc_ast::Mutability; +use rustc_hir::{Expr, ExprKind, MutTy, TyKind, UnOp}; +use rustc_middle::ty; +use rustc_span::sym; + +use crate::{lints::InvalidReferenceCastingDiag, LateContext, LateLintPass, LintContext}; + +declare_lint! { + /// The `invalid_reference_casting` lint checks for casts of `&T` to `&mut T` + /// without using interior mutability. + /// + /// ### Example + /// + /// ```rust,compile_fail + /// # #![deny(invalid_reference_casting)] + /// fn x(r: &i32) { + /// unsafe { + /// *(r as *const i32 as *mut i32) += 1; + /// } + /// } + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Casting `&T` to `&mut T` without using interior mutability is undefined behavior, + /// as it's a violation of Rust reference aliasing requirements. + /// + /// `UnsafeCell` is the only way to obtain aliasable data that is considered + /// mutable. + INVALID_REFERENCE_CASTING, + Allow, + "casts of `&T` to `&mut T` without interior mutability" +} + +declare_lint_pass!(InvalidReferenceCasting => [INVALID_REFERENCE_CASTING]); + +impl<'tcx> LateLintPass<'tcx> for InvalidReferenceCasting { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + let ExprKind::Unary(UnOp::Deref, e) = &expr.kind else { return; }; + + let e = e.peel_blocks(); + let e = if let ExprKind::Cast(e, t) = e.kind + && let TyKind::Ptr(MutTy { mutbl: Mutability::Mut, .. }) = t.kind { + e + } else if let ExprKind::MethodCall(_, expr, [], _) = e.kind + && let Some(def_id) = cx.typeck_results().type_dependent_def_id(e.hir_id) + && cx.tcx.is_diagnostic_item(sym::ptr_cast_mut, def_id) { + expr + } else { + return; + }; + + let e = e.peel_blocks(); + let e = if let ExprKind::Cast(e, t) = e.kind + && let TyKind::Ptr(MutTy { mutbl: Mutability::Not, .. }) = t.kind { + e + } else if let ExprKind::Call(path, [arg]) = e.kind + && let ExprKind::Path(ref qpath) = path.kind + && let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id() + && cx.tcx.is_diagnostic_item(sym::ptr_from_ref, def_id) { + arg + } else { + return; + }; + + let e = e.peel_blocks(); + if let ty::Ref(..) = cx.typeck_results().node_type(e.hir_id).kind() { + cx.emit_spanned_lint(INVALID_REFERENCE_CASTING, expr.span, InvalidReferenceCastingDiag); + } + } +} diff --git a/compiler/rustc_lint/src/traits.rs b/compiler/rustc_lint/src/traits.rs index 7ea1a138b..de1120806 100644 --- a/compiler/rustc_lint/src/traits.rs +++ b/compiler/rustc_lint/src/traits.rs @@ -87,12 +87,11 @@ declare_lint_pass!( impl<'tcx> LateLintPass<'tcx> for DropTraitConstraints { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) { - use rustc_middle::ty::Clause; - use rustc_middle::ty::PredicateKind::*; + use rustc_middle::ty::ClauseKind; let predicates = cx.tcx.explicit_predicates_of(item.owner_id); for &(predicate, span) in predicates.predicates { - let Clause(Clause::Trait(trait_predicate)) = predicate.kind().skip_binder() else { + let ClauseKind::Trait(trait_predicate) = predicate.kind().skip_binder() else { continue }; let def_id = trait_predicate.trait_ref.def_id; diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index 4bf4fda82..ed4fb9860 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -2,7 +2,8 @@ use crate::{ fluent_generated as fluent, lints::{ AtomicOrderingFence, AtomicOrderingLoad, AtomicOrderingStore, ImproperCTypes, - InvalidAtomicOrderingDiag, OnlyCastu8ToChar, OverflowingBinHex, OverflowingBinHexSign, + InvalidAtomicOrderingDiag, InvalidNanComparisons, InvalidNanComparisonsSuggestion, + OnlyCastu8ToChar, OverflowingBinHex, OverflowingBinHexSign, OverflowingBinHexSignBitSub, OverflowingBinHexSub, OverflowingInt, OverflowingIntHelp, OverflowingLiteral, OverflowingUInt, RangeEndpointOutOfRange, UnusedComparisons, UseInclusiveRange, VariantSizeDifferencesDiag, @@ -113,13 +114,35 @@ declare_lint! { "detects enums with widely varying variant sizes" } +declare_lint! { + /// The `invalid_nan_comparisons` lint checks comparison with `f32::NAN` or `f64::NAN` + /// as one of the operand. + /// + /// ### Example + /// + /// ```rust + /// let a = 2.3f32; + /// if a == f32::NAN {} + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// NaN does not compare meaningfully to anything – not + /// even itself – so those comparisons are always false. + INVALID_NAN_COMPARISONS, + Warn, + "detects invalid floating point NaN comparisons" +} + #[derive(Copy, Clone)] pub struct TypeLimits { /// Id of the last visited negated expression negated_expr_id: Option<hir::HirId>, } -impl_lint_pass!(TypeLimits => [UNUSED_COMPARISONS, OVERFLOWING_LITERALS]); +impl_lint_pass!(TypeLimits => [UNUSED_COMPARISONS, OVERFLOWING_LITERALS, INVALID_NAN_COMPARISONS]); impl TypeLimits { pub fn new() -> TypeLimits { @@ -275,10 +298,50 @@ fn report_bin_hex_error( } }, ); + let sign_bit_sub = (!negative) + .then(|| { + let ty::Int(int_ty) = cx.typeck_results().node_type(expr.hir_id).kind() else { + return None; + }; + + let Some(bit_width) = int_ty.bit_width() else { + return None; // isize case + }; + + // Skip if sign bit is not set + if (val & (1 << (bit_width - 1))) == 0 { + return None; + } + + let lit_no_suffix = + if let Some(pos) = repr_str.chars().position(|c| c == 'i' || c == 'u') { + repr_str.split_at(pos).0 + } else { + &repr_str + }; + + Some(OverflowingBinHexSignBitSub { + span: expr.span, + lit_no_suffix, + negative_val: actually.clone(), + int_ty: int_ty.name_str(), + uint_ty: int_ty.to_unsigned().name_str(), + }) + }) + .flatten(); + cx.emit_spanned_lint( OVERFLOWING_LITERALS, expr.span, - OverflowingBinHex { ty: t, lit: repr_str.clone(), dec: val, actually, sign, sub }, + OverflowingBinHex { + ty: t, + lit: repr_str.clone(), + dec: val, + actually, + sign, + sub, + sign_bit_sub, + }, ) } @@ -486,6 +549,68 @@ fn lint_literal<'tcx>( } } +fn lint_nan<'tcx>( + cx: &LateContext<'tcx>, + e: &'tcx hir::Expr<'tcx>, + binop: hir::BinOp, + l: &'tcx hir::Expr<'tcx>, + r: &'tcx hir::Expr<'tcx>, +) { + fn is_nan(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool { + let expr = expr.peel_blocks().peel_borrows(); + match expr.kind { + ExprKind::Path(qpath) => { + let Some(def_id) = cx.typeck_results().qpath_res(&qpath, expr.hir_id).opt_def_id() else { return false; }; + + matches!(cx.tcx.get_diagnostic_name(def_id), Some(sym::f32_nan | sym::f64_nan)) + } + _ => false, + } + } + + fn eq_ne( + e: &hir::Expr<'_>, + l: &hir::Expr<'_>, + r: &hir::Expr<'_>, + f: impl FnOnce(Span, Span) -> InvalidNanComparisonsSuggestion, + ) -> InvalidNanComparisons { + let suggestion = + if let Some(l_span) = l.span.find_ancestor_inside(e.span) && + let Some(r_span) = r.span.find_ancestor_inside(e.span) { + f(l_span, r_span) + } else { + InvalidNanComparisonsSuggestion::Spanless + }; + + InvalidNanComparisons::EqNe { suggestion } + } + + let lint = match binop.node { + hir::BinOpKind::Eq | hir::BinOpKind::Ne if is_nan(cx, l) => { + eq_ne(e, l, r, |l_span, r_span| InvalidNanComparisonsSuggestion::Spanful { + nan_plus_binop: l_span.until(r_span), + float: r_span.shrink_to_hi(), + neg: (binop.node == hir::BinOpKind::Ne).then(|| r_span.shrink_to_lo()), + }) + } + hir::BinOpKind::Eq | hir::BinOpKind::Ne if is_nan(cx, r) => { + eq_ne(e, l, r, |l_span, r_span| InvalidNanComparisonsSuggestion::Spanful { + nan_plus_binop: l_span.shrink_to_hi().to(r_span), + float: l_span.shrink_to_hi(), + neg: (binop.node == hir::BinOpKind::Ne).then(|| l_span.shrink_to_lo()), + }) + } + hir::BinOpKind::Lt | hir::BinOpKind::Le | hir::BinOpKind::Gt | hir::BinOpKind::Ge + if is_nan(cx, l) || is_nan(cx, r) => + { + InvalidNanComparisons::LtLeGtGe + } + _ => return, + }; + + cx.emit_spanned_lint(INVALID_NAN_COMPARISONS, e.span, lint); +} + impl<'tcx> LateLintPass<'tcx> for TypeLimits { fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx hir::Expr<'tcx>) { match e.kind { @@ -496,8 +621,12 @@ impl<'tcx> LateLintPass<'tcx> for TypeLimits { } } hir::ExprKind::Binary(binop, ref l, ref r) => { - if is_comparison(binop) && !check_limits(cx, binop, &l, &r) { - cx.emit_spanned_lint(UNUSED_COMPARISONS, e.span, UnusedComparisons); + if is_comparison(binop) { + if !check_limits(cx, binop, &l, &r) { + cx.emit_spanned_lint(UNUSED_COMPARISONS, e.span, UnusedComparisons); + } else { + lint_nan(cx, e, binop, l, r); + } } } hir::ExprKind::Lit(ref lit) => lint_literal(cx, self, e, lit), @@ -733,12 +862,12 @@ fn get_nullable_type<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<Ty<'t }; return get_nullable_type(cx, inner_field_ty); } - ty::Int(ty) => tcx.mk_mach_int(ty), - ty::Uint(ty) => tcx.mk_mach_uint(ty), - ty::RawPtr(ty_mut) => tcx.mk_ptr(ty_mut), + ty::Int(ty) => Ty::new_int(tcx, ty), + ty::Uint(ty) => Ty::new_uint(tcx, ty), + ty::RawPtr(ty_mut) => Ty::new_ptr(tcx, ty_mut), // As these types are always non-null, the nullable equivalent of // Option<T> of these types are their raw pointer counterparts. - ty::Ref(_region, ty, mutbl) => tcx.mk_ptr(ty::TypeAndMut { ty, mutbl }), + ty::Ref(_region, ty, mutbl) => Ty::new_ptr(tcx, ty::TypeAndMut { ty, mutbl }), ty::FnPtr(..) => { // There is no nullable equivalent for Rust's function pointers -- you // must use an Option<fn(..) -> _> to represent it. @@ -834,12 +963,12 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { substs: SubstsRef<'tcx>, ) -> FfiResult<'tcx> { let field_ty = field.ty(self.cx.tcx, substs); - if field_ty.has_opaque_types() { - self.check_type_for_ffi(cache, field_ty) - } else { - let field_ty = self.cx.tcx.normalize_erasing_regions(self.cx.param_env, field_ty); - self.check_type_for_ffi(cache, field_ty) - } + let field_ty = self + .cx + .tcx + .try_normalize_erasing_regions(self.cx.param_env, field_ty) + .unwrap_or(field_ty); + self.check_type_for_ffi(cache, field_ty) } /// Checks if the given `VariantDef`'s field types are "ffi-safe". @@ -853,39 +982,43 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { ) -> FfiResult<'tcx> { use FfiResult::*; - let transparent_safety = def.repr().transparent().then(|| { - // Can assume that at most one field is not a ZST, so only check - // that field's type for FFI-safety. + let transparent_with_all_zst_fields = if def.repr().transparent() { if let Some(field) = transparent_newtype_field(self.cx.tcx, variant) { - return self.check_field_type_for_ffi(cache, field, substs); + // Transparent newtypes have at most one non-ZST field which needs to be checked.. + match self.check_field_type_for_ffi(cache, field, substs) { + FfiUnsafe { ty, .. } if ty.is_unit() => (), + r => return r, + } + + false } else { - // All fields are ZSTs; this means that the type should behave - // like (), which is FFI-unsafe... except if all fields are PhantomData, - // which is tested for below - FfiUnsafe { ty, reason: fluent::lint_improper_ctypes_struct_zst, help: None } + // ..or have only ZST fields, which is FFI-unsafe (unless those fields are all + // `PhantomData`). + true } - }); - // We can't completely trust repr(C) markings; make sure the fields are - // actually safe. + } else { + false + }; + + // We can't completely trust `repr(C)` markings, so make sure the fields are actually safe. let mut all_phantom = !variant.fields.is_empty(); for field in &variant.fields { - match self.check_field_type_for_ffi(cache, &field, substs) { - FfiSafe => { - all_phantom = false; - } - FfiPhantom(..) if !def.repr().transparent() && def.is_enum() => { - return FfiUnsafe { - ty, - reason: fluent::lint_improper_ctypes_enum_phantomdata, - help: None, - }; - } - FfiPhantom(..) => {} - r => return transparent_safety.unwrap_or(r), + all_phantom &= match self.check_field_type_for_ffi(cache, &field, substs) { + FfiSafe => false, + // `()` fields are FFI-safe! + FfiUnsafe { ty, .. } if ty.is_unit() => false, + FfiPhantom(..) => true, + r @ FfiUnsafe { .. } => return r, } } - if all_phantom { FfiPhantom(ty) } else { transparent_safety.unwrap_or(FfiSafe) } + if all_phantom { + FfiPhantom(ty) + } else if transparent_with_all_zst_fields { + FfiUnsafe { ty, reason: fluent::lint_improper_ctypes_struct_zst, help: None } + } else { + FfiSafe + } } /// Checks if the given type is "ffi-safe" (has a stable, well-defined @@ -1088,25 +1221,19 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } let sig = tcx.erase_late_bound_regions(sig); - if !sig.output().is_unit() { - let r = self.check_type_for_ffi(cache, sig.output()); - match r { - FfiSafe => {} - _ => { - return r; - } - } - } for arg in sig.inputs() { - let r = self.check_type_for_ffi(cache, *arg); - match r { + match self.check_type_for_ffi(cache, *arg) { FfiSafe => {} - _ => { - return r; - } + r => return r, } } - FfiSafe + + let ret_ty = sig.output(); + if ret_ty.is_unit() { + return FfiSafe; + } + + self.check_type_for_ffi(cache, ret_ty) } ty::Foreign(..) => FfiSafe, @@ -1126,7 +1253,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } ty::Param(..) - | ty::Alias(ty::Projection | ty::Inherent, ..) + | ty::Alias(ty::Projection | ty::Inherent | ty::Weak, ..) | ty::Infer(..) | ty::Bound(..) | ty::Error(_) @@ -1188,7 +1315,8 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { if let Some(ty) = self .cx .tcx - .normalize_erasing_regions(self.cx.param_env, ty) + .try_normalize_erasing_regions(self.cx.param_env, ty) + .unwrap_or(ty) .visit_with(&mut ProhibitOpaqueTypes) .break_value() { @@ -1206,16 +1334,12 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { is_static: bool, is_return_type: bool, ) { - // We have to check for opaque types before `normalize_erasing_regions`, - // which will replace opaque types with their underlying concrete type. if self.check_for_opaque_ty(sp, ty) { // We've already emitted an error due to an opaque type. return; } - // it is only OK to use this function because extern fns cannot have - // any generic types right now: - let ty = self.cx.tcx.normalize_erasing_regions(self.cx.param_env, ty); + let ty = self.cx.tcx.try_normalize_erasing_regions(self.cx.param_env, ty).unwrap_or(ty); // C doesn't really support passing arrays by value - the only way to pass an array by value // is through a struct. So, first test that the top level isn't an array, and then @@ -1225,7 +1349,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } // Don't report FFI errors for unit return types. This check exists here, and not in - // `check_foreign_fn` (where it would make more sense) so that normalization has definitely + // the caller (where it would make more sense) so that normalization has definitely // happened. if is_return_type && ty.is_unit() { return; @@ -1241,16 +1365,35 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { None, ); } - // If `ty` is a `repr(transparent)` newtype, and the non-zero-sized type is a generic - // argument, which after substitution, is `()`, then this branch can be hit. - FfiResult::FfiUnsafe { ty, .. } if is_return_type && ty.is_unit() => {} FfiResult::FfiUnsafe { ty, reason, help } => { self.emit_ffi_unsafe_type_lint(ty, sp, reason, help); } } } - fn check_foreign_fn(&mut self, def_id: LocalDefId, decl: &hir::FnDecl<'_>) { + /// Check if a function's argument types and result type are "ffi-safe". + /// + /// For a external ABI function, argument types and the result type are walked to find fn-ptr + /// types that have external ABIs, as these still need checked. + fn check_fn(&mut self, def_id: LocalDefId, decl: &'tcx hir::FnDecl<'_>) { + let sig = self.cx.tcx.fn_sig(def_id).subst_identity(); + let sig = self.cx.tcx.erase_late_bound_regions(sig); + + for (input_ty, input_hir) in iter::zip(sig.inputs(), decl.inputs) { + for (fn_ptr_ty, span) in self.find_fn_ptr_ty_with_external_abi(input_hir, *input_ty) { + self.check_type_for_ffi_and_report_errors(span, fn_ptr_ty, false, false); + } + } + + if let hir::FnRetTy::Return(ref ret_hir) = decl.output { + for (fn_ptr_ty, span) in self.find_fn_ptr_ty_with_external_abi(ret_hir, sig.output()) { + self.check_type_for_ffi_and_report_errors(span, fn_ptr_ty, false, true); + } + } + } + + /// Check if a function's argument types and result type are "ffi-safe". + fn check_foreign_fn(&mut self, def_id: LocalDefId, decl: &'tcx hir::FnDecl<'_>) { let sig = self.cx.tcx.fn_sig(def_id).subst_identity(); let sig = self.cx.tcx.erase_late_bound_regions(sig); @@ -1259,8 +1402,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } if let hir::FnRetTy::Return(ref ret_hir) = decl.output { - let ret_ty = sig.output(); - self.check_type_for_ffi_and_report_errors(ret_hir.span, ret_ty, false, true); + self.check_type_for_ffi_and_report_errors(ret_hir.span, sig.output(), false, true); } } @@ -1275,28 +1417,131 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { SpecAbi::Rust | SpecAbi::RustCall | SpecAbi::RustIntrinsic | SpecAbi::PlatformIntrinsic ) } + + /// Find any fn-ptr types with external ABIs in `ty`. + /// + /// For example, `Option<extern "C" fn()>` returns `extern "C" fn()` + fn find_fn_ptr_ty_with_external_abi( + &self, + hir_ty: &hir::Ty<'tcx>, + ty: Ty<'tcx>, + ) -> Vec<(Ty<'tcx>, Span)> { + struct FnPtrFinder<'parent, 'a, 'tcx> { + visitor: &'parent ImproperCTypesVisitor<'a, 'tcx>, + spans: Vec<Span>, + tys: Vec<Ty<'tcx>>, + } + + impl<'parent, 'a, 'tcx> hir::intravisit::Visitor<'_> for FnPtrFinder<'parent, 'a, 'tcx> { + fn visit_ty(&mut self, ty: &'_ hir::Ty<'_>) { + debug!(?ty); + if let hir::TyKind::BareFn(hir::BareFnTy { abi, .. }) = ty.kind + && !self.visitor.is_internal_abi(*abi) + { + self.spans.push(ty.span); + } + + hir::intravisit::walk_ty(self, ty) + } + } + + impl<'vis, 'a, 'tcx> ty::visit::TypeVisitor<TyCtxt<'tcx>> for FnPtrFinder<'vis, 'a, 'tcx> { + type BreakTy = Ty<'tcx>; + + fn visit_ty(&mut self, ty: Ty<'tcx>) -> ControlFlow<Self::BreakTy> { + if let ty::FnPtr(sig) = ty.kind() && !self.visitor.is_internal_abi(sig.abi()) { + self.tys.push(ty); + } + + ty.super_visit_with(self) + } + } + + let mut visitor = FnPtrFinder { visitor: &*self, spans: Vec::new(), tys: Vec::new() }; + ty.visit_with(&mut visitor); + hir::intravisit::Visitor::visit_ty(&mut visitor, hir_ty); + + iter::zip(visitor.tys.drain(..), visitor.spans.drain(..)).collect() + } } impl<'tcx> LateLintPass<'tcx> for ImproperCTypesDeclarations { - fn check_foreign_item(&mut self, cx: &LateContext<'_>, it: &hir::ForeignItem<'_>) { + fn check_foreign_item(&mut self, cx: &LateContext<'tcx>, it: &hir::ForeignItem<'tcx>) { let mut vis = ImproperCTypesVisitor { cx, mode: CItemKind::Declaration }; let abi = cx.tcx.hir().get_foreign_abi(it.hir_id()); - if !vis.is_internal_abi(abi) { - match it.kind { - hir::ForeignItemKind::Fn(ref decl, _, _) => { - vis.check_foreign_fn(it.owner_id.def_id, decl); - } - hir::ForeignItemKind::Static(ref ty, _) => { - vis.check_foreign_static(it.owner_id, ty.span); - } - hir::ForeignItemKind::Type => (), + match it.kind { + hir::ForeignItemKind::Fn(ref decl, _, _) if !vis.is_internal_abi(abi) => { + vis.check_foreign_fn(it.owner_id.def_id, decl); } + hir::ForeignItemKind::Static(ref ty, _) if !vis.is_internal_abi(abi) => { + vis.check_foreign_static(it.owner_id, ty.span); + } + hir::ForeignItemKind::Fn(ref decl, _, _) => vis.check_fn(it.owner_id.def_id, decl), + hir::ForeignItemKind::Static(..) | hir::ForeignItemKind::Type => (), + } + } +} + +impl ImproperCTypesDefinitions { + fn check_ty_maybe_containing_foreign_fnptr<'tcx>( + &mut self, + cx: &LateContext<'tcx>, + hir_ty: &'tcx hir::Ty<'_>, + ty: Ty<'tcx>, + ) { + let mut vis = ImproperCTypesVisitor { cx, mode: CItemKind::Definition }; + for (fn_ptr_ty, span) in vis.find_fn_ptr_ty_with_external_abi(hir_ty, ty) { + vis.check_type_for_ffi_and_report_errors(span, fn_ptr_ty, true, false); } } } +/// `ImproperCTypesDefinitions` checks items outside of foreign items (e.g. stuff that isn't in +/// `extern "C" { }` blocks): +/// +/// - `extern "<abi>" fn` definitions are checked in the same way as the +/// `ImproperCtypesDeclarations` visitor checks functions if `<abi>` is external (e.g. "C"). +/// - All other items which contain types (e.g. other functions, struct definitions, etc) are +/// checked for extern fn-ptrs with external ABIs. impl<'tcx> LateLintPass<'tcx> for ImproperCTypesDefinitions { + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) { + match item.kind { + hir::ItemKind::Static(ty, ..) + | hir::ItemKind::Const(ty, ..) + | hir::ItemKind::TyAlias(ty, ..) => { + self.check_ty_maybe_containing_foreign_fnptr( + cx, + ty, + cx.tcx.type_of(item.owner_id).subst_identity(), + ); + } + // See `check_fn`.. + hir::ItemKind::Fn(..) => {} + // See `check_field_def`.. + hir::ItemKind::Union(..) | hir::ItemKind::Struct(..) | hir::ItemKind::Enum(..) => {} + // Doesn't define something that can contain a external type to be checked. + hir::ItemKind::Impl(..) + | hir::ItemKind::TraitAlias(..) + | hir::ItemKind::Trait(..) + | hir::ItemKind::OpaqueTy(..) + | hir::ItemKind::GlobalAsm(..) + | hir::ItemKind::ForeignMod { .. } + | hir::ItemKind::Mod(..) + | hir::ItemKind::Macro(..) + | hir::ItemKind::Use(..) + | hir::ItemKind::ExternCrate(..) => {} + } + } + + fn check_field_def(&mut self, cx: &LateContext<'tcx>, field: &'tcx hir::FieldDef<'tcx>) { + self.check_ty_maybe_containing_foreign_fnptr( + cx, + field.ty, + cx.tcx.type_of(field.def_id).subst_identity(), + ); + } + fn check_fn( &mut self, cx: &LateContext<'tcx>, @@ -1315,7 +1560,9 @@ impl<'tcx> LateLintPass<'tcx> for ImproperCTypesDefinitions { }; let mut vis = ImproperCTypesVisitor { cx, mode: CItemKind::Definition }; - if !vis.is_internal_abi(abi) { + if vis.is_internal_abi(abi) { + vis.check_fn(id, decl); + } else { vis.check_foreign_fn(id, decl); } } diff --git a/compiler/rustc_lint/src/unused.rs b/compiler/rustc_lint/src/unused.rs index 8f75fa11d..5015b751e 100644 --- a/compiler/rustc_lint/src/unused.rs +++ b/compiler/rustc_lint/src/unused.rs @@ -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}; @@ -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; @@ -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 @@ -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. @@ -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; @@ -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 { @@ -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 } @@ -261,9 +289,8 @@ impl<'tcx> LateLintPass<'tcx> for UnusedResults { .filter_only_self() .find_map(|(pred, _span)| { // We only look at the `DefId`, so it is safe to skip the binder here. - if let ty::PredicateKind::Clause(ty::Clause::Trait( - ref poly_trait_predicate, - )) = pred.kind().skip_binder() + if let ty::ClauseKind::Trait(ref poly_trait_predicate) = + pred.kind().skip_binder() { let def_id = poly_trait_predicate.trait_ref.def_id; @@ -350,6 +377,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| { @@ -360,6 +388,7 @@ impl<'tcx> LateLintPass<'tcx> for UnusedResults { descr_post_path, 1, false, + expr_is_from_block, ) }) .is_some() @@ -373,6 +402,7 @@ 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); @@ -380,21 +410,51 @@ impl<'tcx> LateLintPass<'tcx> for UnusedResults { 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, ); } } @@ -407,6 +467,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) => { @@ -433,8 +494,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() } + }), }, ); } @@ -556,6 +623,7 @@ trait UnusedDelimLint { followed_by_block: bool, left_pos: Option<BytePos>, right_pos: Option<BytePos>, + is_kw: bool, ); fn is_expr_delims_necessary( @@ -624,6 +692,7 @@ trait UnusedDelimLint { ctx: UnusedDelimsCtx, left_pos: Option<BytePos>, right_pos: Option<BytePos>, + is_kw: bool, ) { // If `value` has `ExprKind::Err`, unused delim lint can be broken. // For example, the following code caused ICE. @@ -667,7 +736,7 @@ trait UnusedDelimLint { left_pos.is_some_and(|s| s >= value.span.lo()), right_pos.is_some_and(|s| s <= value.span.hi()), ); - self.emit_unused_delims(cx, value.span, spans, ctx.into(), keep_space); + self.emit_unused_delims(cx, value.span, spans, ctx.into(), keep_space, is_kw); } fn emit_unused_delims( @@ -677,6 +746,7 @@ trait UnusedDelimLint { spans: Option<(Span, Span)>, msg: &str, keep_space: (bool, bool), + is_kw: bool, ) { let primary_span = if let Some((lo, hi)) = spans { if hi.is_empty() { @@ -690,7 +760,7 @@ trait UnusedDelimLint { let suggestion = spans.map(|(lo, hi)| { let sm = cx.sess().source_map(); let lo_replace = - if keep_space.0 && + if (keep_space.0 || is_kw) && let Ok(snip) = sm.span_to_prev_source(lo) && !snip.ends_with(' ') { " " } else { @@ -720,7 +790,7 @@ trait UnusedDelimLint { fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) { use rustc_ast::ExprKind::*; - let (value, ctx, followed_by_block, left_pos, right_pos) = match e.kind { + let (value, ctx, followed_by_block, left_pos, right_pos, is_kw) = match e.kind { // Do not lint `unused_braces` in `if let` expressions. If(ref cond, ref block, _) if !matches!(cond.kind, Let(_, _, _)) @@ -728,7 +798,7 @@ trait UnusedDelimLint { { let left = e.span.lo() + rustc_span::BytePos(2); let right = block.span.lo(); - (cond, UnusedDelimsCtx::IfCond, true, Some(left), Some(right)) + (cond, UnusedDelimsCtx::IfCond, true, Some(left), Some(right), true) } // Do not lint `unused_braces` in `while let` expressions. @@ -738,27 +808,27 @@ trait UnusedDelimLint { { let left = e.span.lo() + rustc_span::BytePos(5); let right = block.span.lo(); - (cond, UnusedDelimsCtx::WhileCond, true, Some(left), Some(right)) + (cond, UnusedDelimsCtx::WhileCond, true, Some(left), Some(right), true) } ForLoop(_, ref cond, ref block, ..) => { - (cond, UnusedDelimsCtx::ForIterExpr, true, None, Some(block.span.lo())) + (cond, UnusedDelimsCtx::ForIterExpr, true, None, Some(block.span.lo()), true) } Match(ref head, _) if Self::LINT_EXPR_IN_PATTERN_MATCHING_CTX => { let left = e.span.lo() + rustc_span::BytePos(5); - (head, UnusedDelimsCtx::MatchScrutineeExpr, true, Some(left), None) + (head, UnusedDelimsCtx::MatchScrutineeExpr, true, Some(left), None, true) } Ret(Some(ref value)) => { let left = e.span.lo() + rustc_span::BytePos(3); - (value, UnusedDelimsCtx::ReturnValue, false, Some(left), None) + (value, UnusedDelimsCtx::ReturnValue, false, Some(left), None, true) } - Index(_, ref value) => (value, UnusedDelimsCtx::IndexExpr, false, None, None), + Index(_, ref value) => (value, UnusedDelimsCtx::IndexExpr, false, None, None, false), Assign(_, ref value, _) | AssignOp(.., ref value) => { - (value, UnusedDelimsCtx::AssignedValue, false, None, None) + (value, UnusedDelimsCtx::AssignedValue, false, None, None, false) } // either function/method call, or something this lint doesn't care about ref call_or_other => { @@ -778,12 +848,20 @@ trait UnusedDelimLint { return; } for arg in args_to_check { - self.check_unused_delims_expr(cx, arg, ctx, false, None, None); + self.check_unused_delims_expr(cx, arg, ctx, false, None, None, false); } return; } }; - self.check_unused_delims_expr(cx, &value, ctx, followed_by_block, left_pos, right_pos); + self.check_unused_delims_expr( + cx, + &value, + ctx, + followed_by_block, + left_pos, + right_pos, + is_kw, + ); } fn check_stmt(&mut self, cx: &EarlyContext<'_>, s: &ast::Stmt) { @@ -794,7 +872,7 @@ trait UnusedDelimLint { None => UnusedDelimsCtx::AssignedValue, Some(_) => UnusedDelimsCtx::AssignedValueLetElse, }; - self.check_unused_delims_expr(cx, init, ctx, false, None, None); + self.check_unused_delims_expr(cx, init, ctx, false, None, None, false); } } StmtKind::Expr(ref expr) => { @@ -805,6 +883,7 @@ trait UnusedDelimLint { false, None, None, + false, ); } _ => {} @@ -824,6 +903,7 @@ trait UnusedDelimLint { false, None, None, + false, ); } } @@ -879,6 +959,7 @@ impl UnusedDelimLint for UnusedParens { followed_by_block: bool, left_pos: Option<BytePos>, right_pos: Option<BytePos>, + is_kw: bool, ) { match value.kind { ast::ExprKind::Paren(ref inner) => { @@ -893,7 +974,7 @@ impl UnusedDelimLint for UnusedParens { _, ) if node.lazy())) { - self.emit_unused_delims_expr(cx, value, ctx, left_pos, right_pos) + self.emit_unused_delims_expr(cx, value, ctx, left_pos, right_pos, is_kw) } } ast::ExprKind::Let(_, ref expr, _) => { @@ -904,6 +985,7 @@ impl UnusedDelimLint for UnusedParens { followed_by_block, None, None, + false, ); } _ => {} @@ -942,7 +1024,7 @@ impl UnusedParens { .span .find_ancestor_inside(value.span) .map(|inner| (value.span.with_hi(inner.lo()), value.span.with_lo(inner.hi()))); - self.emit_unused_delims(cx, value.span, spans, "pattern", keep_space); + self.emit_unused_delims(cx, value.span, spans, "pattern", keep_space, false); } } } @@ -967,6 +1049,7 @@ impl EarlyLintPass for UnusedParens { true, None, None, + true, ); for stmt in &block.stmts { <Self as UnusedDelimLint>::check_stmt(self, cx, stmt); @@ -985,6 +1068,7 @@ impl EarlyLintPass for UnusedParens { false, None, None, + true, ); } } @@ -1043,6 +1127,7 @@ impl EarlyLintPass for UnusedParens { false, None, None, + false, ); } ast::TyKind::Paren(r) => { @@ -1057,7 +1142,7 @@ impl EarlyLintPass for UnusedParens { .find_ancestor_inside(ty.span) .map(|r| (ty.span.with_hi(r.lo()), ty.span.with_lo(r.hi()))); - self.emit_unused_delims(cx, ty.span, spans, "type", (false, false)); + self.emit_unused_delims(cx, ty.span, spans, "type", (false, false), false); } } self.with_self_ty_parens = false; @@ -1130,6 +1215,7 @@ impl UnusedDelimLint for UnusedBraces { followed_by_block: bool, left_pos: Option<BytePos>, right_pos: Option<BytePos>, + is_kw: bool, ) { match value.kind { ast::ExprKind::Block(ref inner, None) @@ -1170,7 +1256,7 @@ impl UnusedDelimLint for UnusedBraces { && !value.span.from_expansion() && !inner.span.from_expansion() { - self.emit_unused_delims_expr(cx, value, ctx, left_pos, right_pos) + self.emit_unused_delims_expr(cx, value, ctx, left_pos, right_pos, is_kw) } } } @@ -1183,6 +1269,7 @@ impl UnusedDelimLint for UnusedBraces { followed_by_block, None, None, + false, ); } _ => {} @@ -1207,6 +1294,7 @@ impl EarlyLintPass for UnusedBraces { false, None, None, + false, ); } } @@ -1220,6 +1308,7 @@ impl EarlyLintPass for UnusedBraces { false, None, None, + false, ); } } @@ -1233,6 +1322,7 @@ impl EarlyLintPass for UnusedBraces { false, None, None, + false, ); } } @@ -1247,6 +1337,7 @@ impl EarlyLintPass for UnusedBraces { false, None, None, + false, ); } @@ -1258,6 +1349,7 @@ impl EarlyLintPass for UnusedBraces { false, None, None, + false, ); } |