From 754b52669ff505b376e0d00b7ba9a0c18db535d1 Mon Sep 17 00:00:00 2001 From: klensy Date: Sun, 31 Jul 2022 16:44:06 +0300 Subject: [PATCH 1/8] dedupe 'annotate-snippets' crate versions --- Cargo.lock | 12 +++--------- compiler/rustc_errors/Cargo.toml | 2 +- .../src/annotate_snippet_emitter_writer.rs | 6 +++++- compiler/rustc_macros/Cargo.toml | 2 +- 4 files changed, 10 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 058b6198dc425..68c110fa46420 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -82,12 +82,6 @@ dependencies = [ "url 2.2.2", ] -[[package]] -name = "annotate-snippets" -version = "0.8.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d78ea013094e5ea606b1c05fe35f1dd7ea1eb1ea259908d040b25bd5ec677ee5" - [[package]] name = "annotate-snippets" version = "0.9.1" @@ -3861,7 +3855,7 @@ dependencies = [ name = "rustc_errors" version = "0.0.0" dependencies = [ - "annotate-snippets 0.8.0", + "annotate-snippets", "atty", "rustc_data_structures", "rustc_error_messages", @@ -4113,7 +4107,7 @@ dependencies = [ name = "rustc_macros" version = "0.1.0" dependencies = [ - "annotate-snippets 0.8.0", + "annotate-snippets", "fluent-bundle", "fluent-syntax", "proc-macro2", @@ -4713,7 +4707,7 @@ dependencies = [ name = "rustfmt-nightly" version = "1.5.1" dependencies = [ - "annotate-snippets 0.9.1", + "annotate-snippets", "anyhow", "bytecount", "cargo_metadata 0.14.0", diff --git a/compiler/rustc_errors/Cargo.toml b/compiler/rustc_errors/Cargo.toml index 7d7e92c522922..36805aa874fe7 100644 --- a/compiler/rustc_errors/Cargo.toml +++ b/compiler/rustc_errors/Cargo.toml @@ -18,7 +18,7 @@ rustc_lint_defs = { path = "../rustc_lint_defs" } unicode-width = "0.1.4" atty = "0.2" termcolor = "1.0" -annotate-snippets = "0.8.0" +annotate-snippets = "0.9" termize = "0.1.1" serde = { version = "1.0.125", features = ["derive"] } serde_json = "1.0.59" diff --git a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs index 0fcd61d1e58c3..3df562c7edac7 100644 --- a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs +++ b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs @@ -183,7 +183,11 @@ impl AnnotateSnippetEmitterWriter { annotation_type: annotation_type_for_level(*level), }), footer: vec![], - opt: FormatOptions { color: true, anonymized_line_numbers: self.ui_testing }, + opt: FormatOptions { + color: true, + anonymized_line_numbers: self.ui_testing, + margin: None, + }, slices: annotated_files .iter() .map(|(source, line_index, annotations)| { diff --git a/compiler/rustc_macros/Cargo.toml b/compiler/rustc_macros/Cargo.toml index 25b3aadc1c527..547c8debb505d 100644 --- a/compiler/rustc_macros/Cargo.toml +++ b/compiler/rustc_macros/Cargo.toml @@ -7,7 +7,7 @@ edition = "2021" proc-macro = true [dependencies] -annotate-snippets = "0.8.0" +annotate-snippets = "0.9" fluent-bundle = "0.15.2" fluent-syntax = "0.11" synstructure = "0.12.1" From aac82a9e187d314b770baa4bf1bda7ce3d113d01 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Thu, 4 Aug 2022 00:20:06 +0100 Subject: [PATCH 2/8] Add visibility modifier to compat macro --- library/std/src/sys/windows/compat.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/std/src/sys/windows/compat.rs b/library/std/src/sys/windows/compat.rs index ccc90177a2034..e199bca9f03a0 100644 --- a/library/std/src/sys/windows/compat.rs +++ b/library/std/src/sys/windows/compat.rs @@ -151,7 +151,7 @@ impl Module { macro_rules! compat_fn_with_fallback { (pub static $module:ident: &CStr = $name:expr; $( $(#[$meta:meta])* - pub fn $symbol:ident($($argname:ident: $argtype:ty),*) -> $rettype:ty $fallback_body:block + $vis:vis fn $symbol:ident($($argname:ident: $argtype:ty),*) -> $rettype:ty $fallback_body:block )*) => ( pub static $module: &CStr = $name; $( @@ -208,7 +208,7 @@ macro_rules! compat_fn_with_fallback { } } $(#[$meta])* - pub use $symbol::call as $symbol; + $vis use $symbol::call as $symbol; )*) } From c985648593122a6fc8ec146a9ca755b73d0dc788 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Thu, 4 Aug 2022 01:46:14 +0100 Subject: [PATCH 3/8] Remove Windows function preloading --- library/std/src/sys/windows/c.rs | 18 +- library/std/src/sys/windows/compat.rs | 195 ++++++------------- library/std/src/sys/windows/thread_parker.rs | 31 +-- 3 files changed, 86 insertions(+), 158 deletions(-) diff --git a/library/std/src/sys/windows/c.rs b/library/std/src/sys/windows/c.rs index 478068c73ba88..c340baf2a4a2a 100644 --- a/library/std/src/sys/windows/c.rs +++ b/library/std/src/sys/windows/c.rs @@ -1250,19 +1250,15 @@ compat_fn_with_fallback! { } } -compat_fn_optional! { +compat_fn_with_fallback! { pub static SYNCH_API: &CStr = ansi_str!("api-ms-win-core-synch-l1-2-0"); - - // >= Windows 8 / Server 2012 - // https://docs.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-waitonaddress - pub fn WaitOnAddress( - Address: LPVOID, - CompareAddress: LPVOID, - AddressSize: SIZE_T, - dwMilliseconds: DWORD - ) -> BOOL; - pub fn WakeByAddressSingle(Address: LPVOID) -> (); + #[allow(unused)] + fn WakeByAddressSingle(Address: LPVOID) -> () { + crate::sys::windows::thread_parker::unpark_keyed_event(Address) + } } +pub use crate::sys::compat::WaitOnAddress; +pub use WakeByAddressSingle::call as wake_by_address_single_or_unpark_keyed_event; compat_fn_with_fallback! { pub static NTDLL: &CStr = ansi_str!("ntdll"); diff --git a/library/std/src/sys/windows/compat.rs b/library/std/src/sys/windows/compat.rs index e199bca9f03a0..ca5b223513498 100644 --- a/library/std/src/sys/windows/compat.rs +++ b/library/std/src/sys/windows/compat.rs @@ -7,47 +7,17 @@ //! `GetModuleHandle` and `GetProcAddress` to look up DLL entry points at //! runtime. //! -//! This implementation uses a static initializer to look up the DLL entry -//! points. The CRT (C runtime) executes static initializers before `main` -//! is called (for binaries) and before `DllMain` is called (for DLLs). -//! This is the ideal time to look up DLL imports, because we are guaranteed -//! that no other threads will attempt to call these entry points. Thus, -//! we can look up the imports and store them in `static mut` fields -//! without any synchronization. +//! This is implemented simply by storing a function pointer in an atomic +//! and using relaxed ordering to load it. This means that calling it will be no +//! more expensive then calling any other dynamically imported function. //! -//! This has an additional advantage: Because the DLL import lookup happens -//! at module initialization, the cost of these lookups is deterministic, -//! and is removed from the code paths that actually call the DLL imports. -//! That is, there is no unpredictable "cache miss" that occurs when calling -//! a DLL import. For applications that benefit from predictable delays, -//! this is a benefit. This also eliminates the comparison-and-branch -//! from the hot path. -//! -//! Currently, the standard library uses only a small number of dynamic -//! DLL imports. If this number grows substantially, then the cost of -//! performing all of the lookups at initialization time might become -//! substantial. -//! -//! The mechanism of registering a static initializer with the CRT is -//! documented in -//! [CRT Initialization](https://docs.microsoft.com/en-us/cpp/c-runtime-library/crt-initialization?view=msvc-160). -//! It works by contributing a global symbol to the `.CRT$XCU` section. -//! The linker builds a table of all static initializer functions. -//! The CRT startup code then iterates that table, calling each -//! initializer function. -//! -//! # **WARNING!!* -//! The environment that a static initializer function runs in is highly -//! constrained. There are **many** restrictions on what static initializers -//! can safely do. Static initializer functions **MUST NOT** do any of the -//! following (this list is not comprehensive): -//! * touch any other static field that is used by a different static -//! initializer, because the order that static initializers run in -//! is not defined. -//! * call `LoadLibrary` or any other function that acquires the DLL -//! loader lock. -//! * call any Rust function or CRT function that touches any static -//! (global) state. +//! The stored function pointer starts out as an importer function which will +//! swap itself with the real function when it's called for the first time. If +//! the real function can't be imported then a fallback function is used in its +//! place. While this is zero cost for the happy path (where the function is +//! already loaded) it does mean there's some overhead the first time the +//! function is called. In the worst case, multiple threads may all end up +//! importing the same function unnecessarily. use crate::ffi::{c_void, CStr}; use crate::ptr::NonNull; @@ -85,39 +55,6 @@ pub(crate) const fn const_cstr_from_bytes(bytes: &'static [u8]) -> &'static CStr unsafe { crate::ffi::CStr::from_bytes_with_nul_unchecked(bytes) } } -#[used] -#[link_section = ".CRT$XCU"] -static INIT_TABLE_ENTRY: unsafe extern "C" fn() = init; - -/// This is where the magic preloading of symbols happens. -/// -/// Note that any functions included here will be unconditionally included in -/// the final binary, regardless of whether or not they're actually used. -/// -/// Therefore, this is limited to `compat_fn_optional` functions which must be -/// preloaded and any functions which may be more time sensitive, even for the first call. -unsafe extern "C" fn init() { - // There is no locking here. This code is executed before main() is entered, and - // is guaranteed to be single-threaded. - // - // DO NOT do anything interesting or complicated in this function! DO NOT call - // any Rust functions or CRT functions if those functions touch any global state, - // because this function runs during global initialization. For example, DO NOT - // do any dynamic allocation, don't call LoadLibrary, etc. - - if let Some(synch) = Module::new(c::SYNCH_API) { - // These are optional and so we must manually attempt to load them - // before they can be used. - c::WaitOnAddress::preload(synch); - c::WakeByAddressSingle::preload(synch); - } - - if let Some(kernel32) = Module::new(c::KERNEL32) { - // Preloading this means getting a precise time will be as fast as possible. - c::GetSystemTimePreciseAsFileTime::preload(kernel32); - } -} - /// Represents a loaded module. /// /// Note that the modules std depends on must not be unloaded. @@ -196,11 +133,6 @@ macro_rules! compat_fn_with_fallback { $fallback_body } - #[allow(unused)] - pub(in crate::sys) fn preload(module: Module) { - load_from_module(Some(module)); - } - #[inline(always)] pub unsafe fn call($($argname: $argtype),*) -> $rettype { let func: F = mem::transmute(PTR.load(Ordering::Relaxed)); @@ -212,62 +144,61 @@ macro_rules! compat_fn_with_fallback { )*) } -/// A function that either exists or doesn't. +/// Optionally load `WaitOnAddress`. +/// Unlike the dynamic loading described above, this does not have a fallback. /// -/// NOTE: Optional functions must be preloaded in the `init` function above, or they will always be None. -macro_rules! compat_fn_optional { - (pub static $module:ident: &CStr = $name:expr; $( - $(#[$meta:meta])* - pub fn $symbol:ident($($argname:ident: $argtype:ty),*) -> $rettype:ty; - )*) => ( - pub static $module: &CStr = $name; - $( - $(#[$meta])* - pub mod $symbol { - #[allow(unused_imports)] - use super::*; - use crate::mem; - use crate::sync::atomic::{AtomicPtr, Ordering}; - use crate::sys::compat::Module; - use crate::ptr::{self, NonNull}; - - type F = unsafe extern "system" fn($($argtype),*) -> $rettype; - - /// `PTR` will either be `null()` or set to the loaded function. - static PTR: AtomicPtr = AtomicPtr::new(ptr::null_mut()); - - /// Only allow access to the function if it has loaded successfully. - #[inline(always)] - #[cfg(not(miri))] - pub fn option() -> Option { - unsafe { - NonNull::new(PTR.load(Ordering::Relaxed)).map(|f| mem::transmute(f)) - } - } - - // Miri does not understand the way we do preloading - // therefore load the function here instead. - #[cfg(miri)] - pub fn option() -> Option { - let mut func = NonNull::new(PTR.load(Ordering::Relaxed)); - if func.is_none() { - unsafe { Module::new($module).map(preload) }; - func = NonNull::new(PTR.load(Ordering::Relaxed)); - } - unsafe { - func.map(|f| mem::transmute(f)) - } - } +/// This is rexported from sys::c. You should prefer to import +/// from there in case this changes again in the future. +pub mod WaitOnAddress { + use super::*; + use crate::mem; + use crate::ptr; + use crate::sync::atomic::{AtomicBool, AtomicPtr, Ordering}; + use crate::sys::c; + + static MODULE_NAME: &CStr = ansi_str!("api-ms-win-core-synch-l1-2-0"); + static SYMBOL_NAME: &CStr = ansi_str!("WaitOnAddress"); + + // WaitOnAddress function signature. + type F = unsafe extern "system" fn( + Address: c::LPVOID, + CompareAddress: c::LPVOID, + AddressSize: c::SIZE_T, + dwMilliseconds: c::DWORD, + ); + + // A place to store the loaded function atomically. + static WAIT_ON_ADDRESS: AtomicPtr = AtomicPtr::new(ptr::null_mut()); + + // We can skip trying to load again if we already tried. + static LOAD_MODULE: AtomicBool = AtomicBool::new(true); + + #[inline(always)] + pub fn option() -> Option { + let f = WAIT_ON_ADDRESS.load(Ordering::Relaxed); + if !f.is_null() { Some(unsafe { mem::transmute(f) }) } else { try_load() } + } - #[allow(unused)] - pub(in crate::sys) fn preload(module: Module) { - unsafe { - static SYMBOL_NAME: &CStr = ansi_str!(sym $symbol); - if let Some(f) = module.proc_address(SYMBOL_NAME) { - PTR.store(f.as_ptr(), Ordering::Relaxed); - } - } + #[cold] + fn try_load() -> Option { + if LOAD_MODULE.load(Ordering::Acquire) { + // load the module + let mut wait_on_address = None; + if let Some(func) = try_load_inner() { + WAIT_ON_ADDRESS.store(func.as_ptr(), Ordering::Relaxed); + wait_on_address = Some(unsafe { mem::transmute(func) }); } + // Don't try to load the module again even if loading failed. + LOAD_MODULE.store(false, Ordering::Release); + wait_on_address + } else { + None } - )*) + } + + // In the future this could be a `try` block but until then I think it's a + // little bit cleaner as a separate function. + fn try_load_inner() -> Option> { + unsafe { Module::new(MODULE_NAME)?.proc_address(SYMBOL_NAME) } + } } diff --git a/library/std/src/sys/windows/thread_parker.rs b/library/std/src/sys/windows/thread_parker.rs index d876e0f6f3c03..16863c9903ac7 100644 --- a/library/std/src/sys/windows/thread_parker.rs +++ b/library/std/src/sys/windows/thread_parker.rs @@ -197,21 +197,9 @@ impl Parker { // purpose, to make sure every unpark() has a release-acquire ordering // with park(). if self.state.swap(NOTIFIED, Release) == PARKED { - if let Some(wake_by_address_single) = c::WakeByAddressSingle::option() { - unsafe { - wake_by_address_single(self.ptr()); - } - } else { - // If we run NtReleaseKeyedEvent before the waiting thread runs - // NtWaitForKeyedEvent, this (shortly) blocks until we can wake it up. - // If the waiting thread wakes up before we run NtReleaseKeyedEvent - // (e.g. due to a timeout), this blocks until we do wake up a thread. - // To prevent this thread from blocking indefinitely in that case, - // park_impl() will, after seeing the state set to NOTIFIED after - // waking up, call NtWaitForKeyedEvent again to unblock us. - unsafe { - c::NtReleaseKeyedEvent(keyed_event_handle(), self.ptr(), 0, ptr::null_mut()); - } + unsafe { + // This calls either WakeByAddressSingle or unpark_keyed_event (see below). + c::wake_by_address_single_or_unpark_keyed_event(self.ptr()); } } } @@ -221,6 +209,19 @@ impl Parker { } } +// This function signature makes it compatible with c::WakeByAddressSingle +// so that it can be used as a fallback for that function. +pub unsafe extern "C" fn unpark_keyed_event(address: c::LPVOID) { + // If we run NtReleaseKeyedEvent before the waiting thread runs + // NtWaitForKeyedEvent, this (shortly) blocks until we can wake it up. + // If the waiting thread wakes up before we run NtReleaseKeyedEvent + // (e.g. due to a timeout), this blocks until we do wake up a thread. + // To prevent this thread from blocking indefinitely in that case, + // park_impl() will, after seeing the state set to NOTIFIED after + // waking up, call NtWaitForKeyedEvent again to unblock us. + c::NtReleaseKeyedEvent(keyed_event_handle(), address, 0, ptr::null_mut()); +} + fn keyed_event_handle() -> c::HANDLE { const INVALID: c::HANDLE = ptr::invalid_mut(!0); static HANDLE: AtomicPtr = AtomicPtr::new(INVALID); From c62a8ea9df90a14c8c48ccbaffa959ed6ca2a97e Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 4 Aug 2022 02:55:40 +0000 Subject: [PATCH 4/8] Don't point out return span on every E0308 --- compiler/rustc_typeck/src/check/coercion.rs | 23 ++++++++++++++++++- compiler/rustc_typeck/src/check/demand.rs | 22 ------------------ src/test/ui/closures/issue-84128.stderr | 5 ---- .../dont-point-return-on-E0308.rs | 18 +++++++++++++++ .../dont-point-return-on-E0308.stderr | 19 +++++++++++++++ 5 files changed, 59 insertions(+), 28 deletions(-) create mode 100644 src/test/ui/mismatched_types/dont-point-return-on-E0308.rs create mode 100644 src/test/ui/mismatched_types/dont-point-return-on-E0308.stderr diff --git a/compiler/rustc_typeck/src/check/coercion.rs b/compiler/rustc_typeck/src/check/coercion.rs index 2ed5f569b4f3e..b75a2f3edd95a 100644 --- a/compiler/rustc_typeck/src/check/coercion.rs +++ b/compiler/rustc_typeck/src/check/coercion.rs @@ -1648,9 +1648,30 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { ); } - if let (Some(sp), Some(fn_output)) = (fcx.ret_coercion_span.get(), fn_output) { + let ret_coercion_span = fcx.ret_coercion_span.get(); + + if let Some(sp) = ret_coercion_span + // If the closure has an explicit return type annotation, or if + // the closure's return type has been inferred from outside + // requirements (such as an Fn* trait bound), then a type error + // may occur at the first return expression we see in the closure + // (if it conflicts with the declared return type). Skip adding a + // note in this case, since it would be incorrect. + && !fcx.return_type_pre_known + { + err.span_note( + sp, + &format!( + "return type inferred to be `{}` here", + fcx.resolve_vars_if_possible(expected) + ), + ); + } + + if let (Some(sp), Some(fn_output)) = (ret_coercion_span, fn_output) { self.add_impl_trait_explanation(&mut err, cause, fcx, expected, sp, fn_output); } + err } diff --git a/compiler/rustc_typeck/src/check/demand.rs b/compiler/rustc_typeck/src/check/demand.rs index 4de48dc5ba107..0595b9a73bed5 100644 --- a/compiler/rustc_typeck/src/check/demand.rs +++ b/compiler/rustc_typeck/src/check/demand.rs @@ -45,7 +45,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.note_type_is_not_clone(err, expected, expr_ty, expr); self.note_need_for_fn_pointer(err, expected, expr_ty); self.note_internal_mutation_in_method(err, expr, expected, expr_ty); - self.report_closure_inferred_return_type(err, expected); } // Requires that the two types unify, and prints an error message if @@ -1418,25 +1417,4 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { _ => false, } } - - // Report the type inferred by the return statement. - fn report_closure_inferred_return_type(&self, err: &mut Diagnostic, expected: Ty<'tcx>) { - if let Some(sp) = self.ret_coercion_span.get() - // If the closure has an explicit return type annotation, or if - // the closure's return type has been inferred from outside - // requirements (such as an Fn* trait bound), then a type error - // may occur at the first return expression we see in the closure - // (if it conflicts with the declared return type). Skip adding a - // note in this case, since it would be incorrect. - && !self.return_type_pre_known - { - err.span_note( - sp, - &format!( - "return type inferred to be `{}` here", - self.resolve_vars_if_possible(expected) - ), - ); - } - } } diff --git a/src/test/ui/closures/issue-84128.stderr b/src/test/ui/closures/issue-84128.stderr index 09c44d261af0c..59607afec8f8d 100644 --- a/src/test/ui/closures/issue-84128.stderr +++ b/src/test/ui/closures/issue-84128.stderr @@ -6,11 +6,6 @@ LL | Foo(()) | | | arguments to this struct are incorrect | -note: return type inferred to be `{integer}` here - --> $DIR/issue-84128.rs:10:20 - | -LL | return Foo(0); - | ^^^^^^ note: tuple struct defined here --> $DIR/issue-84128.rs:5:8 | diff --git a/src/test/ui/mismatched_types/dont-point-return-on-E0308.rs b/src/test/ui/mismatched_types/dont-point-return-on-E0308.rs new file mode 100644 index 0000000000000..f2ba610e2d1f6 --- /dev/null +++ b/src/test/ui/mismatched_types/dont-point-return-on-E0308.rs @@ -0,0 +1,18 @@ +// edition:2021 + +async fn f(_: &()) {} +//~^ NOTE function defined here +//~| NOTE +// Second note is the span of the underlined argument, I think... + +fn main() { + (|| async { + Err::<(), ()>(())?; + f(()); + //~^ ERROR mismatched types + //~| NOTE arguments to this function are incorrect + //~| NOTE expected `&()`, found `()` + //~| HELP consider borrowing here + Ok::<(), ()>(()) + })(); +} diff --git a/src/test/ui/mismatched_types/dont-point-return-on-E0308.stderr b/src/test/ui/mismatched_types/dont-point-return-on-E0308.stderr new file mode 100644 index 0000000000000..e79ab537b4c0f --- /dev/null +++ b/src/test/ui/mismatched_types/dont-point-return-on-E0308.stderr @@ -0,0 +1,19 @@ +error[E0308]: mismatched types + --> $DIR/dont-point-return-on-E0308.rs:10:11 + | +LL | f(()); + | - ^^ + | | | + | | expected `&()`, found `()` + | | help: consider borrowing here: `&()` + | arguments to this function are incorrect + | +note: function defined here + --> $DIR/dont-point-return-on-E0308.rs:3:10 + | +LL | async fn f(_: &()) {} + | ^ ------ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`. From 1f463ac40735529abd7343bb8a12a1888bb623df Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 4 Aug 2022 03:05:57 +0000 Subject: [PATCH 5/8] Resolve vars before emitting coerce error --- compiler/rustc_typeck/src/check/coercion.rs | 3 +- src/test/ui/expr/if/if-branch-types.stderr | 5 ++ .../ui/expr/if/if-else-type-mismatch.stderr | 10 +++ .../type-mismatch-signature-deduction.stderr | 6 ++ src/test/ui/impl-trait/equality.stderr | 7 +- ...trait-in-return-position-impl-trait.stderr | 4 +- ...type-err-cause-on-impl-trait-return.stderr | 67 +++++++++++++++++-- .../dont-point-return-on-E0308.stderr | 2 +- .../ui/mismatched_types/issue-84976.stderr | 5 ++ src/test/ui/reify-intrinsic.stderr | 3 + .../type-alias-impl-trait/issue-74280.stderr | 2 +- 11 files changed, 102 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_typeck/src/check/coercion.rs b/compiler/rustc_typeck/src/check/coercion.rs index b75a2f3edd95a..b659eab5ca4c0 100644 --- a/compiler/rustc_typeck/src/check/coercion.rs +++ b/compiler/rustc_typeck/src/check/coercion.rs @@ -1478,6 +1478,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { // type) (self.final_ty.unwrap_or(self.expected_ty), expression_ty) }; + let (expected, found) = fcx.resolve_vars_if_possible((expected, found)); let mut err; let mut unsized_return = false; @@ -1663,7 +1664,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { sp, &format!( "return type inferred to be `{}` here", - fcx.resolve_vars_if_possible(expected) + expected ), ); } diff --git a/src/test/ui/expr/if/if-branch-types.stderr b/src/test/ui/expr/if/if-branch-types.stderr index 14f02163a8320..d2bba88211ea8 100644 --- a/src/test/ui/expr/if/if-branch-types.stderr +++ b/src/test/ui/expr/if/if-branch-types.stderr @@ -5,6 +5,11 @@ LL | let x = if true { 10i32 } else { 10u32 }; | ----- ^^^^^ expected `i32`, found `u32` | | | expected because of this + | +help: change the type of the numeric literal from `u32` to `i32` + | +LL | let x = if true { 10i32 } else { 10i32 }; + | ~~~ error: aborting due to previous error diff --git a/src/test/ui/expr/if/if-else-type-mismatch.stderr b/src/test/ui/expr/if/if-else-type-mismatch.stderr index 9fa190d6c9df3..f1fffdb1e7ef8 100644 --- a/src/test/ui/expr/if/if-else-type-mismatch.stderr +++ b/src/test/ui/expr/if/if-else-type-mismatch.stderr @@ -10,6 +10,11 @@ LL | | 2u32 | | ^^^^ expected `i32`, found `u32` LL | | }; | |_____- `if` and `else` have incompatible types + | +help: change the type of the numeric literal from `u32` to `i32` + | +LL | 2i32 + | ~~~ error[E0308]: `if` and `else` have incompatible types --> $DIR/if-else-type-mismatch.rs:8:38 @@ -18,6 +23,11 @@ LL | let _ = if true { 42i32 } else { 42u32 }; | ----- ^^^^^ expected `i32`, found `u32` | | | expected because of this + | +help: change the type of the numeric literal from `u32` to `i32` + | +LL | let _ = if true { 42i32 } else { 42i32 }; + | ~~~ error[E0308]: `if` and `else` have incompatible types --> $DIR/if-else-type-mismatch.rs:13:9 diff --git a/src/test/ui/generator/type-mismatch-signature-deduction.stderr b/src/test/ui/generator/type-mismatch-signature-deduction.stderr index 7938fc8097cd2..b98da1ed8be6e 100644 --- a/src/test/ui/generator/type-mismatch-signature-deduction.stderr +++ b/src/test/ui/generator/type-mismatch-signature-deduction.stderr @@ -11,6 +11,12 @@ note: return type inferred to be `Result<{integer}, _>` here | LL | return Ok(6); | ^^^^^ +help: try wrapping the expression in a variant of `Result` + | +LL | Ok(5) + | +++ + +LL | Err(5) + | ++++ + error[E0271]: type mismatch resolving `<[generator@$DIR/type-mismatch-signature-deduction.rs:7:5: 7:7] as Generator>::Return == i32` --> $DIR/type-mismatch-signature-deduction.rs:5:13 diff --git a/src/test/ui/impl-trait/equality.stderr b/src/test/ui/impl-trait/equality.stderr index f14b447b07730..d4a3495515cf1 100644 --- a/src/test/ui/impl-trait/equality.stderr +++ b/src/test/ui/impl-trait/equality.stderr @@ -12,10 +12,15 @@ error[E0308]: mismatched types --> $DIR/equality.rs:15:5 | LL | fn two(x: bool) -> impl Foo { - | -------- expected `_` because of return type + | -------- expected `i32` because of return type ... LL | 0_u32 | ^^^^^ expected `i32`, found `u32` + | +help: change the type of the numeric literal from `u32` to `i32` + | +LL | 0_i32 + | ~~~ error[E0277]: cannot add `impl Foo` to `u32` --> $DIR/equality.rs:24:11 diff --git a/src/test/ui/impl-trait/object-unsafe-trait-in-return-position-impl-trait.stderr b/src/test/ui/impl-trait/object-unsafe-trait-in-return-position-impl-trait.stderr index 5ca01a593761c..d6f5a1ac25b64 100644 --- a/src/test/ui/impl-trait/object-unsafe-trait-in-return-position-impl-trait.stderr +++ b/src/test/ui/impl-trait/object-unsafe-trait-in-return-position-impl-trait.stderr @@ -2,7 +2,7 @@ error[E0308]: mismatched types --> $DIR/object-unsafe-trait-in-return-position-impl-trait.rs:36:5 | LL | fn can() -> impl NotObjectSafe { - | ------------------ expected `_` because of return type + | ------------------ expected `A` because of return type ... LL | B | ^ expected struct `A`, found struct `B` @@ -11,7 +11,7 @@ error[E0308]: mismatched types --> $DIR/object-unsafe-trait-in-return-position-impl-trait.rs:43:5 | LL | fn cat() -> impl ObjectSafe { - | --------------- expected `_` because of return type + | --------------- expected `A` because of return type ... LL | B | ^ expected struct `A`, found struct `B` diff --git a/src/test/ui/impl-trait/point-to-type-err-cause-on-impl-trait-return.stderr b/src/test/ui/impl-trait/point-to-type-err-cause-on-impl-trait-return.stderr index 10510c1754eda..11c1072f02ccc 100644 --- a/src/test/ui/impl-trait/point-to-type-err-cause-on-impl-trait-return.stderr +++ b/src/test/ui/impl-trait/point-to-type-err-cause-on-impl-trait-return.stderr @@ -2,28 +2,43 @@ error[E0308]: mismatched types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:5:5 | LL | fn foo() -> impl std::fmt::Display { - | ---------------------- expected `_` because of return type + | ---------------------- expected `i32` because of return type ... LL | 1u32 | ^^^^ expected `i32`, found `u32` + | +help: change the type of the numeric literal from `u32` to `i32` + | +LL | 1i32 + | ~~~ error[E0308]: mismatched types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:12:16 | LL | fn bar() -> impl std::fmt::Display { - | ---------------------- expected `_` because of return type + | ---------------------- expected `i32` because of return type ... LL | return 1u32; | ^^^^ expected `i32`, found `u32` + | +help: change the type of the numeric literal from `u32` to `i32` + | +LL | return 1i32; + | ~~~ error[E0308]: mismatched types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:20:9 | LL | fn baz() -> impl std::fmt::Display { - | ---------------------- expected `_` because of return type + | ---------------------- expected `i32` because of return type ... LL | 1u32 | ^^^^ expected `i32`, found `u32` + | +help: you can convert a `u32` to an `i32` and panic if the converted value doesn't fit + | +LL | }.try_into().unwrap() + | ++++++++++++++++++++ error[E0308]: `if` and `else` have incompatible types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:28:9 @@ -36,36 +51,56 @@ LL | | 1u32 | | ^^^^ expected `i32`, found `u32` LL | | } | |_____- `if` and `else` have incompatible types + | +help: change the type of the numeric literal from `u32` to `i32` + | +LL | 1i32 + | ~~~ error[E0308]: mismatched types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:35:14 | LL | fn bat() -> impl std::fmt::Display { - | ---------------------- expected `_` because of return type + | ---------------------- expected `i32` because of return type ... LL | _ => 1u32, | ^^^^ expected `i32`, found `u32` + | +help: you can convert a `u32` to an `i32` and panic if the converted value doesn't fit + | +LL | }.try_into().unwrap() + | ++++++++++++++++++++ error[E0308]: mismatched types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:40:5 | LL | fn can() -> impl std::fmt::Display { - | ---------------------- expected `_` because of return type + | ---------------------- expected `i32` because of return type LL | / match 13 { LL | | 0 => return 0i32, LL | | 1 => 1u32, LL | | _ => 2u32, LL | | } | |_____^ expected `i32`, found `u32` + | +help: you can convert a `u32` to an `i32` and panic if the converted value doesn't fit + | +LL | }.try_into().unwrap() + | ++++++++++++++++++++ error[E0308]: mismatched types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:53:13 | LL | fn cat() -> impl std::fmt::Display { - | ---------------------- expected `_` because of return type + | ---------------------- expected `i32` because of return type ... LL | 1u32 | ^^^^ expected `i32`, found `u32` + | +help: you can convert a `u32` to an `i32` and panic if the converted value doesn't fit + | +LL | }.try_into().unwrap() + | ++++++++++++++++++++ error[E0308]: `match` arms have incompatible types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:61:14 @@ -78,6 +113,11 @@ LL | | 1 => 1u32, LL | | _ => 2u32, LL | | } | |_____- `match` arms have incompatible types + | +help: change the type of the numeric literal from `u32` to `i32` + | +LL | 1 => 1i32, + | ~~~ error[E0308]: `if` and `else` have incompatible types --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:97:9 @@ -90,6 +130,11 @@ LL | | 1u32 | | ^^^^ expected `i32`, found `u32` LL | | } | |_____- `if` and `else` have incompatible types + | +help: change the type of the numeric literal from `u32` to `i32` + | +LL | 1i32 + | ~~~ error[E0746]: return type cannot have an unboxed trait object --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:66:13 @@ -125,6 +170,11 @@ LL | | 1 => 1u32, LL | | _ => 2u32, LL | | } | |_____- `match` arms have incompatible types + | +help: change the type of the numeric literal from `u32` to `i32` + | +LL | 1 => 1i32, + | ~~~ error[E0746]: return type cannot have an unboxed trait object --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:77:13 @@ -164,6 +214,11 @@ LL | | 1u32 | | ^^^^ expected `i32`, found `u32` LL | | } | |_____- `if` and `else` have incompatible types + | +help: change the type of the numeric literal from `u32` to `i32` + | +LL | 1i32 + | ~~~ error[E0746]: return type cannot have an unboxed trait object --> $DIR/point-to-type-err-cause-on-impl-trait-return.rs:85:13 diff --git a/src/test/ui/mismatched_types/dont-point-return-on-E0308.stderr b/src/test/ui/mismatched_types/dont-point-return-on-E0308.stderr index e79ab537b4c0f..13942682d289c 100644 --- a/src/test/ui/mismatched_types/dont-point-return-on-E0308.stderr +++ b/src/test/ui/mismatched_types/dont-point-return-on-E0308.stderr @@ -1,5 +1,5 @@ error[E0308]: mismatched types - --> $DIR/dont-point-return-on-E0308.rs:10:11 + --> $DIR/dont-point-return-on-E0308.rs:11:11 | LL | f(()); | - ^^ diff --git a/src/test/ui/mismatched_types/issue-84976.stderr b/src/test/ui/mismatched_types/issue-84976.stderr index f8f2b1f0f5720..9157566e3a79b 100644 --- a/src/test/ui/mismatched_types/issue-84976.stderr +++ b/src/test/ui/mismatched_types/issue-84976.stderr @@ -3,6 +3,11 @@ error[E0308]: mismatched types | LL | length = { foo(&length) }; | ^^^^^^^^^^^^ expected `u32`, found `i32` + | +help: you can convert an `i32` to a `u32` and panic if the converted value doesn't fit + | +LL | length = { foo(&length).try_into().unwrap() }; + | ++++++++++++++++++++ error[E0308]: mismatched types --> $DIR/issue-84976.rs:17:14 diff --git a/src/test/ui/reify-intrinsic.stderr b/src/test/ui/reify-intrinsic.stderr index 360557fb5201d..f78f1d822bf60 100644 --- a/src/test/ui/reify-intrinsic.stderr +++ b/src/test/ui/reify-intrinsic.stderr @@ -23,6 +23,9 @@ LL | std::intrinsics::unlikely, | = note: expected fn item `extern "rust-intrinsic" fn(_) -> _ {likely}` found fn item `extern "rust-intrinsic" fn(_) -> _ {unlikely}` + = note: different `fn` items always have unique types, even if their signatures are the same + = help: change the expected type to be function pointer `extern "rust-intrinsic" fn(bool) -> bool` + = help: if the expected type is due to type inference, cast the expected `fn` to a function pointer: `likely as extern "rust-intrinsic" fn(bool) -> bool` error: aborting due to 3 previous errors diff --git a/src/test/ui/type-alias-impl-trait/issue-74280.stderr b/src/test/ui/type-alias-impl-trait/issue-74280.stderr index 5ed29e0ac94ff..66886db6eb946 100644 --- a/src/test/ui/type-alias-impl-trait/issue-74280.stderr +++ b/src/test/ui/type-alias-impl-trait/issue-74280.stderr @@ -2,7 +2,7 @@ error[E0308]: mismatched types --> $DIR/issue-74280.rs:9:5 | LL | fn test() -> Test { - | ---- expected `_` because of return type + | ---- expected `()` because of return type LL | let y = || -> Test { () }; LL | 7 | ^ expected `()`, found integer From a0e4c1695854db9bf2f4aae3229026608675307d Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Thu, 4 Aug 2022 12:26:40 +0100 Subject: [PATCH 6/8] Update after code review --- library/std/src/sys/windows/c.rs | 6 ++++++ library/std/src/sys/windows/compat.rs | 12 ++++++------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/library/std/src/sys/windows/c.rs b/library/std/src/sys/windows/c.rs index c340baf2a4a2a..c5a30f8bac86d 100644 --- a/library/std/src/sys/windows/c.rs +++ b/library/std/src/sys/windows/c.rs @@ -1254,10 +1254,16 @@ compat_fn_with_fallback! { pub static SYNCH_API: &CStr = ansi_str!("api-ms-win-core-synch-l1-2-0"); #[allow(unused)] fn WakeByAddressSingle(Address: LPVOID) -> () { + // This fallback is currently tightly coupled to its use in Parker::unpark. + // + // FIXME: If `WakeByAddressSingle` needs to be used anywhere other than + // Parker::unpark then this fallback will be wrong and will need to be decoupled. crate::sys::windows::thread_parker::unpark_keyed_event(Address) } } pub use crate::sys::compat::WaitOnAddress; +// Change exported name of `WakeByAddressSingle` to make the strange fallback +// behaviour clear. pub use WakeByAddressSingle::call as wake_by_address_single_or_unpark_keyed_event; compat_fn_with_fallback! { diff --git a/library/std/src/sys/windows/compat.rs b/library/std/src/sys/windows/compat.rs index ca5b223513498..473544c4d4f7b 100644 --- a/library/std/src/sys/windows/compat.rs +++ b/library/std/src/sys/windows/compat.rs @@ -7,14 +7,14 @@ //! `GetModuleHandle` and `GetProcAddress` to look up DLL entry points at //! runtime. //! -//! This is implemented simply by storing a function pointer in an atomic -//! and using relaxed ordering to load it. This means that calling it will be no -//! more expensive then calling any other dynamically imported function. +//! This is implemented simply by storing a function pointer in an atomic. +//! Loading and calling this function will have little or no overhead +//! compared with calling any other dynamically imported function. //! //! The stored function pointer starts out as an importer function which will //! swap itself with the real function when it's called for the first time. If //! the real function can't be imported then a fallback function is used in its -//! place. While this is zero cost for the happy path (where the function is +//! place. While this is low cost for the happy path (where the function is //! already loaded) it does mean there's some overhead the first time the //! function is called. In the worst case, multiple threads may all end up //! importing the same function unnecessarily. @@ -175,7 +175,7 @@ pub mod WaitOnAddress { #[inline(always)] pub fn option() -> Option { - let f = WAIT_ON_ADDRESS.load(Ordering::Relaxed); + let f = WAIT_ON_ADDRESS.load(Ordering::Acquire); if !f.is_null() { Some(unsafe { mem::transmute(f) }) } else { try_load() } } @@ -185,7 +185,7 @@ pub mod WaitOnAddress { // load the module let mut wait_on_address = None; if let Some(func) = try_load_inner() { - WAIT_ON_ADDRESS.store(func.as_ptr(), Ordering::Relaxed); + WAIT_ON_ADDRESS.store(func.as_ptr(), Ordering::Release); wait_on_address = Some(unsafe { mem::transmute(func) }); } // Don't try to load the module again even if loading failed. From c195f7c0a432d1d77aed3c490509895add77f93f Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Fri, 5 Aug 2022 17:11:47 +0400 Subject: [PATCH 7/8] Optimize `pointer::as_aligned_to` --- library/core/src/ptr/const_ptr.rs | 5 +---- library/core/src/ptr/mut_ptr.rs | 5 +---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/library/core/src/ptr/const_ptr.rs b/library/core/src/ptr/const_ptr.rs index e0655d68d2cfa..2cd6e23c71a3e 100644 --- a/library/core/src/ptr/const_ptr.rs +++ b/library/core/src/ptr/const_ptr.rs @@ -1336,11 +1336,8 @@ impl *const T { panic!("is_aligned_to: align is not a power-of-two"); } - // SAFETY: `is_power_of_two()` will return `false` for zero. - unsafe { core::intrinsics::assume(align != 0) }; - // Cast is needed for `T: !Sized` - self.cast::().addr() % align == 0 + self.cast::().addr() & align - 1 == 0 } } diff --git a/library/core/src/ptr/mut_ptr.rs b/library/core/src/ptr/mut_ptr.rs index fc3dd2a9b25a9..56ad5f7658e3d 100644 --- a/library/core/src/ptr/mut_ptr.rs +++ b/library/core/src/ptr/mut_ptr.rs @@ -1614,11 +1614,8 @@ impl *mut T { panic!("is_aligned_to: align is not a power-of-two"); } - // SAFETY: `is_power_of_two()` will return `false` for zero. - unsafe { core::intrinsics::assume(align != 0) }; - // Cast is needed for `T: !Sized` - self.cast::().addr() % align == 0 + self.cast::().addr() & align - 1 == 0 } } From 64d1c91a31c9c7c1b03868e7c62467b352c82325 Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Fri, 5 Aug 2022 18:45:42 +0200 Subject: [PATCH 8/8] ascii -> ASCII in code comment --- library/core/src/str/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/str/mod.rs b/library/core/src/str/mod.rs index c4f2e283eb3bc..ee66895a7ed2a 100644 --- a/library/core/src/str/mod.rs +++ b/library/core/src/str/mod.rs @@ -2353,7 +2353,7 @@ impl str { #[inline] pub fn is_ascii(&self) -> bool { // We can treat each byte as character here: all multibyte characters - // start with a byte that is not in the ascii range, so we will stop + // start with a byte that is not in the ASCII range, so we will stop // there already. self.as_bytes().is_ascii() }