From 99c1bcfac500bc5bf1cc5992dca4f918540de556 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Thu, 10 Aug 2023 20:35:48 +0000 Subject: [PATCH 1/9] Revert "make MCP510 behavior explicitly opt-in" This reverts commit 2b61a5e17a6bcb552889966f8f932aa680826ab6. --- compiler/rustc_codegen_ssa/src/back/link.rs | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index cd6201648ee9b..53c56ee782e5d 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -2991,25 +2991,10 @@ fn add_lld_args(cmd: &mut dyn Linker, sess: &Session, flavor: LinkerFlavor) { return; } - let self_contained_linker = sess.opts.cg.link_self_contained.linker(); - - // FIXME: some targets default to using `lld`, but users can only override the linker on the CLI - // and cannot yet select the precise linker flavor to opt out of that. See for example issue - // #113597 for the `thumbv6m-none-eabi` target: a driver is used, and its default linker - // conflicts with the target's flavor, causing unexpected arguments being passed. - // - // Until the new `LinkerFlavor`-like CLI options are stabilized, we only adopt MCP510's behavior - // if its dedicated unstable CLI flags are used, to keep the current sub-optimal stable - // behavior. - let using_mcp510 = - self_contained_linker || sess.opts.cg.linker_flavor.is_some_and(|f| f.is_unstable()); - if !using_mcp510 && !unstable_use_lld { - return; - } - // 1. Implement the "self-contained" part of this feature by adding rustc distribution // directories to the tool's search path. - if self_contained_linker || unstable_use_lld { + let self_contained_linker = sess.opts.cg.link_self_contained.linker() || unstable_use_lld; + if self_contained_linker { for path in sess.get_tools_search_paths(false) { cmd.arg({ let mut arg = OsString::from("-B"); From 498d6562c3fc4e9261efe2e8188fb45c64532eef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Thu, 10 Aug 2023 20:36:25 +0000 Subject: [PATCH 2/9] infer no use of lld when using a generic linker driver --- compiler/rustc_target/src/spec/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index 1871239d7de70..4aa6f1f98a2ca 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -338,7 +338,7 @@ impl LinkerFlavor { || stem == "clang++" || stem.ends_with("-clang++") { - (Some(Cc::Yes), None) + (Some(Cc::Yes), Some(Lld::No)) } else if stem == "wasm-ld" || stem.ends_with("-wasm-ld") || stem == "ld.lld" From f887f5a9c6393d1f3eef98cdf13b4491e27b22e4 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 14 Aug 2023 10:38:46 +0200 Subject: [PATCH 3/9] std: add some missing repr(transparent) --- library/core/src/ffi/c_str.rs | 3 +++ library/std/src/ffi/os_str.rs | 3 +++ library/std/src/path.rs | 6 ++++++ library/std/src/sys/wasi/fd.rs | 10 ++++++++-- library/std/src/sys_common/wtf8.rs | 2 ++ 5 files changed, 22 insertions(+), 2 deletions(-) diff --git a/library/core/src/ffi/c_str.rs b/library/core/src/ffi/c_str.rs index b59ec12790d24..aa34569023eb8 100644 --- a/library/core/src/ffi/c_str.rs +++ b/library/core/src/ffi/c_str.rs @@ -88,6 +88,9 @@ use crate::str; // When attribute privacy is implemented, `CStr` should be annotated as `#[repr(transparent)]`. // Anyway, `CStr` representation and layout are considered implementation detail, are // not documented and must not be relied upon. +// For now we just hide this from rustdoc, technically making our doc test builds rely on +// unspecified layout assumptions. We are std, so we can get away with that. +#[cfg_attr(not(doc), repr(transparent))] pub struct CStr { // FIXME: this should not be represented with a DST slice but rather with // just a raw `c_char` along with some form of marker to make diff --git a/library/std/src/ffi/os_str.rs b/library/std/src/ffi/os_str.rs index 67e58fd1b8613..a9907ae10342d 100644 --- a/library/std/src/ffi/os_str.rs +++ b/library/std/src/ffi/os_str.rs @@ -116,6 +116,9 @@ impl crate::sealed::Sealed for OsString {} // When attribute privacy is implemented, `OsStr` should be annotated as `#[repr(transparent)]`. // Anyway, `OsStr` representation and layout are considered implementation details, are // not documented and must not be relied upon. +// For now we just hide this from rustdoc, technically making our doc test builds rely on +// unspecified layout assumptions. We are std, so we can get away with that. +#[cfg_attr(not(doc), repr(transparent))] pub struct OsStr { inner: Slice, } diff --git a/library/std/src/path.rs b/library/std/src/path.rs index 99f7a60f8ab01..ef5fc669037ab 100644 --- a/library/std/src/path.rs +++ b/library/std/src/path.rs @@ -1164,6 +1164,9 @@ impl FusedIterator for Ancestors<'_> {} // When attribute privacy is implemented, `PathBuf` should be annotated as `#[repr(transparent)]`. // Anyway, `PathBuf` representation and layout are considered implementation detail, are // not documented and must not be relied upon. +// For now we just hide this from rustdoc, technically making our doc test builds rely on +// unspecified layout assumptions. We are std, so we can get away with that. +#[cfg_attr(not(doc), repr(transparent))] pub struct PathBuf { inner: OsString, } @@ -1989,6 +1992,9 @@ impl AsRef for PathBuf { // When attribute privacy is implemented, `Path` should be annotated as `#[repr(transparent)]`. // Anyway, `Path` representation and layout are considered implementation detail, are // not documented and must not be relied upon. +// For now we just hide this from rustdoc, technically making our doc test builds rely on +// unspecified layout assumptions. We are std, so we can get away with that. +#[cfg_attr(not(doc), repr(transparent))] pub struct Path { inner: OsStr, } diff --git a/library/std/src/sys/wasi/fd.rs b/library/std/src/sys/wasi/fd.rs index 1b50c2ea6dd57..d7295a799daab 100644 --- a/library/std/src/sys/wasi/fd.rs +++ b/library/std/src/sys/wasi/fd.rs @@ -16,14 +16,20 @@ pub struct WasiFd { fn iovec<'a>(a: &'a mut [IoSliceMut<'_>]) -> &'a [wasi::Iovec] { assert_eq!(mem::size_of::>(), mem::size_of::()); assert_eq!(mem::align_of::>(), mem::align_of::()); - // SAFETY: `IoSliceMut` and `IoVec` have exactly the same memory layout + // SAFETY: `IoSliceMut` and `IoVec` have exactly the same memory layout. + // We decorate our `IoSliceMut` with `repr(transparent)` (see `io.rs`), and + // `crate::io::IoSliceMut` is a `repr(transparent)` wrapper around our type, so this is + // guaranteed. unsafe { mem::transmute(a) } } fn ciovec<'a>(a: &'a [IoSlice<'_>]) -> &'a [wasi::Ciovec] { assert_eq!(mem::size_of::>(), mem::size_of::()); assert_eq!(mem::align_of::>(), mem::align_of::()); - // SAFETY: `IoSlice` and `CIoVec` have exactly the same memory layout + // SAFETY: `IoSlice` and `CIoVec` have exactly the same memory layout. + // We decorate our `IoSlice` with `repr(transparent)` (see `io.rs`), and + // `crate::io::IoSlice` is a `repr(transparent)` wrapper around our type, so this is + // guaranteed. unsafe { mem::transmute(a) } } diff --git a/library/std/src/sys_common/wtf8.rs b/library/std/src/sys_common/wtf8.rs index 195d175cc9bde..67db5ebd89cfc 100644 --- a/library/std/src/sys_common/wtf8.rs +++ b/library/std/src/sys_common/wtf8.rs @@ -459,6 +459,7 @@ impl Wtf8Buf { /// Converts this `Wtf8Buf` into a boxed `Wtf8`. #[inline] pub fn into_box(self) -> Box { + // SAFETY: relies on `Wtf8` being `repr(transparent)`. unsafe { mem::transmute(self.bytes.into_boxed_slice()) } } @@ -511,6 +512,7 @@ impl Extend for Wtf8Buf { /// Similar to `&str`, but can additionally contain surrogate code points /// if they’re not in a surrogate pair. #[derive(Eq, Ord, PartialEq, PartialOrd)] +#[repr(transparent)] pub struct Wtf8 { bytes: [u8], } From fe1a034f16adbed4302e4d27be96a7ec6fb27177 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 14 Aug 2023 22:55:29 +0200 Subject: [PATCH 4/9] actually this doesn't even affect doctests. nice. --- library/core/src/ffi/c_str.rs | 9 +++------ library/std/src/ffi/os_str.rs | 9 +++------ library/std/src/path.rs | 18 ++++++------------ 3 files changed, 12 insertions(+), 24 deletions(-) diff --git a/library/core/src/ffi/c_str.rs b/library/core/src/ffi/c_str.rs index aa34569023eb8..4082b208c1263 100644 --- a/library/core/src/ffi/c_str.rs +++ b/library/core/src/ffi/c_str.rs @@ -82,14 +82,11 @@ use crate::str; #[stable(feature = "core_c_str", since = "1.64.0")] #[rustc_has_incoherent_inherent_impls] #[lang = "CStr"] -// FIXME: // `fn from` in `impl From<&CStr> for Box` current implementation relies // on `CStr` being layout-compatible with `[u8]`. -// When attribute privacy is implemented, `CStr` should be annotated as `#[repr(transparent)]`. -// Anyway, `CStr` representation and layout are considered implementation detail, are -// not documented and must not be relied upon. -// For now we just hide this from rustdoc, technically making our doc test builds rely on -// unspecified layout assumptions. We are std, so we can get away with that. +// However, `CStr` layout is considered an implementation detail and must not be relied upon. We +// want `repr(transparent)` but we don't want it to show up in rustdoc, so we hide it under +// `cfg(doc)`. This is an ad-hoc implementation of attribute privacy. #[cfg_attr(not(doc), repr(transparent))] pub struct CStr { // FIXME: this should not be represented with a DST slice but rather with diff --git a/library/std/src/ffi/os_str.rs b/library/std/src/ffi/os_str.rs index a9907ae10342d..43cecb19b148e 100644 --- a/library/std/src/ffi/os_str.rs +++ b/library/std/src/ffi/os_str.rs @@ -110,14 +110,11 @@ impl crate::sealed::Sealed for OsString {} /// [conversions]: super#conversions #[cfg_attr(not(test), rustc_diagnostic_item = "OsStr")] #[stable(feature = "rust1", since = "1.0.0")] -// FIXME: // `OsStr::from_inner` current implementation relies // on `OsStr` being layout-compatible with `Slice`. -// When attribute privacy is implemented, `OsStr` should be annotated as `#[repr(transparent)]`. -// Anyway, `OsStr` representation and layout are considered implementation details, are -// not documented and must not be relied upon. -// For now we just hide this from rustdoc, technically making our doc test builds rely on -// unspecified layout assumptions. We are std, so we can get away with that. +// However, `OsStr` layout is considered an implementation detail and must not be relied upon. We +// want `repr(transparent)` but we don't want it to show up in rustdoc, so we hide it under +// `cfg(doc)`. This is an ad-hoc implementation of attribute privacy. #[cfg_attr(not(doc), repr(transparent))] pub struct OsStr { inner: Slice, diff --git a/library/std/src/path.rs b/library/std/src/path.rs index ef5fc669037ab..5842c096f1ab7 100644 --- a/library/std/src/path.rs +++ b/library/std/src/path.rs @@ -1158,14 +1158,11 @@ impl FusedIterator for Ancestors<'_> {} /// Which method works best depends on what kind of situation you're in. #[cfg_attr(not(test), rustc_diagnostic_item = "PathBuf")] #[stable(feature = "rust1", since = "1.0.0")] -// FIXME: // `PathBuf::as_mut_vec` current implementation relies // on `PathBuf` being layout-compatible with `Vec`. -// When attribute privacy is implemented, `PathBuf` should be annotated as `#[repr(transparent)]`. -// Anyway, `PathBuf` representation and layout are considered implementation detail, are -// not documented and must not be relied upon. -// For now we just hide this from rustdoc, technically making our doc test builds rely on -// unspecified layout assumptions. We are std, so we can get away with that. +// However, `PathBuf` layout is considered an implementation detail and must not be relied upon. We +// want `repr(transparent)` but we don't want it to show up in rustdoc, so we hide it under +// `cfg(doc)`. This is an ad-hoc implementation of attribute privacy. #[cfg_attr(not(doc), repr(transparent))] pub struct PathBuf { inner: OsString, @@ -1986,14 +1983,11 @@ impl AsRef for PathBuf { /// ``` #[cfg_attr(not(test), rustc_diagnostic_item = "Path")] #[stable(feature = "rust1", since = "1.0.0")] -// FIXME: // `Path::new` current implementation relies // on `Path` being layout-compatible with `OsStr`. -// When attribute privacy is implemented, `Path` should be annotated as `#[repr(transparent)]`. -// Anyway, `Path` representation and layout are considered implementation detail, are -// not documented and must not be relied upon. -// For now we just hide this from rustdoc, technically making our doc test builds rely on -// unspecified layout assumptions. We are std, so we can get away with that. +// However, `Path` layout is considered an implementation detail and must not be relied upon. We +// want `repr(transparent)` but we don't want it to show up in rustdoc, so we hide it under +// `cfg(doc)`. This is an ad-hoc implementation of attribute privacy. #[cfg_attr(not(doc), repr(transparent))] pub struct Path { inner: OsStr, From 55f8c66a601236b422e35f56f7e414a8280c78d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 14 Aug 2023 18:36:03 +0000 Subject: [PATCH 5/9] Point at return type when it influences non-first `match` arm When encountering code like ```rust fn foo() -> i32 { match 0 { 1 => return 0, 2 => "", _ => 1, } } ``` Point at the return type and not at the prior arm, as that arm has type `!` which isn't influencing the arm corresponding to arm `2`. Fix #78124. --- compiler/rustc_hir_typeck/src/_match.rs | 13 ++- compiler/rustc_hir_typeck/src/coercion.rs | 2 +- .../rustc_hir_typeck/src/fn_ctxt/checks.rs | 9 ++- .../src/infer/error_reporting/mod.rs | 80 +++++++++++++++---- .../nice_region_error/static_impl_trait.rs | 2 +- compiler/rustc_middle/src/traits/mod.rs | 2 +- .../src/traits/error_reporting/suggestions.rs | 2 +- .../did_you_mean/compatible-variants.stderr | 2 + ...1632-try-desugar-incompatible-types.stderr | 2 + ...t-arm-doesnt-match-expected-return-type.rs | 21 +++++ ...m-doesnt-match-expected-return-type.stderr | 12 +++ .../remove-question-symbol-with-paren.stderr | 3 + 12 files changed, 129 insertions(+), 21 deletions(-) create mode 100644 tests/ui/match/non-first-arm-doesnt-match-expected-return-type.rs create mode 100644 tests/ui/match/non-first-arm-doesnt-match-expected-return-type.stderr diff --git a/compiler/rustc_hir_typeck/src/_match.rs b/compiler/rustc_hir_typeck/src/_match.rs index 6d926cd8aa132..e565dbfe8d2ec 100644 --- a/compiler/rustc_hir_typeck/src/_match.rs +++ b/compiler/rustc_hir_typeck/src/_match.rs @@ -107,7 +107,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let (span, code) = match prior_arm { // The reason for the first arm to fail is not that the match arms diverge, // but rather that there's a prior obligation that doesn't hold. - None => (arm_span, ObligationCauseCode::BlockTailExpression(arm.body.hir_id)), + None => ( + arm_span, + ObligationCauseCode::BlockTailExpression( + arm.body.hir_id, + scrut.hir_id, + match_src, + ), + ), Some((prior_arm_block_id, prior_arm_ty, prior_arm_span)) => ( expr.span, ObligationCauseCode::MatchExpressionArm(Box::new(MatchExpressionArmCause { @@ -145,7 +152,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { other_arms.remove(0); } - prior_arm = Some((arm_block_id, arm_ty, arm_span)); + if !arm_ty.is_never() { + prior_arm = Some((arm_block_id, arm_ty, arm_span)); + } } // If all of the arms in the `match` diverge, diff --git a/compiler/rustc_hir_typeck/src/coercion.rs b/compiler/rustc_hir_typeck/src/coercion.rs index 1cfdc5b9e7f57..ca616fd1518f9 100644 --- a/compiler/rustc_hir_typeck/src/coercion.rs +++ b/compiler/rustc_hir_typeck/src/coercion.rs @@ -1603,7 +1603,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { ); err.span_label(cause.span, "return type is not `()`"); } - ObligationCauseCode::BlockTailExpression(blk_id) => { + ObligationCauseCode::BlockTailExpression(blk_id, ..) => { let parent_id = fcx.tcx.hir().parent_id(blk_id); err = self.report_return_mismatched_types( cause, diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs index 40f9a95403497..5254f0796d86c 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs @@ -1580,7 +1580,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let coerce = ctxt.coerce.as_mut().unwrap(); if let Some((tail_expr, tail_expr_ty)) = tail_expr_ty { let span = self.get_expr_coercion_span(tail_expr); - let cause = self.cause(span, ObligationCauseCode::BlockTailExpression(blk.hir_id)); + let cause = self.cause( + span, + ObligationCauseCode::BlockTailExpression( + blk.hir_id, + blk.hir_id, + hir::MatchSource::Normal, + ), + ); let ty_for_diagnostic = coerce.merged_ty(); // We use coerce_inner here because we want to augment the error // suggesting to wrap the block in square brackets if it might've diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index 75cca9733060d..717103ed0b439 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -743,6 +743,36 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { ObligationCauseCode::Pattern { origin_expr: false, span: Some(span), .. } => { err.span_label(span, "expected due to this"); } + ObligationCauseCode::BlockTailExpression( + _, + scrut_hir_id, + hir::MatchSource::TryDesugar, + ) => { + if let Some(ty::error::ExpectedFound { expected, .. }) = exp_found { + let scrut_expr = self.tcx.hir().expect_expr(scrut_hir_id); + let scrut_ty = if let hir::ExprKind::Call(_, args) = &scrut_expr.kind { + let arg_expr = args.first().expect("try desugaring call w/out arg"); + self.typeck_results.as_ref().and_then(|typeck_results| { + typeck_results.expr_ty_opt(arg_expr) + }) + } else { + bug!("try desugaring w/out call expr as scrutinee"); + }; + + match scrut_ty { + Some(ty) if expected == ty => { + let source_map = self.tcx.sess.source_map(); + err.span_suggestion( + source_map.end_point(cause.span()), + "try removing this `?`", + "", + Applicability::MachineApplicable, + ); + } + _ => {} + } + } + }, ObligationCauseCode::MatchExpressionArm(box MatchExpressionArmCause { arm_block_id, arm_span, @@ -1973,7 +2003,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { trace: &TypeTrace<'tcx>, terr: TypeError<'tcx>, ) -> Vec { - use crate::traits::ObligationCauseCode::MatchExpressionArm; + use crate::traits::ObligationCauseCode::{BlockTailExpression, MatchExpressionArm}; let mut suggestions = Vec::new(); let span = trace.cause.span(); let values = self.resolve_vars_if_possible(trace.values); @@ -1991,11 +2021,17 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { // specify a byte literal (ty::Uint(ty::UintTy::U8), ty::Char) => { if let Ok(code) = self.tcx.sess().source_map().span_to_snippet(span) - && let Some(code) = code.strip_prefix('\'').and_then(|s| s.strip_suffix('\'')) - && !code.starts_with("\\u") // forbid all Unicode escapes - && code.chars().next().is_some_and(|c| c.is_ascii()) // forbids literal Unicode characters beyond ASCII + && let Some(code) = + code.strip_prefix('\'').and_then(|s| s.strip_suffix('\'')) + // forbid all Unicode escapes + && !code.starts_with("\\u") + // forbids literal Unicode characters beyond ASCII + && code.chars().next().is_some_and(|c| c.is_ascii()) { - suggestions.push(TypeErrorAdditionalDiags::MeantByteLiteral { span, code: escape_literal(code) }) + suggestions.push(TypeErrorAdditionalDiags::MeantByteLiteral { + span, + code: escape_literal(code), + }) } } // If a character was expected and the found expression is a string literal @@ -2006,7 +2042,10 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { && let Some(code) = code.strip_prefix('"').and_then(|s| s.strip_suffix('"')) && code.chars().count() == 1 { - suggestions.push(TypeErrorAdditionalDiags::MeantCharLiteral { span, code: escape_literal(code) }) + suggestions.push(TypeErrorAdditionalDiags::MeantCharLiteral { + span, + code: escape_literal(code), + }) } } // If a string was expected and the found expression is a character literal, @@ -2016,7 +2055,10 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { if let Some(code) = code.strip_prefix('\'').and_then(|s| s.strip_suffix('\'')) { - suggestions.push(TypeErrorAdditionalDiags::MeantStrLiteral { span, code: escape_literal(code) }) + suggestions.push(TypeErrorAdditionalDiags::MeantStrLiteral { + span, + code: escape_literal(code), + }) } } } @@ -2025,17 +2067,24 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { (ty::Bool, ty::Tuple(list)) => if list.len() == 0 { suggestions.extend(self.suggest_let_for_letchains(&trace.cause, span)); } - (ty::Array(_, _), ty::Array(_, _)) => suggestions.extend(self.suggest_specify_actual_length(terr, trace, span)), + (ty::Array(_, _), ty::Array(_, _)) => { + suggestions.extend(self.suggest_specify_actual_length(terr, trace, span)) + } _ => {} } } let code = trace.cause.code(); - if let &MatchExpressionArm(box MatchExpressionArmCause { source, .. }) = code - && let hir::MatchSource::TryDesugar = source - && let Some((expected_ty, found_ty, _, _)) = self.values_str(trace.values) - { - suggestions.push(TypeErrorAdditionalDiags::TryCannotConvert { found: found_ty.content(), expected: expected_ty.content() }); - } + if let &(MatchExpressionArm(box MatchExpressionArmCause { source, .. }) + | BlockTailExpression(.., source) + ) = code + && let hir::MatchSource::TryDesugar = source + && let Some((expected_ty, found_ty, _, _)) = self.values_str(trace.values) + { + suggestions.push(TypeErrorAdditionalDiags::TryCannotConvert { + found: found_ty.content(), + expected: expected_ty.content(), + }); + } suggestions } @@ -2905,6 +2954,9 @@ impl<'tcx> ObligationCauseExt<'tcx> for ObligationCause<'tcx> { CompareImplItemObligation { kind: ty::AssocKind::Const, .. } => { ObligationCauseFailureCode::ConstCompat { span, subdiags } } + BlockTailExpression(.., hir::MatchSource::TryDesugar) => { + ObligationCauseFailureCode::TryCompat { span, subdiags } + } MatchExpressionArm(box MatchExpressionArmCause { source, .. }) => match source { hir::MatchSource::TryDesugar => { ObligationCauseFailureCode::TryCompat { span, subdiags } diff --git a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs index d08b6ba5e47d9..3cfda0cc5c05c 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs @@ -146,7 +146,7 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> { if let SubregionOrigin::Subtype(box TypeTrace { cause, .. }) = sub_origin { if let ObligationCauseCode::ReturnValue(hir_id) - | ObligationCauseCode::BlockTailExpression(hir_id) = cause.code() + | ObligationCauseCode::BlockTailExpression(hir_id, ..) = cause.code() { let parent_id = tcx.hir().get_parent_item(*hir_id); if let Some(fn_decl) = tcx.hir().fn_decl_by_hir_id(parent_id.into()) { diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index 2d655041c32e8..1845d42bf7f04 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -402,7 +402,7 @@ pub enum ObligationCauseCode<'tcx> { OpaqueReturnType(Option<(Ty<'tcx>, Span)>), /// Block implicit return - BlockTailExpression(hir::HirId), + BlockTailExpression(hir::HirId, hir::HirId, hir::MatchSource), /// #[feature(trivial_bounds)] is not enabled TrivialBound, diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index d071cf76fd3ac..5e075984238ee 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -2700,7 +2700,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { | ObligationCauseCode::MatchImpl(..) | ObligationCauseCode::ReturnType | ObligationCauseCode::ReturnValue(_) - | ObligationCauseCode::BlockTailExpression(_) + | ObligationCauseCode::BlockTailExpression(..) | ObligationCauseCode::AwaitableExpr(_) | ObligationCauseCode::ForLoopIterator | ObligationCauseCode::QuestionMark diff --git a/tests/ui/did_you_mean/compatible-variants.stderr b/tests/ui/did_you_mean/compatible-variants.stderr index 7b88d93ead112..f2bbd8ced8f24 100644 --- a/tests/ui/did_you_mean/compatible-variants.stderr +++ b/tests/ui/did_you_mean/compatible-variants.stderr @@ -61,6 +61,8 @@ LL + Some(()) error[E0308]: `?` operator has incompatible types --> $DIR/compatible-variants.rs:35:5 | +LL | fn d() -> Option<()> { + | ---------- expected `Option<()>` because of return type LL | c()? | ^^^^ expected `Option<()>`, found `()` | diff --git a/tests/ui/issues/issue-51632-try-desugar-incompatible-types.stderr b/tests/ui/issues/issue-51632-try-desugar-incompatible-types.stderr index 7180a3d2426d1..c92da53dbc48b 100644 --- a/tests/ui/issues/issue-51632-try-desugar-incompatible-types.stderr +++ b/tests/ui/issues/issue-51632-try-desugar-incompatible-types.stderr @@ -1,6 +1,8 @@ error[E0308]: `?` operator has incompatible types --> $DIR/issue-51632-try-desugar-incompatible-types.rs:8:5 | +LL | fn forbidden_narratives() -> Result { + | ----------------- expected `Result` because of return type LL | missing_discourses()? | ^^^^^^^^^^^^^^^^^^^^^ expected `Result`, found `isize` | diff --git a/tests/ui/match/non-first-arm-doesnt-match-expected-return-type.rs b/tests/ui/match/non-first-arm-doesnt-match-expected-return-type.rs new file mode 100644 index 0000000000000..85b1ef7555edc --- /dev/null +++ b/tests/ui/match/non-first-arm-doesnt-match-expected-return-type.rs @@ -0,0 +1,21 @@ +#![allow(unused)] + +fn test(shouldwe: Option, shouldwe2: Option) -> u32 { + //~^ NOTE expected `u32` because of return type + match shouldwe { + Some(val) => { + match shouldwe2 { + Some(val) => { + return val; + } + None => (), //~ ERROR mismatched types + //~^ NOTE expected `u32`, found `()` + } + } + None => return 12, + } +} + +fn main() { + println!("returned {}", test(None, Some(5))); +} diff --git a/tests/ui/match/non-first-arm-doesnt-match-expected-return-type.stderr b/tests/ui/match/non-first-arm-doesnt-match-expected-return-type.stderr new file mode 100644 index 0000000000000..e6d93b8b5f57b --- /dev/null +++ b/tests/ui/match/non-first-arm-doesnt-match-expected-return-type.stderr @@ -0,0 +1,12 @@ +error[E0308]: mismatched types + --> $DIR/non-first-arm-doesnt-match-expected-return-type.rs:11:25 + | +LL | fn test(shouldwe: Option, shouldwe2: Option) -> u32 { + | --- expected `u32` because of return type +... +LL | None => (), + | ^^ expected `u32`, found `()` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/suggestions/remove-question-symbol-with-paren.stderr b/tests/ui/suggestions/remove-question-symbol-with-paren.stderr index 39e35f733a1d2..40b9cf2dcd4d4 100644 --- a/tests/ui/suggestions/remove-question-symbol-with-paren.stderr +++ b/tests/ui/suggestions/remove-question-symbol-with-paren.stderr @@ -1,6 +1,9 @@ error[E0308]: `?` operator has incompatible types --> $DIR/remove-question-symbol-with-paren.rs:5:6 | +LL | fn foo() -> Option<()> { + | ---------- expected `Option<()>` because of return type +LL | let x = Some(()); LL | (x?) | ^^ expected `Option<()>`, found `()` | From 5021dde1a094884eb16aa5370e2f9e32d4e1fce4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 14 Aug 2023 19:25:01 +0000 Subject: [PATCH 6/9] Move scrutinee `HirId` into `MatchSource::TryDesugar` --- compiler/rustc_ast_lowering/src/expr.rs | 2 +- compiler/rustc_hir/src/hir.rs | 4 ++-- compiler/rustc_hir_typeck/src/_match.rs | 12 +++--------- compiler/rustc_hir_typeck/src/coercion.rs | 2 +- compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs | 6 +----- .../rustc_infer/src/infer/error_reporting/mod.rs | 12 +++++------- compiler/rustc_middle/src/thir.rs | 1 + compiler/rustc_middle/src/thir/visit.rs | 2 +- compiler/rustc_middle/src/traits/mod.rs | 3 +-- .../src/build/custom/parse/instruction.rs | 2 +- compiler/rustc_mir_build/src/build/expr/into.rs | 2 +- compiler/rustc_mir_build/src/thir/cx/expr.rs | 1 + .../rustc_mir_build/src/thir/pattern/check_match.rs | 8 +++++--- compiler/rustc_mir_build/src/thir/print.rs | 2 +- compiler/rustc_passes/src/check_const.rs | 2 +- src/tools/clippy/clippy_lints/src/dereference.rs | 3 ++- src/tools/clippy/clippy_lints/src/matches/mod.rs | 2 +- src/tools/clippy/clippy_lints/src/matches/try_err.rs | 2 +- .../clippy/clippy_lints/src/methods/clone_on_copy.rs | 2 +- .../clippy/clippy_lints/src/methods/str_splitn.rs | 2 +- .../clippy_lints/src/needless_question_mark.rs | 2 +- .../clippy/clippy_lints/src/question_mark_used.rs | 2 +- .../clippy_lints/src/redundant_closure_call.rs | 2 +- src/tools/clippy/clippy_lints/src/returns.rs | 2 +- .../clippy/clippy_lints/src/unit_types/unit_arg.rs | 2 +- .../clippy/clippy_lints/src/useless_conversion.rs | 2 +- .../clippy/clippy_utils/src/check_proc_macro.rs | 2 +- src/tools/clippy/clippy_utils/src/lib.rs | 2 +- src/tools/clippy/clippy_utils/src/visitors.rs | 2 +- 29 files changed, 41 insertions(+), 49 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index b23cee14f7586..7408b4fb0af48 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -1648,7 +1648,7 @@ impl<'hir> LoweringContext<'_, 'hir> { hir::ExprKind::Match( scrutinee, arena_vec![self; break_arm, continue_arm], - hir::MatchSource::TryDesugar, + hir::MatchSource::TryDesugar(scrutinee.hir_id), ) } diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 6340f1dcca1c2..0bfd62d68b295 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -2148,7 +2148,7 @@ pub enum MatchSource { /// A desugared `for _ in _ { .. }` loop. ForLoopDesugar, /// A desugared `?` operator. - TryDesugar, + TryDesugar(HirId), /// A desugared `.await`. AwaitDesugar, /// A desugared `format_args!()`. @@ -2162,7 +2162,7 @@ impl MatchSource { match self { Normal => "match", ForLoopDesugar => "for", - TryDesugar => "?", + TryDesugar(_) => "?", AwaitDesugar => ".await", FormatArgs => "format_args!()", } diff --git a/compiler/rustc_hir_typeck/src/_match.rs b/compiler/rustc_hir_typeck/src/_match.rs index e565dbfe8d2ec..5d710239ff35d 100644 --- a/compiler/rustc_hir_typeck/src/_match.rs +++ b/compiler/rustc_hir_typeck/src/_match.rs @@ -107,14 +107,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let (span, code) = match prior_arm { // The reason for the first arm to fail is not that the match arms diverge, // but rather that there's a prior obligation that doesn't hold. - None => ( - arm_span, - ObligationCauseCode::BlockTailExpression( - arm.body.hir_id, - scrut.hir_id, - match_src, - ), - ), + None => { + (arm_span, ObligationCauseCode::BlockTailExpression(arm.body.hir_id, match_src)) + } Some((prior_arm_block_id, prior_arm_ty, prior_arm_span)) => ( expr.span, ObligationCauseCode::MatchExpressionArm(Box::new(MatchExpressionArmCause { @@ -127,7 +122,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { scrut_span: scrut.span, source: match_src, prior_arms: other_arms.clone(), - scrut_hir_id: scrut.hir_id, opt_suggest_box_span, })), ), diff --git a/compiler/rustc_hir_typeck/src/coercion.rs b/compiler/rustc_hir_typeck/src/coercion.rs index ca616fd1518f9..f618699fa2be9 100644 --- a/compiler/rustc_hir_typeck/src/coercion.rs +++ b/compiler/rustc_hir_typeck/src/coercion.rs @@ -1751,7 +1751,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { ) && !in_external_macro(fcx.tcx.sess, cond_expr.span) && !matches!( cond_expr.kind, - hir::ExprKind::Match(.., hir::MatchSource::TryDesugar) + hir::ExprKind::Match(.., hir::MatchSource::TryDesugar(_)) ) { err.span_label(cond_expr.span, "expected this to be `()`"); diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs index 5254f0796d86c..817115012a467 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs @@ -1582,11 +1582,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let span = self.get_expr_coercion_span(tail_expr); let cause = self.cause( span, - ObligationCauseCode::BlockTailExpression( - blk.hir_id, - blk.hir_id, - hir::MatchSource::Normal, - ), + ObligationCauseCode::BlockTailExpression(blk.hir_id, hir::MatchSource::Normal), ); let ty_for_diagnostic = coerce.merged_ty(); // We use coerce_inner here because we want to augment the error diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index 717103ed0b439..0b676850dc718 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -745,8 +745,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { } ObligationCauseCode::BlockTailExpression( _, - scrut_hir_id, - hir::MatchSource::TryDesugar, + hir::MatchSource::TryDesugar(scrut_hir_id), ) => { if let Some(ty::error::ExpectedFound { expected, .. }) = exp_found { let scrut_expr = self.tcx.hir().expect_expr(scrut_hir_id); @@ -782,12 +781,11 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { prior_arm_ty, source, ref prior_arms, - scrut_hir_id, opt_suggest_box_span, scrut_span, .. }) => match source { - hir::MatchSource::TryDesugar => { + hir::MatchSource::TryDesugar(scrut_hir_id) => { if let Some(ty::error::ExpectedFound { expected, .. }) = exp_found { let scrut_expr = self.tcx.hir().expect_expr(scrut_hir_id); let scrut_ty = if let hir::ExprKind::Call(_, args) = &scrut_expr.kind { @@ -2077,7 +2075,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { if let &(MatchExpressionArm(box MatchExpressionArmCause { source, .. }) | BlockTailExpression(.., source) ) = code - && let hir::MatchSource::TryDesugar = source + && let hir::MatchSource::TryDesugar(_) = source && let Some((expected_ty, found_ty, _, _)) = self.values_str(trace.values) { suggestions.push(TypeErrorAdditionalDiags::TryCannotConvert { @@ -2954,11 +2952,11 @@ impl<'tcx> ObligationCauseExt<'tcx> for ObligationCause<'tcx> { CompareImplItemObligation { kind: ty::AssocKind::Const, .. } => { ObligationCauseFailureCode::ConstCompat { span, subdiags } } - BlockTailExpression(.., hir::MatchSource::TryDesugar) => { + BlockTailExpression(.., hir::MatchSource::TryDesugar(_)) => { ObligationCauseFailureCode::TryCompat { span, subdiags } } MatchExpressionArm(box MatchExpressionArmCause { source, .. }) => match source { - hir::MatchSource::TryDesugar => { + hir::MatchSource::TryDesugar(_) => { ObligationCauseFailureCode::TryCompat { span, subdiags } } _ => ObligationCauseFailureCode::MatchCompat { span, subdiags }, diff --git a/compiler/rustc_middle/src/thir.rs b/compiler/rustc_middle/src/thir.rs index 8e2e71fd87984..ebc1c11902bcb 100644 --- a/compiler/rustc_middle/src/thir.rs +++ b/compiler/rustc_middle/src/thir.rs @@ -346,6 +346,7 @@ pub enum ExprKind<'tcx> { /// A `match` expression. Match { scrutinee: ExprId, + scrutinee_hir_id: hir::HirId, arms: Box<[ArmId]>, }, /// A block. diff --git a/compiler/rustc_middle/src/thir/visit.rs b/compiler/rustc_middle/src/thir/visit.rs index 55ec17423ec93..681400dbb9481 100644 --- a/compiler/rustc_middle/src/thir/visit.rs +++ b/compiler/rustc_middle/src/thir/visit.rs @@ -70,7 +70,7 @@ pub fn walk_expr<'a, 'tcx: 'a, V: Visitor<'a, 'tcx>>(visitor: &mut V, expr: &Exp visitor.visit_expr(&visitor.thir()[expr]); } Loop { body } => visitor.visit_expr(&visitor.thir()[body]), - Match { scrutinee, ref arms } => { + Match { scrutinee, ref arms, .. } => { visitor.visit_expr(&visitor.thir()[scrutinee]); for &arm in &**arms { visitor.visit_arm(&visitor.thir()[arm]); diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index 1845d42bf7f04..3465759b91340 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -402,7 +402,7 @@ pub enum ObligationCauseCode<'tcx> { OpaqueReturnType(Option<(Ty<'tcx>, Span)>), /// Block implicit return - BlockTailExpression(hir::HirId, hir::HirId, hir::MatchSource), + BlockTailExpression(hir::HirId, hir::MatchSource), /// #[feature(trivial_bounds)] is not enabled TrivialBound, @@ -543,7 +543,6 @@ pub struct MatchExpressionArmCause<'tcx> { pub scrut_span: Span, pub source: hir::MatchSource, pub prior_arms: Vec, - pub scrut_hir_id: hir::HirId, pub opt_suggest_box_span: Option, } diff --git a/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs b/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs index 295cddb1e15b4..fe5190900e940 100644 --- a/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs +++ b/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs @@ -65,7 +65,7 @@ impl<'tcx, 'body> ParseCtxt<'tcx, 'body> { let target = self.parse_block(args[1])?; self.parse_call(args[2], destination, target) }, - ExprKind::Match { scrutinee, arms } => { + ExprKind::Match { scrutinee, arms, .. } => { let discr = self.parse_operand(*scrutinee)?; self.parse_match(arms, expr.span).map(|t| TerminatorKind::SwitchInt { discr, targets: t }) }, diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs index c750727903fdc..a5c86e31a2921 100644 --- a/compiler/rustc_mir_build/src/build/expr/into.rs +++ b/compiler/rustc_mir_build/src/build/expr/into.rs @@ -47,7 +47,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ExprKind::Block { block: ast_block } => { this.ast_block(destination, block, ast_block, source_info) } - ExprKind::Match { scrutinee, ref arms } => { + ExprKind::Match { scrutinee, ref arms, .. } => { this.match_expr(destination, expr_span, block, &this.thir[scrutinee], arms) } ExprKind::If { cond, then, else_opt, if_then_scope } => { diff --git a/compiler/rustc_mir_build/src/thir/cx/expr.rs b/compiler/rustc_mir_build/src/thir/cx/expr.rs index 994ac8a3286ca..a07ea7f2e1d7d 100644 --- a/compiler/rustc_mir_build/src/thir/cx/expr.rs +++ b/compiler/rustc_mir_build/src/thir/cx/expr.rs @@ -733,6 +733,7 @@ impl<'tcx> Cx<'tcx> { }, hir::ExprKind::Match(ref discr, ref arms, _) => ExprKind::Match { scrutinee: self.mirror_expr(discr), + scrutinee_hir_id: discr.hir_id, arms: arms.iter().map(|a| self.convert_arm(a)).collect(), }, hir::ExprKind::Loop(ref body, ..) => { diff --git a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs index a786e65966423..383e80851f0d7 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs @@ -135,10 +135,12 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for MatchVisitor<'a, '_, 'tcx> { }); return; } - ExprKind::Match { scrutinee, box ref arms } => { + ExprKind::Match { scrutinee, scrutinee_hir_id, box ref arms } => { let source = match ex.span.desugaring_kind() { Some(DesugaringKind::ForLoop) => hir::MatchSource::ForLoopDesugar, - Some(DesugaringKind::QuestionMark) => hir::MatchSource::TryDesugar, + Some(DesugaringKind::QuestionMark) => { + hir::MatchSource::TryDesugar(scrutinee_hir_id) + } Some(DesugaringKind::Await) => hir::MatchSource::AwaitDesugar, _ => hir::MatchSource::Normal, }; @@ -277,7 +279,7 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> { | hir::MatchSource::FormatArgs => report_arm_reachability(&cx, &report), // Unreachable patterns in try and await expressions occur when one of // the arms are an uninhabited type. Which is OK. - hir::MatchSource::AwaitDesugar | hir::MatchSource::TryDesugar => {} + hir::MatchSource::AwaitDesugar | hir::MatchSource::TryDesugar(_) => {} } // Check if the match is exhaustive. diff --git a/compiler/rustc_mir_build/src/thir/print.rs b/compiler/rustc_mir_build/src/thir/print.rs index 903dbeeadfa26..3b6276cfeb0e6 100644 --- a/compiler/rustc_mir_build/src/thir/print.rs +++ b/compiler/rustc_mir_build/src/thir/print.rs @@ -321,7 +321,7 @@ impl<'a, 'tcx> ThirPrinter<'a, 'tcx> { print_indented!(self, format!("pat: {:?}", pat), depth_lvl + 1); print_indented!(self, "}", depth_lvl); } - Match { scrutinee, arms } => { + Match { scrutinee, arms, .. } => { print_indented!(self, "Match {", depth_lvl); print_indented!(self, "scrutinee:", depth_lvl + 1); self.print_expr(*scrutinee, depth_lvl + 2); diff --git a/compiler/rustc_passes/src/check_const.rs b/compiler/rustc_passes/src/check_const.rs index e70817d7b7c55..a39f7e7ffccdd 100644 --- a/compiler/rustc_passes/src/check_const.rs +++ b/compiler/rustc_passes/src/check_const.rs @@ -45,7 +45,7 @@ impl NonConstExpr { Self::Loop(ForLoop) | Self::Match(ForLoopDesugar) => &[sym::const_for], - Self::Match(TryDesugar) => &[sym::const_try], + Self::Match(TryDesugar(_)) => &[sym::const_try], // All other expressions are allowed. Self::Loop(Loop | While) | Self::Match(Normal | FormatArgs) => &[], diff --git a/src/tools/clippy/clippy_lints/src/dereference.rs b/src/tools/clippy/clippy_lints/src/dereference.rs index bc011a6c354cb..58c2785500013 100644 --- a/src/tools/clippy/clippy_lints/src/dereference.rs +++ b/src/tools/clippy/clippy_lints/src/dereference.rs @@ -802,7 +802,8 @@ fn in_postfix_position<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> boo match parent.kind { ExprKind::Call(child, _) | ExprKind::MethodCall(_, child, _, _) | ExprKind::Index(child, _, _) if child.hir_id == e.hir_id => true, - ExprKind::Field(_, _) | ExprKind::Match(_, _, MatchSource::TryDesugar | MatchSource::AwaitDesugar) => true, + ExprKind::Match(.., MatchSource::TryDesugar(_) | MatchSource::AwaitDesugar) + | ExprKind::Field(_, _) => true, _ => false, } } else { diff --git a/src/tools/clippy/clippy_lints/src/matches/mod.rs b/src/tools/clippy/clippy_lints/src/matches/mod.rs index 6d16d18875408..930386a60aa02 100644 --- a/src/tools/clippy/clippy_lints/src/matches/mod.rs +++ b/src/tools/clippy/clippy_lints/src/matches/mod.rs @@ -1038,7 +1038,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches { wild_in_or_pats::check(cx, arms); } - if source == MatchSource::TryDesugar { + if let MatchSource::TryDesugar(_) = source { try_err::check(cx, expr, ex); } diff --git a/src/tools/clippy/clippy_lints/src/matches/try_err.rs b/src/tools/clippy/clippy_lints/src/matches/try_err.rs index 99a748489b470..0fd6f533db0d5 100644 --- a/src/tools/clippy/clippy_lints/src/matches/try_err.rs +++ b/src/tools/clippy/clippy_lints/src/matches/try_err.rs @@ -80,7 +80,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, scrutine /// Finds function return type by examining return expressions in match arms. fn find_return_type<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx ExprKind<'_>) -> Option> { - if let ExprKind::Match(_, arms, MatchSource::TryDesugar) = expr { + if let ExprKind::Match(_, arms, MatchSource::TryDesugar(_)) = expr { for arm in *arms { if let ExprKind::Ret(Some(ret)) = arm.body.kind { return Some(cx.typeck_results().expr_ty(ret)); diff --git a/src/tools/clippy/clippy_lints/src/methods/clone_on_copy.rs b/src/tools/clippy/clippy_lints/src/methods/clone_on_copy.rs index 7eb325ee7b5e7..eb4f003d38ae5 100644 --- a/src/tools/clippy/clippy_lints/src/methods/clone_on_copy.rs +++ b/src/tools/clippy/clippy_lints/src/methods/clone_on_copy.rs @@ -64,7 +64,7 @@ pub(super) fn check( ExprKind::Path(QPath::LangItem(rustc_hir::LangItem::TryTraitBranch, _, _)) ), ExprKind::MethodCall(_, self_arg, ..) if expr.hir_id == self_arg.hir_id => true, - ExprKind::Match(_, _, MatchSource::TryDesugar | MatchSource::AwaitDesugar) + ExprKind::Match(_, _, MatchSource::TryDesugar(_) | MatchSource::AwaitDesugar) | ExprKind::Field(..) | ExprKind::Index(..) => true, _ => false, diff --git a/src/tools/clippy/clippy_lints/src/methods/str_splitn.rs b/src/tools/clippy/clippy_lints/src/methods/str_splitn.rs index 41986551da472..7016ad0a80f15 100644 --- a/src/tools/clippy/clippy_lints/src/methods/str_splitn.rs +++ b/src/tools/clippy/clippy_lints/src/methods/str_splitn.rs @@ -236,7 +236,7 @@ fn indirect_usage<'tcx>( !matches!( node, Node::Expr(Expr { - kind: ExprKind::Match(.., MatchSource::TryDesugar), + kind: ExprKind::Match(.., MatchSource::TryDesugar(_)), .. }) ) diff --git a/src/tools/clippy/clippy_lints/src/needless_question_mark.rs b/src/tools/clippy/clippy_lints/src/needless_question_mark.rs index e2a7ba02a043c..7b0f7eaf1f063 100644 --- a/src/tools/clippy/clippy_lints/src/needless_question_mark.rs +++ b/src/tools/clippy/clippy_lints/src/needless_question_mark.rs @@ -122,7 +122,7 @@ fn check(cx: &LateContext<'_>, expr: &Expr<'_>) { } else { return; }; - if let ExprKind::Match(inner_expr_with_q, _, MatchSource::TryDesugar) = &arg.kind; + if let ExprKind::Match(inner_expr_with_q, _, MatchSource::TryDesugar(_)) = &arg.kind; if let ExprKind::Call(called, [inner_expr]) = &inner_expr_with_q.kind; if let ExprKind::Path(QPath::LangItem(LangItem::TryTraitBranch, ..)) = &called.kind; if expr.span.ctxt() == inner_expr.span.ctxt(); diff --git a/src/tools/clippy/clippy_lints/src/question_mark_used.rs b/src/tools/clippy/clippy_lints/src/question_mark_used.rs index ff66b8a009531..d0de33e3c4f14 100644 --- a/src/tools/clippy/clippy_lints/src/question_mark_used.rs +++ b/src/tools/clippy/clippy_lints/src/question_mark_used.rs @@ -34,7 +34,7 @@ declare_lint_pass!(QuestionMarkUsed => [QUESTION_MARK_USED]); impl<'tcx> LateLintPass<'tcx> for QuestionMarkUsed { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if let ExprKind::Match(_, _, MatchSource::TryDesugar) = expr.kind { + if let ExprKind::Match(_, _, MatchSource::TryDesugar(_)) = expr.kind { if !span_is_local(expr.span) { return; } diff --git a/src/tools/clippy/clippy_lints/src/redundant_closure_call.rs b/src/tools/clippy/clippy_lints/src/redundant_closure_call.rs index 4e3efe97b8c61..fc49b58e0a777 100644 --- a/src/tools/clippy/clippy_lints/src/redundant_closure_call.rs +++ b/src/tools/clippy/clippy_lints/src/redundant_closure_call.rs @@ -52,7 +52,7 @@ impl ReturnVisitor { impl<'tcx> Visitor<'tcx> for ReturnVisitor { fn visit_expr(&mut self, ex: &'tcx hir::Expr<'tcx>) { - if let hir::ExprKind::Ret(_) | hir::ExprKind::Match(.., hir::MatchSource::TryDesugar) = ex.kind { + if let hir::ExprKind::Ret(_) | hir::ExprKind::Match(.., hir::MatchSource::TryDesugar(_)) = ex.kind { self.found_return = true; } else { hir_visit::walk_expr(self, ex); diff --git a/src/tools/clippy/clippy_lints/src/returns.rs b/src/tools/clippy/clippy_lints/src/returns.rs index 351bacf5691f3..d6b9a49d2fe05 100644 --- a/src/tools/clippy/clippy_lints/src/returns.rs +++ b/src/tools/clippy/clippy_lints/src/returns.rs @@ -164,7 +164,7 @@ impl<'tcx> LateLintPass<'tcx> for Return { if !in_external_macro(cx.sess(), stmt.span) && let StmtKind::Semi(expr) = stmt.kind && let ExprKind::Ret(Some(ret)) = expr.kind - && let ExprKind::Match(.., MatchSource::TryDesugar) = ret.kind + && let ExprKind::Match(.., MatchSource::TryDesugar(_)) = ret.kind // Ensure this is not the final stmt, otherwise removing it would cause a compile error && let OwnerNode::Item(item) = cx.tcx.hir().owner(cx.tcx.hir().get_parent_item(expr.hir_id)) && let ItemKind::Fn(_, _, body) = item.kind diff --git a/src/tools/clippy/clippy_lints/src/unit_types/unit_arg.rs b/src/tools/clippy/clippy_lints/src/unit_types/unit_arg.rs index dd120599c04e1..462b1aa8153e7 100644 --- a/src/tools/clippy/clippy_lints/src/unit_types/unit_arg.rs +++ b/src/tools/clippy/clippy_lints/src/unit_types/unit_arg.rs @@ -42,7 +42,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { if cx.typeck_results().expr_ty(arg).is_unit() && !utils::is_unit_literal(arg) { !matches!( &arg.kind, - ExprKind::Match(.., MatchSource::TryDesugar) | ExprKind::Path(..) + ExprKind::Match(.., MatchSource::TryDesugar(_)) | ExprKind::Path(..) ) } else { false diff --git a/src/tools/clippy/clippy_lints/src/useless_conversion.rs b/src/tools/clippy/clippy_lints/src/useless_conversion.rs index 92b694d307600..bd4dc07a42bfa 100644 --- a/src/tools/clippy/clippy_lints/src/useless_conversion.rs +++ b/src/tools/clippy/clippy_lints/src/useless_conversion.rs @@ -113,7 +113,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion { } match e.kind { - ExprKind::Match(_, arms, MatchSource::TryDesugar) => { + ExprKind::Match(_, arms, MatchSource::TryDesugar(_)) => { let (ExprKind::Ret(Some(e)) | ExprKind::Break(_, Some(e))) = arms[0].body.kind else { return; }; diff --git a/src/tools/clippy/clippy_utils/src/check_proc_macro.rs b/src/tools/clippy/clippy_utils/src/check_proc_macro.rs index 60fab1ec41aeb..6be8b8bb91696 100644 --- a/src/tools/clippy/clippy_utils/src/check_proc_macro.rs +++ b/src/tools/clippy/clippy_utils/src/check_proc_macro.rs @@ -149,7 +149,7 @@ fn expr_search_pat(tcx: TyCtxt<'_>, e: &Expr<'_>) -> (Pat, Pat) { (Pat::Str("for"), Pat::Str("}")) }, ExprKind::Match(_, _, MatchSource::Normal) => (Pat::Str("match"), Pat::Str("}")), - ExprKind::Match(e, _, MatchSource::TryDesugar) => (expr_search_pat(tcx, e).0, Pat::Str("?")), + ExprKind::Match(e, _, MatchSource::TryDesugar(_)) => (expr_search_pat(tcx, e).0, Pat::Str("?")), ExprKind::Match(e, _, MatchSource::AwaitDesugar) | ExprKind::Yield(e, YieldSource::Await { .. }) => { (expr_search_pat(tcx, e).0, Pat::Str("await")) }, diff --git a/src/tools/clippy/clippy_utils/src/lib.rs b/src/tools/clippy/clippy_utils/src/lib.rs index 171b7faf21960..09576d67b23be 100644 --- a/src/tools/clippy/clippy_utils/src/lib.rs +++ b/src/tools/clippy/clippy_utils/src/lib.rs @@ -1765,7 +1765,7 @@ pub fn is_try<'tcx>(cx: &LateContext<'_>, expr: &'tcx Expr<'tcx>) -> Option<&'tc if let ExprKind::Match(_, arms, ref source) = expr.kind { // desugared from a `?` operator - if *source == MatchSource::TryDesugar { + if let MatchSource::TryDesugar(_) = *source { return Some(expr); } diff --git a/src/tools/clippy/clippy_utils/src/visitors.rs b/src/tools/clippy/clippy_utils/src/visitors.rs index f83988a6e325a..3b47a451345eb 100644 --- a/src/tools/clippy/clippy_utils/src/visitors.rs +++ b/src/tools/clippy/clippy_utils/src/visitors.rs @@ -161,7 +161,7 @@ pub fn for_each_expr_with_closures<'tcx, B, C: Continue>( /// returns `true` if expr contains match expr desugared from try fn contains_try(expr: &hir::Expr<'_>) -> bool { for_each_expr(expr, |e| { - if matches!(e.kind, hir::ExprKind::Match(_, _, hir::MatchSource::TryDesugar)) { + if matches!(e.kind, hir::ExprKind::Match(_, _, hir::MatchSource::TryDesugar(_))) { ControlFlow::Break(()) } else { ControlFlow::Continue(()) From 298ca6732b242a88bc6038d56a0d0b6abac2521a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 14 Aug 2023 19:27:12 +0000 Subject: [PATCH 7/9] review comment: add claryfing comment --- compiler/rustc_hir_typeck/src/_match.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/compiler/rustc_hir_typeck/src/_match.rs b/compiler/rustc_hir_typeck/src/_match.rs index 5d710239ff35d..7ad9f51ba705d 100644 --- a/compiler/rustc_hir_typeck/src/_match.rs +++ b/compiler/rustc_hir_typeck/src/_match.rs @@ -147,6 +147,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } if !arm_ty.is_never() { + // When a match arm has type `!`, then it doesn't influence the expected type for + // the following arm. If all of the prior arms are `!`, then the influence comes + // from elsewhere and we shouldn't point to any previous arm. prior_arm = Some((arm_block_id, arm_ty, arm_span)); } } From 58aa903848028ede4c8ccd584cb67f3780b082e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 14 Aug 2023 22:00:46 +0000 Subject: [PATCH 8/9] bless clippy test --- .../clippy/tests/ui/if_same_then_else2.rs | 2 +- .../clippy/tests/ui/if_same_then_else2.stderr | 21 +------------------ 2 files changed, 2 insertions(+), 21 deletions(-) diff --git a/src/tools/clippy/tests/ui/if_same_then_else2.rs b/src/tools/clippy/tests/ui/if_same_then_else2.rs index 0b171f21d0cc4..c545434efe5b5 100644 --- a/src/tools/clippy/tests/ui/if_same_then_else2.rs +++ b/src/tools/clippy/tests/ui/if_same_then_else2.rs @@ -98,7 +98,7 @@ fn if_same_then_else2() -> Result<&'static str, ()> { }; if true { - //~^ ERROR: this `if` has identical blocks + // FIXME: should emit "this `if` has identical blocks" Ok("foo")?; } else { Ok("foo")?; diff --git a/src/tools/clippy/tests/ui/if_same_then_else2.stderr b/src/tools/clippy/tests/ui/if_same_then_else2.stderr index 56e5f3e45b22c..37fe787d1de3a 100644 --- a/src/tools/clippy/tests/ui/if_same_then_else2.stderr +++ b/src/tools/clippy/tests/ui/if_same_then_else2.stderr @@ -82,25 +82,6 @@ LL | | f32::NAN LL | | }; | |_____^ -error: this `if` has identical blocks - --> $DIR/if_same_then_else2.rs:100:13 - | -LL | if true { - | _____________^ -LL | | -LL | | Ok("foo")?; -LL | | } else { - | |_____^ - | -note: same as this - --> $DIR/if_same_then_else2.rs:103:12 - | -LL | } else { - | ____________^ -LL | | Ok("foo")?; -LL | | } - | |_____^ - error: this `if` has identical blocks --> $DIR/if_same_then_else2.rs:124:20 | @@ -122,5 +103,5 @@ LL | | return Ok(&foo[0..]); LL | | } | |_____^ -error: aborting due to 6 previous errors +error: aborting due to 5 previous errors From dc946649f5a813bc29e76f3879f4fc4f02cc3bab Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 14 Aug 2023 21:47:23 +0000 Subject: [PATCH 9/9] Clean up some bad ui testing annotations --- tests/ui/generic-associated-types/issue-86218.rs | 1 - tests/ui/generic-associated-types/issue-90014-tait.rs | 1 - .../generic-associated-types/issue-90014-tait.stderr | 8 ++++---- tests/ui/internal/internal-unstable.rs | 2 -- tests/ui/internal/internal-unstable.stderr | 10 +++++----- tests/ui/parser/issues/issue-70583-block-is-empty-2.rs | 8 ++++++-- .../parser/issues/issue-70583-block-is-empty-2.stderr | 3 ++- .../where-clauses/where-clause-bounds-inconsistency.rs | 1 - 8 files changed, 17 insertions(+), 17 deletions(-) diff --git a/tests/ui/generic-associated-types/issue-86218.rs b/tests/ui/generic-associated-types/issue-86218.rs index 61cfdd35a8926..397a0f2c64903 100644 --- a/tests/ui/generic-associated-types/issue-86218.rs +++ b/tests/ui/generic-associated-types/issue-86218.rs @@ -17,7 +17,6 @@ trait Yay { impl<'a> Yay<&'a ()> for () { type InnerStream<'s> = impl Stream + 's; - //^ ERROR does not fulfill the required lifetime fn foo<'s>() -> Self::InnerStream<'s> { () } diff --git a/tests/ui/generic-associated-types/issue-90014-tait.rs b/tests/ui/generic-associated-types/issue-90014-tait.rs index bc3a4e1296513..1ce5cd3198767 100644 --- a/tests/ui/generic-associated-types/issue-90014-tait.rs +++ b/tests/ui/generic-associated-types/issue-90014-tait.rs @@ -13,7 +13,6 @@ struct Foo<'a>(&'a mut ()); impl Foo<'_> { type Fut<'a> = impl Future; - //^ ERROR: the type `&mut ()` does not fulfill the required lifetime fn make_fut<'a>(&'a self) -> Self::Fut<'a> { async { () } diff --git a/tests/ui/generic-associated-types/issue-90014-tait.stderr b/tests/ui/generic-associated-types/issue-90014-tait.stderr index 8330a387ecd55..1dec7edce508c 100644 --- a/tests/ui/generic-associated-types/issue-90014-tait.stderr +++ b/tests/ui/generic-associated-types/issue-90014-tait.stderr @@ -1,18 +1,18 @@ error[E0308]: mismatched types - --> $DIR/issue-90014-tait.rs:19:9 + --> $DIR/issue-90014-tait.rs:18:9 | LL | type Fut<'a> = impl Future; | ------------------------ the expected future -... +LL | LL | fn make_fut<'a>(&'a self) -> Self::Fut<'a> { | ------------- expected `Foo<'_>::Fut<'a>` because of return type LL | async { () } | ^^^^^^^^^^^^ expected future, found `async` block | = note: expected opaque type `Foo<'_>::Fut<'a>` - found `async` block `[async block@$DIR/issue-90014-tait.rs:19:9: 19:21]` + found `async` block `[async block@$DIR/issue-90014-tait.rs:18:9: 18:21]` note: this item must have the opaque type in its signature in order to be able to register hidden types - --> $DIR/issue-90014-tait.rs:18:8 + --> $DIR/issue-90014-tait.rs:17:8 | LL | fn make_fut<'a>(&'a self) -> Self::Fut<'a> { | ^^^^^^^^ diff --git a/tests/ui/internal/internal-unstable.rs b/tests/ui/internal/internal-unstable.rs index b8987d3e13c65..1eb27fbdc3a63 100644 --- a/tests/ui/internal/internal-unstable.rs +++ b/tests/ui/internal/internal-unstable.rs @@ -8,7 +8,6 @@ extern crate internal_unstable; struct Baz { #[allow_internal_unstable] - //^ WARN `#[allow_internal_unstable]` is ignored on struct fields and match arms baz: u8, } @@ -50,7 +49,6 @@ fn main() { match true { #[allow_internal_unstable] - //^ WARN `#[allow_internal_unstable]` is ignored on struct fields and match arms _ => {} } } diff --git a/tests/ui/internal/internal-unstable.stderr b/tests/ui/internal/internal-unstable.stderr index f0f9bfb8d234f..b7c47365c2d27 100644 --- a/tests/ui/internal/internal-unstable.stderr +++ b/tests/ui/internal/internal-unstable.stderr @@ -1,5 +1,5 @@ error[E0658]: use of unstable library feature 'function' - --> $DIR/internal-unstable.rs:41:25 + --> $DIR/internal-unstable.rs:40:25 | LL | pass_through_allow!(internal_unstable::unstable()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -7,7 +7,7 @@ LL | pass_through_allow!(internal_unstable::unstable()); = help: add `#![feature(function)]` to the crate attributes to enable error[E0658]: use of unstable library feature 'function' - --> $DIR/internal-unstable.rs:43:27 + --> $DIR/internal-unstable.rs:42:27 | LL | pass_through_noallow!(internal_unstable::unstable()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -15,7 +15,7 @@ LL | pass_through_noallow!(internal_unstable::unstable()); = help: add `#![feature(function)]` to the crate attributes to enable error[E0658]: use of unstable library feature 'function' - --> $DIR/internal-unstable.rs:47:22 + --> $DIR/internal-unstable.rs:46:22 | LL | println!("{:?}", internal_unstable::unstable()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -23,7 +23,7 @@ LL | println!("{:?}", internal_unstable::unstable()); = help: add `#![feature(function)]` to the crate attributes to enable error[E0658]: use of unstable library feature 'function' - --> $DIR/internal-unstable.rs:49:10 + --> $DIR/internal-unstable.rs:48:10 | LL | bar!(internal_unstable::unstable()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -31,7 +31,7 @@ LL | bar!(internal_unstable::unstable()); = help: add `#![feature(function)]` to the crate attributes to enable error[E0658]: use of unstable library feature 'function' - --> $DIR/internal-unstable.rs:19:9 + --> $DIR/internal-unstable.rs:18:9 | LL | internal_unstable::unstable(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/ui/parser/issues/issue-70583-block-is-empty-2.rs b/tests/ui/parser/issues/issue-70583-block-is-empty-2.rs index 80f53338a689e..92ff0ef643e34 100644 --- a/tests/ui/parser/issues/issue-70583-block-is-empty-2.rs +++ b/tests/ui/parser/issues/issue-70583-block-is-empty-2.rs @@ -6,9 +6,13 @@ pub enum ErrorHandled { impl ErrorHandled { pub fn assert_reported(self) { match self { + //~^ NOTE this delimiter might not be properly closed... ErrorHandled::Reported => {}} - //^~ ERROR block is empty, you might have not meant to close it + //~^ NOTE block is empty, you might have not meant to close it + //~| NOTE as it matches this but it has different indentation ErrorHandled::TooGeneric => panic!(), } } -} //~ ERROR unexpected closing delimiter: `}` +} +//~^ ERROR unexpected closing delimiter: `}` +//~| NOTE unexpected closing delimiter diff --git a/tests/ui/parser/issues/issue-70583-block-is-empty-2.stderr b/tests/ui/parser/issues/issue-70583-block-is-empty-2.stderr index 9ae94c701869b..c590e04bb3ded 100644 --- a/tests/ui/parser/issues/issue-70583-block-is-empty-2.stderr +++ b/tests/ui/parser/issues/issue-70583-block-is-empty-2.stderr @@ -1,8 +1,9 @@ error: unexpected closing delimiter: `}` - --> $DIR/issue-70583-block-is-empty-2.rs:14:1 + --> $DIR/issue-70583-block-is-empty-2.rs:16:1 | LL | match self { | - this delimiter might not be properly closed... +LL | LL | ErrorHandled::Reported => {}} | --- ...as it matches this but it has different indentation | | diff --git a/tests/ui/where-clauses/where-clause-bounds-inconsistency.rs b/tests/ui/where-clauses/where-clause-bounds-inconsistency.rs index cf7d06b6179b0..ea60fa708764f 100644 --- a/tests/ui/where-clauses/where-clause-bounds-inconsistency.rs +++ b/tests/ui/where-clauses/where-clause-bounds-inconsistency.rs @@ -14,7 +14,6 @@ trait Trait { impl Trait for bool { fn a(&self, _: T) {} - //^~ This gets rejected but should be accepted fn b(&self, _: T) where T: Bound {} fn c(&self, _: T) {} fn d(&self, _: T) where T: Bound {}