From bdefcf52559d5dbb6f966cb708ebd01e57ef4d56 Mon Sep 17 00:00:00 2001 From: Qiu Chaofan Date: Wed, 11 Jan 2023 11:27:29 +0800 Subject: [PATCH 01/16] Recognize AIX style archive kind --- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 1 + compiler/rustc_codegen_llvm/src/llvm/mod.rs | 1 + compiler/rustc_codegen_ssa/src/back/archive.rs | 1 + compiler/rustc_llvm/llvm-wrapper/ArchiveWrapper.cpp | 3 +++ 4 files changed, 6 insertions(+) diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 509cb0fef565e..0c7f5d41c7cfc 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -552,6 +552,7 @@ pub enum ArchiveKind { K_BSD, K_DARWIN, K_COFF, + K_AIXBIG, } // LLVMRustThinLTOData diff --git a/compiler/rustc_codegen_llvm/src/llvm/mod.rs b/compiler/rustc_codegen_llvm/src/llvm/mod.rs index f820e7523712c..4f5cc575da6e5 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/mod.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/mod.rs @@ -137,6 +137,7 @@ impl FromStr for ArchiveKind { "bsd" => Ok(ArchiveKind::K_BSD), "darwin" => Ok(ArchiveKind::K_DARWIN), "coff" => Ok(ArchiveKind::K_COFF), + "aix_big" => Ok(ArchiveKind::K_AIXBIG), _ => Err(()), } } diff --git a/compiler/rustc_codegen_ssa/src/back/archive.rs b/compiler/rustc_codegen_ssa/src/back/archive.rs index 66ec8f5f57d21..1c464b3eca497 100644 --- a/compiler/rustc_codegen_ssa/src/back/archive.rs +++ b/compiler/rustc_codegen_ssa/src/back/archive.rs @@ -233,6 +233,7 @@ impl<'a> ArArchiveBuilder<'a> { "bsd" => ArchiveKind::Bsd, "darwin" => ArchiveKind::Darwin, "coff" => ArchiveKind::Coff, + "aix_big" => ArchiveKind::AixBig, kind => { self.sess.emit_fatal(UnknownArchiveKind { kind }); } diff --git a/compiler/rustc_llvm/llvm-wrapper/ArchiveWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/ArchiveWrapper.cpp index 448a1f62f69ed..35d6b9ed7a44f 100644 --- a/compiler/rustc_llvm/llvm-wrapper/ArchiveWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/ArchiveWrapper.cpp @@ -39,6 +39,7 @@ enum class LLVMRustArchiveKind { BSD, DARWIN, COFF, + AIX_BIG, }; static Archive::Kind fromRust(LLVMRustArchiveKind Kind) { @@ -51,6 +52,8 @@ static Archive::Kind fromRust(LLVMRustArchiveKind Kind) { return Archive::K_DARWIN; case LLVMRustArchiveKind::COFF: return Archive::K_COFF; + case LLVMRustArchiveKind::AIX_BIG: + return Archive::K_AIXBIG; default: report_fatal_error("Bad ArchiveKind."); } From ee0513cd264e547b187103c22c12b70dbf1cd47f Mon Sep 17 00:00:00 2001 From: Qiu Chaofan Date: Wed, 1 Feb 2023 12:29:19 +0800 Subject: [PATCH 02/16] Bump version of object and related crates --- Cargo.lock | 69 +++++++++---------- compiler/rustc_codegen_ssa/Cargo.toml | 2 +- library/backtrace | 2 +- library/std/Cargo.toml | 6 +- .../issue-71394-no-from-impl.stderr | 1 + 5 files changed, 39 insertions(+), 41 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 51332919fe755..df9839915f634 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4,12 +4,12 @@ version = 3 [[package]] name = "addr2line" -version = "0.17.0" +version = "0.19.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b9ecd88a8c8378ca913a680cd98f0f13ac67383d35993f86c90a70e3f137816b" +checksum = "a76fd60b23679b7d19bd066031410fb7e458ccc5e958eb5c325888ce4baedc97" dependencies = [ "compiler_builtins", - "gimli", + "gimli 0.27.2", "rustc-std-workspace-alloc", "rustc-std-workspace-core", ] @@ -106,11 +106,11 @@ checksum = "98161a4e3e2184da77bb14f02184cdd111e83bbbcc9979dfee3c44b9a85f5602" [[package]] name = "ar_archive_writer" -version = "0.1.1" +version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "276881980556fdadeb88aa1ffc667e4d2e8fe72531dfabcb7a82bb3c9ea9ba31" +checksum = "c2c584cce02ebcebfa2cf43e413e13dcb339bc17f7146be01fbd8d69d298c49c" dependencies = [ - "object 0.29.0", + "object", ] [[package]] @@ -188,16 +188,16 @@ checksum = "d468802bab17cbc0cc575e9b053f41e72aa36bfa6b7f55e3529ffa43161b97fa" [[package]] name = "backtrace" -version = "0.3.66" +version = "0.3.67" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cab84319d616cfb654d03394f38ab7e6f0919e181b1b57e1fd15e7fb4077d9a7" +checksum = "233d376d6d185f2a3093e58f283f60f880315b6c60075b01f36b3b85154564ca" dependencies = [ "addr2line", "cc", "cfg-if", "libc", "miniz_oxide", - "object 0.29.0", + "object", "rustc-demangle", ] @@ -1519,13 +1519,11 @@ checksum = "37ab347416e802de484e4d03c7316c48f1ecb56574dfd4a46a80f173ce1de04d" [[package]] name = "flate2" -version = "1.0.23" +version = "1.0.25" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b39522e96686d38f4bc984b9198e3a0613264abaebaff2c5c918bfa6b6da09af" +checksum = "a8a2db397cb1c8772f31494cb8917e48cd1e64f0fa7efac59fbd741a0a8ce841" dependencies = [ - "cfg-if", "crc32fast", - "libc", "libz-sys", "miniz_oxide", ] @@ -1775,12 +1773,20 @@ version = "0.26.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "22030e2c5a68ec659fde1e949a745124b48e6fa8b045b7ed5bd1fe4ccc5c4e5d" dependencies = [ - "compiler_builtins", "fallible-iterator", "indexmap", + "stable_deref_trait", +] + +[[package]] +name = "gimli" +version = "0.27.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ad0a93d233ebf96623465aad4046a8d3aa4da22d4f4beba5388838c8a434bbb4" +dependencies = [ + "compiler_builtins", "rustc-std-workspace-alloc", "rustc-std-workspace-core", - "stable_deref_trait", ] [[package]] @@ -2961,9 +2967,9 @@ dependencies = [ [[package]] name = "libz-sys" -version = "1.1.3" +version = "1.1.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "de5435b8549c16d423ed0c03dbaafe57cf6c3344744f1242520d59c9d8ecec66" +checksum = "9702761c3935f8cc2f101793272e202c72b99da8f4224a19ddcf1279a6450bbf" dependencies = [ "cc", "libc", @@ -3223,9 +3229,9 @@ checksum = "68354c5c6bd36d73ff3feceb05efa59b6acb7626617f4962be322a825e61f79a" [[package]] name = "miniz_oxide" -version = "0.5.3" +version = "0.6.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6f5c75688da582b8ffc1f1799e9db273f32133c49e048f614d22ec3256773ccc" +checksum = "b275950c28b37e794e8c55d88aeb5e139d0ce23fdbbeda68f8d7174abdf9e8fa" dependencies = [ "adler", "compiler_builtins", @@ -3361,29 +3367,20 @@ dependencies = [ "libc", ] -[[package]] -name = "object" -version = "0.29.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "21158b2c33aa6d4561f1c0a6ea283ca92bc54802a93b263e910746d679a7eb53" -dependencies = [ - "compiler_builtins", - "memchr", - "rustc-std-workspace-alloc", - "rustc-std-workspace-core", -] - [[package]] name = "object" version = "0.30.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8d864c91689fdc196779b98dba0aceac6118594c2df6ee5d943eb6a8df4d107a" dependencies = [ + "compiler_builtins", "crc32fast", "flate2", "hashbrown 0.13.1", "indexmap", "memchr", + "rustc-std-workspace-alloc", + "rustc-std-workspace-core", ] [[package]] @@ -4411,7 +4408,7 @@ dependencies = [ "cstr", "libc", "measureme", - "object 0.30.1", + "object", "rustc-demangle", "rustc_ast", "rustc_attr", @@ -4447,7 +4444,7 @@ dependencies = [ "itertools", "jobserver", "libc", - "object 0.30.1", + "object", "pathdiff", "regex", "rustc_arena", @@ -5942,7 +5939,7 @@ dependencies = [ "hermit-abi 0.3.0", "libc", "miniz_oxide", - "object 0.29.0", + "object", "panic_abort", "panic_unwind", "profiler_builtins", @@ -6193,9 +6190,9 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "da8fbf660a019b6bf11ea95762041464aa9099cc293b6a66d77cea5107619671" dependencies = [ - "gimli", + "gimli 0.26.2", "hashbrown 0.12.3", - "object 0.30.1", + "object", "tracing", ] diff --git a/compiler/rustc_codegen_ssa/Cargo.toml b/compiler/rustc_codegen_ssa/Cargo.toml index c55991e00d3ae..9a57bdcad62e4 100644 --- a/compiler/rustc_codegen_ssa/Cargo.toml +++ b/compiler/rustc_codegen_ssa/Cargo.toml @@ -7,7 +7,7 @@ edition = "2021" test = false [dependencies] -ar_archive_writer = "0.1.1" +ar_archive_writer = "0.1.2" bitflags = "1.2.1" cc = "1.0.69" itertools = "0.10.1" diff --git a/library/backtrace b/library/backtrace index 07872f28cd8a6..8ad84ca5ad88a 160000 --- a/library/backtrace +++ b/library/backtrace @@ -1 +1 @@ -Subproject commit 07872f28cd8a65c3c7428811548dc85f1f2fb05b +Subproject commit 8ad84ca5ad88ade697637387e7cb4d7c3cf4bde8 diff --git a/library/std/Cargo.toml b/library/std/Cargo.toml index 598a4bf928290..ed9841ebe9687 100644 --- a/library/std/Cargo.toml +++ b/library/std/Cargo.toml @@ -23,11 +23,11 @@ hashbrown = { version = "0.12", default-features = false, features = ['rustc-dep std_detect = { path = "../stdarch/crates/std_detect", default-features = false, features = ['rustc-dep-of-std'] } # Dependencies of the `backtrace` crate -addr2line = { version = "0.17.0", optional = true, default-features = false } +addr2line = { version = "0.19.0", optional = true, default-features = false } rustc-demangle = { version = "0.1.21", features = ['rustc-dep-of-std'] } -miniz_oxide = { version = "0.5.0", optional = true, default-features = false } +miniz_oxide = { version = "0.6.0", optional = true, default-features = false } [dependencies.object] -version = "0.29.0" +version = "0.30.0" optional = true default-features = false features = ['read_core', 'elf', 'macho', 'pe', 'unaligned', 'archive'] diff --git a/tests/ui/suggestions/issue-71394-no-from-impl.stderr b/tests/ui/suggestions/issue-71394-no-from-impl.stderr index a5e6f5b5ffcb0..b1fb5a765da8c 100644 --- a/tests/ui/suggestions/issue-71394-no-from-impl.stderr +++ b/tests/ui/suggestions/issue-71394-no-from-impl.stderr @@ -5,6 +5,7 @@ LL | let _: &[i8] = data.into(); | ^^^^ the trait `From<&[u8]>` is not implemented for `&[i8]` | = help: the following other types implement trait `From`: + <&'input [u8] as From>> <[T; LANES] as From>> <[bool; LANES] as From>> = note: required for `&[u8]` to implement `Into<&[i8]>` From 27e9ee9baeb8149e3f3d56572061c008109cd134 Mon Sep 17 00:00:00 2001 From: Andre Bogus Date: Wed, 15 Mar 2023 16:29:52 +0100 Subject: [PATCH 03/16] move Option::as_slice to intrinsic --- compiler/rustc_hir/src/lang_items.rs | 1 + .../rustc_hir_analysis/src/check/intrinsic.rs | 15 +++ .../src/lower_intrinsics.rs | 30 +++++ compiler/rustc_span/src/symbol.rs | 1 + library/core/src/intrinsics.rs | 6 + library/core/src/option.rs | 108 ++++++------------ ...insics.option_payload.LowerIntrinsics.diff | 54 +++++++++ tests/mir-opt/lower_intrinsics.rs | 9 ++ 8 files changed, 152 insertions(+), 72 deletions(-) create mode 100644 tests/mir-opt/lower_intrinsics.option_payload.LowerIntrinsics.diff diff --git a/compiler/rustc_hir/src/lang_items.rs b/compiler/rustc_hir/src/lang_items.rs index 72ff317d45d6e..0863d65d8f9c8 100644 --- a/compiler/rustc_hir/src/lang_items.rs +++ b/compiler/rustc_hir/src/lang_items.rs @@ -301,6 +301,7 @@ language_item_table! { Context, sym::Context, context, Target::Struct, GenericRequirement::None; FuturePoll, sym::poll, future_poll_fn, Target::Method(MethodKind::Trait { body: false }), GenericRequirement::None; + Option, sym::Option, option_type, Target::Enum, GenericRequirement::None; OptionSome, sym::Some, option_some_variant, Target::Variant, GenericRequirement::None; OptionNone, sym::None, option_none_variant, Target::Variant, GenericRequirement::None; diff --git a/compiler/rustc_hir_analysis/src/check/intrinsic.rs b/compiler/rustc_hir_analysis/src/check/intrinsic.rs index 8c364a4f3b2b8..2f9fc7adb5212 100644 --- a/compiler/rustc_hir_analysis/src/check/intrinsic.rs +++ b/compiler/rustc_hir_analysis/src/check/intrinsic.rs @@ -222,6 +222,21 @@ pub fn check_intrinsic_type(tcx: TyCtxt<'_>, it: &hir::ForeignItem<'_>) { ], tcx.mk_ptr(ty::TypeAndMut { ty: param(0), mutbl: hir::Mutability::Not }), ), + sym::option_payload_ptr => { + let option_def_id = tcx.require_lang_item(hir::LangItem::Option, None); + let p0 = param(0); + ( + 1, + vec![tcx.mk_ptr(ty::TypeAndMut { + ty: tcx.mk_adt( + tcx.adt_def(option_def_id), + tcx.mk_substs_from_iter([ty::GenericArg::from(p0)].into_iter()), + ), + mutbl: hir::Mutability::Not, + })], + tcx.mk_ptr(ty::TypeAndMut { ty: p0, mutbl: hir::Mutability::Not }), + ) + } sym::ptr_mask => ( 1, vec![ diff --git a/compiler/rustc_mir_transform/src/lower_intrinsics.rs b/compiler/rustc_mir_transform/src/lower_intrinsics.rs index 5d7382305ae14..46eab1184bdad 100644 --- a/compiler/rustc_mir_transform/src/lower_intrinsics.rs +++ b/compiler/rustc_mir_transform/src/lower_intrinsics.rs @@ -6,6 +6,7 @@ use rustc_middle::ty::subst::SubstsRef; use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_span::symbol::{sym, Symbol}; use rustc_span::Span; +use rustc_target::abi::VariantIdx; pub struct LowerIntrinsics; @@ -191,6 +192,35 @@ impl<'tcx> MirPass<'tcx> for LowerIntrinsics { terminator.kind = TerminatorKind::Goto { target }; } } + sym::option_payload_ptr => { + if let (Some(target), Some(arg)) = (*target, args[0].place()) { + let ty::RawPtr(ty::TypeAndMut { ty: dest_ty, .. }) = + destination.ty(local_decls, tcx).ty.kind() + else { bug!(); }; + + block.statements.push(Statement { + source_info: terminator.source_info, + kind: StatementKind::Assign(Box::new(( + *destination, + Rvalue::AddressOf( + Mutability::Not, + arg.project_deeper( + &[ + PlaceElem::Deref, + PlaceElem::Downcast( + Some(sym::Some), + VariantIdx::from_u32(1), + ), + PlaceElem::Field(Field::from_u32(0), *dest_ty), + ], + tcx, + ), + ), + ))), + }); + terminator.kind = TerminatorKind::Goto { target }; + } + } _ if intrinsic_name.as_str().starts_with("simd_shuffle") => { validate_simd_shuffle(tcx, args, terminator.source_info.span); } diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 0154c719ef608..139fb48e90e06 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1043,6 +1043,7 @@ symbols! { optin_builtin_traits, option, option_env, + option_payload_ptr, options, or, or_patterns, diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index ee8846675ce25..7482b8b0862e2 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -2214,6 +2214,12 @@ extern "rust-intrinsic" { where G: FnOnce, F: FnOnce; + + #[cfg(not(bootstrap))] + /// This method creates a pointer to any `Some` value. If the argument is + /// `None`, an invalid within-bounds pointer (that is still acceptable for + /// constructing an empty slice) is returned. + pub fn option_payload_ptr(arg: *const Option) -> *const T; } // Some functions are defined here because they accidentally got made diff --git a/library/core/src/option.rs b/library/core/src/option.rs index 0f2475a8bdea6..cba597e66aa13 100644 --- a/library/core/src/option.rs +++ b/library/core/src/option.rs @@ -559,6 +559,7 @@ use crate::{ /// The `Option` type. See [the module level documentation](self) for more. #[derive(Copy, PartialOrd, Eq, Ord, Debug, Hash)] #[rustc_diagnostic_item = "Option"] +#[cfg_attr(not(bootstrap), lang = "Option")] #[stable(feature = "rust1", since = "1.0.0")] pub enum Option { /// No value. @@ -735,48 +736,6 @@ impl Option { } } - /// This is a guess at how many bytes into the option the payload can be found. - /// - /// For niche-optimized types it's correct because it's pigeon-holed to only - /// one possible place. For other types, it's usually correct today, but - /// tweaks to the layout algorithm (particularly expansions of - /// `-Z randomize-layout`) might make it incorrect at any point. - /// - /// It's guaranteed to be a multiple of alignment (so will always give a - /// correctly-aligned location) and to be within the allocated object, so - /// is valid to use with `offset` and to use for a zero-sized read. - /// - /// FIXME: This is a horrible hack, but allows a nice optimization. It should - /// be replaced with `offset_of!` once that works on enum variants. - const SOME_BYTE_OFFSET_GUESS: isize = { - let some_uninit = Some(mem::MaybeUninit::::uninit()); - let payload_ref = some_uninit.as_ref().unwrap(); - // SAFETY: `as_ref` gives an address inside the existing `Option`, - // so both pointers are derived from the same thing and the result - // cannot overflow an `isize`. - let offset = unsafe { <*const _>::byte_offset_from(payload_ref, &some_uninit) }; - - // The offset is into the object, so it's guaranteed to be non-negative. - assert!(offset >= 0); - - // The payload and the overall option are aligned, - // so the offset will be a multiple of the alignment too. - assert!((offset as usize) % mem::align_of::() == 0); - - let max_offset = mem::size_of::() - mem::size_of::(); - if offset as usize <= max_offset { - // There's enough space after this offset for a `T` to exist without - // overflowing the bounds of the object, so let's try it. - offset - } else { - // The offset guess is definitely wrong, so use the address - // of the original option since we have it already. - // This also correctly handles the case of layout-optimized enums - // where `max_offset == 0` and thus this is the only possibility. - 0 - } - }; - /// Returns a slice of the contained value, if any. If this is `None`, an /// empty slice is returned. This can be useful to have a single type of /// iterator over an `Option` or slice. @@ -809,28 +768,29 @@ impl Option { #[must_use] #[unstable(feature = "option_as_slice", issue = "108545")] pub fn as_slice(&self) -> &[T] { - let payload_ptr: *const T = - // The goal here is that both arms here are calculating exactly - // the same pointer, and thus it'll be folded away when the guessed - // offset is correct, but if the guess is wrong for some reason - // it'll at least still be sound, just no longer optimal. - if let Some(payload) = self { - payload - } else { - let self_ptr: *const Self = self; - // SAFETY: `SOME_BYTE_OFFSET_GUESS` guarantees that its value is - // such that this will be in-bounds of the object. - unsafe { self_ptr.byte_offset(Self::SOME_BYTE_OFFSET_GUESS).cast() } - }; - let len = usize::from(self.is_some()); + #[cfg(bootstrap)] + match self { + Some(value) => slice::from_ref(value), + None => &[], + } + #[cfg(not(bootstrap))] // SAFETY: When the `Option` is `Some`, we're using the actual pointer // to the payload, with a length of 1, so this is equivalent to // `slice::from_ref`, and thus is safe. // When the `Option` is `None`, the length used is 0, so to be safe it // just needs to be aligned, which it is because `&self` is aligned and // the offset used is a multiple of alignment. - unsafe { slice::from_raw_parts(payload_ptr, len) } + // + // In the new version, the intrinsic always returns a pointer to an + // in-bounds and correctly aligned position for a `T` (even if in the + // `None` case it's just padding). + unsafe { + slice::from_raw_parts( + crate::intrinsics::option_payload_ptr(crate::ptr::from_ref(self)), + usize::from(self.is_some()), + ) + } } /// Returns a mutable slice of the contained value, if any. If this is @@ -875,28 +835,32 @@ impl Option { #[must_use] #[unstable(feature = "option_as_slice", issue = "108545")] pub fn as_mut_slice(&mut self) -> &mut [T] { - let payload_ptr: *mut T = - // The goal here is that both arms here are calculating exactly - // the same pointer, and thus it'll be folded away when the guessed - // offset is correct, but if the guess is wrong for some reason - // it'll at least still be sound, just no longer optimal. - if let Some(payload) = self { - payload - } else { - let self_ptr: *mut Self = self; - // SAFETY: `SOME_BYTE_OFFSET_GUESS` guarantees that its value is - // such that this will be in-bounds of the object. - unsafe { self_ptr.byte_offset(Self::SOME_BYTE_OFFSET_GUESS).cast() } - }; - let len = usize::from(self.is_some()); + #[cfg(bootstrap)] + match self { + Some(value) => slice::from_mut(value), + None => &mut [], + } + #[cfg(not(bootstrap))] // SAFETY: When the `Option` is `Some`, we're using the actual pointer // to the payload, with a length of 1, so this is equivalent to // `slice::from_mut`, and thus is safe. // When the `Option` is `None`, the length used is 0, so to be safe it // just needs to be aligned, which it is because `&self` is aligned and // the offset used is a multiple of alignment. - unsafe { slice::from_raw_parts_mut(payload_ptr, len) } + // + // In the new version, the intrinsic creates a `*const T` from a + // mutable reference so it is safe to cast back to a mutable pointer + // here. As with `as_slice`, the intrinsic always returns a pointer to + // an in-bounds and correctly aligned position for a `T` (even if in + // the `None` case it's just padding). + unsafe { + slice::from_raw_parts_mut( + crate::intrinsics::option_payload_ptr(crate::ptr::from_mut(self).cast_const()) + .cast_mut(), + usize::from(self.is_some()), + ) + } } ///////////////////////////////////////////////////////////////////////// diff --git a/tests/mir-opt/lower_intrinsics.option_payload.LowerIntrinsics.diff b/tests/mir-opt/lower_intrinsics.option_payload.LowerIntrinsics.diff new file mode 100644 index 0000000000000..e535141e772f8 --- /dev/null +++ b/tests/mir-opt/lower_intrinsics.option_payload.LowerIntrinsics.diff @@ -0,0 +1,54 @@ +- // MIR for `option_payload` before LowerIntrinsics ++ // MIR for `option_payload` after LowerIntrinsics + + fn option_payload(_1: &Option, _2: &Option) -> () { + debug o => _1; // in scope 0 at $DIR/lower_intrinsics.rs:+0:23: +0:24 + debug p => _2; // in scope 0 at $DIR/lower_intrinsics.rs:+0:42: +0:43 + let mut _0: (); // return place in scope 0 at $DIR/lower_intrinsics.rs:+0:62: +0:62 + let mut _4: *const std::option::Option; // in scope 0 at $DIR/lower_intrinsics.rs:+2:55: +2:56 + let mut _6: *const std::option::Option; // in scope 0 at $DIR/lower_intrinsics.rs:+3:55: +3:56 + scope 1 { + let _3: *const usize; // in scope 1 at $DIR/lower_intrinsics.rs:+2:13: +2:15 + scope 2 { + debug _x => _3; // in scope 2 at $DIR/lower_intrinsics.rs:+2:13: +2:15 + let _5: *const std::string::String; // in scope 2 at $DIR/lower_intrinsics.rs:+3:13: +3:15 + scope 3 { + debug _y => _5; // in scope 3 at $DIR/lower_intrinsics.rs:+3:13: +3:15 + } + } + } + + bb0: { + StorageLive(_3); // scope 1 at $DIR/lower_intrinsics.rs:+2:13: +2:15 + StorageLive(_4); // scope 1 at $DIR/lower_intrinsics.rs:+2:55: +2:56 + _4 = &raw const (*_1); // scope 1 at $DIR/lower_intrinsics.rs:+2:55: +2:56 +- _3 = option_payload_ptr::(move _4) -> bb1; // scope 1 at $DIR/lower_intrinsics.rs:+2:18: +2:57 +- // mir::Constant +- // + span: $DIR/lower_intrinsics.rs:99:18: 99:54 +- // + literal: Const { ty: unsafe extern "rust-intrinsic" fn(*const Option) -> *const usize {option_payload_ptr::}, val: Value() } ++ _3 = &raw const (((*_4) as Some).0: usize); // scope 1 at $DIR/lower_intrinsics.rs:+2:18: +2:57 ++ goto -> bb1; // scope 1 at $DIR/lower_intrinsics.rs:+2:18: +2:57 + } + + bb1: { + StorageDead(_4); // scope 1 at $DIR/lower_intrinsics.rs:+2:56: +2:57 + StorageLive(_5); // scope 2 at $DIR/lower_intrinsics.rs:+3:13: +3:15 + StorageLive(_6); // scope 2 at $DIR/lower_intrinsics.rs:+3:55: +3:56 + _6 = &raw const (*_2); // scope 2 at $DIR/lower_intrinsics.rs:+3:55: +3:56 +- _5 = option_payload_ptr::(move _6) -> bb2; // scope 2 at $DIR/lower_intrinsics.rs:+3:18: +3:57 +- // mir::Constant +- // + span: $DIR/lower_intrinsics.rs:100:18: 100:54 +- // + literal: Const { ty: unsafe extern "rust-intrinsic" fn(*const Option) -> *const String {option_payload_ptr::}, val: Value() } ++ _5 = &raw const (((*_6) as Some).0: std::string::String); // scope 2 at $DIR/lower_intrinsics.rs:+3:18: +3:57 ++ goto -> bb2; // scope 2 at $DIR/lower_intrinsics.rs:+3:18: +3:57 + } + + bb2: { + StorageDead(_6); // scope 2 at $DIR/lower_intrinsics.rs:+3:56: +3:57 + _0 = const (); // scope 1 at $DIR/lower_intrinsics.rs:+1:5: +4:6 + StorageDead(_5); // scope 2 at $DIR/lower_intrinsics.rs:+4:5: +4:6 + StorageDead(_3); // scope 1 at $DIR/lower_intrinsics.rs:+4:5: +4:6 + return; // scope 0 at $DIR/lower_intrinsics.rs:+5:2: +5:2 + } + } + diff --git a/tests/mir-opt/lower_intrinsics.rs b/tests/mir-opt/lower_intrinsics.rs index a0a1df4e5ca86..f07e2816f4f41 100644 --- a/tests/mir-opt/lower_intrinsics.rs +++ b/tests/mir-opt/lower_intrinsics.rs @@ -91,3 +91,12 @@ pub fn read_via_copy_uninhabited(r: &Never) -> Never { } pub enum Never {} + +// EMIT_MIR lower_intrinsics.option_payload.LowerIntrinsics.diff +#[cfg(not(bootstrap))] +pub fn option_payload(o: &Option, p: &Option) { + unsafe { + let _x = core::intrinsics::option_payload_ptr(o); + let _y = core::intrinsics::option_payload_ptr(p); + } +} From 2ec7f6c8d54e821303ba5a3da31858dbe009e283 Mon Sep 17 00:00:00 2001 From: ozkanonur Date: Sat, 18 Mar 2023 10:23:30 +0300 Subject: [PATCH 04/16] refactor `fn bootstrap::builder::Builder::compiler_for` - check compiler stage before forcing for stage2. - check if download_rustc is not set before forcing for stage1. Signed-off-by: ozkanonur --- src/bootstrap/builder.rs | 4 ++-- src/bootstrap/lib.rs | 13 +++++++------ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index bb07ca1908e1c..c68452db0aff6 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -919,9 +919,9 @@ impl<'a> Builder<'a> { host: TargetSelection, target: TargetSelection, ) -> Compiler { - if self.build.force_use_stage2() { + if self.build.force_use_stage2(stage) { self.compiler(2, self.config.build) - } else if self.build.force_use_stage1(Compiler { stage, host }, target) { + } else if self.build.force_use_stage1(stage, target) { self.compiler(1, self.config.build) } else { self.compiler(stage, host) diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index 22ddf87221595..e4b4192cf0882 100644 --- a/src/bootstrap/lib.rs +++ b/src/bootstrap/lib.rs @@ -1198,19 +1198,20 @@ impl Build { /// /// When all of these conditions are met the build will lift artifacts from /// the previous stage forward. - fn force_use_stage1(&self, compiler: Compiler, target: TargetSelection) -> bool { + fn force_use_stage1(&self, stage: u32, target: TargetSelection) -> bool { !self.config.full_bootstrap - && compiler.stage >= 2 + && !self.config.download_rustc() + && stage >= 2 && (self.hosts.iter().any(|h| *h == target) || target == self.build) } /// Checks whether the `compiler` compiling for `target` should be forced to /// use a stage2 compiler instead. /// - /// When we download the pre-compiled version of rustc it should be forced to - /// use a stage2 compiler. - fn force_use_stage2(&self) -> bool { - self.config.download_rustc() + /// When we download the pre-compiled version of rustc and compiler stage is >= 2, + /// it should be forced to use a stage2 compiler. + fn force_use_stage2(&self, stage: u32) -> bool { + self.config.download_rustc() && stage >= 2 } /// Given `num` in the form "a.b.c" return a "release string" which From c9ddb73184290e0698060a80b0b5727d6ee11098 Mon Sep 17 00:00:00 2001 From: Ezra Shaw Date: Fri, 17 Mar 2023 21:35:43 +1300 Subject: [PATCH 05/16] refactor: refactor identifier parsing somewhat --- compiler/rustc_parse/messages.ftl | 2 +- compiler/rustc_parse/src/errors.rs | 6 ++--- .../rustc_parse/src/parser/diagnostics.rs | 17 +++++++++--- compiler/rustc_parse/src/parser/item.rs | 2 +- compiler/rustc_parse/src/parser/mod.rs | 27 +++++++++---------- 5 files changed, 31 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_parse/messages.ftl b/compiler/rustc_parse/messages.ftl index c39ada95a4ec4..96765c296e79b 100644 --- a/compiler/rustc_parse/messages.ftl +++ b/compiler/rustc_parse/messages.ftl @@ -336,7 +336,7 @@ parse_expected_identifier_found_reserved_keyword = expected identifier, found re parse_expected_identifier_found_doc_comment = expected identifier, found doc comment parse_expected_identifier = expected identifier -parse_sugg_escape_to_use_as_identifier = escape `{$ident_name}` to use it as an identifier +parse_sugg_escape_identifier = escape `{$ident_name}` to use it as an identifier parse_sugg_remove_comma = remove this comma diff --git a/compiler/rustc_parse/src/errors.rs b/compiler/rustc_parse/src/errors.rs index af0c3026c6605..2def5c5b32f17 100644 --- a/compiler/rustc_parse/src/errors.rs +++ b/compiler/rustc_parse/src/errors.rs @@ -888,12 +888,12 @@ pub(crate) struct InvalidMetaItem { #[derive(Subdiagnostic)] #[suggestion( - parse_sugg_escape_to_use_as_identifier, + parse_sugg_escape_identifier, style = "verbose", applicability = "maybe-incorrect", code = "r#" )] -pub(crate) struct SuggEscapeToUseAsIdentifier { +pub(crate) struct SuggEscapeIdentifier { #[primary_span] pub span: Span, pub ident_name: String, @@ -937,7 +937,7 @@ impl ExpectedIdentifierFound { pub(crate) struct ExpectedIdentifier { pub span: Span, pub token: Token, - pub suggest_raw: Option, + pub suggest_raw: Option, pub suggest_remove_comma: Option, pub help_cannot_start_number: Option, } diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 5b12bcc182222..a9f24ab44ea84 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -6,14 +6,14 @@ use super::{ use crate::errors::{ AmbiguousPlus, AttributeOnParamType, BadQPathStage2, BadTypePlus, BadTypePlusSub, ComparisonOperatorsCannotBeChained, ComparisonOperatorsCannotBeChainedSugg, - ConstGenericWithoutBraces, ConstGenericWithoutBracesSugg, DocCommentOnParamType, - DoubleColonInBound, ExpectedIdentifier, ExpectedSemi, ExpectedSemiSugg, + ConstGenericWithoutBraces, ConstGenericWithoutBracesSugg, DocCommentDoesNotDocumentAnything, + DocCommentOnParamType, DoubleColonInBound, ExpectedIdentifier, ExpectedSemi, ExpectedSemiSugg, GenericParamsWithoutAngleBrackets, GenericParamsWithoutAngleBracketsSugg, HelpIdentifierStartsWithNumber, InInTypo, IncorrectAwait, IncorrectSemicolon, IncorrectUseOfAwait, ParenthesesInForHead, ParenthesesInForHeadSugg, PatternMethodParamWithoutBody, QuestionMarkInType, QuestionMarkInTypeSugg, SelfParamNotFirst, StructLiteralBodyWithoutPath, StructLiteralBodyWithoutPathSugg, StructLiteralNeedingParens, - StructLiteralNeedingParensSugg, SuggEscapeToUseAsIdentifier, SuggRemoveComma, + StructLiteralNeedingParensSugg, SuggEscapeIdentifier, SuggRemoveComma, UnexpectedConstInGenericParam, UnexpectedConstParamDeclaration, UnexpectedConstParamDeclarationSugg, UnmatchedAngleBrackets, UseEqInstead, }; @@ -268,7 +268,16 @@ impl<'a> Parser<'a> { self.sess.source_map().span_to_snippet(span) } + /// Emits an error with suggestions if an identifier was expected but not found. pub(super) fn expected_ident_found(&mut self) -> DiagnosticBuilder<'a, ErrorGuaranteed> { + if let TokenKind::DocComment(..) = self.prev_token.kind { + return DocCommentDoesNotDocumentAnything { + span: self.prev_token.span, + missing_comma: None, + } + .into_diagnostic(&self.sess.span_diagnostic); + } + let valid_follow = &[ TokenKind::Eq, TokenKind::Colon, @@ -286,7 +295,7 @@ impl<'a> Parser<'a> { if ident.is_raw_guess() && self.look_ahead(1, |t| valid_follow.contains(&t.kind)) => { - Some(SuggEscapeToUseAsIdentifier { + Some(SuggEscapeIdentifier { span: ident.span.shrink_to_lo(), // `Symbol::to_string()` is different from `Symbol::into_diagnostic_arg()`, // which uses `Symbol::to_ident_string()` and "helpfully" adds an implicit `r#` diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index 85cc8ca02a977..b7415dc8fb84c 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -1744,7 +1744,7 @@ impl<'a> Parser<'a> { /// Parses a field identifier. Specialized version of `parse_ident_common` /// for better diagnostics and suggestions. fn parse_field_ident(&mut self, adt_ty: &str, lo: Span) -> PResult<'a, Ident> { - let (ident, is_raw) = self.ident_or_err()?; + let (ident, is_raw) = self.ident_or_err(true)?; if !is_raw && ident.is_reserved() { let snapshot = self.create_snapshot_for_diagnostic(); let err = if self.check_fn_front_matter(false, Case::Sensitive) { diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index 3251dd6d0c6fb..6991520895ddd 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -42,8 +42,7 @@ use thin_vec::ThinVec; use tracing::debug; use crate::errors::{ - DocCommentDoesNotDocumentAnything, IncorrectVisibilityRestriction, MismatchedClosingDelimiter, - NonStringAbiLiteral, + IncorrectVisibilityRestriction, MismatchedClosingDelimiter, NonStringAbiLiteral, }; bitflags::bitflags! { @@ -552,19 +551,8 @@ impl<'a> Parser<'a> { self.parse_ident_common(true) } - fn ident_or_err(&mut self) -> PResult<'a, (Ident, /* is_raw */ bool)> { - self.token.ident().ok_or_else(|| match self.prev_token.kind { - TokenKind::DocComment(..) => DocCommentDoesNotDocumentAnything { - span: self.prev_token.span, - missing_comma: None, - } - .into_diagnostic(&self.sess.span_diagnostic), - _ => self.expected_ident_found(), - }) - } - fn parse_ident_common(&mut self, recover: bool) -> PResult<'a, Ident> { - let (ident, is_raw) = self.ident_or_err()?; + let (ident, is_raw) = self.ident_or_err(recover)?; if !is_raw && ident.is_reserved() { let mut err = self.expected_ident_found(); if recover { @@ -577,6 +565,17 @@ impl<'a> Parser<'a> { Ok(ident) } + fn ident_or_err(&mut self, _recover: bool) -> PResult<'a, (Ident, /* is_raw */ bool)> { + let result = self.token.ident().ok_or_else(|| self.expected_ident_found()); + + let (ident, is_raw) = match result { + Ok(ident) => ident, + Err(err) => return Err(err), + }; + + Ok((ident, is_raw)) + } + /// Checks if the next token is `tok`, and returns `true` if so. /// /// This method will automatically add `tok` to `expected_tokens` if `tok` is not From b4e17a5098f0413b01c90c8505e0f01e8bea50de Mon Sep 17 00:00:00 2001 From: Ezra Shaw Date: Fri, 17 Mar 2023 21:41:26 +1300 Subject: [PATCH 06/16] refactor: improve "ident starts with number" error --- compiler/rustc_parse/src/errors.rs | 5 ++- .../rustc_parse/src/parser/diagnostics.rs | 34 +++++++++++++------ compiler/rustc_parse/src/parser/pat.rs | 6 +--- compiler/rustc_span/src/lib.rs | 12 +++++++ .../parser/integer-literal-start-ident.stderr | 6 +++- tests/ui/parser/issues/issue-104088.stderr | 18 ++++++++-- 6 files changed, 61 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_parse/src/errors.rs b/compiler/rustc_parse/src/errors.rs index 2def5c5b32f17..a9d116012ae5b 100644 --- a/compiler/rustc_parse/src/errors.rs +++ b/compiler/rustc_parse/src/errors.rs @@ -986,7 +986,10 @@ impl<'a, G: EmissionGuarantee> IntoDiagnostic<'a, G> for ExpectedIdentifier { #[derive(Subdiagnostic)] #[help(parse_invalid_identifier_with_leading_number)] -pub(crate) struct HelpIdentifierStartsWithNumber; +pub(crate) struct HelpIdentifierStartsWithNumber { + #[primary_span] + pub num_span: Span, +} pub(crate) struct ExpectedSemi { pub span: Span, diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index a9f24ab44ea84..47d1108491530 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -38,7 +38,7 @@ use rustc_errors::{ use rustc_session::errors::ExprParenthesesNeeded; use rustc_span::source_map::Spanned; use rustc_span::symbol::{kw, sym, Ident}; -use rustc_span::{Span, SpanSnippetError, DUMMY_SP}; +use rustc_span::{Span, SpanSnippetError, Symbol, DUMMY_SP}; use std::mem::take; use std::ops::{Deref, DerefMut}; use thin_vec::{thin_vec, ThinVec}; @@ -309,8 +309,11 @@ impl<'a> Parser<'a> { && self.look_ahead(1, |t| t.is_ident())) .then_some(SuggRemoveComma { span: self.token.span }); - let help_cannot_start_number = - self.is_lit_bad_ident().then_some(HelpIdentifierStartsWithNumber); + let help_cannot_start_number = self.is_lit_bad_ident().map(|(len, _valid_portion)| { + let (invalid, _valid) = self.token.span.split_at(len as u32); + + HelpIdentifierStartsWithNumber { num_span: invalid } + }); let err = ExpectedIdentifier { span: self.token.span, @@ -378,13 +381,24 @@ impl<'a> Parser<'a> { /// Checks if the current token is a integer or float literal and looks like /// it could be a invalid identifier with digits at the start. - pub(super) fn is_lit_bad_ident(&mut self) -> bool { - matches!(self.token.uninterpolate().kind, token::Literal(Lit { kind: token::LitKind::Integer | token::LitKind::Float, .. }) - // ensure that the integer literal is followed by a *invalid* - // suffix: this is how we know that it is a identifier with an - // invalid beginning. - if rustc_ast::MetaItemLit::from_token(&self.token).is_none() - ) + /// + /// Returns the number of characters (bytes) composing the invalid portion + /// of the identifier and the valid portion of the identifier. + pub(super) fn is_lit_bad_ident(&mut self) -> Option<(usize, Symbol)> { + // ensure that the integer literal is followed by a *invalid* + // suffix: this is how we know that it is a identifier with an + // invalid beginning. + if let token::Literal(Lit { + kind: token::LitKind::Integer | token::LitKind::Float, + symbol, + suffix, + }) = self.token.uninterpolate().kind + && rustc_ast::MetaItemLit::from_token(&self.token).is_none() + { + Some((symbol.as_str().len(), suffix.unwrap())) + } else { + None + } } pub(super) fn expected_one_of_not_found( diff --git a/compiler/rustc_parse/src/parser/pat.rs b/compiler/rustc_parse/src/parser/pat.rs index fc9f1d1330a72..d9af241584868 100644 --- a/compiler/rustc_parse/src/parser/pat.rs +++ b/compiler/rustc_parse/src/parser/pat.rs @@ -348,10 +348,6 @@ impl<'a> Parser<'a> { lo = self.token.span; } - if self.is_lit_bad_ident() { - return Err(self.expected_ident_found()); - } - let pat = if self.check(&token::BinOp(token::And)) || self.token.kind == token::AndAnd { self.parse_pat_deref(expected)? } else if self.check(&token::OpenDelim(Delimiter::Parenthesis)) { @@ -395,7 +391,7 @@ impl<'a> Parser<'a> { } else { PatKind::Lit(const_expr) } - } else if self.can_be_ident_pat() { + } else if self.can_be_ident_pat() || self.is_lit_bad_ident().is_some() { // Parse `ident @ pat` // This can give false positives and parse nullary enums, // they are dealt with later in resolve. diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs index 873cd33f6a4f2..02cffc762bed3 100644 --- a/compiler/rustc_span/src/lib.rs +++ b/compiler/rustc_span/src/lib.rs @@ -795,6 +795,18 @@ impl Span { }) } + /// Splits a span into two composite spans around a certain position. + pub fn split_at(self, pos: u32) -> (Span, Span) { + let len = self.hi().0 - self.lo().0; + debug_assert!(pos <= len); + + let split_pos = BytePos(self.lo().0 + pos); + ( + Span::new(self.lo(), split_pos, self.ctxt(), self.parent()), + Span::new(split_pos, self.hi(), self.ctxt(), self.parent()), + ) + } + /// Returns a `Span` that would enclose both `self` and `end`. /// /// Note that this can also be used to extend the span "backwards": diff --git a/tests/ui/parser/integer-literal-start-ident.stderr b/tests/ui/parser/integer-literal-start-ident.stderr index 51c37a0d24c3c..b2c6612965608 100644 --- a/tests/ui/parser/integer-literal-start-ident.stderr +++ b/tests/ui/parser/integer-literal-start-ident.stderr @@ -4,7 +4,11 @@ error: expected identifier, found `1main` LL | fn 1main() {} | ^^^^^ expected identifier | - = help: identifiers cannot start with a number +help: identifiers cannot start with a number + --> $DIR/integer-literal-start-ident.rs:1:4 + | +LL | fn 1main() {} + | ^ error: aborting due to previous error diff --git a/tests/ui/parser/issues/issue-104088.stderr b/tests/ui/parser/issues/issue-104088.stderr index 6511a313149f4..08ea1c891c172 100644 --- a/tests/ui/parser/issues/issue-104088.stderr +++ b/tests/ui/parser/issues/issue-104088.stderr @@ -4,7 +4,11 @@ error: expected identifier, found `1x` LL | let 1x = 123; | ^^ expected identifier | - = help: identifiers cannot start with a number +help: identifiers cannot start with a number + --> $DIR/issue-104088.rs:6:9 + | +LL | let 1x = 123; + | ^ error: expected identifier, found `2x` --> $DIR/issue-104088.rs:11:9 @@ -12,7 +16,11 @@ error: expected identifier, found `2x` LL | let 2x: i32 = 123; | ^^ expected identifier | - = help: identifiers cannot start with a number +help: identifiers cannot start with a number + --> $DIR/issue-104088.rs:11:9 + | +LL | let 2x: i32 = 123; + | ^ error: expected identifier, found `23name` --> $DIR/issue-104088.rs:22:9 @@ -20,7 +28,11 @@ error: expected identifier, found `23name` LL | let 23name = 123; | ^^^^^^ expected identifier | - = help: identifiers cannot start with a number +help: identifiers cannot start with a number + --> $DIR/issue-104088.rs:22:9 + | +LL | let 23name = 123; + | ^^ error[E0308]: mismatched types --> $DIR/issue-104088.rs:16:12 From 05b5046633e9f594f955e0365a1219d1a96a5b54 Mon Sep 17 00:00:00 2001 From: Ezra Shaw Date: Fri, 17 Mar 2023 22:27:17 +1300 Subject: [PATCH 07/16] feat: implement error recovery in `expected_ident_found` --- .../rustc_parse/src/parser/diagnostics.rs | 84 +++++++++++++------ compiler/rustc_parse/src/parser/item.rs | 8 +- compiler/rustc_parse/src/parser/mod.rs | 13 ++- compiler/rustc_parse/src/parser/pat.rs | 10 ++- tests/ui/parser/ident-recovery.rs | 16 ++++ tests/ui/parser/ident-recovery.stderr | 42 ++++++++++ tests/ui/parser/issues/issue-104088.rs | 23 ++--- tests/ui/parser/issues/issue-104088.stderr | 48 +++++++---- 8 files changed, 175 insertions(+), 69 deletions(-) create mode 100644 tests/ui/parser/ident-recovery.rs create mode 100644 tests/ui/parser/ident-recovery.stderr diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 47d1108491530..9544afd3d6df9 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -269,13 +269,18 @@ impl<'a> Parser<'a> { } /// Emits an error with suggestions if an identifier was expected but not found. - pub(super) fn expected_ident_found(&mut self) -> DiagnosticBuilder<'a, ErrorGuaranteed> { + /// + /// Returns a possibly recovered identifier. + pub(super) fn expected_ident_found( + &mut self, + recover: bool, + ) -> PResult<'a, (Ident, /* is_raw */ bool)> { if let TokenKind::DocComment(..) = self.prev_token.kind { - return DocCommentDoesNotDocumentAnything { + return Err(DocCommentDoesNotDocumentAnything { span: self.prev_token.span, missing_comma: None, } - .into_diagnostic(&self.sess.span_diagnostic); + .into_diagnostic(&self.sess.span_diagnostic)); } let valid_follow = &[ @@ -290,34 +295,51 @@ impl<'a> Parser<'a> { TokenKind::CloseDelim(Delimiter::Parenthesis), ]; - let suggest_raw = match self.token.ident() { - Some((ident, false)) - if ident.is_raw_guess() - && self.look_ahead(1, |t| valid_follow.contains(&t.kind)) => - { - Some(SuggEscapeIdentifier { - span: ident.span.shrink_to_lo(), - // `Symbol::to_string()` is different from `Symbol::into_diagnostic_arg()`, - // which uses `Symbol::to_ident_string()` and "helpfully" adds an implicit `r#` - ident_name: ident.name.to_string(), - }) - } - _ => None, - }; + let mut recovered_ident = None; + // we take this here so that the correct original token is retained in + // the diagnostic, regardless of eager recovery. + let bad_token = self.token.clone(); + + // suggest prepending a keyword in identifier position with `r#` + let suggest_raw = if let Some((ident, false)) = self.token.ident() + && ident.is_raw_guess() + && self.look_ahead(1, |t| valid_follow.contains(&t.kind)) + { + recovered_ident = Some((ident, true)); + + // `Symbol::to_string()` is different from `Symbol::into_diagnostic_arg()`, + // which uses `Symbol::to_ident_string()` and "helpfully" adds an implicit `r#` + let ident_name = ident.name.to_string(); + + Some(SuggEscapeIdentifier { + span: ident.span.shrink_to_lo(), + ident_name + }) + } else { None }; + + let suggest_remove_comma = + if self.token == token::Comma && self.look_ahead(1, |t| t.is_ident()) { + if recover { + self.bump(); + recovered_ident = self.ident_or_err(false).ok(); + }; + + Some(SuggRemoveComma { span: bad_token.span }) + } else { + None + }; - let suggest_remove_comma = (self.token == token::Comma - && self.look_ahead(1, |t| t.is_ident())) - .then_some(SuggRemoveComma { span: self.token.span }); + let help_cannot_start_number = self.is_lit_bad_ident().map(|(len, valid_portion)| { + let (invalid, valid) = self.token.span.split_at(len as u32); - let help_cannot_start_number = self.is_lit_bad_ident().map(|(len, _valid_portion)| { - let (invalid, _valid) = self.token.span.split_at(len as u32); + recovered_ident = Some((Ident::new(valid_portion, valid), false)); HelpIdentifierStartsWithNumber { num_span: invalid } }); let err = ExpectedIdentifier { - span: self.token.span, - token: self.token.clone(), + span: bad_token.span, + token: bad_token, suggest_raw, suggest_remove_comma, help_cannot_start_number, @@ -326,6 +348,7 @@ impl<'a> Parser<'a> { // if the token we have is a `<` // it *might* be a misplaced generic + // FIXME: could we recover with this? if self.token == token::Lt { // all keywords that could have generic applied let valid_prev_keywords = @@ -376,7 +399,16 @@ impl<'a> Parser<'a> { } } - err + if let Some(recovered_ident) = recovered_ident && recover { + err.emit(); + Ok(recovered_ident) + } else { + Err(err) + } + } + + pub(super) fn expected_ident_found_err(&mut self) -> DiagnosticBuilder<'a, ErrorGuaranteed> { + self.expected_ident_found(false).unwrap_err() } /// Checks if the current token is a integer or float literal and looks like @@ -392,7 +424,7 @@ impl<'a> Parser<'a> { kind: token::LitKind::Integer | token::LitKind::Float, symbol, suffix, - }) = self.token.uninterpolate().kind + }) = self.token.kind && rustc_ast::MetaItemLit::from_token(&self.token).is_none() { Some((symbol.as_str().len(), suffix.unwrap())) diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index b7415dc8fb84c..ae8fe90e9d611 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -1181,7 +1181,7 @@ impl<'a> Parser<'a> { defaultness: Defaultness, ) -> PResult<'a, ItemInfo> { let impl_span = self.token.span; - let mut err = self.expected_ident_found(); + let mut err = self.expected_ident_found_err(); // Only try to recover if this is implementing a trait for a type let mut impl_info = match self.parse_item_impl(attrs, defaultness) { @@ -1776,7 +1776,7 @@ impl<'a> Parser<'a> { Err(err) => { err.cancel(); self.restore_snapshot(snapshot); - self.expected_ident_found() + self.expected_ident_found_err() } } } else if self.eat_keyword(kw::Struct) { @@ -1792,11 +1792,11 @@ impl<'a> Parser<'a> { Err(err) => { err.cancel(); self.restore_snapshot(snapshot); - self.expected_ident_found() + self.expected_ident_found_err() } } } else { - let mut err = self.expected_ident_found(); + let mut err = self.expected_ident_found_err(); if self.eat_keyword_noexpect(kw::Let) && let removal_span = self.prev_token.span.until(self.token.span) && let Ok(ident) = self.parse_ident_common(false) diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index 6991520895ddd..53c25a80c4bf3 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -553,8 +553,9 @@ impl<'a> Parser<'a> { fn parse_ident_common(&mut self, recover: bool) -> PResult<'a, Ident> { let (ident, is_raw) = self.ident_or_err(recover)?; + if !is_raw && ident.is_reserved() { - let mut err = self.expected_ident_found(); + let mut err = self.expected_ident_found_err(); if recover { err.emit(); } else { @@ -565,12 +566,16 @@ impl<'a> Parser<'a> { Ok(ident) } - fn ident_or_err(&mut self, _recover: bool) -> PResult<'a, (Ident, /* is_raw */ bool)> { - let result = self.token.ident().ok_or_else(|| self.expected_ident_found()); + fn ident_or_err(&mut self, recover: bool) -> PResult<'a, (Ident, /* is_raw */ bool)> { + let result = self.token.ident().ok_or_else(|| self.expected_ident_found(recover)); let (ident, is_raw) = match result { Ok(ident) => ident, - Err(err) => return Err(err), + Err(err) => match err { + // we recovered! + Ok(ident) => ident, + Err(err) => return Err(err), + }, }; Ok((ident, is_raw)) diff --git a/compiler/rustc_parse/src/parser/pat.rs b/compiler/rustc_parse/src/parser/pat.rs index d9af241584868..2246002f5d32a 100644 --- a/compiler/rustc_parse/src/parser/pat.rs +++ b/compiler/rustc_parse/src/parser/pat.rs @@ -391,7 +391,13 @@ impl<'a> Parser<'a> { } else { PatKind::Lit(const_expr) } - } else if self.can_be_ident_pat() || self.is_lit_bad_ident().is_some() { + // Don't eagerly error on semantically invalid tokens when matching + // declarative macros, as the input to those doesn't have to be + // semantically valid. For attribute/derive proc macros this is not the + // case, so doing the recovery for them is fine. + } else if self.can_be_ident_pat() + || (self.is_lit_bad_ident().is_some() && self.may_recover()) + { // Parse `ident @ pat` // This can give false positives and parse nullary enums, // they are dealt with later in resolve. @@ -590,7 +596,7 @@ impl<'a> Parser<'a> { // Make sure we don't allow e.g. `let mut $p;` where `$p:pat`. if let token::Interpolated(nt) = &self.token.kind { if let token::NtPat(_) = **nt { - self.expected_ident_found().emit(); + self.expected_ident_found_err().emit(); } } diff --git a/tests/ui/parser/ident-recovery.rs b/tests/ui/parser/ident-recovery.rs new file mode 100644 index 0000000000000..7575372b940fd --- /dev/null +++ b/tests/ui/parser/ident-recovery.rs @@ -0,0 +1,16 @@ +fn ,comma() { + //~^ ERROR expected identifier, found `,` + struct Foo { + x: i32,, + //~^ ERROR expected identifier, found `,` + y: u32, + } +} + +fn break() { +//~^ ERROR expected identifier, found keyword `break` + let continue = 5; + //~^ ERROR expected identifier, found keyword `continue` +} + +fn main() {} diff --git a/tests/ui/parser/ident-recovery.stderr b/tests/ui/parser/ident-recovery.stderr new file mode 100644 index 0000000000000..e9a55026d1245 --- /dev/null +++ b/tests/ui/parser/ident-recovery.stderr @@ -0,0 +1,42 @@ +error: expected identifier, found `,` + --> $DIR/ident-recovery.rs:1:4 + | +LL | fn ,comma() { + | ^ + | | + | expected identifier + | help: remove this comma + +error: expected identifier, found `,` + --> $DIR/ident-recovery.rs:4:16 + | +LL | x: i32,, + | ^ + | | + | expected identifier + | help: remove this comma + +error: expected identifier, found keyword `break` + --> $DIR/ident-recovery.rs:10:4 + | +LL | fn break() { + | ^^^^^ expected identifier, found keyword + | +help: escape `break` to use it as an identifier + | +LL | fn r#break() { + | ++ + +error: expected identifier, found keyword `continue` + --> $DIR/ident-recovery.rs:12:9 + | +LL | let continue = 5; + | ^^^^^^^^ expected identifier, found keyword + | +help: escape `continue` to use it as an identifier + | +LL | let r#continue = 5; + | ++ + +error: aborting due to 4 previous errors + diff --git a/tests/ui/parser/issues/issue-104088.rs b/tests/ui/parser/issues/issue-104088.rs index 86988c8cd21da..3dc636b6a33bd 100644 --- a/tests/ui/parser/issues/issue-104088.rs +++ b/tests/ui/parser/issues/issue-104088.rs @@ -1,26 +1,19 @@ -fn test() { +fn 1234test() { +//~^ ERROR expected identifier, found `1234test` if let 123 = 123 { println!("yes"); } -} - -fn test_2() { - let 1x = 123; - //~^ ERROR expected identifier, found `1x` -} - -fn test_3() { - let 2x: i32 = 123; - //~^ ERROR expected identifier, found `2x` -} -fn test_4() { if let 2e1 = 123 { //~^ ERROR mismatched types } -} -fn test_5() { let 23name = 123; //~^ ERROR expected identifier, found `23name` + + let 2x: i32 = 123; + //~^ ERROR expected identifier, found `2x` + + let 1x = 123; + //~^ ERROR expected identifier, found `1x` } fn main() {} diff --git a/tests/ui/parser/issues/issue-104088.stderr b/tests/ui/parser/issues/issue-104088.stderr index 08ea1c891c172..8b751759d69a6 100644 --- a/tests/ui/parser/issues/issue-104088.stderr +++ b/tests/ui/parser/issues/issue-104088.stderr @@ -1,47 +1,59 @@ -error: expected identifier, found `1x` - --> $DIR/issue-104088.rs:6:9 +error: expected identifier, found `1234test` + --> $DIR/issue-104088.rs:1:4 | -LL | let 1x = 123; - | ^^ expected identifier +LL | fn 1234test() { + | ^^^^^^^^ expected identifier | help: identifiers cannot start with a number - --> $DIR/issue-104088.rs:6:9 + --> $DIR/issue-104088.rs:1:4 | -LL | let 1x = 123; - | ^ +LL | fn 1234test() { + | ^^^^ + +error: expected identifier, found `23name` + --> $DIR/issue-104088.rs:9:9 + | +LL | let 23name = 123; + | ^^^^^^ expected identifier + | +help: identifiers cannot start with a number + --> $DIR/issue-104088.rs:9:9 + | +LL | let 23name = 123; + | ^^ error: expected identifier, found `2x` - --> $DIR/issue-104088.rs:11:9 + --> $DIR/issue-104088.rs:12:9 | LL | let 2x: i32 = 123; | ^^ expected identifier | help: identifiers cannot start with a number - --> $DIR/issue-104088.rs:11:9 + --> $DIR/issue-104088.rs:12:9 | LL | let 2x: i32 = 123; | ^ -error: expected identifier, found `23name` - --> $DIR/issue-104088.rs:22:9 +error: expected identifier, found `1x` + --> $DIR/issue-104088.rs:15:9 | -LL | let 23name = 123; - | ^^^^^^ expected identifier +LL | let 1x = 123; + | ^^ expected identifier | help: identifiers cannot start with a number - --> $DIR/issue-104088.rs:22:9 + --> $DIR/issue-104088.rs:15:9 | -LL | let 23name = 123; - | ^^ +LL | let 1x = 123; + | ^ error[E0308]: mismatched types - --> $DIR/issue-104088.rs:16:12 + --> $DIR/issue-104088.rs:5:12 | LL | if let 2e1 = 123 { | ^^^ --- this expression has type `{integer}` | | | expected integer, found floating-point number -error: aborting due to 4 previous errors +error: aborting due to 5 previous errors For more information about this error, try `rustc --explain E0308`. From e4a4064480c34df9776bcb7c739b17b6b04dc6e7 Mon Sep 17 00:00:00 2001 From: Krasimir Georgiev Date: Mon, 20 Mar 2023 14:39:35 +0000 Subject: [PATCH 08/16] adapt tests/codegen/vec-shrink-panik for LLVM 17 After https://github.com/llvm/llvm-project/commit/0d4a709bb876824a0afa5f86e138e8ffdcaf7661 LLVM now doesn't generate references to panic_cannot_unwind: @nikic: https://rust-lang.zulipchat.com/#narrow/stream/187780-t-compiler.2Fwg-llvm/topic/a.20couple.20codegen.20test.20failures.20after.20llvm.200d4a709bb876824a/near/342664944 >Okay, so LLVM now realizes that double panic is not possible, so that's fine. --- tests/codegen/vec-shrink-panik.rs | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/tests/codegen/vec-shrink-panik.rs b/tests/codegen/vec-shrink-panik.rs index aa6589dc35bb1..b3c3483fea982 100644 --- a/tests/codegen/vec-shrink-panik.rs +++ b/tests/codegen/vec-shrink-panik.rs @@ -1,3 +1,8 @@ +// revisions: old new +// LLVM 17 realizes double panic is not possible and doesn't generate calls +// to panic_cannot_unwind. +// [old]ignore-llvm-version: 17 - 99 +// [new]min-llvm-version: 17 // compile-flags: -O // ignore-debug: the debug assertions get in the way #![crate_type = "lib"] @@ -18,11 +23,11 @@ pub fn shrink_to_fit(vec: &mut Vec) { pub fn issue71861(vec: Vec) -> Box<[u32]> { // CHECK-NOT: panic - // Call to panic_cannot_unwind in case of double-panic is expected, - // but other panics are not. + // Call to panic_cannot_unwind in case of double-panic is expected + // on LLVM 16 and older, but other panics are not. // CHECK: cleanup - // CHECK-NEXT: ; call core::panicking::panic_cannot_unwind - // CHECK-NEXT: panic_cannot_unwind + // old-NEXT: ; call core::panicking::panic_cannot_unwind + // old-NEXT: panic_cannot_unwind // CHECK-NOT: panic vec.into_boxed_slice() @@ -34,14 +39,14 @@ pub fn issue75636<'a>(iter: &[&'a str]) -> Box<[&'a str]> { // CHECK-NOT: panic // Call to panic_cannot_unwind in case of double-panic is expected, - // but other panics are not. + // on LLVM 16 and older, but other panics are not. // CHECK: cleanup - // CHECK-NEXT: ; call core::panicking::panic_cannot_unwind - // CHECK-NEXT: panic_cannot_unwind + // old-NEXT: ; call core::panicking::panic_cannot_unwind + // old-NEXT: panic_cannot_unwind // CHECK-NOT: panic iter.iter().copied().collect() } -// CHECK: ; core::panicking::panic_cannot_unwind -// CHECK: declare void @{{.*}}panic_cannot_unwind +// old: ; core::panicking::panic_cannot_unwind +// old: declare void @{{.*}}panic_cannot_unwind From 12c138531f047cadaef63176f4428dc3a9db44e1 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 20 Mar 2023 20:25:27 +0100 Subject: [PATCH 09/16] Update browser-ui-test version to 0.14.5 --- .../docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version b/src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version index 9c550b2d728c5..6fd113fcfd8aa 100644 --- a/src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version +++ b/src/ci/docker/host-x86_64/x86_64-gnu-tools/browser-ui-test.version @@ -1 +1 @@ -0.14.4 \ No newline at end of file +0.14.5 \ No newline at end of file From ab1573a88730e31fb541a13c9927c3769e850834 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 20 Mar 2023 20:24:54 +0100 Subject: [PATCH 10/16] Add GUI test for "Auto-hide item contents for large items" setting --- ...setting-auto-hide-content-large-items.goml | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 tests/rustdoc-gui/setting-auto-hide-content-large-items.goml diff --git a/tests/rustdoc-gui/setting-auto-hide-content-large-items.goml b/tests/rustdoc-gui/setting-auto-hide-content-large-items.goml new file mode 100644 index 0000000000000..0ebb96d787046 --- /dev/null +++ b/tests/rustdoc-gui/setting-auto-hide-content-large-items.goml @@ -0,0 +1,51 @@ +// This test ensures that the "Auto-hide item contents for large items" setting is working as +// expected. + +// We need to disable this check because `implementors/test_docs/trait.Iterator.js` doesn't exist. +fail-on-request-error: false + +define-function: ( + "check-setting", + (storage_value, setting_attribute_value, toggle_attribute_value), + block { + assert-local-storage: {"rustdoc-auto-hide-large-items": |storage_value|} + click: "#settings-menu" + wait-for: "#settings" + assert-property: ("#auto-hide-large-items", {"checked": |setting_attribute_value|}) + assert-attribute: (".item-decl .type-contents-toggle", {"open": |toggle_attribute_value|}) + } +) + +goto: "file://" + |DOC_PATH| + "/lib2/scroll_traits/trait.Iterator.html" + +// We check that the setting is enabled by default and is working. +call-function: ("check-setting", { + "storage_value": null, + "setting_attribute_value": "true", + "toggle_attribute_value": null, +}) + +// Now we change its value. +click: "#auto-hide-large-items" +assert-local-storage: {"rustdoc-auto-hide-large-items": "false"} + +// We check that the changes were applied as expected. +reload: + +call-function: ("check-setting", { + "storage_value": "false", + "setting_attribute_value": "false", + "toggle_attribute_value": "", +}) + +// And now we re-enable the setting. +click: "#auto-hide-large-items" +assert-local-storage: {"rustdoc-auto-hide-large-items": "true"} + +// And we check everything is back the way it was before. +reload: +call-function: ("check-setting", { + "storage_value": "true", + "setting_attribute_value": "true", + "toggle_attribute_value": null, +}) From 47f24a881bb05293c4d922218d9dfed7e29511cd Mon Sep 17 00:00:00 2001 From: lcnr Date: Tue, 21 Mar 2023 16:26:23 +0100 Subject: [PATCH 11/16] new solver cleanup + coherence --- compiler/rustc_middle/src/traits/solve.rs | 11 ++- .../src/solve/assembly.rs | 69 +++++++++++-------- .../src/solve/eval_ctxt.rs | 9 ++- .../rustc_trait_selection/src/solve/mod.rs | 19 +++-- .../src/solve/project_goals.rs | 6 +- .../src/solve/search_graph/mod.rs | 13 +++- .../src/solve/trait_goals.rs | 27 +++++++- .../rustc_trait_selection/src/traits/mod.rs | 2 +- .../new-solver/coherence/issue-102048.rs | 44 ++++++++++++ .../new-solver/coherence/issue-102048.stderr | 12 ++++ .../coherence-conflict.next.stderr | 11 +++ ...t.stderr => coherence-conflict.old.stderr} | 2 +- .../reservation-impl/coherence-conflict.rs | 3 +- .../{no-use.stderr => no-use.next.stderr} | 2 +- .../traits/reservation-impl/no-use.old.stderr | 13 ++++ tests/ui/traits/reservation-impl/no-use.rs | 3 +- .../traits/reservation-impl/non-lattice-ok.rs | 6 ++ tests/ui/traits/reservation-impl/ok.rs | 3 + 18 files changed, 203 insertions(+), 52 deletions(-) create mode 100644 tests/ui/traits/new-solver/coherence/issue-102048.rs create mode 100644 tests/ui/traits/new-solver/coherence/issue-102048.stderr create mode 100644 tests/ui/traits/reservation-impl/coherence-conflict.next.stderr rename tests/ui/traits/reservation-impl/{coherence-conflict.stderr => coherence-conflict.old.stderr} (91%) rename tests/ui/traits/reservation-impl/{no-use.stderr => no-use.next.stderr} (93%) create mode 100644 tests/ui/traits/reservation-impl/no-use.old.stderr diff --git a/compiler/rustc_middle/src/traits/solve.rs b/compiler/rustc_middle/src/traits/solve.rs index 92d3e73e683cd..f47f4cbdb4db7 100644 --- a/compiler/rustc_middle/src/traits/solve.rs +++ b/compiler/rustc_middle/src/traits/solve.rs @@ -63,14 +63,13 @@ impl Certainty { (Certainty::Yes, Certainty::Yes) => Certainty::Yes, (Certainty::Yes, Certainty::Maybe(_)) => other, (Certainty::Maybe(_), Certainty::Yes) => self, - (Certainty::Maybe(MaybeCause::Overflow), Certainty::Maybe(MaybeCause::Overflow)) => { + (Certainty::Maybe(MaybeCause::Ambiguity), Certainty::Maybe(MaybeCause::Ambiguity)) => { Certainty::Maybe(MaybeCause::Overflow) } - // If at least one of the goals is ambiguous, hide the overflow as the ambiguous goal - // may still result in failure. - (Certainty::Maybe(MaybeCause::Ambiguity), Certainty::Maybe(_)) - | (Certainty::Maybe(_), Certainty::Maybe(MaybeCause::Ambiguity)) => { - Certainty::Maybe(MaybeCause::Ambiguity) + (Certainty::Maybe(MaybeCause::Ambiguity), Certainty::Maybe(MaybeCause::Overflow)) + | (Certainty::Maybe(MaybeCause::Overflow), Certainty::Maybe(MaybeCause::Ambiguity)) + | (Certainty::Maybe(MaybeCause::Overflow), Certainty::Maybe(MaybeCause::Overflow)) => { + Certainty::Maybe(MaybeCause::Overflow) } } } diff --git a/compiler/rustc_trait_selection/src/solve/assembly.rs b/compiler/rustc_trait_selection/src/solve/assembly.rs index 76cde1a669225..8cb09108e83ca 100644 --- a/compiler/rustc_trait_selection/src/solve/assembly.rs +++ b/compiler/rustc_trait_selection/src/solve/assembly.rs @@ -2,7 +2,8 @@ #[cfg(doc)] use super::trait_goals::structural_traits::*; -use super::EvalCtxt; +use super::{EvalCtxt, SolverMode}; +use crate::traits::coherence; use itertools::Itertools; use rustc_hir::def_id::DefId; use rustc_infer::traits::query::NoSolution; @@ -87,6 +88,8 @@ pub(super) enum CandidateSource { pub(super) trait GoalKind<'tcx>: TypeFoldable> + Copy + Eq { fn self_ty(self) -> Ty<'tcx>; + fn trait_ref(self, tcx: TyCtxt<'tcx>) -> ty::TraitRef<'tcx>; + fn with_self_ty(self, tcx: TyCtxt<'tcx>, self_ty: Ty<'tcx>) -> Self; fn trait_def_id(self, tcx: TyCtxt<'tcx>) -> DefId; @@ -244,15 +247,16 @@ impl<'tcx> EvalCtxt<'_, 'tcx> { self.assemble_object_bound_candidates(goal, &mut candidates); + self.assemble_coherence_unknowable_candidates(goal, &mut candidates); + candidates } /// If the self type of a goal is a projection, computing the relevant candidates is difficult. /// /// To deal with this, we first try to normalize the self type and add the candidates for the normalized - /// self type to the list of candidates in case that succeeds. Note that we can't just eagerly return in - /// this case as projections as self types add - // FIXME complete the unfinished sentence above + /// self type to the list of candidates in case that succeeds. We also have to consider candidates with the + /// projection as a self type as well fn assemble_candidates_after_normalizing_self_ty>( &mut self, goal: Goal<'tcx, G>, @@ -468,14 +472,40 @@ impl<'tcx> EvalCtxt<'_, 'tcx> { } } + fn assemble_coherence_unknowable_candidates>( + &mut self, + goal: Goal<'tcx, G>, + candidates: &mut Vec>, + ) { + match self.solver_mode() { + SolverMode::Normal => return, + SolverMode::Coherence => { + let trait_ref = goal.predicate.trait_ref(self.tcx()); + match coherence::trait_ref_is_knowable(self.tcx(), trait_ref) { + Ok(()) => {} + Err(_) => match self + .evaluate_added_goals_and_make_canonical_response(Certainty::AMBIGUOUS) + { + Ok(result) => candidates + .push(Candidate { source: CandidateSource::BuiltinImpl, result }), + // FIXME: This will be reachable at some point if we're in + // `assemble_candidates_after_normalizing_self_ty` and we get a + // universe error. We'll deal with it at this point. + Err(NoSolution) => bug!("coherence candidate resulted in NoSolution"), + }, + } + } + } + } + #[instrument(level = "debug", skip(self), ret)] - pub(super) fn merge_candidates_and_discard_reservation_impls( + pub(super) fn merge_candidates( &mut self, mut candidates: Vec>, ) -> QueryResult<'tcx> { match candidates.len() { 0 => return Err(NoSolution), - 1 => return Ok(self.discard_reservation_impl(candidates.pop().unwrap()).result), + 1 => return Ok(candidates.pop().unwrap().result), _ => {} } @@ -483,10 +513,8 @@ impl<'tcx> EvalCtxt<'_, 'tcx> { let mut i = 0; 'outer: while i < candidates.len() { for j in (0..candidates.len()).filter(|&j| i != j) { - if self.trait_candidate_should_be_dropped_in_favor_of( - &candidates[i], - &candidates[j], - ) { + if self.candidate_should_be_dropped_in_favor_of(&candidates[i], &candidates[j]) + { debug!(candidate = ?candidates[i], "Dropping candidate #{}/{}", i, candidates.len()); candidates.swap_remove(i); continue 'outer; @@ -511,11 +539,10 @@ impl<'tcx> EvalCtxt<'_, 'tcx> { } } - // FIXME: What if there are >1 candidates left with the same response, and one is a reservation impl? - Ok(self.discard_reservation_impl(candidates.pop().unwrap()).result) + Ok(candidates.pop().unwrap().result) } - fn trait_candidate_should_be_dropped_in_favor_of( + fn candidate_should_be_dropped_in_favor_of( &self, candidate: &Candidate<'tcx>, other: &Candidate<'tcx>, @@ -528,20 +555,4 @@ impl<'tcx> EvalCtxt<'_, 'tcx> { | (CandidateSource::BuiltinImpl, _) => false, } } - - fn discard_reservation_impl(&mut self, mut candidate: Candidate<'tcx>) -> Candidate<'tcx> { - if let CandidateSource::Impl(def_id) = candidate.source { - if let ty::ImplPolarity::Reservation = self.tcx().impl_polarity(def_id) { - debug!("Selected reservation impl"); - // We assemble all candidates inside of a probe so by - // making a new canonical response here our result will - // have no constraints. - candidate.result = self - .evaluate_added_goals_and_make_canonical_response(Certainty::AMBIGUOUS) - .unwrap(); - } - } - - candidate - } } diff --git a/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs b/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs index 9541292235795..4b85be69e61bd 100644 --- a/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs +++ b/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs @@ -17,6 +17,7 @@ use rustc_span::DUMMY_SP; use std::ops::ControlFlow; use super::search_graph::{self, OverflowHandler}; +use super::SolverMode; use super::{search_graph::SearchGraph, Goal}; pub struct EvalCtxt<'a, 'tcx> { @@ -78,7 +79,9 @@ impl<'tcx> InferCtxtEvalExt<'tcx> for InferCtxt<'tcx> { &self, goal: Goal<'tcx, ty::Predicate<'tcx>>, ) -> Result<(bool, Certainty), NoSolution> { - let mut search_graph = search_graph::SearchGraph::new(self.tcx); + let mode = if self.intercrate { SolverMode::Coherence } else { SolverMode::Normal }; + + let mut search_graph = search_graph::SearchGraph::new(self.tcx, mode); let mut ecx = EvalCtxt { search_graph: &mut search_graph, @@ -101,6 +104,10 @@ impl<'tcx> InferCtxtEvalExt<'tcx> for InferCtxt<'tcx> { } impl<'a, 'tcx> EvalCtxt<'a, 'tcx> { + pub(super) fn solver_mode(&self) -> SolverMode { + self.search_graph.solver_mode() + } + /// The entry point of the solver. /// /// This function deals with (coinductive) cycles, overflow, and caching diff --git a/compiler/rustc_trait_selection/src/solve/mod.rs b/compiler/rustc_trait_selection/src/solve/mod.rs index 606c2eaa51051..89f4056a58db4 100644 --- a/compiler/rustc_trait_selection/src/solve/mod.rs +++ b/compiler/rustc_trait_selection/src/solve/mod.rs @@ -9,10 +9,6 @@ //! FIXME(@lcnr): Write that section. If you read this before then ask me //! about it on zulip. -// FIXME: Instead of using `infcx.canonicalize_query` we have to add a new routine which -// preserves universes and creates a unique var (in the highest universe) for each -// appearance of a region. - // FIXME: uses of `infcx.at` need to enable deferred projection equality once that's implemented. use rustc_hir::def_id::DefId; @@ -41,6 +37,19 @@ mod trait_goals; pub use eval_ctxt::{EvalCtxt, InferCtxtEvalExt}; pub use fulfill::FulfillmentCtxt; +#[derive(Debug, Clone, Copy)] +enum SolverMode { + /// Ordinary trait solving, using everywhere except for coherence. + Normal, + /// Trait solving during coherence. There are a few notable differences + /// between coherence and ordinary trait solving. + /// + /// Most importantly, trait solving during coherence must not be incomplete, + /// i.e. return `Err(NoSolution)` for goals for which a solution exists. + /// This means that we must not make any guesses or arbitrary choices. + Coherence, +} + trait CanonicalResponseExt { fn has_no_inference_or_external_constraints(&self) -> bool; } @@ -255,7 +264,7 @@ impl<'tcx> EvalCtxt<'_, 'tcx> { return Err(NoSolution); } - // FIXME(-Ztreat-solver=next): We should instead try to find a `Certainty::Yes` response with + // FIXME(-Ztrait-solver=next): We should instead try to find a `Certainty::Yes` response with // a subset of the constraints that all the other responses have. let one = candidates[0]; if candidates[1..].iter().all(|resp| resp == &one) { diff --git a/compiler/rustc_trait_selection/src/solve/project_goals.rs b/compiler/rustc_trait_selection/src/solve/project_goals.rs index 93d77c39f9580..998859966528b 100644 --- a/compiler/rustc_trait_selection/src/solve/project_goals.rs +++ b/compiler/rustc_trait_selection/src/solve/project_goals.rs @@ -34,7 +34,7 @@ impl<'tcx> EvalCtxt<'_, 'tcx> { // projection cache in the solver. if self.term_is_fully_unconstrained(goal) { let candidates = self.assemble_and_evaluate_candidates(goal); - self.merge_candidates_and_discard_reservation_impls(candidates) + self.merge_candidates(candidates) } else { let predicate = goal.predicate; let unconstrained_rhs = self.next_term_infer_of_kind(predicate.term); @@ -56,6 +56,10 @@ impl<'tcx> assembly::GoalKind<'tcx> for ProjectionPredicate<'tcx> { self.self_ty() } + fn trait_ref(self, tcx: TyCtxt<'tcx>) -> ty::TraitRef<'tcx> { + self.projection_ty.trait_ref(tcx) + } + fn with_self_ty(self, tcx: TyCtxt<'tcx>, self_ty: Ty<'tcx>) -> Self { self.with_self_ty(tcx, self_ty) } diff --git a/compiler/rustc_trait_selection/src/solve/search_graph/mod.rs b/compiler/rustc_trait_selection/src/solve/search_graph/mod.rs index 83d77a69c0020..b94c44cbdd038 100644 --- a/compiler/rustc_trait_selection/src/solve/search_graph/mod.rs +++ b/compiler/rustc_trait_selection/src/solve/search_graph/mod.rs @@ -1,8 +1,9 @@ mod cache; mod overflow; +pub(super) use overflow::OverflowHandler; + use self::cache::ProvisionalEntry; -pub(super) use crate::solve::search_graph::overflow::OverflowHandler; use cache::ProvisionalCache; use overflow::OverflowData; use rustc_index::vec::IndexVec; @@ -11,6 +12,8 @@ use rustc_middle::traits::solve::{CanonicalGoal, Certainty, MaybeCause, QueryRes use rustc_middle::ty::TyCtxt; use std::{collections::hash_map::Entry, mem}; +use super::SolverMode; + rustc_index::newtype_index! { pub struct StackDepth {} } @@ -21,6 +24,7 @@ struct StackElem<'tcx> { } pub(super) struct SearchGraph<'tcx> { + mode: SolverMode, /// The stack of goals currently being computed. /// /// An element is *deeper* in the stack if its index is *lower*. @@ -30,14 +34,19 @@ pub(super) struct SearchGraph<'tcx> { } impl<'tcx> SearchGraph<'tcx> { - pub(super) fn new(tcx: TyCtxt<'tcx>) -> SearchGraph<'tcx> { + pub(super) fn new(tcx: TyCtxt<'tcx>, mode: SolverMode) -> SearchGraph<'tcx> { Self { + mode, stack: Default::default(), overflow_data: OverflowData::new(tcx), provisional_cache: ProvisionalCache::empty(), } } + pub(super) fn solver_mode(&self) -> SolverMode { + self.mode + } + pub(super) fn is_empty(&self) -> bool { self.stack.is_empty() && self.provisional_cache.is_empty() } diff --git a/compiler/rustc_trait_selection/src/solve/trait_goals.rs b/compiler/rustc_trait_selection/src/solve/trait_goals.rs index 8ab55c79fc450..2ebdfc8fe72a1 100644 --- a/compiler/rustc_trait_selection/src/solve/trait_goals.rs +++ b/compiler/rustc_trait_selection/src/solve/trait_goals.rs @@ -2,7 +2,7 @@ use std::iter; -use super::{assembly, EvalCtxt}; +use super::{assembly, EvalCtxt, SolverMode}; use rustc_hir::def_id::DefId; use rustc_hir::LangItem; use rustc_infer::traits::query::NoSolution; @@ -20,6 +20,10 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> { self.self_ty() } + fn trait_ref(self, _: TyCtxt<'tcx>) -> ty::TraitRef<'tcx> { + self.trait_ref + } + fn with_self_ty(self, tcx: TyCtxt<'tcx>, self_ty: Ty<'tcx>) -> Self { self.with_self_ty(tcx, self_ty) } @@ -43,6 +47,22 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> { return Err(NoSolution); } + let impl_polarity = tcx.impl_polarity(impl_def_id); + // An upper bound of the certainty of this goal, used to lower the certainty + // of reservation impl to ambiguous during coherence. + let maximal_certainty = match impl_polarity { + ty::ImplPolarity::Positive | ty::ImplPolarity::Negative => { + match impl_polarity == goal.predicate.polarity { + true => Certainty::Yes, + false => return Err(NoSolution), + } + } + ty::ImplPolarity::Reservation => match ecx.solver_mode() { + SolverMode::Normal => return Err(NoSolution), + SolverMode::Coherence => Certainty::AMBIGUOUS, + }, + }; + ecx.probe(|ecx| { let impl_substs = ecx.fresh_substs_for_item(impl_def_id); let impl_trait_ref = impl_trait_ref.subst(tcx, impl_substs); @@ -55,7 +75,8 @@ impl<'tcx> assembly::GoalKind<'tcx> for TraitPredicate<'tcx> { .into_iter() .map(|pred| goal.with(tcx, pred)); ecx.add_goals(where_clause_bounds); - ecx.evaluate_added_goals_and_make_canonical_response(Certainty::Yes) + + ecx.evaluate_added_goals_and_make_canonical_response(maximal_certainty) }) } @@ -547,6 +568,6 @@ impl<'tcx> EvalCtxt<'_, 'tcx> { goal: Goal<'tcx, TraitPredicate<'tcx>>, ) -> QueryResult<'tcx> { let candidates = self.assemble_and_evaluate_candidates(goal); - self.merge_candidates_and_discard_reservation_impls(candidates) + self.merge_candidates(candidates) } } diff --git a/compiler/rustc_trait_selection/src/traits/mod.rs b/compiler/rustc_trait_selection/src/traits/mod.rs index bfeda88a6d40c..2de420b52940a 100644 --- a/compiler/rustc_trait_selection/src/traits/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/mod.rs @@ -4,7 +4,7 @@ pub mod auto_trait; mod chalk_fulfill; -mod coherence; +pub(crate) mod coherence; pub mod const_evaluatable; mod engine; pub mod error_reporting; diff --git a/tests/ui/traits/new-solver/coherence/issue-102048.rs b/tests/ui/traits/new-solver/coherence/issue-102048.rs new file mode 100644 index 0000000000000..11636bfeb5509 --- /dev/null +++ b/tests/ui/traits/new-solver/coherence/issue-102048.rs @@ -0,0 +1,44 @@ +// This must fail coherence. +// +// Getting this to pass was fairly difficult, so here's an explanation +// of what's happening: +// +// Normalizing projections currently tries to replace them with inference variables +// while emitting a nested `Projection` obligation. This cannot be done if the projection +// has bound variables which is the case here. +// +// So the projections stay until after normalization. When unifying two projections we +// currently treat them as if they are injective, so we **incorrectly** unify their +// substs. This means that coherence for the two impls ends up unifying `?T` and `?U` +// as it tries to unify `>::Assoc` with `>::Assoc`. +// +// `impl1` therefore has the projection `>::Assoc` and we have the +// assumption `?T: for<'a> WithAssoc2<'a, Assoc = i32>` in the `param_env`, so we normalize +// that to `i32`. We then try to unify `i32` from `impl1` with `u32` from `impl2` which fails, +// causing coherence to consider these two impls distinct. + +// compile-flags: -Ztrait-solver=next +pub trait Trait {} + +pub trait WithAssoc1<'a> { + type Assoc; +} +pub trait WithAssoc2<'a> { + type Assoc; +} + +// impl 1 +impl Trait fn(>::Assoc, >::Assoc)> for (T, U) +where + T: for<'a> WithAssoc1<'a> + for<'a> WithAssoc2<'a, Assoc = i32>, + U: for<'a> WithAssoc2<'a>, +{ +} + +// impl 2 +impl Trait fn(>::Assoc, u32)> for (T, U) where + U: for<'a> WithAssoc1<'a> //~^ ERROR conflicting implementations of trait +{ +} + +fn main() {} diff --git a/tests/ui/traits/new-solver/coherence/issue-102048.stderr b/tests/ui/traits/new-solver/coherence/issue-102048.stderr new file mode 100644 index 0000000000000..17a43838fe275 --- /dev/null +++ b/tests/ui/traits/new-solver/coherence/issue-102048.stderr @@ -0,0 +1,12 @@ +error[E0119]: conflicting implementations of trait `Trait fn(<_ as WithAssoc1<'a>>::Assoc, <_ as WithAssoc2<'a>>::Assoc)>` for type `(_, _)` + --> $DIR/issue-102048.rs:39:1 + | +LL | impl Trait fn(>::Assoc, >::Assoc)> for (T, U) + | --------------------------------------------------------------------------------------------------- first implementation here +... +LL | impl Trait fn(>::Assoc, u32)> for (T, U) where + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `(_, _)` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0119`. diff --git a/tests/ui/traits/reservation-impl/coherence-conflict.next.stderr b/tests/ui/traits/reservation-impl/coherence-conflict.next.stderr new file mode 100644 index 0000000000000..e5a3c3f5cc4b7 --- /dev/null +++ b/tests/ui/traits/reservation-impl/coherence-conflict.next.stderr @@ -0,0 +1,11 @@ +error[E0119]: conflicting implementations of trait `OtherTrait` for type `()` + --> $DIR/coherence-conflict.rs:12:1 + | +LL | impl OtherTrait for () {} + | ---------------------- first implementation here +LL | impl OtherTrait for T {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `()` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0119`. diff --git a/tests/ui/traits/reservation-impl/coherence-conflict.stderr b/tests/ui/traits/reservation-impl/coherence-conflict.old.stderr similarity index 91% rename from tests/ui/traits/reservation-impl/coherence-conflict.stderr rename to tests/ui/traits/reservation-impl/coherence-conflict.old.stderr index a811d7e32016b..393350ea3f12a 100644 --- a/tests/ui/traits/reservation-impl/coherence-conflict.stderr +++ b/tests/ui/traits/reservation-impl/coherence-conflict.old.stderr @@ -1,5 +1,5 @@ error[E0119]: conflicting implementations of trait `OtherTrait` for type `()` - --> $DIR/coherence-conflict.rs:11:1 + --> $DIR/coherence-conflict.rs:12:1 | LL | impl OtherTrait for () {} | ---------------------- first implementation here diff --git a/tests/ui/traits/reservation-impl/coherence-conflict.rs b/tests/ui/traits/reservation-impl/coherence-conflict.rs index fa4a309315b47..6bbd90f94dc39 100644 --- a/tests/ui/traits/reservation-impl/coherence-conflict.rs +++ b/tests/ui/traits/reservation-impl/coherence-conflict.rs @@ -1,5 +1,6 @@ // check that reservation impls are accounted for in negative reasoning. - +// revisions: old next +//[next] compile-flags: -Ztrait-solver=next #![feature(rustc_attrs)] trait MyTrait {} diff --git a/tests/ui/traits/reservation-impl/no-use.stderr b/tests/ui/traits/reservation-impl/no-use.next.stderr similarity index 93% rename from tests/ui/traits/reservation-impl/no-use.stderr rename to tests/ui/traits/reservation-impl/no-use.next.stderr index cefb2a8792f17..542e3a28adf28 100644 --- a/tests/ui/traits/reservation-impl/no-use.stderr +++ b/tests/ui/traits/reservation-impl/no-use.next.stderr @@ -1,5 +1,5 @@ error[E0277]: the trait bound `(): MyTrait` is not satisfied - --> $DIR/no-use.rs:10:26 + --> $DIR/no-use.rs:11:26 | LL | <() as MyTrait>::foo(&()); | -------------------- ^^^ the trait `MyTrait` is not implemented for `()` diff --git a/tests/ui/traits/reservation-impl/no-use.old.stderr b/tests/ui/traits/reservation-impl/no-use.old.stderr new file mode 100644 index 0000000000000..542e3a28adf28 --- /dev/null +++ b/tests/ui/traits/reservation-impl/no-use.old.stderr @@ -0,0 +1,13 @@ +error[E0277]: the trait bound `(): MyTrait` is not satisfied + --> $DIR/no-use.rs:11:26 + | +LL | <() as MyTrait>::foo(&()); + | -------------------- ^^^ the trait `MyTrait` is not implemented for `()` + | | + | required by a bound introduced by this call + | + = help: the trait `MyTrait` is implemented for `()` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0277`. diff --git a/tests/ui/traits/reservation-impl/no-use.rs b/tests/ui/traits/reservation-impl/no-use.rs index 65a55d9e20936..864f1791fd0a7 100644 --- a/tests/ui/traits/reservation-impl/no-use.rs +++ b/tests/ui/traits/reservation-impl/no-use.rs @@ -1,5 +1,6 @@ // check that reservation impls can't be used as normal impls in positive reasoning. - +// revisions: old next +//[next] compile-flags: -Ztrait-solver=next #![feature(rustc_attrs)] trait MyTrait { fn foo(&self); } diff --git a/tests/ui/traits/reservation-impl/non-lattice-ok.rs b/tests/ui/traits/reservation-impl/non-lattice-ok.rs index a71051243c893..7787904d9b22d 100644 --- a/tests/ui/traits/reservation-impl/non-lattice-ok.rs +++ b/tests/ui/traits/reservation-impl/non-lattice-ok.rs @@ -30,6 +30,12 @@ // // [ii]: https://smallcultfollowing.com/babysteps/blog/2016/09/24/intersection-impls/ + +// check that reservation impls can't be used as normal impls in positive reasoning. + +// revisions: old next +//[next] compile-flags: -Ztrait-solver=next + #![feature(rustc_attrs, never_type)] trait MyTrait {} diff --git a/tests/ui/traits/reservation-impl/ok.rs b/tests/ui/traits/reservation-impl/ok.rs index 611c8d8841323..8ff6645a2b3d3 100644 --- a/tests/ui/traits/reservation-impl/ok.rs +++ b/tests/ui/traits/reservation-impl/ok.rs @@ -3,6 +3,9 @@ // rpass test for reservation impls. Not 100% required because `From` uses them, // but still. +// revisions: old next +//[next] compile-flags: -Ztrait-solver=next + #![feature(rustc_attrs)] use std::mem; From 938434ab82609c8de83028fa80bde4d6d28c282a Mon Sep 17 00:00:00 2001 From: lcnr Date: Tue, 21 Mar 2023 16:34:04 +0100 Subject: [PATCH 12/16] enable `intercrate` in the solver `InferCtxt` --- compiler/rustc_infer/src/infer/mod.rs | 4 ++-- .../rustc_trait_selection/src/solve/eval_ctxt.rs | 10 ++++++++-- .../rustc_trait_selection/src/traits/coherence.rs | 14 ++++++++++---- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index aeb4ddb421259..ed5fd590934f3 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -585,8 +585,8 @@ impl<'tcx> InferCtxtBuilder<'tcx> { self } - pub fn intercrate(mut self) -> Self { - self.intercrate = true; + pub fn intercrate(mut self, intercrate: bool) -> Self { + self.intercrate = intercrate; self } diff --git a/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs b/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs index 4b85be69e61bd..c492c8c0aea05 100644 --- a/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs +++ b/compiler/rustc_trait_selection/src/solve/eval_ctxt.rs @@ -127,8 +127,14 @@ impl<'a, 'tcx> EvalCtxt<'a, 'tcx> { // // The actual solver logic happens in `ecx.compute_goal`. search_graph.with_new_goal(tcx, canonical_goal, |search_graph| { - let (ref infcx, goal, var_values) = - tcx.infer_ctxt().build_with_canonical(DUMMY_SP, &canonical_goal); + let intercrate = match search_graph.solver_mode() { + SolverMode::Normal => false, + SolverMode::Coherence => true, + }; + let (ref infcx, goal, var_values) = tcx + .infer_ctxt() + .intercrate(intercrate) + .build_with_canonical(DUMMY_SP, &canonical_goal); let mut ecx = EvalCtxt { infcx, var_values, diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index f4cfe4ec0b0ce..5bb0cdda1af0f 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -95,8 +95,11 @@ pub fn overlapping_impls( return None; } - let infcx = - tcx.infer_ctxt().with_opaque_type_inference(DefiningAnchor::Bubble).intercrate().build(); + let infcx = tcx + .infer_ctxt() + .with_opaque_type_inference(DefiningAnchor::Bubble) + .intercrate(true) + .build(); let selcx = &mut SelectionContext::new(&infcx); let overlaps = overlap(selcx, skip_leak_check, impl1_def_id, impl2_def_id, overlap_mode).is_some(); @@ -107,8 +110,11 @@ pub fn overlapping_impls( // In the case where we detect an error, run the check again, but // this time tracking intercrate ambiguity causes for better // diagnostics. (These take time and can lead to false errors.) - let infcx = - tcx.infer_ctxt().with_opaque_type_inference(DefiningAnchor::Bubble).intercrate().build(); + let infcx = tcx + .infer_ctxt() + .with_opaque_type_inference(DefiningAnchor::Bubble) + .intercrate(true) + .build(); let selcx = &mut SelectionContext::new(&infcx); selcx.enable_tracking_intercrate_ambiguity_causes(); Some(overlap(selcx, skip_leak_check, impl1_def_id, impl2_def_id, overlap_mode).unwrap()) From a7ec045be8133fe679c34a7eaba989ca6975d53e Mon Sep 17 00:00:00 2001 From: lcnr Date: Tue, 21 Mar 2023 16:38:40 +0100 Subject: [PATCH 13/16] disable global caching during coherence --- compiler/rustc_trait_selection/src/solve/search_graph/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_trait_selection/src/solve/search_graph/mod.rs b/compiler/rustc_trait_selection/src/solve/search_graph/mod.rs index b94c44cbdd038..219890b9dc42c 100644 --- a/compiler/rustc_trait_selection/src/solve/search_graph/mod.rs +++ b/compiler/rustc_trait_selection/src/solve/search_graph/mod.rs @@ -254,7 +254,8 @@ impl<'tcx> SearchGraph<'tcx> { // dependencies, our non-root goal may no longer appear as child of the root goal. // // See https://github.com/rust-lang/rust/pull/108071 for some additional context. - let should_cache_globally = !self.overflow_data.did_overflow() || self.stack.is_empty(); + let should_cache_globally = matches!(self.solver_mode(), SolverMode::Normal) + && (!self.overflow_data.did_overflow() || self.stack.is_empty()); if should_cache_globally { tcx.new_solver_evaluation_cache.insert( current_goal.goal, From f86b0358f81aa67ff5ca8abf90935226fcab4f40 Mon Sep 17 00:00:00 2001 From: lcnr Date: Tue, 21 Mar 2023 16:39:24 +0100 Subject: [PATCH 14/16] woops --- compiler/rustc_middle/src/traits/solve.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_middle/src/traits/solve.rs b/compiler/rustc_middle/src/traits/solve.rs index f47f4cbdb4db7..512d67f34b97d 100644 --- a/compiler/rustc_middle/src/traits/solve.rs +++ b/compiler/rustc_middle/src/traits/solve.rs @@ -64,7 +64,7 @@ impl Certainty { (Certainty::Yes, Certainty::Maybe(_)) => other, (Certainty::Maybe(_), Certainty::Yes) => self, (Certainty::Maybe(MaybeCause::Ambiguity), Certainty::Maybe(MaybeCause::Ambiguity)) => { - Certainty::Maybe(MaybeCause::Overflow) + Certainty::Maybe(MaybeCause::Ambiguity) } (Certainty::Maybe(MaybeCause::Ambiguity), Certainty::Maybe(MaybeCause::Overflow)) | (Certainty::Maybe(MaybeCause::Overflow), Certainty::Maybe(MaybeCause::Ambiguity)) From 75718081ee140ad9b18281cc5319a9436101b9da Mon Sep 17 00:00:00 2001 From: James Farrell Date: Tue, 21 Mar 2023 17:12:02 +0000 Subject: [PATCH 15/16] Ignore the vendor directory for tidy tests. When running `x.py test` on a downloaded source distribution (e.g. https://static.rust-lang.org/dist/rustc--src.tar.gz), the crates in the vendor directory contain a number of executable files that cause the tidy test to fail with the following message: tidy error: binary checked into source: I see 26 such errors with the 1.68.0 source distribution. A few of these are .rs source files with incorrect executable permission, but most are scripts that are correctly marked executable. --- src/tools/tidy/src/walk.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tools/tidy/src/walk.rs b/src/tools/tidy/src/walk.rs index 2ade22c209f5a..67a4df19fcc7a 100644 --- a/src/tools/tidy/src/walk.rs +++ b/src/tools/tidy/src/walk.rs @@ -29,6 +29,7 @@ pub fn filter_dirs(path: &Path) -> bool { // Filter RLS output directories "target/rls", "src/bootstrap/target", + "vendor", ]; skip.iter().any(|p| path.ends_with(p)) } From 293f21c876278df7943eea889dacd977155a2f62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Tue, 21 Mar 2023 01:09:00 +0100 Subject: [PATCH 16/16] iat selection: erase regions in self type --- .../rustc_hir_analysis/src/astconv/mod.rs | 82 ++++++++++++------- .../issue-109299-1.rs | 12 +++ .../issue-109299-1.stderr | 15 ++++ .../associated-inherent-types/issue-109299.rs | 12 +++ .../issue-109299.stderr | 11 +++ 5 files changed, 101 insertions(+), 31 deletions(-) create mode 100644 tests/ui/associated-inherent-types/issue-109299-1.rs create mode 100644 tests/ui/associated-inherent-types/issue-109299-1.stderr create mode 100644 tests/ui/associated-inherent-types/issue-109299.rs create mode 100644 tests/ui/associated-inherent-types/issue-109299.stderr diff --git a/compiler/rustc_hir_analysis/src/astconv/mod.rs b/compiler/rustc_hir_analysis/src/astconv/mod.rs index 6a27383121d2d..9d3f8c2b3e74f 100644 --- a/compiler/rustc_hir_analysis/src/astconv/mod.rs +++ b/compiler/rustc_hir_analysis/src/astconv/mod.rs @@ -31,6 +31,7 @@ use rustc_infer::infer::{InferCtxt, TyCtxtInferExt}; use rustc_infer::traits::ObligationCause; use rustc_middle::infer::unify_key::{ConstVariableOrigin, ConstVariableOriginKind}; use rustc_middle::middle::stability::AllowUnstable; +use rustc_middle::ty::fold::FnMutDelegate; use rustc_middle::ty::subst::{self, GenericArgKind, InternalSubsts, SubstsRef}; use rustc_middle::ty::DynKind; use rustc_middle::ty::GenericParamDefKind; @@ -2226,47 +2227,66 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { let param_env = tcx.param_env(block.owner.to_def_id()); let cause = ObligationCause::misc(span, block.owner.def_id); + let mut fulfillment_errors = Vec::new(); - let mut applicable_candidates: Vec<_> = candidates - .iter() - .filter_map(|&(impl_, (assoc_item, def_scope))| { - infcx.probe(|_| { - let ocx = ObligationCtxt::new_in_snapshot(&infcx); + let mut applicable_candidates: Vec<_> = infcx.probe(|_| { + let universe = infcx.create_next_universe(); + + // Regions are not considered during selection. + let self_ty = tcx.replace_escaping_bound_vars_uncached( + self_ty, + FnMutDelegate { + regions: &mut |_| tcx.lifetimes.re_erased, + types: &mut |bv| { + tcx.mk_placeholder(ty::PlaceholderType { universe, name: bv.kind }) + }, + consts: &mut |bv, ty| { + tcx.mk_const(ty::PlaceholderConst { universe, name: bv }, ty) + }, + }, + ); - let impl_ty = tcx.type_of(impl_); - let impl_substs = infcx.fresh_item_substs(impl_); - let impl_ty = impl_ty.subst(tcx, impl_substs); - let impl_ty = ocx.normalize(&cause, param_env, impl_ty); + candidates + .iter() + .filter_map(|&(impl_, (assoc_item, def_scope))| { + infcx.probe(|_| { + let ocx = ObligationCtxt::new_in_snapshot(&infcx); - // Check that the Self-types can be related. - // FIXME(fmease): Should we use `eq` here? - ocx.sup(&ObligationCause::dummy(), param_env, impl_ty, self_ty).ok()?; + let impl_ty = tcx.type_of(impl_); + let impl_substs = infcx.fresh_item_substs(impl_); + let impl_ty = impl_ty.subst(tcx, impl_substs); + let impl_ty = ocx.normalize(&cause, param_env, impl_ty); - // Check whether the impl imposes obligations we have to worry about. - let impl_bounds = tcx.predicates_of(impl_); - let impl_bounds = impl_bounds.instantiate(tcx, impl_substs); + // Check that the Self-types can be related. + // FIXME(fmease): Should we use `eq` here? + ocx.sup(&ObligationCause::dummy(), param_env, impl_ty, self_ty).ok()?; - let impl_bounds = ocx.normalize(&cause, param_env, impl_bounds); + // Check whether the impl imposes obligations we have to worry about. + let impl_bounds = tcx.predicates_of(impl_); + let impl_bounds = impl_bounds.instantiate(tcx, impl_substs); - let impl_obligations = traits::predicates_for_generics( - |_, _| cause.clone(), - param_env, - impl_bounds, - ); + let impl_bounds = ocx.normalize(&cause, param_env, impl_bounds); - ocx.register_obligations(impl_obligations); + let impl_obligations = traits::predicates_for_generics( + |_, _| cause.clone(), + param_env, + impl_bounds, + ); - let mut errors = ocx.select_where_possible(); - if !errors.is_empty() { - fulfillment_errors.append(&mut errors); - return None; - } + ocx.register_obligations(impl_obligations); + + let mut errors = ocx.select_where_possible(); + if !errors.is_empty() { + fulfillment_errors.append(&mut errors); + return None; + } - // FIXME(fmease): Unsolved vars can escape this InferCtxt snapshot. - Some((assoc_item, def_scope, infcx.resolve_vars_if_possible(impl_substs))) + // FIXME(fmease): Unsolved vars can escape this InferCtxt snapshot. + Some((assoc_item, def_scope, infcx.resolve_vars_if_possible(impl_substs))) + }) }) - }) - .collect(); + .collect() + }); if applicable_candidates.len() > 1 { return Err(self.complain_about_ambiguous_inherent_assoc_type( diff --git a/tests/ui/associated-inherent-types/issue-109299-1.rs b/tests/ui/associated-inherent-types/issue-109299-1.rs new file mode 100644 index 0000000000000..6f95273116b78 --- /dev/null +++ b/tests/ui/associated-inherent-types/issue-109299-1.rs @@ -0,0 +1,12 @@ +#![feature(inherent_associated_types, non_lifetime_binders, type_alias_impl_trait)] +#![allow(incomplete_features)] + +struct Lexer(T); + +impl Lexer { + type Cursor = (); +} + +type X = impl for Fn() -> Lexer::Cursor; //~ ERROR associated type `Cursor` not found for `Lexer` in the current scope + +fn main() {} diff --git a/tests/ui/associated-inherent-types/issue-109299-1.stderr b/tests/ui/associated-inherent-types/issue-109299-1.stderr new file mode 100644 index 0000000000000..dc59b56ee207d --- /dev/null +++ b/tests/ui/associated-inherent-types/issue-109299-1.stderr @@ -0,0 +1,15 @@ +error[E0220]: associated type `Cursor` not found for `Lexer` in the current scope + --> $DIR/issue-109299-1.rs:10:40 + | +LL | struct Lexer(T); + | --------------- associated item `Cursor` not found for this struct +... +LL | type X = impl for Fn() -> Lexer::Cursor; + | ^^^^^^ associated item not found in `Lexer` + | + = note: the associated type was found for + - `Lexer` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0220`. diff --git a/tests/ui/associated-inherent-types/issue-109299.rs b/tests/ui/associated-inherent-types/issue-109299.rs new file mode 100644 index 0000000000000..84e4f9e72527a --- /dev/null +++ b/tests/ui/associated-inherent-types/issue-109299.rs @@ -0,0 +1,12 @@ +#![feature(inherent_associated_types)] +#![allow(incomplete_features)] + +struct Lexer<'d>(&'d ()); + +impl Lexer<'d> { //~ ERROR use of undeclared lifetime name `'d` + type Cursor = (); +} + +fn test(_: Lexer::Cursor) {} + +fn main() {} diff --git a/tests/ui/associated-inherent-types/issue-109299.stderr b/tests/ui/associated-inherent-types/issue-109299.stderr new file mode 100644 index 0000000000000..63f50732d3c50 --- /dev/null +++ b/tests/ui/associated-inherent-types/issue-109299.stderr @@ -0,0 +1,11 @@ +error[E0261]: use of undeclared lifetime name `'d` + --> $DIR/issue-109299.rs:6:12 + | +LL | impl Lexer<'d> { + | - ^^ undeclared lifetime + | | + | help: consider introducing lifetime `'d` here: `<'d>` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0261`.