From 6017de46f7864cabcf4770f11c93f319314a250f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 22 Feb 2024 18:01:12 +0000 Subject: [PATCH 01/10] When encountering `<&T as Clone>::clone(x)` because `T: Clone`, suggest `#[derive(Clone)]` CC #40699. --- compiler/rustc_lint/messages.ftl | 1 + compiler/rustc_lint/src/lints.rs | 6 ++ compiler/rustc_lint/src/noop_method_call.rs | 12 +++- tests/ui/lint/noop-method-call.fixed | 64 ------------------ tests/ui/lint/noop-method-call.rs | 1 - tests/ui/lint/noop-method-call.stderr | 74 +++++++++++++++++---- 6 files changed, 80 insertions(+), 78 deletions(-) delete mode 100644 tests/ui/lint/noop-method-call.fixed diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 1548646c04a28..496ffeddbffbc 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -429,6 +429,7 @@ lint_non_upper_case_global = {$sort} `{$name}` should have an upper case name lint_noop_method_call = call to `.{$method}()` on a reference in this situation does nothing .suggestion = remove this redundant call .note = the type `{$orig_ty}` does not implement `{$trait_}`, so calling `{$method}` on `&{$orig_ty}` copies the reference, which does not do anything and can be removed + .derive_suggestion = if you meant to clone `{$orig_ty}`, implement `Clone` for it lint_only_cast_u8_to_char = only `u8` can be cast into `char` .suggestion = use a `char` literal instead diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 5fb2ddf91990c..a25ddbce8626a 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1314,6 +1314,12 @@ pub struct NoopMethodCallDiag<'a> { pub trait_: Symbol, #[suggestion(code = "", applicability = "machine-applicable")] pub label: Span, + #[suggestion( + lint_derive_suggestion, + code = "#[derive(Clone)]\n", + applicability = "maybe-incorrect" + )] + pub suggest_derive: Option, } #[derive(LintDiagnostic)] diff --git a/compiler/rustc_lint/src/noop_method_call.rs b/compiler/rustc_lint/src/noop_method_call.rs index 26c5e4fb4837d..970d411fb0657 100644 --- a/compiler/rustc_lint/src/noop_method_call.rs +++ b/compiler/rustc_lint/src/noop_method_call.rs @@ -121,10 +121,20 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall { let orig_ty = expr_ty.peel_refs(); if receiver_ty == expr_ty { + let suggest_derive = match orig_ty.kind() { + ty::Adt(def, _) => Some(cx.tcx.def_span(def.did()).shrink_to_lo()), + _ => None, + }; cx.emit_span_lint( NOOP_METHOD_CALL, span, - NoopMethodCallDiag { method: call.ident.name, orig_ty, trait_, label: span }, + NoopMethodCallDiag { + method: call.ident.name, + orig_ty, + trait_, + label: span, + suggest_derive, + }, ); } else { match name { diff --git a/tests/ui/lint/noop-method-call.fixed b/tests/ui/lint/noop-method-call.fixed deleted file mode 100644 index 279dc8a3cd0cc..0000000000000 --- a/tests/ui/lint/noop-method-call.fixed +++ /dev/null @@ -1,64 +0,0 @@ -//@ check-pass -//@ run-rustfix - -#![feature(rustc_attrs)] -#![allow(unused)] - -use std::borrow::Borrow; -use std::ops::Deref; - -struct PlainType(T); - -#[derive(Clone)] -struct CloneType(T); - -fn check(mut encoded: &[u8]) { - let _ = &mut encoded; - //~^ WARN call to `.clone()` on a reference in this situation does nothing - let _ = &encoded; - //~^ WARN call to `.clone()` on a reference in this situation does nothing -} - -fn main() { - let non_clone_type_ref = &PlainType(1u32); - let non_clone_type_ref_clone: &PlainType = non_clone_type_ref; - //~^ WARN call to `.clone()` on a reference in this situation does nothing - - let clone_type_ref = &CloneType(1u32); - let clone_type_ref_clone: CloneType = clone_type_ref.clone(); - - - let non_deref_type = &PlainType(1u32); - let non_deref_type_deref: &PlainType = non_deref_type; - //~^ WARN call to `.deref()` on a reference in this situation does nothing - - let non_borrow_type = &PlainType(1u32); - let non_borrow_type_borrow: &PlainType = non_borrow_type; - //~^ WARN call to `.borrow()` on a reference in this situation does nothing - - // Borrowing a &&T does not warn since it has collapsed the double reference - let non_borrow_type = &&PlainType(1u32); - let non_borrow_type_borrow: &PlainType = non_borrow_type.borrow(); -} - -fn generic(non_clone_type: &PlainType) { - non_clone_type; - //~^ WARN call to `.clone()` on a reference in this situation does nothing -} - -fn non_generic(non_clone_type: &PlainType) { - non_clone_type; - //~^ WARN call to `.clone()` on a reference in this situation does nothing -} - -struct DiagnosticClone; -impl Clone for DiagnosticClone { - #[rustc_diagnostic_item = "other_clone"] - fn clone(&self) -> Self { - DiagnosticClone - } -} - -fn with_other_diagnostic_item(x: DiagnosticClone) { - x.clone(); -} diff --git a/tests/ui/lint/noop-method-call.rs b/tests/ui/lint/noop-method-call.rs index 447a4c62410b0..8db5c889d1c37 100644 --- a/tests/ui/lint/noop-method-call.rs +++ b/tests/ui/lint/noop-method-call.rs @@ -1,5 +1,4 @@ //@ check-pass -//@ run-rustfix #![feature(rustc_attrs)] #![allow(unused)] diff --git a/tests/ui/lint/noop-method-call.stderr b/tests/ui/lint/noop-method-call.stderr index d04f44022eefd..8823644ac2d5f 100644 --- a/tests/ui/lint/noop-method-call.stderr +++ b/tests/ui/lint/noop-method-call.stderr @@ -1,5 +1,5 @@ warning: call to `.clone()` on a reference in this situation does nothing - --> $DIR/noop-method-call.rs:16:25 + --> $DIR/noop-method-call.rs:15:25 | LL | let _ = &mut encoded.clone(); | ^^^^^^^^ help: remove this redundant call @@ -8,7 +8,7 @@ LL | let _ = &mut encoded.clone(); = note: `#[warn(noop_method_call)]` on by default warning: call to `.clone()` on a reference in this situation does nothing - --> $DIR/noop-method-call.rs:18:21 + --> $DIR/noop-method-call.rs:17:21 | LL | let _ = &encoded.clone(); | ^^^^^^^^ help: remove this redundant call @@ -16,44 +16,94 @@ LL | let _ = &encoded.clone(); = note: the type `[u8]` does not implement `Clone`, so calling `clone` on `&[u8]` copies the reference, which does not do anything and can be removed warning: call to `.clone()` on a reference in this situation does nothing - --> $DIR/noop-method-call.rs:24:71 + --> $DIR/noop-method-call.rs:23:71 | LL | let non_clone_type_ref_clone: &PlainType = non_clone_type_ref.clone(); - | ^^^^^^^^ help: remove this redundant call + | ^^^^^^^^ | = note: the type `PlainType` does not implement `Clone`, so calling `clone` on `&PlainType` copies the reference, which does not do anything and can be removed +help: remove this redundant call + | +LL - let non_clone_type_ref_clone: &PlainType = non_clone_type_ref.clone(); +LL + let non_clone_type_ref_clone: &PlainType = non_clone_type_ref; + | +help: if you meant to clone `PlainType`, implement `Clone` for it + | +LL + #[derive(Clone)] +LL | struct PlainType(T); + | warning: call to `.deref()` on a reference in this situation does nothing - --> $DIR/noop-method-call.rs:32:63 + --> $DIR/noop-method-call.rs:31:63 | LL | let non_deref_type_deref: &PlainType = non_deref_type.deref(); - | ^^^^^^^^ help: remove this redundant call + | ^^^^^^^^ | = note: the type `PlainType` does not implement `Deref`, so calling `deref` on `&PlainType` copies the reference, which does not do anything and can be removed +help: remove this redundant call + | +LL - let non_deref_type_deref: &PlainType = non_deref_type.deref(); +LL + let non_deref_type_deref: &PlainType = non_deref_type; + | +help: if you meant to clone `PlainType`, implement `Clone` for it + | +LL + #[derive(Clone)] +LL | struct PlainType(T); + | warning: call to `.borrow()` on a reference in this situation does nothing - --> $DIR/noop-method-call.rs:36:66 + --> $DIR/noop-method-call.rs:35:66 | LL | let non_borrow_type_borrow: &PlainType = non_borrow_type.borrow(); - | ^^^^^^^^^ help: remove this redundant call + | ^^^^^^^^^ | = note: the type `PlainType` does not implement `Borrow`, so calling `borrow` on `&PlainType` copies the reference, which does not do anything and can be removed +help: remove this redundant call + | +LL - let non_borrow_type_borrow: &PlainType = non_borrow_type.borrow(); +LL + let non_borrow_type_borrow: &PlainType = non_borrow_type; + | +help: if you meant to clone `PlainType`, implement `Clone` for it + | +LL + #[derive(Clone)] +LL | struct PlainType(T); + | warning: call to `.clone()` on a reference in this situation does nothing - --> $DIR/noop-method-call.rs:45:19 + --> $DIR/noop-method-call.rs:44:19 | LL | non_clone_type.clone(); - | ^^^^^^^^ help: remove this redundant call + | ^^^^^^^^ | = note: the type `PlainType` does not implement `Clone`, so calling `clone` on `&PlainType` copies the reference, which does not do anything and can be removed +help: remove this redundant call + | +LL - non_clone_type.clone(); +LL + non_clone_type; + | +help: if you meant to clone `PlainType`, implement `Clone` for it + | +LL + #[derive(Clone)] +LL | struct PlainType(T); + | warning: call to `.clone()` on a reference in this situation does nothing - --> $DIR/noop-method-call.rs:50:19 + --> $DIR/noop-method-call.rs:49:19 | LL | non_clone_type.clone(); - | ^^^^^^^^ help: remove this redundant call + | ^^^^^^^^ | = note: the type `PlainType` does not implement `Clone`, so calling `clone` on `&PlainType` copies the reference, which does not do anything and can be removed +help: remove this redundant call + | +LL - non_clone_type.clone(); +LL + non_clone_type; + | +help: if you meant to clone `PlainType`, implement `Clone` for it + | +LL + #[derive(Clone)] +LL | struct PlainType(T); + | warning: 7 warnings emitted From 52227edfaf467ac6abe0be085f97884c4502014e Mon Sep 17 00:00:00 2001 From: onur-ozkan Date: Thu, 22 Feb 2024 22:04:51 +0300 Subject: [PATCH 02/10] set `llvm.assertions` to false in compiler profile Having this set to true disrupts compiler development workflows for people who use `llvm.download-ci-llvm = true` because we don't provide ci-llvm on the `rustc-alt-builds` server. Therefore, it is kept off by default. Signed-off-by: onur-ozkan --- src/bootstrap/defaults/config.compiler.toml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/bootstrap/defaults/config.compiler.toml b/src/bootstrap/defaults/config.compiler.toml index 178c6e9056c69..e276f12621188 100644 --- a/src/bootstrap/defaults/config.compiler.toml +++ b/src/bootstrap/defaults/config.compiler.toml @@ -19,9 +19,9 @@ lto = "off" frame-pointers = true [llvm] -# This enables debug-assertions in LLVM, -# catching logic errors in codegen much earlier in the process. -assertions = true +# Having this set to true disrupts compiler development workflows for people who use `llvm.download-ci-llvm = true` +# because we don't provide ci-llvm on the `rustc-alt-builds` server. Therefore, it is kept off by default. +assertions = false # Enable warnings during the LLVM compilation (when LLVM is changed, causing a compilation) enable-warnings = true # Will download LLVM from CI if available on your platform. From fa2921bdca25960fbe68ad5fa14410f254a6c69e Mon Sep 17 00:00:00 2001 From: lcnr Date: Thu, 22 Feb 2024 22:18:43 +0100 Subject: [PATCH 03/10] woops, soundly generalizing is hard I ended up getting confused while trying to flip the variances when flipping the order. Should be all right now --- .../src/type_check/relate_tys.rs | 14 ++++--- .../src/infer/relate/generalize.rs | 42 +++++++++++-------- 2 files changed, 33 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_borrowck/src/type_check/relate_tys.rs b/compiler/rustc_borrowck/src/type_check/relate_tys.rs index dd355c3525c30..61b803ea38d4b 100644 --- a/compiler/rustc_borrowck/src/type_check/relate_tys.rs +++ b/compiler/rustc_borrowck/src/type_check/relate_tys.rs @@ -123,7 +123,11 @@ impl<'me, 'bccx, 'tcx> NllTypeRelating<'me, 'bccx, 'tcx> { // `handle_opaque_type` cannot handle subtyping, so to support subtyping // we instead eagerly generalize here. This is a bit of a mess but will go // away once we're using the new solver. - let mut enable_subtyping = |ty, ty_is_expected| { + // + // Given `opaque rel B`, we create a new infer var `ty_vid` constrain it + // by using `ty_vid rel B` and then finally and end by equating `ty_vid` to + // the opaque. + let mut enable_subtyping = |ty, opaque_is_expected| { let ty_vid = infcx.next_ty_var_id_in_universe( TypeVariableOrigin { kind: TypeVariableOriginKind::MiscVariable, @@ -132,7 +136,7 @@ impl<'me, 'bccx, 'tcx> NllTypeRelating<'me, 'bccx, 'tcx> { ty::UniverseIndex::ROOT, ); - let variance = if ty_is_expected { + let variance = if opaque_is_expected { self.ambient_variance } else { self.ambient_variance.xform(ty::Contravariant) @@ -140,7 +144,7 @@ impl<'me, 'bccx, 'tcx> NllTypeRelating<'me, 'bccx, 'tcx> { self.type_checker.infcx.instantiate_ty_var( self, - ty_is_expected, + opaque_is_expected, ty_vid, variance, ty, @@ -149,8 +153,8 @@ impl<'me, 'bccx, 'tcx> NllTypeRelating<'me, 'bccx, 'tcx> { }; let (a, b) = match (a.kind(), b.kind()) { - (&ty::Alias(ty::Opaque, ..), _) => (a, enable_subtyping(b, false)?), - (_, &ty::Alias(ty::Opaque, ..)) => (enable_subtyping(a, true)?, b), + (&ty::Alias(ty::Opaque, ..), _) => (a, enable_subtyping(b, true)?), + (_, &ty::Alias(ty::Opaque, ..)) => (enable_subtyping(a, false)?, b), _ => unreachable!( "expected at least one opaque type in `relate_opaques`, got {a} and {b}." ), diff --git a/compiler/rustc_infer/src/infer/relate/generalize.rs b/compiler/rustc_infer/src/infer/relate/generalize.rs index 90f10a0eba9e5..1494730978b67 100644 --- a/compiler/rustc_infer/src/infer/relate/generalize.rs +++ b/compiler/rustc_infer/src/infer/relate/generalize.rs @@ -26,13 +26,13 @@ impl<'tcx> InferCtxt<'tcx> { /// This is *not* expected to be used anywhere except for an implementation of /// `TypeRelation`. Do not use this, and instead please use `At::eq`, for all /// other usecases (i.e. setting the value of a type var). - #[instrument(level = "debug", skip(self, relation, target_is_expected))] + #[instrument(level = "debug", skip(self, relation))] pub fn instantiate_ty_var>( &self, relation: &mut R, target_is_expected: bool, target_vid: ty::TyVid, - ambient_variance: ty::Variance, + instantiation_variance: ty::Variance, source_ty: Ty<'tcx>, ) -> RelateResult<'tcx, ()> { debug_assert!(self.inner.borrow_mut().type_variables().probe(target_vid).is_unknown()); @@ -46,7 +46,7 @@ impl<'tcx> InferCtxt<'tcx> { // // We then relate `generalized_ty <: source_ty`,adding constraints like `'x: '?2` and `?1 <: ?3`. let Generalization { value_may_be_infer: generalized_ty, has_unconstrained_ty_var } = - self.generalize(relation.span(), target_vid, ambient_variance, source_ty)?; + self.generalize(relation.span(), target_vid, instantiation_variance, source_ty)?; // Constrain `b_vid` to the generalized type `generalized_ty`. if let &ty::Infer(ty::TyVar(generalized_vid)) = generalized_ty.kind() { @@ -73,7 +73,7 @@ impl<'tcx> InferCtxt<'tcx> { // the alias can be normalized to something which does not // mention `?0`. if self.next_trait_solver() { - let (lhs, rhs, direction) = match ambient_variance { + let (lhs, rhs, direction) = match instantiation_variance { ty::Variance::Invariant => { (generalized_ty.into(), source_ty.into(), AliasRelationDirection::Equate) } @@ -106,22 +106,28 @@ impl<'tcx> InferCtxt<'tcx> { } } } else { - // HACK: make sure that we `a_is_expected` continues to be - // correct when relating the generalized type with the source. + // NOTE: The `instantiation_variance` is not the same variance as + // used by the relation. When instantiating `b`, `target_is_expected` + // is flipped and the `instantion_variance` is also flipped. To + // constrain the `generalized_ty` while using the original relation, + // we therefore only have to flip the arguments. + // + // ```ignore + // ?a rel B + // instantiate_ty_var(?a, B) # expected and variance not flipped + // B' rel B + // ``` + // or + // ```ignore + // A rel ?b + // instantiate_ty_var(?b, A) # expected and variance flipped + // A rel A' + // ``` if target_is_expected == relation.a_is_expected() { - relation.relate_with_variance( - ambient_variance, - ty::VarianceDiagInfo::default(), - generalized_ty, - source_ty, - )?; + relation.relate(generalized_ty, source_ty)?; } else { - relation.relate_with_variance( - ambient_variance.xform(ty::Contravariant), - ty::VarianceDiagInfo::default(), - source_ty, - generalized_ty, - )?; + debug!("flip relation"); + relation.relate(source_ty, generalized_ty)?; } } From 4f83e50f98f60d192e258b091d39d85c7d346310 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 22 Feb 2024 14:41:32 +1100 Subject: [PATCH 04/10] Revert some `span_bug`s to `span_delayed_bug`. Fixes #121410. Fixes #121414. Fixes #121418. Fixes #121431. --- compiler/rustc_ast_lowering/src/lib.rs | 4 +++- .../src/collect/generics_of.rs | 2 +- .../src/mem_categorization.rs | 3 ++- .../rustc_trait_selection/src/traits/mod.rs | 4 +--- tests/ui/lowering/span-bug-issue-121431.rs | 4 ++++ .../ui/lowering/span-bug-issue-121431.stderr | 9 ++++++++ .../effects/span-bug-issue-121418.rs | 13 +++++++++++ .../effects/span-bug-issue-121418.stderr | 22 +++++++++++++++++++ tests/ui/traits/span-bug-issue-121414.rs | 15 +++++++++++++ tests/ui/traits/span-bug-issue-121414.stderr | 20 +++++++++++++++++ tests/ui/typeck/span-bug-issue-121410.rs | 15 +++++++++++++ tests/ui/typeck/span-bug-issue-121410.stderr | 14 ++++++++++++ 12 files changed, 119 insertions(+), 6 deletions(-) create mode 100644 tests/ui/lowering/span-bug-issue-121431.rs create mode 100644 tests/ui/lowering/span-bug-issue-121431.stderr create mode 100644 tests/ui/rfcs/rfc-2632-const-trait-impl/effects/span-bug-issue-121418.rs create mode 100644 tests/ui/rfcs/rfc-2632-const-trait-impl/effects/span-bug-issue-121418.stderr create mode 100644 tests/ui/traits/span-bug-issue-121414.rs create mode 100644 tests/ui/traits/span-bug-issue-121414.stderr create mode 100644 tests/ui/typeck/span-bug-issue-121410.rs create mode 100644 tests/ui/typeck/span-bug-issue-121410.stderr diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index ef843da7307d3..a5be91bb87209 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -1636,7 +1636,9 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { if let Some(old_def_id) = self.orig_opt_local_def_id(param) { old_def_id } else { - self.dcx().span_bug(lifetime.ident.span, "no def-id for fresh lifetime"); + self.dcx() + .span_delayed_bug(lifetime.ident.span, "no def-id for fresh lifetime"); + continue; } } diff --git a/compiler/rustc_hir_analysis/src/collect/generics_of.rs b/compiler/rustc_hir_analysis/src/collect/generics_of.rs index 410a069f9568f..9cc6c16c12639 100644 --- a/compiler/rustc_hir_analysis/src/collect/generics_of.rs +++ b/compiler/rustc_hir_analysis/src/collect/generics_of.rs @@ -315,7 +315,7 @@ pub(super) fn generics_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Generics { if is_host_effect { if let Some(idx) = host_effect_index { - tcx.dcx().span_bug( + tcx.dcx().span_delayed_bug( param.span, format!("parent also has host effect param? index: {idx}, def: {def_id:?}"), ); diff --git a/compiler/rustc_hir_typeck/src/mem_categorization.rs b/compiler/rustc_hir_typeck/src/mem_categorization.rs index c300ec7444b2e..1a860aa406791 100644 --- a/compiler/rustc_hir_typeck/src/mem_categorization.rs +++ b/compiler/rustc_hir_typeck/src/mem_categorization.rs @@ -582,7 +582,8 @@ impl<'a, 'tcx> MemCategorizationContext<'a, 'tcx> { match ty.kind() { ty::Tuple(args) => Ok(args.len()), _ => { - self.tcx().dcx().span_bug(span, "tuple pattern not applied to a tuple"); + self.tcx().dcx().span_delayed_bug(span, "tuple pattern not applied to a tuple"); + Err(()) } } } diff --git a/compiler/rustc_trait_selection/src/traits/mod.rs b/compiler/rustc_trait_selection/src/traits/mod.rs index cac5379674790..9eec60ea06c21 100644 --- a/compiler/rustc_trait_selection/src/traits/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/mod.rs @@ -172,9 +172,7 @@ fn do_normalize_predicates<'tcx>( // the normalized predicates. let errors = infcx.resolve_regions(&outlives_env); if !errors.is_empty() { - // @lcnr: Let's still ICE here for now. I want a test case - // for that. - tcx.dcx().span_bug( + tcx.dcx().span_delayed_bug( span, format!("failed region resolution while normalizing {elaborated_env:?}: {errors:?}"), ); diff --git a/tests/ui/lowering/span-bug-issue-121431.rs b/tests/ui/lowering/span-bug-issue-121431.rs new file mode 100644 index 0000000000000..b855577bcfbff --- /dev/null +++ b/tests/ui/lowering/span-bug-issue-121431.rs @@ -0,0 +1,4 @@ +fn bug() -> impl CallbackMarker< Item = [(); { |_: &mut ()| 3; 4 }] > {} +//~^ ERROR cannot find trait `CallbackMarker` in this scope + +fn main() {} diff --git a/tests/ui/lowering/span-bug-issue-121431.stderr b/tests/ui/lowering/span-bug-issue-121431.stderr new file mode 100644 index 0000000000000..595500b580640 --- /dev/null +++ b/tests/ui/lowering/span-bug-issue-121431.stderr @@ -0,0 +1,9 @@ +error[E0405]: cannot find trait `CallbackMarker` in this scope + --> $DIR/span-bug-issue-121431.rs:1:21 + | +LL | fn bug() -> impl CallbackMarker< Item = [(); { |_: &mut ()| 3; 4 }] > {} + | ^^^^^^^^^^^^^^ not found in this scope + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0405`. diff --git a/tests/ui/rfcs/rfc-2632-const-trait-impl/effects/span-bug-issue-121418.rs b/tests/ui/rfcs/rfc-2632-const-trait-impl/effects/span-bug-issue-121418.rs new file mode 100644 index 0000000000000..97e89f96fe1b0 --- /dev/null +++ b/tests/ui/rfcs/rfc-2632-const-trait-impl/effects/span-bug-issue-121418.rs @@ -0,0 +1,13 @@ +#![feature(const_trait_impl)] +#![feature(effects)] + +struct S; +trait T {} + +impl const dyn T { + //~^ ERROR inherent impls cannot be `const` + //~| ERROR the const parameter `host` is not constrained by the impl trait, self type, or + pub const fn new() -> std::sync::Mutex {} +} + +fn main() {} diff --git a/tests/ui/rfcs/rfc-2632-const-trait-impl/effects/span-bug-issue-121418.stderr b/tests/ui/rfcs/rfc-2632-const-trait-impl/effects/span-bug-issue-121418.stderr new file mode 100644 index 0000000000000..11577d9ec1d0f --- /dev/null +++ b/tests/ui/rfcs/rfc-2632-const-trait-impl/effects/span-bug-issue-121418.stderr @@ -0,0 +1,22 @@ +error: inherent impls cannot be `const` + --> $DIR/span-bug-issue-121418.rs:7:12 + | +LL | impl const dyn T { + | ----- ^^^^^ inherent impl for this type + | | + | `const` because of this + | + = note: only trait implementations may be annotated with `const` + +error[E0207]: the const parameter `host` is not constrained by the impl trait, self type, or predicates + --> $DIR/span-bug-issue-121418.rs:7:6 + | +LL | impl const dyn T { + | ^^^^^ unconstrained const parameter + | + = note: expressions using a const parameter must map each value to a distinct output value + = note: proving the result of expressions other than the parameter are unique is not supported + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0207`. diff --git a/tests/ui/traits/span-bug-issue-121414.rs b/tests/ui/traits/span-bug-issue-121414.rs new file mode 100644 index 0000000000000..6fbe2c179c652 --- /dev/null +++ b/tests/ui/traits/span-bug-issue-121414.rs @@ -0,0 +1,15 @@ +trait Bar { + type Type; +} +struct Foo<'a>(&'a ()); +impl<'a> Bar for Foo<'f> { //~ ERROR undeclared lifetime + type Type = u32; +} + +fn test() //~ ERROR implementation of `Bar` is not general enough +where + for<'a> as Bar>::Type: Sized, +{ +} + +fn main() {} diff --git a/tests/ui/traits/span-bug-issue-121414.stderr b/tests/ui/traits/span-bug-issue-121414.stderr new file mode 100644 index 0000000000000..3c97f64e781e3 --- /dev/null +++ b/tests/ui/traits/span-bug-issue-121414.stderr @@ -0,0 +1,20 @@ +error[E0261]: use of undeclared lifetime name `'f` + --> $DIR/span-bug-issue-121414.rs:5:22 + | +LL | impl<'a> Bar for Foo<'f> { + | - ^^ undeclared lifetime + | | + | help: consider introducing lifetime `'f` here: `'f,` + +error: implementation of `Bar` is not general enough + --> $DIR/span-bug-issue-121414.rs:9:4 + | +LL | fn test() + | ^^^^ implementation of `Bar` is not general enough + | + = note: `Bar` would have to be implemented for the type `Foo<'0>`, for any lifetime `'0`... + = note: ...but `Bar` is actually implemented for the type `Foo<'1>`, for some specific lifetime `'1` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0261`. diff --git a/tests/ui/typeck/span-bug-issue-121410.rs b/tests/ui/typeck/span-bug-issue-121410.rs new file mode 100644 index 0000000000000..324b398d74f77 --- /dev/null +++ b/tests/ui/typeck/span-bug-issue-121410.rs @@ -0,0 +1,15 @@ +fn test_missing_unsafe_warning_on_repr_packed() { + struct Foo { + x: String, + } + + let foo = Foo { x: String::new() }; + + let c = || { + let (_, t2) = foo.x; //~ ERROR mismatched types + }; + + c(); +} + +fn main() {} diff --git a/tests/ui/typeck/span-bug-issue-121410.stderr b/tests/ui/typeck/span-bug-issue-121410.stderr new file mode 100644 index 0000000000000..f745ac51a5e3e --- /dev/null +++ b/tests/ui/typeck/span-bug-issue-121410.stderr @@ -0,0 +1,14 @@ +error[E0308]: mismatched types + --> $DIR/span-bug-issue-121410.rs:9:13 + | +LL | let (_, t2) = foo.x; + | ^^^^^^^ ----- this expression has type `String` + | | + | expected `String`, found `(_, _)` + | + = note: expected struct `String` + found tuple `(_, _)` + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0308`. From dabacb7431dbca5ea337ed5a456ecf756e2533af Mon Sep 17 00:00:00 2001 From: lcnr Date: Thu, 22 Feb 2024 23:10:46 +0100 Subject: [PATCH 05/10] fix CI --- compiler/rustc_infer/src/infer/relate/generalize.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_infer/src/infer/relate/generalize.rs b/compiler/rustc_infer/src/infer/relate/generalize.rs index 1494730978b67..eecdbbe5179e3 100644 --- a/compiler/rustc_infer/src/infer/relate/generalize.rs +++ b/compiler/rustc_infer/src/infer/relate/generalize.rs @@ -112,13 +112,13 @@ impl<'tcx> InferCtxt<'tcx> { // constrain the `generalized_ty` while using the original relation, // we therefore only have to flip the arguments. // - // ```ignore + // ```ignore (not code) // ?a rel B // instantiate_ty_var(?a, B) # expected and variance not flipped // B' rel B // ``` // or - // ```ignore + // ```ignore (not code) // A rel ?b // instantiate_ty_var(?b, A) # expected and variance flipped // A rel A' From 109321ac47d0ffff5d03aad60f3df0c56e412db4 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 23 Feb 2024 09:37:33 +1100 Subject: [PATCH 06/10] Revert some `span_bug`s to `span_delayed_bug`. Fixes #121445. Fixes #121457. --- .../src/check/compare_impl_item/refine.rs | 12 +++----- compiler/rustc_hir_typeck/src/expr.rs | 13 ++++++-- .../in-trait/span-bug-issue-121457.rs | 18 +++++++++++ .../in-trait/span-bug-issue-121457.stderr | 30 +++++++++++++++++++ tests/ui/never_type/span-bug-issue-121445.rs | 15 ++++++++++ .../never_type/span-bug-issue-121445.stderr | 22 ++++++++++++++ 6 files changed, 99 insertions(+), 11 deletions(-) create mode 100644 tests/ui/impl-trait/in-trait/span-bug-issue-121457.rs create mode 100644 tests/ui/impl-trait/in-trait/span-bug-issue-121457.stderr create mode 100644 tests/ui/never_type/span-bug-issue-121445.rs create mode 100644 tests/ui/never_type/span-bug-issue-121445.stderr diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs index 7bdbab4325cbf..b5e69b8e3766e 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs +++ b/compiler/rustc_hir_analysis/src/check/compare_impl_item/refine.rs @@ -154,10 +154,8 @@ pub(super) fn check_refining_return_position_impl_trait_in_trait<'tcx>( trait_m_sig.inputs_and_output, )); if !ocx.select_all_or_error().is_empty() { - // This code path is not reached in any tests, but may be reachable. If - // this is triggered, it should be converted to `delayed_bug` and the - // triggering case turned into a test. - tcx.dcx().bug("encountered errors when checking RPITIT refinement (selection)"); + tcx.dcx().delayed_bug("encountered errors when checking RPITIT refinement (selection)"); + return; } let outlives_env = OutlivesEnvironment::with_bounds( param_env, @@ -165,10 +163,8 @@ pub(super) fn check_refining_return_position_impl_trait_in_trait<'tcx>( ); let errors = infcx.resolve_regions(&outlives_env); if !errors.is_empty() { - // This code path is not reached in any tests, but may be reachable. If - // this is triggered, it should be converted to `delayed_bug` and the - // triggering case turned into a test. - tcx.dcx().bug("encountered errors when checking RPITIT refinement (regions)"); + tcx.dcx().delayed_bug("encountered errors when checking RPITIT refinement (regions)"); + return; } // Resolve any lifetime variables that may have been introduced during normalization. let Ok((trait_bounds, impl_bounds)) = infcx.fully_resolve((trait_bounds, impl_bounds)) else { diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index 89cc46dc5ab80..6b52dd4392e39 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -76,9 +76,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // While we don't allow *arbitrary* coercions here, we *do* allow // coercions from ! to `expected`. if ty.is_never() { - if let Some(_) = self.typeck_results.borrow().adjustments().get(expr.hir_id) { - self.dcx() - .span_bug(expr.span, "expression with never type wound up being adjusted"); + if let Some(adjustments) = self.typeck_results.borrow().adjustments().get(expr.hir_id) { + let reported = self.dcx().span_delayed_bug( + expr.span, + "expression with never type wound up being adjusted", + ); + return if let [Adjustment { kind: Adjust::NeverToAny, target }] = &adjustments[..] { + target.to_owned() + } else { + Ty::new_error(self.tcx(), reported) + }; } let adj_ty = self.next_ty_var(TypeVariableOrigin { diff --git a/tests/ui/impl-trait/in-trait/span-bug-issue-121457.rs b/tests/ui/impl-trait/in-trait/span-bug-issue-121457.rs new file mode 100644 index 0000000000000..10167ee935262 --- /dev/null +++ b/tests/ui/impl-trait/in-trait/span-bug-issue-121457.rs @@ -0,0 +1,18 @@ +pub trait Iterable { + type Item<'a> + where + Self: 'a; + + fn iter(&self) -> impl Iterator; +} + +impl<'a, I: 'a + Iterable> Iterable for &'a I { + type Item = u32; + //~^ ERROR lifetime parameters or bounds on type `Item` do not match the trait declaration + + fn iter(&self) -> impl for<'missing> Iterator> {} + //~^ ERROR binding for associated type `Item` references lifetime `'missing` + //~| ERROR `()` is not an iterator +} + +fn main() {} diff --git a/tests/ui/impl-trait/in-trait/span-bug-issue-121457.stderr b/tests/ui/impl-trait/in-trait/span-bug-issue-121457.stderr new file mode 100644 index 0000000000000..96c3644f89348 --- /dev/null +++ b/tests/ui/impl-trait/in-trait/span-bug-issue-121457.stderr @@ -0,0 +1,30 @@ +error[E0582]: binding for associated type `Item` references lifetime `'missing`, which does not appear in the trait input types + --> $DIR/span-bug-issue-121457.rs:13:51 + | +LL | fn iter(&self) -> impl for<'missing> Iterator> {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error[E0195]: lifetime parameters or bounds on type `Item` do not match the trait declaration + --> $DIR/span-bug-issue-121457.rs:10:14 + | +LL | type Item<'a> + | ---- lifetimes in impl do not match this type in trait +LL | where +LL | Self: 'a; + | -- this bound might be missing in the impl +... +LL | type Item = u32; + | ^ lifetimes do not match type in trait + +error[E0277]: `()` is not an iterator + --> $DIR/span-bug-issue-121457.rs:13:23 + | +LL | fn iter(&self) -> impl for<'missing> Iterator> {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `()` is not an iterator + | + = help: the trait `Iterator` is not implemented for `()` + +error: aborting due to 3 previous errors + +Some errors have detailed explanations: E0195, E0277, E0582. +For more information about an error, try `rustc --explain E0195`. diff --git a/tests/ui/never_type/span-bug-issue-121445.rs b/tests/ui/never_type/span-bug-issue-121445.rs new file mode 100644 index 0000000000000..2fe22529c4e9d --- /dev/null +++ b/tests/ui/never_type/span-bug-issue-121445.rs @@ -0,0 +1,15 @@ +#![feature(never_type)] + +fn test2() { + let x: !; + let c2 = SingleVariant::Points(0) + | match x { //~ ERROR no implementation for `SingleVariant | ()` + _ => (), + }; +} + +enum SingleVariant { + Points(u32), +} + +fn main() {} diff --git a/tests/ui/never_type/span-bug-issue-121445.stderr b/tests/ui/never_type/span-bug-issue-121445.stderr new file mode 100644 index 0000000000000..b211afa236fef --- /dev/null +++ b/tests/ui/never_type/span-bug-issue-121445.stderr @@ -0,0 +1,22 @@ +error[E0369]: no implementation for `SingleVariant | ()` + --> $DIR/span-bug-issue-121445.rs:6:9 + | +LL | let c2 = SingleVariant::Points(0) + | ------------------------ SingleVariant +LL | | match x { + | _________^_- +LL | | _ => (), +LL | | }; + | |_________- () + | +note: an implementation of `BitOr<()>` might be missing for `SingleVariant` + --> $DIR/span-bug-issue-121445.rs:11:1 + | +LL | enum SingleVariant { + | ^^^^^^^^^^^^^^^^^^ must implement `BitOr<()>` +note: the trait `BitOr` must be implemented + --> $SRC_DIR/core/src/ops/bit.rs:LL:COL + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0369`. From 21bb1a435907294e50a95359fedcf5ebd5de1959 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 23 Feb 2024 10:47:53 +1100 Subject: [PATCH 07/10] Allow for a missing `adt_def` in `NamePrivacyVisitor`. This was caused by 72b172bdf6 in #121206. That commit removed an early return from `analysis` when there are stashed errors. As a result, it's possible to reach privacy analysis when there are stashed errors, which means more code paths can be reached. One such code path was handled in that commit, where a `span_bug` was changed to a `span_delayed_bug`. This commit handles another such code path uncovered by fuzzing, in much the same way. Fixes #121455. --- compiler/rustc_privacy/src/lib.rs | 5 ++++- tests/ui/privacy/unreachable-issue-121455.rs | 6 ++++++ tests/ui/privacy/unreachable-issue-121455.stderr | 9 +++++++++ 3 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 tests/ui/privacy/unreachable-issue-121455.rs create mode 100644 tests/ui/privacy/unreachable-issue-121455.stderr diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index 9d8a9f5fce3e4..1c6bd88712821 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -988,7 +988,10 @@ impl<'tcx> Visitor<'tcx> for NamePrivacyVisitor<'tcx> { fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) { if let hir::ExprKind::Struct(qpath, fields, ref base) = expr.kind { let res = self.typeck_results().qpath_res(qpath, expr.hir_id); - let adt = self.typeck_results().expr_ty(expr).ty_adt_def().unwrap(); + let Some(adt) = self.typeck_results().expr_ty(expr).ty_adt_def() else { + self.tcx.dcx().span_delayed_bug(expr.span, "no adt_def for expression"); + return; + }; let variant = adt.variant_of_res(res); if let Some(base) = *base { // If the expression uses FRU we need to make sure all the unmentioned fields diff --git a/tests/ui/privacy/unreachable-issue-121455.rs b/tests/ui/privacy/unreachable-issue-121455.rs new file mode 100644 index 0000000000000..5da30d6ed6397 --- /dev/null +++ b/tests/ui/privacy/unreachable-issue-121455.rs @@ -0,0 +1,6 @@ +fn test(s: &Self::Id) { +//~^ ERROR failed to resolve: `Self` is only available in impls, traits, and type definitions + match &s[0..3] {} +} + +fn main() {} diff --git a/tests/ui/privacy/unreachable-issue-121455.stderr b/tests/ui/privacy/unreachable-issue-121455.stderr new file mode 100644 index 0000000000000..864e950a98eb2 --- /dev/null +++ b/tests/ui/privacy/unreachable-issue-121455.stderr @@ -0,0 +1,9 @@ +error[E0433]: failed to resolve: `Self` is only available in impls, traits, and type definitions + --> $DIR/unreachable-issue-121455.rs:1:13 + | +LL | fn test(s: &Self::Id) { + | ^^^^ `Self` is only available in impls, traits, and type definitions + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0433`. From 938e594d62e66cf1d15452a7ed29e2eec4d2efef Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 23 Feb 2024 11:12:04 +1100 Subject: [PATCH 08/10] Remove an unnecessary `if let`. --- compiler/rustc_hir_typeck/src/expr.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index 6b52dd4392e39..81440b0562e24 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -76,16 +76,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // While we don't allow *arbitrary* coercions here, we *do* allow // coercions from ! to `expected`. if ty.is_never() { - if let Some(adjustments) = self.typeck_results.borrow().adjustments().get(expr.hir_id) { + if let Some(_) = self.typeck_results.borrow().adjustments().get(expr.hir_id) { let reported = self.dcx().span_delayed_bug( expr.span, "expression with never type wound up being adjusted", ); - return if let [Adjustment { kind: Adjust::NeverToAny, target }] = &adjustments[..] { - target.to_owned() - } else { - Ty::new_error(self.tcx(), reported) - }; + return Ty::new_error(self.tcx(), reported); } let adj_ty = self.next_ty_var(TypeVariableOrigin { From 9137c1e01eb30165c6d48fb474f843067b514e16 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 2 Jan 2024 20:40:45 +1100 Subject: [PATCH 09/10] coverage: Use variable name `this` in `CoverageGraph::from_mir` This makes it easier to see that we're manipulating the instance that is being constructed, and is a lot less verbose than `basic_coverage_blocks`. --- compiler/rustc_mir_transform/src/coverage/graph.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/graph.rs b/compiler/rustc_mir_transform/src/coverage/graph.rs index c6badbe78a49f..c97192435ce48 100644 --- a/compiler/rustc_mir_transform/src/coverage/graph.rs +++ b/compiler/rustc_mir_transform/src/coverage/graph.rs @@ -52,19 +52,18 @@ impl CoverageGraph { } } - let mut basic_coverage_blocks = - Self { bcbs, bb_to_bcb, successors, predecessors, dominators: None }; - let dominators = dominators::dominators(&basic_coverage_blocks); - basic_coverage_blocks.dominators = Some(dominators); + let mut this = Self { bcbs, bb_to_bcb, successors, predecessors, dominators: None }; + + this.dominators = Some(dominators::dominators(&this)); // The coverage graph's entry-point node (bcb0) always starts with bb0, // which never has predecessors. Any other blocks merged into bcb0 can't // have multiple (coverage-relevant) predecessors, so bcb0 always has // zero in-edges. - assert!(basic_coverage_blocks[START_BCB].leader_bb() == mir::START_BLOCK); - assert!(basic_coverage_blocks.predecessors[START_BCB].is_empty()); + assert!(this[START_BCB].leader_bb() == mir::START_BLOCK); + assert!(this.predecessors[START_BCB].is_empty()); - basic_coverage_blocks + this } fn compute_basic_coverage_blocks( From 41da3d6f2fa3ef3eb49c576d82c6879d4b336ef8 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 23 Feb 2024 13:20:33 +1100 Subject: [PATCH 10/10] Explicitly call `emit_stashed_diagnostics`. Commit 72b172b in #121206 changed things so that `emit_stashed_diagnostics` is only called from `run_compiler`. But rustfmt doesn't use `run_compiler`, so it needs to call `emit_stashed_diagnostics` itself to avoid an abort in `DiagCtxtInner::drop` when stashed diagnostics occur. Fixes #121450. --- src/tools/rustfmt/src/parse/parser.rs | 18 +++++++++++++----- src/tools/rustfmt/src/test/parser.rs | 7 +++++++ src/tools/rustfmt/tests/parser/stashed-diag.rs | 3 +++ 3 files changed, 23 insertions(+), 5 deletions(-) create mode 100644 src/tools/rustfmt/tests/parser/stashed-diag.rs diff --git a/src/tools/rustfmt/src/parse/parser.rs b/src/tools/rustfmt/src/parse/parser.rs index 31226cf8c307d..cca14353b5c18 100644 --- a/src/tools/rustfmt/src/parse/parser.rs +++ b/src/tools/rustfmt/src/parse/parser.rs @@ -163,13 +163,21 @@ impl<'a> Parser<'a> { fn parse_crate_mod(&mut self) -> Result { let mut parser = AssertUnwindSafe(&mut self.parser); - match catch_unwind(move || parser.parse_crate_mod()) { - Ok(Ok(k)) => Ok(k), - Ok(Err(db)) => { + // rustfmt doesn't use `run_compiler` like other tools, so it must emit + // any stashed diagnostics itself, otherwise the `DiagCtxt` will assert + // when dropped. The final result here combines the parsing result and + // the `emit_stashed_diagnostics` result. + let parse_res = catch_unwind(move || parser.parse_crate_mod()); + let stashed_res = self.parser.dcx().emit_stashed_diagnostics(); + let err = Err(ParserError::ParsePanicError); + match (parse_res, stashed_res) { + (Ok(Ok(k)), None) => Ok(k), + (Ok(Ok(_)), Some(_guar)) => err, + (Ok(Err(db)), _) => { db.emit(); - Err(ParserError::ParseError) + err } - Err(_) => Err(ParserError::ParsePanicError), + (Err(_), _) => err, } } } diff --git a/src/tools/rustfmt/src/test/parser.rs b/src/tools/rustfmt/src/test/parser.rs index ae4a4f94d9280..da2a2ba62e009 100644 --- a/src/tools/rustfmt/src/test/parser.rs +++ b/src/tools/rustfmt/src/test/parser.rs @@ -55,3 +55,10 @@ fn crate_parsing_errors_on_unclosed_delims() { let filename = "tests/parser/unclosed-delims/issue_4466.rs"; assert_parser_error(filename); } + +#[test] +fn crate_parsing_stashed_diag() { + // See also https://github.com/rust-lang/rust/issues/121450 + let filename = "tests/parser/stashed-diag.rs"; + assert_parser_error(filename); +} diff --git a/src/tools/rustfmt/tests/parser/stashed-diag.rs b/src/tools/rustfmt/tests/parser/stashed-diag.rs new file mode 100644 index 0000000000000..3b0b543e6101a --- /dev/null +++ b/src/tools/rustfmt/tests/parser/stashed-diag.rs @@ -0,0 +1,3 @@ +#![u={static N;}] + +fn main() {}