From b4b991e66f0a1fd0e517ed7f038129350a7ad6ce Mon Sep 17 00:00:00 2001 From: surechen Date: Tue, 23 Jul 2024 10:24:45 +0800 Subject: [PATCH 1/4] Suggest adding Result return type for associated method in E0277. For following: ```rust struct A; impl A { fn test4(&self) { let mut _file = File::create("foo.txt")?; //~^ ERROR the `?` operator can only be used in a method } ``` Suggest: ```rust impl A { fn test4(&self) -> Result<(), Box> { let mut _file = File::create("foo.txt")?; //~^ ERROR the `?` operator can only be used in a method Ok(()) } } ``` For #125997 --- .../src/error_reporting/traits/suggestions.rs | 41 +++++++++++++++--- ...turn-from-residual-sugg-issue-125997.fixed | 19 ++++++++ .../return-from-residual-sugg-issue-125997.rs | 15 +++++++ ...urn-from-residual-sugg-issue-125997.stderr | 43 ++++++++++++++++++- 4 files changed, 111 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs index 885216e62165e..9b949323a360a 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs @@ -4612,6 +4612,9 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { }) } + // For E0277 when use `?` operator, suggest adding + // a suitable return type in `FnSig`, and a default + // return value at the end of the function's body. pub(super) fn suggest_add_result_as_return_type( &self, obligation: &PredicateObligation<'tcx>, @@ -4622,19 +4625,47 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { return; } + // Only suggest for local function and associated method, + // because this suggest adding both return type in + // the `FnSig` and a default return value in the body, so it + // is not suitable for foreign function without a local body, + // and neighter for trait method which may be also implemented + // in other place, so shouldn't change it's FnSig. + fn choose_suggest_items<'tcx, 'hir>( + tcx: TyCtxt<'tcx>, + node: hir::Node<'hir>, + ) -> Option<(&'hir hir::FnDecl<'hir>, hir::BodyId)> { + match node { + hir::Node::Item(item) if let hir::ItemKind::Fn(sig, _, body_id) = item.kind => { + Some((sig.decl, body_id)) + } + hir::Node::ImplItem(item) + if let hir::ImplItemKind::Fn(sig, body_id) = item.kind => + { + let parent = tcx.parent_hir_node(item.hir_id()); + if let hir::Node::Item(item) = parent + && let hir::ItemKind::Impl(imp) = item.kind + && imp.of_trait.is_none() + { + return Some((sig.decl, body_id)); + } + None + } + _ => None, + } + } + let node = self.tcx.hir_node_by_def_id(obligation.cause.body_id); - if let hir::Node::Item(item) = node - && let hir::ItemKind::Fn(sig, _, body_id) = item.kind - && let hir::FnRetTy::DefaultReturn(ret_span) = sig.decl.output + if let Some((fn_decl, body_id)) = choose_suggest_items(self.tcx, node) + && let hir::FnRetTy::DefaultReturn(ret_span) = fn_decl.output && self.tcx.is_diagnostic_item(sym::FromResidual, trait_pred.def_id()) && trait_pred.skip_binder().trait_ref.args.type_at(0).is_unit() && let ty::Adt(def, _) = trait_pred.skip_binder().trait_ref.args.type_at(1).kind() && self.tcx.is_diagnostic_item(sym::Result, def.did()) { - let body = self.tcx.hir().body(body_id); let mut sugg_spans = vec![(ret_span, " -> Result<(), Box>".to_string())]; - + let body = self.tcx.hir().body(body_id); if let hir::ExprKind::Block(b, _) = body.value.kind && b.expr.is_none() { diff --git a/tests/ui/return/return-from-residual-sugg-issue-125997.fixed b/tests/ui/return/return-from-residual-sugg-issue-125997.fixed index b2eca69aeb902..a5a133998259b 100644 --- a/tests/ui/return/return-from-residual-sugg-issue-125997.fixed +++ b/tests/ui/return/return-from-residual-sugg-issue-125997.fixed @@ -33,6 +33,25 @@ macro_rules! mac { }; } +struct A; + +impl A { + fn test4(&self) -> Result<(), Box> { + let mut _file = File::create("foo.txt")?; + //~^ ERROR the `?` operator can only be used in a method + + Ok(()) +} + + fn test5(&self) -> Result<(), Box> { + let mut _file = File::create("foo.txt")?; + //~^ ERROR the `?` operator can only be used in a method + println!(); + + Ok(()) +} +} + fn main() -> Result<(), Box> { let mut _file = File::create("foo.txt")?; //~^ ERROR the `?` operator can only be used in a function diff --git a/tests/ui/return/return-from-residual-sugg-issue-125997.rs b/tests/ui/return/return-from-residual-sugg-issue-125997.rs index dd8550a388b86..30ca27eae45ef 100644 --- a/tests/ui/return/return-from-residual-sugg-issue-125997.rs +++ b/tests/ui/return/return-from-residual-sugg-issue-125997.rs @@ -27,6 +27,21 @@ macro_rules! mac { }; } +struct A; + +impl A { + fn test4(&self) { + let mut _file = File::create("foo.txt")?; + //~^ ERROR the `?` operator can only be used in a method + } + + fn test5(&self) { + let mut _file = File::create("foo.txt")?; + //~^ ERROR the `?` operator can only be used in a method + println!(); + } +} + fn main() { let mut _file = File::create("foo.txt")?; //~^ ERROR the `?` operator can only be used in a function diff --git a/tests/ui/return/return-from-residual-sugg-issue-125997.stderr b/tests/ui/return/return-from-residual-sugg-issue-125997.stderr index ef938f0213dfb..a59f38c2ec644 100644 --- a/tests/ui/return/return-from-residual-sugg-issue-125997.stderr +++ b/tests/ui/return/return-from-residual-sugg-issue-125997.stderr @@ -37,8 +37,47 @@ LL + Ok(()) LL + } | +error[E0277]: the `?` operator can only be used in a method that returns `Result` or `Option` (or another type that implements `FromResidual`) + --> $DIR/return-from-residual-sugg-issue-125997.rs:34:48 + | +LL | fn test4(&self) { + | --------------- this function should return `Result` or `Option` to accept `?` +LL | let mut _file = File::create("foo.txt")?; + | ^ cannot use the `?` operator in a method that returns `()` + | + = help: the trait `FromResidual>` is not implemented for `()` +help: consider adding return type + | +LL ~ fn test4(&self) -> Result<(), Box> { +LL | let mut _file = File::create("foo.txt")?; +LL | +LL ~ +LL + Ok(()) +LL + } + | + +error[E0277]: the `?` operator can only be used in a method that returns `Result` or `Option` (or another type that implements `FromResidual`) + --> $DIR/return-from-residual-sugg-issue-125997.rs:39:48 + | +LL | fn test5(&self) { + | --------------- this function should return `Result` or `Option` to accept `?` +LL | let mut _file = File::create("foo.txt")?; + | ^ cannot use the `?` operator in a method that returns `()` + | + = help: the trait `FromResidual>` is not implemented for `()` +help: consider adding return type + | +LL ~ fn test5(&self) -> Result<(), Box> { +LL | let mut _file = File::create("foo.txt")?; +LL | +LL | println!(); +LL ~ +LL + Ok(()) +LL + } + | + error[E0277]: the `?` operator can only be used in a function that returns `Result` or `Option` (or another type that implements `FromResidual`) - --> $DIR/return-from-residual-sugg-issue-125997.rs:31:44 + --> $DIR/return-from-residual-sugg-issue-125997.rs:46:44 | LL | fn main() { | --------- this function should return `Result` or `Option` to accept `?` @@ -81,6 +120,6 @@ LL + Ok(()) LL + } | -error: aborting due to 4 previous errors +error: aborting due to 6 previous errors For more information about this error, try `rustc --explain E0277`. From 7fd62320feddcb3fa85a3ab25bc4c0ac55ddac7c Mon Sep 17 00:00:00 2001 From: Luca Versari Date: Sat, 17 Aug 2024 20:45:45 +0200 Subject: [PATCH 2/4] Fix order of normalization and recursion in const folding. Fixes #126831. Without this patch, type normalization is not always idempotent, which leads to all sorts of bugs in places that assume that normalizing a normalized type does nothing. --- .../src/traits/query/normalize.rs | 8 +++--- .../const-generics/const-ty-is-normalized.rs | 25 +++++++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) create mode 100644 tests/ui/const-generics/const-ty-is-normalized.rs diff --git a/compiler/rustc_trait_selection/src/traits/query/normalize.rs b/compiler/rustc_trait_selection/src/traits/query/normalize.rs index 247b6e4823c4b..cb96db5f7a22e 100644 --- a/compiler/rustc_trait_selection/src/traits/query/normalize.rs +++ b/compiler/rustc_trait_selection/src/traits/query/normalize.rs @@ -333,14 +333,14 @@ impl<'cx, 'tcx> FallibleTypeFolder> for QueryNormalizer<'cx, 'tcx> return Ok(constant); } - let constant = constant.try_super_fold_with(self)?; - debug!(?constant, ?self.param_env); - Ok(crate::traits::with_replaced_escaping_bound_vars( + let constant = crate::traits::with_replaced_escaping_bound_vars( self.infcx, &mut self.universes, constant, |constant| constant.normalize(self.infcx.tcx, self.param_env), - )) + ); + debug!(?constant, ?self.param_env); + constant.try_super_fold_with(self) } #[inline] diff --git a/tests/ui/const-generics/const-ty-is-normalized.rs b/tests/ui/const-generics/const-ty-is-normalized.rs new file mode 100644 index 0000000000000..784145f735ed3 --- /dev/null +++ b/tests/ui/const-generics/const-ty-is-normalized.rs @@ -0,0 +1,25 @@ +//@ compile-flags: -Cdebuginfo=2 --crate-type=lib +//@ build-pass +#![feature(adt_const_params)] + +const N_ISLANDS: usize = 4; + +pub type Matrix = [[usize; N_ISLANDS]; N_ISLANDS]; + +const EMPTY_MATRIX: Matrix = [[0; N_ISLANDS]; N_ISLANDS]; + +const fn to_matrix() -> Matrix { + EMPTY_MATRIX +} + +const BRIDGE_MATRIX: [[usize; N_ISLANDS]; N_ISLANDS] = to_matrix(); + +pub struct Walk { + _p: (), +} + +impl Walk<0, BRIDGE_MATRIX> { + pub const fn new() -> Self { + Self { _p: () } + } +} From 1687c55168f3837506afcd2240a8a0b6eadcc1eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= <39484203+jieyouxu@users.noreply.github.com> Date: Sat, 17 Aug 2024 17:06:26 +0800 Subject: [PATCH 3/4] bootstrap: fix clean's `remove_dir_all` implementation ... by using `std::fs::remove_dir_all`, which handles a bunch of edge cases including read-only files and symlinks which are extremely tricky on Windows. --- src/bootstrap/src/core/build_steps/clean.rs | 93 ++++----------------- 1 file changed, 15 insertions(+), 78 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/clean.rs b/src/bootstrap/src/core/build_steps/clean.rs index f608e5d715e49..bcbe490c36a0f 100644 --- a/src/bootstrap/src/core/build_steps/clean.rs +++ b/src/bootstrap/src/core/build_steps/clean.rs @@ -6,7 +6,6 @@ //! directory unless the `--all` flag is present. use std::fs; -use std::io::{self, ErrorKind}; use std::path::Path; use crate::core::builder::{crate_description, Builder, RunConfig, ShouldRun, Step}; @@ -101,11 +100,11 @@ fn clean(build: &Build, all: bool, stage: Option) { return; } - rm_rf("tmp".as_ref()); + remove_dir_recursive("tmp"); // Clean the entire build directory if all { - rm_rf(&build.out); + remove_dir_recursive(&build.out); return; } @@ -136,17 +135,17 @@ fn clean_specific_stage(build: &Build, stage: u32) { } let path = t!(entry.path().canonicalize()); - rm_rf(&path); + remove_dir_recursive(&path); } } } fn clean_default(build: &Build) { - rm_rf(&build.out.join("tmp")); - rm_rf(&build.out.join("dist")); - rm_rf(&build.out.join("bootstrap").join(".last-warned-change-id")); - rm_rf(&build.out.join("bootstrap-shims-dump")); - rm_rf(&build.out.join("rustfmt.stamp")); + remove_dir_recursive(build.out.join("tmp")); + remove_dir_recursive(build.out.join("dist")); + remove_dir_recursive(build.out.join("bootstrap").join(".last-warned-change-id")); + remove_dir_recursive(build.out.join("bootstrap-shims-dump")); + remove_dir_recursive(build.out.join("rustfmt.stamp")); let mut hosts: Vec<_> = build.hosts.iter().map(|t| build.out.join(t)).collect(); // After cross-compilation, artifacts of the host architecture (which may differ from build.host) @@ -166,78 +165,16 @@ fn clean_default(build: &Build) { continue; } let path = t!(entry.path().canonicalize()); - rm_rf(&path); + remove_dir_recursive(&path); } } } -fn rm_rf(path: &Path) { - match path.symlink_metadata() { - Err(e) => { - if e.kind() == ErrorKind::NotFound { - return; - } - panic!("failed to get metadata for file {}: {}", path.display(), e); - } - Ok(metadata) => { - if metadata.file_type().is_file() || metadata.file_type().is_symlink() { - do_op(path, "remove file", |p| match fs::remove_file(p) { - #[cfg(windows)] - Err(e) - if e.kind() == std::io::ErrorKind::PermissionDenied - && p.file_name().and_then(std::ffi::OsStr::to_str) - == Some("bootstrap.exe") => - { - eprintln!("WARNING: failed to delete '{}'.", p.display()); - Ok(()) - } - r => r, - }); - - return; - } - - for file in t!(fs::read_dir(path)) { - rm_rf(&t!(file).path()); - } - - do_op(path, "remove dir", |p| match fs::remove_dir(p) { - // Check for dir not empty on Windows - // FIXME: Once `ErrorKind::DirectoryNotEmpty` is stabilized, - // match on `e.kind()` instead. - #[cfg(windows)] - Err(e) if e.raw_os_error() == Some(145) => Ok(()), - r => r, - }); - } - }; -} - -fn do_op(path: &Path, desc: &str, mut f: F) -where - F: FnMut(&Path) -> io::Result<()>, -{ - match f(path) { - Ok(()) => {} - // On windows we can't remove a readonly file, and git will often clone files as readonly. - // As a result, we have some special logic to remove readonly files on windows. - // This is also the reason that we can't use things like fs::remove_dir_all(). - #[cfg(windows)] - Err(ref e) if e.kind() == ErrorKind::PermissionDenied => { - let m = t!(path.symlink_metadata()); - let mut p = m.permissions(); - p.set_readonly(false); - t!(fs::set_permissions(path, p)); - f(path).unwrap_or_else(|e| { - // Delete symlinked directories on Windows - if m.file_type().is_symlink() && path.is_dir() && fs::remove_dir(path).is_ok() { - return; - } - panic!("failed to {} {}: {}", desc, path.display(), e); - }); - } - Err(e) => { - panic!("failed to {} {}: {}", desc, path.display(), e); - } +/// Wrapper for [`std::fs::remove_dir_all`] that panics on failure and prints the `path` we failed +/// on. +fn remove_dir_recursive>(path: P) { + let path = path.as_ref(); + if let Err(e) = fs::remove_dir_all(path) { + panic!("failed to `remove_dir_all` at `{}`: {e}", path.display()); } } From b8531e88198543e319b74c2171496fdba359d7a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Kr=C3=BCger?= Date: Sun, 18 Aug 2024 12:50:01 +0200 Subject: [PATCH 4/4] crashes: more tests --- tests/crashes/129150.rs | 7 +++++++ tests/crashes/129166.rs | 7 +++++++ tests/crashes/129205.rs | 5 +++++ tests/crashes/129209.rs | 11 +++++++++++ tests/crashes/129214.rs | 30 ++++++++++++++++++++++++++++++ tests/crashes/129216.rs | 11 +++++++++++ tests/crashes/129219.rs | 26 ++++++++++++++++++++++++++ 7 files changed, 97 insertions(+) create mode 100644 tests/crashes/129150.rs create mode 100644 tests/crashes/129166.rs create mode 100644 tests/crashes/129205.rs create mode 100644 tests/crashes/129209.rs create mode 100644 tests/crashes/129214.rs create mode 100644 tests/crashes/129216.rs create mode 100644 tests/crashes/129219.rs diff --git a/tests/crashes/129150.rs b/tests/crashes/129150.rs new file mode 100644 index 0000000000000..9f8c2ba173939 --- /dev/null +++ b/tests/crashes/129150.rs @@ -0,0 +1,7 @@ +//@ known-bug: rust-lang/rust#129150 +//@ only-x86_64 +use std::arch::x86_64::_mm_blend_ps; + +pub fn main() { + _mm_blend_ps(1, 2, &const {} ); +} diff --git a/tests/crashes/129166.rs b/tests/crashes/129166.rs new file mode 100644 index 0000000000000..d3635d410db5e --- /dev/null +++ b/tests/crashes/129166.rs @@ -0,0 +1,7 @@ +//@ known-bug: rust-lang/rust#129166 + +fn main() { + #[cfg_eval] + #[cfg] + 0 +} diff --git a/tests/crashes/129205.rs b/tests/crashes/129205.rs new file mode 100644 index 0000000000000..f328fca247ac5 --- /dev/null +++ b/tests/crashes/129205.rs @@ -0,0 +1,5 @@ +//@ known-bug: rust-lang/rust#129205 + +fn x() { + T::try_from(); +} diff --git a/tests/crashes/129209.rs b/tests/crashes/129209.rs new file mode 100644 index 0000000000000..249fa41552ebf --- /dev/null +++ b/tests/crashes/129209.rs @@ -0,0 +1,11 @@ +//@ known-bug: rust-lang/rust#129209 + +impl< + const N: usize = { + static || { + Foo([0; X]); + } + }, + > PartialEq for True +{ +} diff --git a/tests/crashes/129214.rs b/tests/crashes/129214.rs new file mode 100644 index 0000000000000..e14b9f379d62a --- /dev/null +++ b/tests/crashes/129214.rs @@ -0,0 +1,30 @@ +//@ known-bug: rust-lang/rust#129214 +//@ compile-flags: -Zvalidate-mir -Copt-level=3 --crate-type=lib + +trait to_str {} + +trait map { + fn map(&self, f: F) -> Vec + where + F: FnMut(&Box) -> U; +} +impl map for Vec { + fn map(&self, mut f: F) -> Vec + where + F: FnMut(&T) -> U, + { + let mut r = Vec::new(); + for i in self { + r.push(f(i)); + } + r + } +} + +fn foo>(x: T) -> Vec { + x.map(|_e| "hi".to_string()) +} + +pub fn main() { + assert_eq!(foo(vec![1]), ["hi".to_string()]); +} diff --git a/tests/crashes/129216.rs b/tests/crashes/129216.rs new file mode 100644 index 0000000000000..f63ed152113e7 --- /dev/null +++ b/tests/crashes/129216.rs @@ -0,0 +1,11 @@ +//@ known-bug: rust-lang/rust#129216 + +trait Mirror { + type Assoc; +} + +struct Foo; + +fn main() { + ::Assoc::new(); +} diff --git a/tests/crashes/129219.rs b/tests/crashes/129219.rs new file mode 100644 index 0000000000000..effbfcd8b8e45 --- /dev/null +++ b/tests/crashes/129219.rs @@ -0,0 +1,26 @@ +//@ known-bug: rust-lang/rust#129219 +//@ compile-flags: -Zmir-opt-level=5 -Zvalidate-mir --edition=2018 + +use core::marker::Unsize; + +pub trait CastTo: Unsize {} + +impl CastTo for U {} + +impl Cast for T {} +pub trait Cast { + fn cast(&self) -> &T + where + Self: CastTo, + { + self + } +} + +pub trait Foo {} +impl Foo for [i32; 0] {} + +fn main() { + let x: &dyn Foo = &[]; + let x = x.cast::<[i32]>(); +}