From f03cf9916a1932f47b788a9de9256269cc172fe9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20S=C3=A1nchez=20Mu=C3=B1oz?= Date: Tue, 26 May 2020 17:38:22 +0200 Subject: [PATCH 01/36] Add a fast path for `std::thread::panicking`. This is done by adding a global atomic variable (non-TLS) that counts how many threads are panicking. In order to check if the current thread is panicking, this variable is read and, if it is zero, no thread (including the one where `panicking` is being called) is panicking and `panicking` can return `false` immediately without needing to access TLS. If the global counter is not zero, the local counter is accessed from TLS to check if the current thread is panicking. --- src/libstd/panicking.rs | 63 ++++++++++++++++++++++++++++++++--------- src/libstd/rt.rs | 2 +- 2 files changed, 51 insertions(+), 14 deletions(-) diff --git a/src/libstd/panicking.rs b/src/libstd/panicking.rs index bf381896a22bb..46196960e718b 100644 --- a/src/libstd/panicking.rs +++ b/src/libstd/panicking.rs @@ -170,7 +170,7 @@ pub fn take_hook() -> Box) + 'static + Sync + Send> { fn default_hook(info: &PanicInfo<'_>) { // If this is a double panic, make sure that we print a backtrace // for this panic. Otherwise only print it if logging is enabled. - let backtrace_env = if update_panic_count(0) >= 2 { + let backtrace_env = if panic_count::get() >= 2 { RustBacktrace::Print(backtrace_rs::PrintFmt::Full) } else { backtrace::rust_backtrace_env() @@ -222,19 +222,56 @@ fn default_hook(info: &PanicInfo<'_>) { #[cfg(not(test))] #[doc(hidden)] #[unstable(feature = "update_panic_count", issue = "none")] -pub fn update_panic_count(amt: isize) -> usize { +pub mod panic_count { use crate::cell::Cell; - thread_local! { static PANIC_COUNT: Cell = Cell::new(0) } + use crate::sync::atomic::{AtomicUsize, Ordering}; + + // Panic count for the current thread. + thread_local! { static LOCAL_PANIC_COUNT: Cell = Cell::new(0) } + + // Sum of panic counts from all threads. The purpose of this is to have + // a fast path in `is_zero` (which is used by `panicking`). Access to + // this variable can be always be done with relaxed ordering because + // it is always guaranteed that, if `GLOBAL_PANIC_COUNT` is zero, + // `LOCAL_PANIC_COUNT` will be zero. + static GLOBAL_PANIC_COUNT: AtomicUsize = AtomicUsize::new(0); + + pub fn increase() -> usize { + GLOBAL_PANIC_COUNT.fetch_add(1, Ordering::Relaxed); + LOCAL_PANIC_COUNT.with(|c| { + let next = c.get() + 1; + c.set(next); + next + }) + } + + pub fn decrease() -> usize { + GLOBAL_PANIC_COUNT.fetch_sub(1, Ordering::Relaxed); + LOCAL_PANIC_COUNT.with(|c| { + let next = c.get() - 1; + c.set(next); + next + }) + } - PANIC_COUNT.with(|c| { - let next = (c.get() as isize + amt) as usize; - c.set(next); - next - }) + pub fn get() -> usize { + LOCAL_PANIC_COUNT.with(|c| c.get()) + } + + pub fn is_zero() -> bool { + if GLOBAL_PANIC_COUNT.load(Ordering::Relaxed) == 0 { + // Fast path: if `GLOBAL_PANIC_COUNT` is zero, all threads + // (including the current one) will have `LOCAL_PANIC_COUNT` + // equal to zero, so TLS access can be avoided. + true + } else { + LOCAL_PANIC_COUNT.with(|c| c.get() == 0) + } + } } #[cfg(test)] -pub use realstd::rt::update_panic_count; +pub use realstd::rt::panic_count; /// Invoke a closure, capturing the cause of an unwinding panic if one occurs. pub unsafe fn r#try R>(f: F) -> Result> { @@ -284,7 +321,7 @@ pub unsafe fn r#try R>(f: F) -> Result> #[cold] unsafe fn cleanup(payload: *mut u8) -> Box { let obj = Box::from_raw(__rust_panic_cleanup(payload)); - update_panic_count(-1); + panic_count::decrease(); obj } @@ -314,7 +351,7 @@ pub unsafe fn r#try R>(f: F) -> Result> /// Determines whether the current thread is unwinding because of panic. pub fn panicking() -> bool { - update_panic_count(0) != 0 + !panic_count::is_zero() } /// The entry point for panicking with a formatted message. @@ -452,7 +489,7 @@ fn rust_panic_with_hook( message: Option<&fmt::Arguments<'_>>, location: &Location<'_>, ) -> ! { - let panics = update_panic_count(1); + let panics = panic_count::increase(); // If this is the third nested call (e.g., panics == 2, this is 0-indexed), // the panic hook probably triggered the last panic, otherwise the @@ -514,7 +551,7 @@ fn rust_panic_with_hook( /// This is the entry point for `resume_unwind`. /// It just forwards the payload to the panic runtime. pub fn rust_panic_without_hook(payload: Box) -> ! { - update_panic_count(1); + panic_count::increase(); struct RewrapBox(Box); diff --git a/src/libstd/rt.rs b/src/libstd/rt.rs index 2426b2dead712..fb825ab16ebd7 100644 --- a/src/libstd/rt.rs +++ b/src/libstd/rt.rs @@ -15,7 +15,7 @@ #![doc(hidden)] // Re-export some of our utilities which are expected by other crates. -pub use crate::panicking::{begin_panic, begin_panic_fmt, update_panic_count}; +pub use crate::panicking::{begin_panic, begin_panic_fmt, panic_count}; // To reduce the generated code of the new `lang_start`, this function is doing // the real work. From 1b47e8fa998f6d76d536594955336fe0327f894e Mon Sep 17 00:00:00 2001 From: Tshepang Lekhonkhobe Date: Sat, 20 Jun 2020 10:56:47 +0200 Subject: [PATCH 02/36] examples should be of type bool --- config.toml.example | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config.toml.example b/config.toml.example index bc6760334170b..a918feb278f62 100644 --- a/config.toml.example +++ b/config.toml.example @@ -313,11 +313,11 @@ # Whether or not debug assertions are enabled for the compiler and standard # library. -#debug-assertions = debug +#debug-assertions = false # Whether or not debug assertions are enabled for the standard library. # Overrides the `debug-assertions` option, if defined. -#debug-assertions-std = debug-assertions +#debug-assertions-std = false # Debuginfo level for most of Rust code, corresponds to the `-C debuginfo=N` option of `rustc`. # `0` - no debug info From 16e741c8e941f887a5db02ebd60d9a69624fbc0b Mon Sep 17 00:00:00 2001 From: Tshepang Lekhonkhobe Date: Sat, 20 Jun 2020 13:22:13 +0200 Subject: [PATCH 03/36] explain the logic a bit --- config.toml.example | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/config.toml.example b/config.toml.example index a918feb278f62..e214bf048980d 100644 --- a/config.toml.example +++ b/config.toml.example @@ -313,10 +313,12 @@ # Whether or not debug assertions are enabled for the compiler and standard # library. +# Defaults to rust.debug value. #debug-assertions = false # Whether or not debug assertions are enabled for the standard library. # Overrides the `debug-assertions` option, if defined. +# Defaults to rust.debug value. #debug-assertions-std = false # Debuginfo level for most of Rust code, corresponds to the `-C debuginfo=N` option of `rustc`. @@ -326,16 +328,21 @@ # Can be overridden for specific subsets of Rust code (rustc, std or tools). # Debuginfo for tests run with compiletest is not controlled by this option # and needs to be enabled separately with `debuginfo-level-tests`. -#debuginfo-level = if debug { 2 } else { 0 } +# +# If debug is true, this defaults to 2. +#debuginfo-level = 0 # Debuginfo level for the compiler. -#debuginfo-level-rustc = debuginfo-level +# Defaults to rust.debuginfo-level value. +#debuginfo-level-rustc = 0 # Debuginfo level for the standard library. -#debuginfo-level-std = debuginfo-level +# Defaults to rust.debuginfo-level value. +#debuginfo-level-std = 0 # Debuginfo level for the tools. -#debuginfo-level-tools = debuginfo-level +# Defaults to rust.debuginfo-level value. +#debuginfo-level-tools = 0 # Debuginfo level for the test suites run with compiletest. # FIXME(#61117): Some tests fail when this option is enabled. From 9766a93163cfb1447e4e7c4f9a516c77257b54be Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Mon, 22 Jun 2020 15:36:09 +0200 Subject: [PATCH 04/36] Document the mod keyword --- src/libstd/keyword_docs.rs | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/libstd/keyword_docs.rs b/src/libstd/keyword_docs.rs index a4996d9eee810..c966d8d34e999 100644 --- a/src/libstd/keyword_docs.rs +++ b/src/libstd/keyword_docs.rs @@ -913,10 +913,28 @@ mod match_keyword {} // /// Organize code into [modules]. /// -/// The documentation for this keyword is [not yet complete]. Pull requests welcome! +/// Use `mod` to create new [modules] to encapsulate code, including other +/// modules: /// +/// ``` +/// mod foo { +/// mod bar { +/// type MyType = (u8, u8); +/// fn baz() {} +/// } +/// } +/// ``` +/// +/// Like [`struct`]s and [`enum`]s, a module and its content are private by +/// default, unaccessible to code outside of the module. +/// +/// To learn more about allowing access, see the documentation for the [`pub`] +/// keyword. +/// +/// [`enum`]: keyword.enum.html +/// [`pub`]: keyword.pub.html +/// [`struct`]: keyword.struct.html /// [modules]: ../reference/items/modules.html -/// [not yet complete]: https://github.com/rust-lang/rust/issues/34601 mod mod_keyword {} #[doc(keyword = "move")] From 3c46e36d39cc0f833075e59936cf96a60f6348e4 Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Mon, 22 Jun 2020 16:27:23 +0200 Subject: [PATCH 05/36] Document the mut keyword --- src/libstd/keyword_docs.rs | 45 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/src/libstd/keyword_docs.rs b/src/libstd/keyword_docs.rs index a4996d9eee810..7d54f9ed2d20f 100644 --- a/src/libstd/keyword_docs.rs +++ b/src/libstd/keyword_docs.rs @@ -967,9 +967,50 @@ mod move_keyword {} // /// A mutable binding, reference, or pointer. /// -/// The documentation for this keyword is [not yet complete]. Pull requests welcome! +/// `mut` can be used in several situations. The first is mutable bindings, +/// which can be used anywhere you can bind a value to a variable name. Some +/// examples: /// -/// [not yet complete]: https://github.com/rust-lang/rust/issues/34601 +/// ``` +/// // A mutable binding in the parameter list of a function. +/// fn foo(mut x: u8, y: u8) -> u8 { +/// x += y; +/// x +/// } +/// +/// // A mutable binding for a variable. +/// let mut a = 5; +/// a = 6; +/// +/// assert_eq!(foo(3, 4), 7); +/// assert_eq!(a, 6); +/// ``` +/// +/// The second is references. They can be created from `mut` bindings and must +/// be unique: no other binding can have a mutable reference, nor a simple +/// reference. +/// +/// ``` +/// // Taking a mutable reference. +/// fn push_two(v: &mut Vec) { +/// v.push(2); +/// } +/// +/// // You cannot take a mutable reference to a non-mutable variable. +/// let mut v = vec![0, 1]; +/// // Passing a mutable reference. +/// push_two(&mut v); +/// +/// assert_eq!(v, vec![0, 1, 2]); +/// ``` +/// +/// Mutable pointers work much like mutable references, with the added +/// possibility of being nul. The syntax is `*mut Type`. +/// +/// You can find more information on mutable references and pointers in the +/// [Reference]. +/// +/// [Reference]: ../reference/types/pointer.html#mutable-references-mut mod mut_keyword {} #[doc(keyword = "pub")] From e84552e5dcb5b21de1528d7c42406a4b9ae9afe9 Mon Sep 17 00:00:00 2001 From: Tshepang Lekhonkhobe Date: Mon, 22 Jun 2020 17:07:42 +0200 Subject: [PATCH 06/36] be more consistent with "defaults" placement --- config.toml.example | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/config.toml.example b/config.toml.example index e214bf048980d..9ffaac0bbd002 100644 --- a/config.toml.example +++ b/config.toml.example @@ -173,9 +173,7 @@ # Python interpreter to use for various tasks throughout the build, notably # rustdoc tests, the lldb python interpreter, and some dist bits and pieces. -# -# Defaults to the Python interpreter used to execute x.py. -#python = "python" +#python = "python" # defaults to the Python interpreter used to execute x.py # Force Cargo to check that Cargo.lock describes the precise dependency # set that all the Cargo.toml files create, instead of updating it. @@ -313,13 +311,11 @@ # Whether or not debug assertions are enabled for the compiler and standard # library. -# Defaults to rust.debug value. -#debug-assertions = false +#debug-assertions = false # defaults to rust.debug value # Whether or not debug assertions are enabled for the standard library. # Overrides the `debug-assertions` option, if defined. -# Defaults to rust.debug value. -#debug-assertions-std = false +#debug-assertions-std = false # defaults to rust.debug value # Debuginfo level for most of Rust code, corresponds to the `-C debuginfo=N` option of `rustc`. # `0` - no debug info @@ -328,21 +324,16 @@ # Can be overridden for specific subsets of Rust code (rustc, std or tools). # Debuginfo for tests run with compiletest is not controlled by this option # and needs to be enabled separately with `debuginfo-level-tests`. -# -# If debug is true, this defaults to 2. -#debuginfo-level = 0 +#debuginfo-level = 0 # defaults to 2 if debug is true # Debuginfo level for the compiler. -# Defaults to rust.debuginfo-level value. -#debuginfo-level-rustc = 0 +#debuginfo-level-rustc = 0 # defaults to rust.debuginfo-level value # Debuginfo level for the standard library. -# Defaults to rust.debuginfo-level value. -#debuginfo-level-std = 0 +#debuginfo-level-std = 0 # defaults to rust.debuginfo-level value # Debuginfo level for the tools. -# Defaults to rust.debuginfo-level value. -#debuginfo-level-tools = 0 +#debuginfo-level-tools = 0 # defaults to rust.debuginfo-level value # Debuginfo level for the test suites run with compiletest. # FIXME(#61117): Some tests fail when this option is enabled. From ef24faf130b35964cce159bd130631b6e90ed0ea Mon Sep 17 00:00:00 2001 From: Charles Lew Date: Sun, 10 May 2020 09:10:15 +0800 Subject: [PATCH 07/36] Refactor non_ascii_idents lints, exclude ascii pair for confusable_idents lint. --- src/librustc_lint/non_ascii_idents.rs | 235 ++++++++---------- src/librustc_session/parse.rs | 3 +- .../lint-confusable-idents.rs | 10 +- .../lint-confusable-idents.stderr | 8 +- .../lint-non-ascii-idents.rs | 4 +- .../lint-non-ascii-idents.stderr | 8 +- .../lint-uncommon-codepoints.rs | 4 +- .../lint-uncommon-codepoints.stderr | 8 +- src/test/ui/parser/issue-62524.rs | 2 + src/test/ui/parser/issue-62524.stderr | 6 +- 10 files changed, 131 insertions(+), 157 deletions(-) diff --git a/src/librustc_lint/non_ascii_idents.rs b/src/librustc_lint/non_ascii_idents.rs index 064b0255397ce..90bd7ad4acf71 100644 --- a/src/librustc_lint/non_ascii_idents.rs +++ b/src/librustc_lint/non_ascii_idents.rs @@ -1,9 +1,7 @@ use crate::{EarlyContext, EarlyLintPass, LintContext}; use rustc_ast::ast; use rustc_data_structures::fx::FxHashMap; -use rustc_span::symbol::{Ident, SymbolStr}; -use std::hash::{Hash, Hasher}; -use std::ops::Deref; +use rustc_span::symbol::SymbolStr; declare_lint! { pub NON_ASCII_IDENTS, @@ -19,158 +17,133 @@ declare_lint! { crate_level_only } -// FIXME: Change this to warn. declare_lint! { pub CONFUSABLE_IDENTS, - Allow, + Warn, "detects visually confusable pairs between identifiers", crate_level_only } declare_lint_pass!(NonAsciiIdents => [NON_ASCII_IDENTS, UNCOMMON_CODEPOINTS, CONFUSABLE_IDENTS]); -enum CowBoxSymStr { - Interned(SymbolStr), - Owned(Box), -} - -impl Deref for CowBoxSymStr { - type Target = str; - - fn deref(&self) -> &str { - match self { - CowBoxSymStr::Interned(interned) => interned, - CowBoxSymStr::Owned(ref owned) => owned, - } - } -} - -impl Hash for CowBoxSymStr { - #[inline] - fn hash(&self, state: &mut H) { - Hash::hash(&**self, state) - } -} - -impl PartialEq for CowBoxSymStr { - #[inline] - fn eq(&self, other: &CowBoxSymStr) -> bool { - PartialEq::eq(&**self, &**other) - } -} - -impl Eq for CowBoxSymStr {} - -fn calc_skeleton(symbol_str: SymbolStr, buffer: &'_ mut String) -> CowBoxSymStr { - use std::mem::swap; - use unicode_security::confusable_detection::skeleton; - buffer.clear(); - buffer.extend(skeleton(&symbol_str)); - if symbol_str == *buffer { - CowBoxSymStr::Interned(symbol_str) - } else { - let mut owned = String::new(); - swap(buffer, &mut owned); - CowBoxSymStr::Owned(owned.into_boxed_str()) - } -} - -fn is_in_ascii_confusable_closure(c: char) -> bool { - // FIXME: move this table to `unicode_security` crate. - // data here corresponds to Unicode 13. - const ASCII_CONFUSABLE_CLOSURE: &[(u64, u64)] = &[(0x00, 0x7f), (0xba, 0xba), (0x2080, 0x2080)]; - let c = c as u64; - for &(range_start, range_end) in ASCII_CONFUSABLE_CLOSURE { - if c >= range_start && c <= range_end { - return true; - } - } - false -} - -fn is_in_ascii_confusable_closure_relevant_list(c: char) -> bool { - // FIXME: move this table to `unicode_security` crate. - // data here corresponds to Unicode 13. - const ASCII_CONFUSABLE_CLOSURE_RELEVANT_LIST: &[u64] = &[ - 0x22, 0x25, 0x27, 0x2f, 0x30, 0x31, 0x49, 0x4f, 0x60, 0x6c, 0x6d, 0x6e, 0x72, 0x7c, 0xba, - 0x2080, - ]; - let c = c as u64; - for &item in ASCII_CONFUSABLE_CLOSURE_RELEVANT_LIST { - if c == item { - return true; - } - } - false -} - impl EarlyLintPass for NonAsciiIdents { fn check_crate(&mut self, cx: &EarlyContext<'_>, _: &ast::Crate) { use rustc_session::lint::Level; - if cx.builder.lint_level(CONFUSABLE_IDENTS).0 == Level::Allow { + use rustc_span::Span; + use unicode_security::GeneralSecurityProfile; + use utils::CowBoxSymStr; + + let check_non_ascii_idents = cx.builder.lint_level(NON_ASCII_IDENTS).0 != Level::Allow; + let check_uncommon_codepoints = + cx.builder.lint_level(UNCOMMON_CODEPOINTS).0 != Level::Allow; + let check_confusable_idents = cx.builder.lint_level(CONFUSABLE_IDENTS).0 != Level::Allow; + + if !check_non_ascii_idents && !check_uncommon_codepoints && !check_confusable_idents { return; } + + let mut has_non_ascii_idents = false; let symbols = cx.sess.parse_sess.symbol_gallery.symbols.lock(); - let mut symbol_strs_and_spans = Vec::with_capacity(symbols.len()); - let mut in_fast_path = true; - for (symbol, sp) in symbols.iter() { - // fast path + for (symbol, &sp) in symbols.iter() { let symbol_str = symbol.as_str(); - if !symbol_str.chars().all(is_in_ascii_confusable_closure) { - // fallback to slow path. - symbol_strs_and_spans.clear(); - in_fast_path = false; - break; + if symbol_str.is_ascii() { + continue; } - if symbol_str.chars().any(is_in_ascii_confusable_closure_relevant_list) { - symbol_strs_and_spans.push((symbol_str, *sp)); + has_non_ascii_idents = true; + cx.struct_span_lint(NON_ASCII_IDENTS, sp, |lint| { + lint.build("identifier contains non-ASCII characters").emit() + }); + if check_uncommon_codepoints + && !symbol_str.chars().all(GeneralSecurityProfile::identifier_allowed) + { + cx.struct_span_lint(UNCOMMON_CODEPOINTS, sp, |lint| { + lint.build("identifier contains uncommon Unicode codepoints").emit() + }) } } - if !in_fast_path { - // slow path - for (symbol, sp) in symbols.iter() { + + if has_non_ascii_idents && check_confusable_idents { + let mut skeleton_map: FxHashMap = + FxHashMap::with_capacity_and_hasher(symbols.len(), Default::default()); + let mut str_buf = String::new(); + for (symbol, &sp) in symbols.iter() { + fn calc_skeleton(symbol_str: &SymbolStr, buffer: &mut String) -> CowBoxSymStr { + use std::mem::replace; + use unicode_security::confusable_detection::skeleton; + buffer.clear(); + buffer.extend(skeleton(symbol_str)); + if *symbol_str == *buffer { + CowBoxSymStr::Interned(symbol_str.clone()) + } else { + let owned = replace(buffer, String::new()); + CowBoxSymStr::Owned(owned.into_boxed_str()) + } + } let symbol_str = symbol.as_str(); - symbol_strs_and_spans.push((symbol_str, *sp)); + let is_ascii = symbol_str.is_ascii(); + let skeleton = calc_skeleton(&symbol_str, &mut str_buf); + skeleton_map + .entry(skeleton) + .and_modify(|(existing_symbolstr, existing_span, existing_is_ascii)| { + if !*existing_is_ascii || !is_ascii { + cx.struct_span_lint(CONFUSABLE_IDENTS, sp, |lint| { + lint.build(&format!( + "identifier pair considered confusable between `{}` and `{}`", + existing_symbolstr, symbol_str + )) + .span_label( + *existing_span, + "this is where the previous identifier occurred", + ) + .emit(); + }); + } + if *existing_is_ascii && !is_ascii { + *existing_symbolstr = symbol_str.clone(); + *existing_span = sp; + *existing_is_ascii = is_ascii; + } + }) + .or_insert((symbol_str, sp, is_ascii)); } } - drop(symbols); - symbol_strs_and_spans.sort_by_key(|x| x.0.clone()); - let mut skeleton_map = - FxHashMap::with_capacity_and_hasher(symbol_strs_and_spans.len(), Default::default()); - let mut str_buf = String::new(); - for (symbol_str, sp) in symbol_strs_and_spans { - let skeleton = calc_skeleton(symbol_str.clone(), &mut str_buf); - skeleton_map - .entry(skeleton) - .and_modify(|(existing_symbolstr, existing_span)| { - cx.struct_span_lint(CONFUSABLE_IDENTS, sp, |lint| { - lint.build(&format!( - "identifier pair considered confusable between `{}` and `{}`", - existing_symbolstr, symbol_str - )) - .span_label( - *existing_span, - "this is where the previous identifier occurred", - ) - .emit(); - }); - }) - .or_insert((symbol_str, sp)); + } +} + +mod utils { + use rustc_span::symbol::SymbolStr; + use std::hash::{Hash, Hasher}; + use std::ops::Deref; + + pub(super) enum CowBoxSymStr { + Interned(SymbolStr), + Owned(Box), + } + + impl Deref for CowBoxSymStr { + type Target = str; + + fn deref(&self) -> &str { + match self { + CowBoxSymStr::Interned(interned) => interned, + CowBoxSymStr::Owned(ref owned) => owned, + } } } - fn check_ident(&mut self, cx: &EarlyContext<'_>, ident: Ident) { - use unicode_security::GeneralSecurityProfile; - let name_str = ident.name.as_str(); - if name_str.is_ascii() { - return; + + impl Hash for CowBoxSymStr { + #[inline] + fn hash(&self, state: &mut H) { + Hash::hash(&**self, state) } - cx.struct_span_lint(NON_ASCII_IDENTS, ident.span, |lint| { - lint.build("identifier contains non-ASCII characters").emit() - }); - if !name_str.chars().all(GeneralSecurityProfile::identifier_allowed) { - cx.struct_span_lint(UNCOMMON_CODEPOINTS, ident.span, |lint| { - lint.build("identifier contains uncommon Unicode codepoints").emit() - }) + } + + impl PartialEq for CowBoxSymStr { + #[inline] + fn eq(&self, other: &CowBoxSymStr) -> bool { + PartialEq::eq(&**self, &**other) } } + + impl Eq for CowBoxSymStr {} } diff --git a/src/librustc_session/parse.rs b/src/librustc_session/parse.rs index ddbc95fb1b0b8..f4e5da4d54f46 100644 --- a/src/librustc_session/parse.rs +++ b/src/librustc_session/parse.rs @@ -13,6 +13,7 @@ use rustc_span::hygiene::ExpnId; use rustc_span::source_map::{FilePathMapping, SourceMap}; use rustc_span::{MultiSpan, Span, Symbol}; +use std::collections::BTreeMap; use std::path::PathBuf; use std::str; @@ -63,7 +64,7 @@ impl GatedSpans { #[derive(Default)] pub struct SymbolGallery { /// All symbols occurred and their first occurrance span. - pub symbols: Lock>, + pub symbols: Lock>, } impl SymbolGallery { diff --git a/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-confusable-idents.rs b/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-confusable-idents.rs index 12093837d2630..e15ed2e70b896 100644 --- a/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-confusable-idents.rs +++ b/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-confusable-idents.rs @@ -2,8 +2,14 @@ #![deny(confusable_idents)] #![allow(uncommon_codepoints, non_upper_case_globals)] -const ｓ: usize = 42; //~ ERROR identifier pair considered confusable +const ｓ: usize = 42; fn main() { - let s = "rust"; + let s = "rust"; //~ ERROR identifier pair considered confusable + not_affected(); +} + +fn not_affected() { + let s1 = 1; + let sl = 'l'; } diff --git a/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-confusable-idents.stderr b/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-confusable-idents.stderr index 40ee18acb3cd4..218f94f7b5829 100644 --- a/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-confusable-idents.stderr +++ b/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-confusable-idents.stderr @@ -1,11 +1,11 @@ -error: identifier pair considered confusable between `s` and `ｓ` - --> $DIR/lint-confusable-idents.rs:5:7 +error: identifier pair considered confusable between `ｓ` and `s` + --> $DIR/lint-confusable-idents.rs:8:9 | LL | const ｓ: usize = 42; - | ^^ + | -- this is where the previous identifier occurred ... LL | let s = "rust"; - | - this is where the previous identifier occurred + | ^ | note: the lint level is defined here --> $DIR/lint-confusable-idents.rs:2:9 diff --git a/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-non-ascii-idents.rs b/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-non-ascii-idents.rs index 057329a0a650c..20d00cf701a15 100644 --- a/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-non-ascii-idents.rs +++ b/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-non-ascii-idents.rs @@ -7,5 +7,7 @@ fn coöperation() {} //~ ERROR identifier contains non-ASCII characters fn main() { let naïveté = 2; //~ ERROR identifier contains non-ASCII characters - println!("{}", naïveté); //~ ERROR identifier contains non-ASCII characters + + // using the same identifier the second time won't trigger the lint. + println!("{}", naïveté); } diff --git a/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-non-ascii-idents.stderr b/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-non-ascii-idents.stderr index 6c9f0866c017a..048b6ff5d687f 100644 --- a/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-non-ascii-idents.stderr +++ b/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-non-ascii-idents.stderr @@ -22,11 +22,5 @@ error: identifier contains non-ASCII characters LL | let naïveté = 2; | ^^^^^^^ -error: identifier contains non-ASCII characters - --> $DIR/lint-non-ascii-idents.rs:10:20 - | -LL | println!("{}", naïveté); - | ^^^^^^^ - -error: aborting due to 4 previous errors +error: aborting due to 3 previous errors diff --git a/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-uncommon-codepoints.rs b/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-uncommon-codepoints.rs index 7ac0d035d5bf1..b5e251e047b5a 100644 --- a/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-uncommon-codepoints.rs +++ b/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-uncommon-codepoints.rs @@ -7,5 +7,7 @@ fn dijkstra() {} //~ ERROR identifier contains uncommon Unicode codepoints fn main() { let ㇻㇲㇳ = "rust"; //~ ERROR identifier contains uncommon Unicode codepoints - println!("{}", ㇻㇲㇳ); //~ ERROR identifier contains uncommon Unicode codepoints + + // using the same identifier the second time won't trigger the lint. + println!("{}", ㇻㇲㇳ); } diff --git a/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-uncommon-codepoints.stderr b/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-uncommon-codepoints.stderr index b270bd1f051c2..05ea3d5de7dbc 100644 --- a/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-uncommon-codepoints.stderr +++ b/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-uncommon-codepoints.stderr @@ -22,11 +22,5 @@ error: identifier contains uncommon Unicode codepoints LL | let ㇻㇲㇳ = "rust"; | ^^^^^^ -error: identifier contains uncommon Unicode codepoints - --> $DIR/lint-uncommon-codepoints.rs:10:20 - | -LL | println!("{}", ㇻㇲㇳ); - | ^^^^^^ - -error: aborting due to 4 previous errors +error: aborting due to 3 previous errors diff --git a/src/test/ui/parser/issue-62524.rs b/src/test/ui/parser/issue-62524.rs index 57de4b87b0fe6..5259dfe2e656c 100644 --- a/src/test/ui/parser/issue-62524.rs +++ b/src/test/ui/parser/issue-62524.rs @@ -1,4 +1,6 @@ // ignore-tidy-trailing-newlines // error-pattern: aborting due to 3 previous errors +#![allow(uncommon_codepoints)] + y![ Ϥ, \ No newline at end of file diff --git a/src/test/ui/parser/issue-62524.stderr b/src/test/ui/parser/issue-62524.stderr index 8191c9682cefd..d5e07622b11b9 100644 --- a/src/test/ui/parser/issue-62524.stderr +++ b/src/test/ui/parser/issue-62524.stderr @@ -1,5 +1,5 @@ error: this file contains an unclosed delimiter - --> $DIR/issue-62524.rs:4:3 + --> $DIR/issue-62524.rs:6:3 | LL | y![ | - unclosed delimiter @@ -7,7 +7,7 @@ LL | Ϥ, | ^ error: macros that expand to items must be delimited with braces or followed by a semicolon - --> $DIR/issue-62524.rs:3:3 + --> $DIR/issue-62524.rs:5:3 | LL | y![ | ___^ @@ -24,7 +24,7 @@ LL | Ϥ,; | ^ error: cannot find macro `y` in this scope - --> $DIR/issue-62524.rs:3:1 + --> $DIR/issue-62524.rs:5:1 | LL | y![ | ^ From b65ea1bef1391a3af9118884952718fc905f8e02 Mon Sep 17 00:00:00 2001 From: Tshepang Lekhonkhobe Date: Tue, 23 Jun 2020 00:29:55 +0200 Subject: [PATCH 08/36] place non-obvious defaults on a separate line See https://github.com/rust-lang/rust/pull/73538#discussion_r443809593 for the motivation --- config.toml.example | 40 ++++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/config.toml.example b/config.toml.example index 9ffaac0bbd002..79b06d5cf9962 100644 --- a/config.toml.example +++ b/config.toml.example @@ -118,18 +118,24 @@ # nightlies are already produced for. The current platform must be able to run # binaries of this build triple and the nightly will be used to bootstrap the # first compiler. -#build = "x86_64-unknown-linux-gnu" # defaults to your host platform +# +# Defaults to host platform +#build = "x86_64-unknown-linux-gnu" # In addition to the build triple, other triples to produce full compiler # toolchains for. Each of these triples will be bootstrapped from the build # triple and then will continue to bootstrap themselves. This platform must # currently be able to run all of the triples provided here. -#host = ["x86_64-unknown-linux-gnu"] # defaults to just the build triple +# +# Defaults to just the build triple +#host = ["x86_64-unknown-linux-gnu"] # In addition to all host triples, other triples to produce the standard library # for. Each host triple will be used to produce a copy of the standard library # for each target triple. -#target = ["x86_64-unknown-linux-gnu"] # defaults to just the build triple +# +# Defaults to just the build triple +#target = ["x86_64-unknown-linux-gnu"] # Use this directory to store build artifacts. # You can use "$ROOT" to indicate the root of the git repository. @@ -173,7 +179,9 @@ # Python interpreter to use for various tasks throughout the build, notably # rustdoc tests, the lldb python interpreter, and some dist bits and pieces. -#python = "python" # defaults to the Python interpreter used to execute x.py +# +# Defaults to the Python interpreter used to execute x.py +#python = "python" # Force Cargo to check that Cargo.lock describes the precise dependency # set that all the Cargo.toml files create, instead of updating it. @@ -311,11 +319,15 @@ # Whether or not debug assertions are enabled for the compiler and standard # library. -#debug-assertions = false # defaults to rust.debug value +# +# Defaults to rust.debug value +#debug-assertions = false # Whether or not debug assertions are enabled for the standard library. # Overrides the `debug-assertions` option, if defined. -#debug-assertions-std = false # defaults to rust.debug value +# +# Defaults to rust.debug value +#debug-assertions-std = false # Debuginfo level for most of Rust code, corresponds to the `-C debuginfo=N` option of `rustc`. # `0` - no debug info @@ -324,16 +336,24 @@ # Can be overridden for specific subsets of Rust code (rustc, std or tools). # Debuginfo for tests run with compiletest is not controlled by this option # and needs to be enabled separately with `debuginfo-level-tests`. -#debuginfo-level = 0 # defaults to 2 if debug is true +# +# Defaults to 2 if debug is true +#debuginfo-level = 0 # Debuginfo level for the compiler. -#debuginfo-level-rustc = 0 # defaults to rust.debuginfo-level value +# +# Defaults to rust.debuginfo-level value +#debuginfo-level-rustc = 0 # Debuginfo level for the standard library. -#debuginfo-level-std = 0 # defaults to rust.debuginfo-level value +# +# Defaults to rust.debuginfo-level value +#debuginfo-level-std = 0 # Debuginfo level for the tools. -#debuginfo-level-tools = 0 # defaults to rust.debuginfo-level value +# +# Defaults to rust.debuginfo-level value +#debuginfo-level-tools = 0 # Debuginfo level for the test suites run with compiletest. # FIXME(#61117): Some tests fail when this option is enabled. From 9bb414faffbb5e31ecc41edb78e0047b0b8b1221 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Tue, 23 Jun 2020 16:25:01 -0700 Subject: [PATCH 09/36] Fix ptr doc warnings. --- src/libcore/ptr/const_ptr.rs | 2 -- src/libcore/ptr/mut_ptr.rs | 2 -- 2 files changed, 4 deletions(-) diff --git a/src/libcore/ptr/const_ptr.rs b/src/libcore/ptr/const_ptr.rs index 395b3879cfd0c..64a506a6377f2 100644 --- a/src/libcore/ptr/const_ptr.rs +++ b/src/libcore/ptr/const_ptr.rs @@ -316,7 +316,6 @@ impl *const T { /// differently have not been explored. This method should not be used to introduce such /// differences, and it should also not be stabilized before we have a better understanding /// of this issue. - /// ``` #[unstable(feature = "const_raw_ptr_comparison", issue = "53020")] #[rustc_const_unstable(feature = "const_raw_ptr_comparison", issue = "53020")] #[inline] @@ -349,7 +348,6 @@ impl *const T { /// differently have not been explored. This method should not be used to introduce such /// differences, and it should also not be stabilized before we have a better understanding /// of this issue. - /// ``` #[unstable(feature = "const_raw_ptr_comparison", issue = "53020")] #[rustc_const_unstable(feature = "const_raw_ptr_comparison", issue = "53020")] #[inline] diff --git a/src/libcore/ptr/mut_ptr.rs b/src/libcore/ptr/mut_ptr.rs index b86ef5b13b353..6b5cd9fdb854d 100644 --- a/src/libcore/ptr/mut_ptr.rs +++ b/src/libcore/ptr/mut_ptr.rs @@ -294,7 +294,6 @@ impl *mut T { /// differently have not been explored. This method should not be used to introduce such /// differences, and it should also not be stabilized before we have a better understanding /// of this issue. - /// ``` #[unstable(feature = "const_raw_ptr_comparison", issue = "53020")] #[rustc_const_unstable(feature = "const_raw_ptr_comparison", issue = "53020")] #[inline] @@ -327,7 +326,6 @@ impl *mut T { /// differently have not been explored. This method should not be used to introduce such /// differences, and it should also not be stabilized before we have a better understanding /// of this issue. - /// ``` #[unstable(feature = "const_raw_ptr_comparison", issue = "53020")] #[rustc_const_unstable(feature = "const_raw_ptr_comparison", issue = "53020")] #[inline] From 5aab1a9a88dd30d622a5303f26d9ad213c551473 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 23 Jun 2020 17:32:06 -0700 Subject: [PATCH 10/36] Tweak binop errors * Suggest potentially missing binop trait bound (fix #73416) * Use structured suggestion for dereference in binop --- src/librustc_typeck/check/op.rs | 135 +++++++++++++----- src/test/ui/binary-op-on-double-ref.fixed | 9 ++ src/test/ui/binary-op-on-double-ref.rs | 1 + src/test/ui/binary-op-on-double-ref.stderr | 7 +- src/test/ui/issues/issue-35668.stderr | 6 + src/test/ui/issues/issue-5239-1.stderr | 2 +- .../missing-trait-bound-for-op.fixed | 13 ++ .../suggestions/missing-trait-bound-for-op.rs | 13 ++ .../missing-trait-bound-for-op.stderr | 17 +++ .../trait-resolution-in-overloaded-op.stderr | 6 + 10 files changed, 169 insertions(+), 40 deletions(-) create mode 100644 src/test/ui/binary-op-on-double-ref.fixed create mode 100644 src/test/ui/suggestions/missing-trait-bound-for-op.fixed create mode 100644 src/test/ui/suggestions/missing-trait-bound-for-op.rs create mode 100644 src/test/ui/suggestions/missing-trait-bound-for-op.stderr diff --git a/src/librustc_typeck/check/op.rs b/src/librustc_typeck/check/op.rs index fe50870911647..7acf884318573 100644 --- a/src/librustc_typeck/check/op.rs +++ b/src/librustc_typeck/check/op.rs @@ -9,7 +9,9 @@ use rustc_middle::ty::adjustment::{ Adjust, Adjustment, AllowTwoPhase, AutoBorrow, AutoBorrowMutability, }; use rustc_middle::ty::TyKind::{Adt, Array, Char, FnDef, Never, Ref, Str, Tuple, Uint}; -use rustc_middle::ty::{self, suggest_constraining_type_param, Ty, TyCtxt, TypeFoldable}; +use rustc_middle::ty::{ + self, suggest_constraining_type_param, Ty, TyCtxt, TypeFoldable, TypeVisitor, +}; use rustc_span::symbol::Ident; use rustc_span::Span; use rustc_trait_selection::infer::InferCtxtExt; @@ -254,6 +256,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if !lhs_ty.references_error() && !rhs_ty.references_error() { let source_map = self.tcx.sess.source_map(); + let note = |err: &mut DiagnosticBuilder<'_>, missing_trait| { + err.note(&format!( + "the trait `{}` is not implemented for `{}`", + missing_trait, lhs_ty + )); + }; match is_assign { IsAssign::Yes => { let mut err = struct_span_err!( @@ -286,10 +294,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { rty.peel_refs(), lstring, ); - err.span_suggestion( - lhs_expr.span, + err.span_suggestion_verbose( + lhs_expr.span.shrink_to_lo(), msg, - format!("*{}", lstring), + "*".to_string(), rustc_errors::Applicability::MachineApplicable, ); suggested_deref = true; @@ -310,6 +318,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { _ => None, }; if let Some(missing_trait) = missing_trait { + let mut visitor = TypeParamVisitor(vec![]); + visitor.visit_ty(lhs_ty); + + let mut sugg = false; if op.node == hir::BinOpKind::Add && self.check_str_addition( lhs_expr, rhs_expr, lhs_ty, rhs_ty, &mut err, true, op, @@ -318,18 +330,33 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // This has nothing here because it means we did string // concatenation (e.g., "Hello " += "World!"). This means // we don't want the note in the else clause to be emitted - } else if let ty::Param(p) = lhs_ty.kind { - suggest_constraining_param( - self.tcx, - self.body_id, - &mut err, - lhs_ty, - rhs_ty, - missing_trait, - p, - false, - ); - } else if !suggested_deref { + sugg = true; + } else if let [ty] = &visitor.0[..] { + if let ty::Param(p) = ty.kind { + // FIXME: This *guesses* that constraining the type param + // will make the operation available, but this is only true + // when the corresponding trait has a blanked + // implementation, like the following: + // `impl<'a> PartialEq for &'a [T] where T: PartialEq {}` + // The correct thing to do would be to verify this + // projection would hold. + if *ty != lhs_ty { + note(&mut err, missing_trait); + } + suggest_constraining_param( + self.tcx, + self.body_id, + &mut err, + ty, + rhs_ty, + missing_trait, + p, + false, + ); + sugg = true; + } + } + if !sugg && !suggested_deref { suggest_impl_missing(&mut err, lhs_ty, &missing_trait); } } @@ -458,18 +485,27 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { .is_ok() } { if let Ok(lstring) = source_map.span_to_snippet(lhs_expr.span) { - err.help(&format!( - "`{}` can be used on '{}', you can \ - dereference `{2}`: `*{2}`", - op.node.as_str(), - rty.peel_refs(), - lstring - )); + err.span_suggestion_verbose( + lhs_expr.span.shrink_to_lo(), + &format!( + "`{}` can be used on `{}`, you can dereference \ + `{}`", + op.node.as_str(), + rty.peel_refs(), + lstring, + ), + "*".to_string(), + Applicability::MachineApplicable, + ); suggested_deref = true; } } } if let Some(missing_trait) = missing_trait { + let mut visitor = TypeParamVisitor(vec![]); + visitor.visit_ty(lhs_ty); + + let mut sugg = false; if op.node == hir::BinOpKind::Add && self.check_str_addition( lhs_expr, rhs_expr, lhs_ty, rhs_ty, &mut err, false, op, @@ -478,18 +514,33 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // This has nothing here because it means we did string // concatenation (e.g., "Hello " + "World!"). This means // we don't want the note in the else clause to be emitted - } else if let ty::Param(p) = lhs_ty.kind { - suggest_constraining_param( - self.tcx, - self.body_id, - &mut err, - lhs_ty, - rhs_ty, - missing_trait, - p, - use_output, - ); - } else if !suggested_deref && !involves_fn { + sugg = true; + } else if let [ty] = &visitor.0[..] { + if let ty::Param(p) = ty.kind { + // FIXME: This *guesses* that constraining the type param + // will make the operation available, but this is only true + // when the corresponding trait has a blanked + // implementation, like the following: + // `impl<'a> PartialEq for &'a [T] where T: PartialEq {}` + // The correct thing to do would be to verify this + // projection would hold. + if *ty != lhs_ty { + note(&mut err, missing_trait); + } + suggest_constraining_param( + self.tcx, + self.body_id, + &mut err, + ty, + rhs_ty, + missing_trait, + p, + use_output, + ); + sugg = true; + } + } + if !sugg && !suggested_deref && !involves_fn { suggest_impl_missing(&mut err, lhs_ty, &missing_trait); } } @@ -928,8 +979,7 @@ fn suggest_impl_missing(err: &mut DiagnosticBuilder<'_>, ty: Ty<'_>, missing_tra if let Adt(def, _) = ty.peel_refs().kind { if def.did.is_local() { err.note(&format!( - "an implementation of `{}` might \ - be missing for `{}`", + "an implementation of `{}` might be missing for `{}`", missing_trait, ty )); } @@ -975,3 +1025,14 @@ fn suggest_constraining_param( err.span_label(span, msg); } } + +struct TypeParamVisitor<'tcx>(Vec>); + +impl<'tcx> TypeVisitor<'tcx> for TypeParamVisitor<'tcx> { + fn visit_ty(&mut self, ty: Ty<'tcx>) -> bool { + if let ty::Param(_) = ty.kind { + self.0.push(ty); + } + ty.super_visit_with(self) + } +} diff --git a/src/test/ui/binary-op-on-double-ref.fixed b/src/test/ui/binary-op-on-double-ref.fixed new file mode 100644 index 0000000000000..de9dc19af29be --- /dev/null +++ b/src/test/ui/binary-op-on-double-ref.fixed @@ -0,0 +1,9 @@ +// run-rustfix +fn main() { + let v = vec![1, 2, 3, 4, 5, 6, 7, 8, 9]; + let vr = v.iter().filter(|x| { + *x % 2 == 0 + //~^ ERROR cannot mod `&&{integer}` by `{integer}` + }); + println!("{:?}", vr); +} diff --git a/src/test/ui/binary-op-on-double-ref.rs b/src/test/ui/binary-op-on-double-ref.rs index 67e01b9327db1..2616c560cbefb 100644 --- a/src/test/ui/binary-op-on-double-ref.rs +++ b/src/test/ui/binary-op-on-double-ref.rs @@ -1,3 +1,4 @@ +// run-rustfix fn main() { let v = vec![1, 2, 3, 4, 5, 6, 7, 8, 9]; let vr = v.iter().filter(|x| { diff --git a/src/test/ui/binary-op-on-double-ref.stderr b/src/test/ui/binary-op-on-double-ref.stderr index 6c405333ec681..02b0488488c55 100644 --- a/src/test/ui/binary-op-on-double-ref.stderr +++ b/src/test/ui/binary-op-on-double-ref.stderr @@ -1,12 +1,15 @@ error[E0369]: cannot mod `&&{integer}` by `{integer}` - --> $DIR/binary-op-on-double-ref.rs:4:11 + --> $DIR/binary-op-on-double-ref.rs:5:11 | LL | x % 2 == 0 | - ^ - {integer} | | | &&{integer} | - = help: `%` can be used on '{integer}', you can dereference `x`: `*x` +help: `%` can be used on `{integer}`, you can dereference `x` + | +LL | *x % 2 == 0 + | ^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-35668.stderr b/src/test/ui/issues/issue-35668.stderr index 98e8e6366b99b..c8f1f88a4708e 100644 --- a/src/test/ui/issues/issue-35668.stderr +++ b/src/test/ui/issues/issue-35668.stderr @@ -5,6 +5,12 @@ LL | a.iter().map(|a| a*a) | -^- &T | | | &T + | + = note: the trait `std::ops::Mul` is not implemented for `&T` +help: consider restricting type parameter `T` + | +LL | fn func<'a, T: std::ops::Mul>(a: &'a [T]) -> impl Iterator { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-5239-1.stderr b/src/test/ui/issues/issue-5239-1.stderr index f4f0f17d00199..d0c71e73463c1 100644 --- a/src/test/ui/issues/issue-5239-1.stderr +++ b/src/test/ui/issues/issue-5239-1.stderr @@ -9,7 +9,7 @@ LL | let x = |ref x: isize| { x += 1; }; help: `+=` can be used on 'isize', you can dereference `x` | LL | let x = |ref x: isize| { *x += 1; }; - | ^^ + | ^ error: aborting due to previous error diff --git a/src/test/ui/suggestions/missing-trait-bound-for-op.fixed b/src/test/ui/suggestions/missing-trait-bound-for-op.fixed new file mode 100644 index 0000000000000..02886bb845cf7 --- /dev/null +++ b/src/test/ui/suggestions/missing-trait-bound-for-op.fixed @@ -0,0 +1,13 @@ +// run-rustfix + +pub fn strip_prefix<'a, T: std::cmp::PartialEq>(s: &'a [T], prefix: &[T]) -> Option<&'a [T]> { + let n = prefix.len(); + if n <= s.len() { + let (head, tail) = s.split_at(n); + if head == prefix { //~ ERROR binary operation `==` cannot be applied to type `&[T]` + return Some(tail); + } + } + None +} +fn main() {} diff --git a/src/test/ui/suggestions/missing-trait-bound-for-op.rs b/src/test/ui/suggestions/missing-trait-bound-for-op.rs new file mode 100644 index 0000000000000..aa4ef467360d9 --- /dev/null +++ b/src/test/ui/suggestions/missing-trait-bound-for-op.rs @@ -0,0 +1,13 @@ +// run-rustfix + +pub fn strip_prefix<'a, T>(s: &'a [T], prefix: &[T]) -> Option<&'a [T]> { + let n = prefix.len(); + if n <= s.len() { + let (head, tail) = s.split_at(n); + if head == prefix { //~ ERROR binary operation `==` cannot be applied to type `&[T]` + return Some(tail); + } + } + None +} +fn main() {} diff --git a/src/test/ui/suggestions/missing-trait-bound-for-op.stderr b/src/test/ui/suggestions/missing-trait-bound-for-op.stderr new file mode 100644 index 0000000000000..dab4e575be1fa --- /dev/null +++ b/src/test/ui/suggestions/missing-trait-bound-for-op.stderr @@ -0,0 +1,17 @@ +error[E0369]: binary operation `==` cannot be applied to type `&[T]` + --> $DIR/missing-trait-bound-for-op.rs:7:17 + | +LL | if head == prefix { + | ---- ^^ ------ &[T] + | | + | &[T] + | + = note: the trait `std::cmp::PartialEq` is not implemented for `&[T]` +help: consider restricting type parameter `T` + | +LL | pub fn strip_prefix<'a, T: std::cmp::PartialEq>(s: &'a [T], prefix: &[T]) -> Option<&'a [T]> { + | ^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0369`. diff --git a/src/test/ui/traits/trait-resolution-in-overloaded-op.stderr b/src/test/ui/traits/trait-resolution-in-overloaded-op.stderr index 29216f36f5f31..1c424ce7da669 100644 --- a/src/test/ui/traits/trait-resolution-in-overloaded-op.stderr +++ b/src/test/ui/traits/trait-resolution-in-overloaded-op.stderr @@ -5,6 +5,12 @@ LL | a * b | - ^ - f64 | | | &T + | + = note: the trait `std::ops::Mul` is not implemented for `&T` +help: consider further restricting this bound + | +LL | fn foo + std::ops::Mul>(a: &T, b: f64) -> f64 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to previous error From b23baa78dc96cd12a70556de6bc472af3ef2e302 Mon Sep 17 00:00:00 2001 From: Tshepang Lekhonkhobe Date: Wed, 24 Jun 2020 04:47:20 +0200 Subject: [PATCH 11/36] fix See https://github.com/rust-lang/rust/pull/73538/commits/b65ea1bef1391a3af9118884952718fc905f8e02#r444376289 --- config.toml.example | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.toml.example b/config.toml.example index 79b06d5cf9962..c0be7dded17bc 100644 --- a/config.toml.example +++ b/config.toml.example @@ -326,7 +326,7 @@ # Whether or not debug assertions are enabled for the standard library. # Overrides the `debug-assertions` option, if defined. # -# Defaults to rust.debug value +# Defaults to rust.debug-assertions value #debug-assertions-std = false # Debuginfo level for most of Rust code, corresponds to the `-C debuginfo=N` option of `rustc`. From d8ea10c95fb5cf1674666fe4eae10ed761e2bfaa Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Wed, 24 Jun 2020 09:23:55 +0200 Subject: [PATCH 12/36] Document the return keyword Apply suggestions from code review Co-authored-by: Josh Triplett --- src/libstd/keyword_docs.rs | 50 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 2 deletions(-) diff --git a/src/libstd/keyword_docs.rs b/src/libstd/keyword_docs.rs index a4996d9eee810..b2658529e57cf 100644 --- a/src/libstd/keyword_docs.rs +++ b/src/libstd/keyword_docs.rs @@ -1000,9 +1000,55 @@ mod ref_keyword {} // /// Return a value from a function. /// -/// The documentation for this keyword is [not yet complete]. Pull requests welcome! +/// A `return` marks the end of an execution path in a function: /// -/// [not yet complete]: https://github.com/rust-lang/rust/issues/34601 +/// ``` +/// fn foo() -> i32 { +/// return 3; +/// } +/// assert_eq!(foo(), 3); +/// ``` +/// +/// `return` is not needed when the returned value is the last expression in the +/// function. In this case the `;` is omitted: +/// +/// ``` +/// fn foo() -> i32 { +/// 3 +/// } +/// assert_eq!(foo(), 3); +/// ``` +/// +/// `return` returns from the function immediately (an "early return"): +/// +/// ```no_run +/// use std::fs::File; +/// use std::io::{Error, ErrorKind, Read, Result}; +/// +/// fn main() -> Result<()> { +/// let mut file = match File::open("foo.txt") { +/// Ok(f) => f, +/// Err(e) => return Err(e), +/// }; +/// +/// let mut contents = String::new(); +/// let size = match file.read_to_string(&mut contents) { +/// Ok(s) => s, +/// Err(e) => return Err(e), +/// }; +/// +/// if contents.contains("impossible!") { +/// return Err(Error::new(ErrorKind::Other, "oh no!")); +/// } +/// +/// if size > 9000 { +/// return Err(Error::new(ErrorKind::Other, "over 9000!")); +/// } +/// +/// assert_eq!(contents, "Hello, world!"); +/// Ok(()) +/// } +/// ``` mod return_keyword {} #[doc(keyword = "self")] From 65becefd4ac378c9c6aa0e7becb7eaa1fc12b84f Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 24 Jun 2020 13:19:06 +0200 Subject: [PATCH 13/36] Clean up E0701 explanation --- src/librustc_error_codes/error_codes/E0701.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_error_codes/error_codes/E0701.md b/src/librustc_error_codes/error_codes/E0701.md index 87f416ada1803..4965e64310591 100644 --- a/src/librustc_error_codes/error_codes/E0701.md +++ b/src/librustc_error_codes/error_codes/E0701.md @@ -1,7 +1,7 @@ This error indicates that a `#[non_exhaustive]` attribute was incorrectly placed on something other than a struct or enum. -Examples of erroneous code: +Erroneous code example: ```compile_fail,E0701 #[non_exhaustive] From d36d351afcb439144621159d8642892cde26eff6 Mon Sep 17 00:00:00 2001 From: Nathan Corbyn Date: Tue, 23 Jun 2020 16:46:24 +0100 Subject: [PATCH 14/36] Implement intrinsic --- src/libcore/intrinsics.rs | 15 ++++++++ src/libcore/lib.rs | 1 + src/libcore/mem/mod.rs | 27 +++++++++++++ src/librustc_codegen_llvm/intrinsic.rs | 2 +- src/librustc_mir/interpret/intrinsics.rs | 14 ++++++- src/librustc_span/symbol.rs | 1 + src/librustc_typeck/check/intrinsic.rs | 6 ++- src/test/ui/consts/const-variant-count.rs | 46 +++++++++++++++++++++++ 8 files changed, 107 insertions(+), 5 deletions(-) create mode 100644 src/test/ui/consts/const-variant-count.rs diff --git a/src/libcore/intrinsics.rs b/src/libcore/intrinsics.rs index 50e321f9c7158..4074087e1e67f 100644 --- a/src/libcore/intrinsics.rs +++ b/src/libcore/intrinsics.rs @@ -1917,6 +1917,15 @@ extern "rust-intrinsic" { #[rustc_const_unstable(feature = "const_discriminant", issue = "69821")] pub fn discriminant_value(v: &T) -> ::Discriminant; + /// Returns the number of variants of the type `T` cast to a `usize`; + /// if `T` has no variants, returns 0. Uninhabited variants will be counted. + /// + /// The to-be-stabilized version of this intrinsic is + /// [`std::mem::variant_count`](../../std/mem/fn.variant_count.html) + #[rustc_const_unstable(feature = "variant_count", issue = "73662")] + #[cfg(not(bootstrap))] + pub fn variant_count() -> usize; + /// Rust's "try catch" construct which invokes the function pointer `try_fn` /// with the data pointer `data`. /// @@ -1960,6 +1969,12 @@ extern "rust-intrinsic" { pub fn ptr_guaranteed_ne(ptr: *const T, other: *const T) -> bool; } +#[rustc_const_unstable(feature = "variant_count", issue = "73662")] +#[cfg(bootstrap)] +pub const fn variant_count() -> usize { + 0 +} + // Some functions are defined here because they accidentally got made // available in this module on stable. See . // (`transmute` also falls into this category, but it cannot be wrapped due to the diff --git a/src/libcore/lib.rs b/src/libcore/lib.rs index 4eb2fdbd07868..2b26e5303a89c 100644 --- a/src/libcore/lib.rs +++ b/src/libcore/lib.rs @@ -124,6 +124,7 @@ #![feature(unsized_locals)] #![feature(untagged_unions)] #![feature(unwind_attributes)] +#![feature(variant_count)] #![feature(doc_alias)] #![feature(mmx_target_feature)] #![feature(tbm_target_feature)] diff --git a/src/libcore/mem/mod.rs b/src/libcore/mem/mod.rs index 066bb8b3dc787..d041571f26079 100644 --- a/src/libcore/mem/mod.rs +++ b/src/libcore/mem/mod.rs @@ -999,3 +999,30 @@ impl fmt::Debug for Discriminant { pub const fn discriminant(v: &T) -> Discriminant { Discriminant(intrinsics::discriminant_value(v)) } + +/// Returns the number of variants in the enum type `T`. +/// +/// If `T` is not an enum, calling this function will not result in undefined behavior, but the +/// return value is unspecified. Equally, if `T` is an enum with more variants than `usize::MAX` +/// the return value is unspecified. Uninhabited variants will be counted. +/// +/// # Examples +/// +/// ``` +/// use std::mem; +/// +/// enum Void {} +/// enum Foo { A(&'static str), B(i32), C(i32) } +/// +/// assert_eq!(mem::variant_count::(), 0); +/// assert_eq!(mem::variant_count::(), 3); +/// +/// assert_eq!(mem::variant_count::>(), 2); +/// assert_eq!(mem::variant_count::>(), 2); +/// ``` +#[inline(always)] +#[unstable(feature = "variant_count", issue = "73662")] +#[rustc_const_unstable(feature = "variant_count", issue = "73662")] +pub const fn variant_count() -> usize { + intrinsics::variant_count::() +} diff --git a/src/librustc_codegen_llvm/intrinsic.rs b/src/librustc_codegen_llvm/intrinsic.rs index 0a8525f06fa3d..e43c814a6125e 100644 --- a/src/librustc_codegen_llvm/intrinsic.rs +++ b/src/librustc_codegen_llvm/intrinsic.rs @@ -213,7 +213,7 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> { } } "size_of" | "pref_align_of" | "min_align_of" | "needs_drop" | "type_id" - | "type_name" => { + | "type_name" | "variant_count" => { let value = self .tcx .const_eval_instance(ty::ParamEnv::reveal_all(), instance, None) diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs index 6ac1e6be03674..88ba28dab82e1 100644 --- a/src/librustc_mir/interpret/intrinsics.rs +++ b/src/librustc_mir/interpret/intrinsics.rs @@ -69,6 +69,13 @@ crate fn eval_nullary_intrinsic<'tcx>( ConstValue::from_machine_usize(n, &tcx) } sym::type_id => ConstValue::from_u64(tcx.type_id_hash(tp_ty)), + sym::variant_count => { + if let ty::Adt(ref adt, _) = tp_ty.kind { + ConstValue::from_machine_usize(adt.variants.len() as u64, &tcx) + } else { + ConstValue::from_machine_usize(0u64, &tcx) + } + } other => bug!("`{}` is not a zero arg intrinsic", other), }) } @@ -109,10 +116,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { | sym::needs_drop | sym::size_of | sym::type_id - | sym::type_name => { + | sym::type_name + | sym::variant_count => { let gid = GlobalId { instance, promoted: None }; let ty = match intrinsic_name { - sym::min_align_of | sym::pref_align_of | sym::size_of => self.tcx.types.usize, + sym::min_align_of | sym::pref_align_of | sym::size_of | sym::variant_count => { + self.tcx.types.usize + } sym::needs_drop => self.tcx.types.bool, sym::type_id => self.tcx.types.u64, sym::type_name => self.tcx.mk_static_str(), diff --git a/src/librustc_span/symbol.rs b/src/librustc_span/symbol.rs index 06d1f36622b94..07b05d37ed303 100644 --- a/src/librustc_span/symbol.rs +++ b/src/librustc_span/symbol.rs @@ -830,6 +830,7 @@ symbols! { v1, val, var, + variant_count, vec, Vec, version, diff --git a/src/librustc_typeck/check/intrinsic.rs b/src/librustc_typeck/check/intrinsic.rs index ef6c7c14404a7..1c0b22ca7370b 100644 --- a/src/librustc_typeck/check/intrinsic.rs +++ b/src/librustc_typeck/check/intrinsic.rs @@ -75,7 +75,7 @@ pub fn intrinsic_operation_unsafety(intrinsic: &str) -> hir::Unsafety { | "saturating_sub" | "rotate_left" | "rotate_right" | "ctpop" | "ctlz" | "cttz" | "bswap" | "bitreverse" | "discriminant_value" | "type_id" | "likely" | "unlikely" | "ptr_guaranteed_eq" | "ptr_guaranteed_ne" | "minnumf32" | "minnumf64" | "maxnumf32" - | "maxnumf64" | "type_name" => hir::Unsafety::Normal, + | "maxnumf64" | "type_name" | "variant_count" => hir::Unsafety::Normal, _ => hir::Unsafety::Unsafe, } } @@ -137,7 +137,9 @@ pub fn check_intrinsic_type(tcx: TyCtxt<'_>, it: &hir::ForeignItem<'_>) { let unsafety = intrinsic_operation_unsafety(&name[..]); let (n_tps, inputs, output) = match &name[..] { "breakpoint" => (0, Vec::new(), tcx.mk_unit()), - "size_of" | "pref_align_of" | "min_align_of" => (1, Vec::new(), tcx.types.usize), + "size_of" | "pref_align_of" | "min_align_of" | "variant_count" => { + (1, Vec::new(), tcx.types.usize) + } "size_of_val" | "min_align_of_val" => { (1, vec![tcx.mk_imm_ptr(param(0))], tcx.types.usize) } diff --git a/src/test/ui/consts/const-variant-count.rs b/src/test/ui/consts/const-variant-count.rs new file mode 100644 index 0000000000000..f21f6e9c29168 --- /dev/null +++ b/src/test/ui/consts/const-variant-count.rs @@ -0,0 +1,46 @@ +// run-pass +#![allow(dead_code)] +#![feature(variant_count)] + +use std::mem::variant_count; + +enum Void {} + +enum Foo { + A, + B, + C, +} + +enum Bar { + A, + B, + C, + D(usize), + E { field_1: usize, field_2: Foo }, +} + +struct Baz { + a: u32, + b: *const u8, +} + +const TEST_VOID: usize = variant_count::(); +const TEST_FOO: usize = variant_count::(); +const TEST_BAR: usize = variant_count::(); + +const NO_ICE_STRUCT: usize = variant_count::(); +const NO_ICE_BOOL: usize = variant_count::(); +const NO_ICE_PRIM: usize = variant_count::<*const u8>(); + +fn main() { + assert_eq!(TEST_VOID, 0); + assert_eq!(TEST_FOO, 3); + assert_eq!(TEST_BAR, 5); + assert_eq!(variant_count::(), 0); + assert_eq!(variant_count::(), 3); + assert_eq!(variant_count::(), 5); + assert_eq!(variant_count::>(), 2); + assert_eq!(variant_count::>(), 2); + assert_eq!(variant_count::>(), 2); +} From c2dfc25c0eeb74d6c723312d612cd6899058971c Mon Sep 17 00:00:00 2001 From: Nathan Corbyn Date: Wed, 24 Jun 2020 15:10:10 +0100 Subject: [PATCH 15/36] Fix tests --- src/libcore/mem/mod.rs | 2 ++ src/test/ui/consts/const-variant-count.rs | 1 + 2 files changed, 3 insertions(+) diff --git a/src/libcore/mem/mod.rs b/src/libcore/mem/mod.rs index d041571f26079..2be105e68eb79 100644 --- a/src/libcore/mem/mod.rs +++ b/src/libcore/mem/mod.rs @@ -1009,6 +1009,8 @@ pub const fn discriminant(v: &T) -> Discriminant { /// # Examples /// /// ``` +/// # #![feature(never_type)] +/// /// use std::mem; /// /// enum Void {} diff --git a/src/test/ui/consts/const-variant-count.rs b/src/test/ui/consts/const-variant-count.rs index f21f6e9c29168..455419d2c7f1d 100644 --- a/src/test/ui/consts/const-variant-count.rs +++ b/src/test/ui/consts/const-variant-count.rs @@ -1,6 +1,7 @@ // run-pass #![allow(dead_code)] #![feature(variant_count)] +#![feature(never_type)] use std::mem::variant_count; From 493199626baf8a0a8f6d3a19a089165d18b3f1fb Mon Sep 17 00:00:00 2001 From: Nathan Corbyn Date: Wed, 24 Jun 2020 15:36:04 +0100 Subject: [PATCH 16/36] Fix tests --- src/libcore/mem/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libcore/mem/mod.rs b/src/libcore/mem/mod.rs index 2be105e68eb79..1bd7ae3a34ebb 100644 --- a/src/libcore/mem/mod.rs +++ b/src/libcore/mem/mod.rs @@ -1010,6 +1010,7 @@ pub const fn discriminant(v: &T) -> Discriminant { /// /// ``` /// # #![feature(never_type)] +/// # #![feature(variant_count)] /// /// use std::mem; /// From 771a1d8e0ad4e6702aaab91cb5e58adbf3ec3708 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20S=C3=A1nchez=20Mu=C3=B1oz?= Date: Wed, 24 Jun 2020 18:17:27 +0200 Subject: [PATCH 17/36] Make `std::panicking::panic_count::is_zero` inline and move the slow path into a separate cold function. --- src/libstd/panicking.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/libstd/panicking.rs b/src/libstd/panicking.rs index 46196960e718b..9a5d5e2b5ae9f 100644 --- a/src/libstd/panicking.rs +++ b/src/libstd/panicking.rs @@ -258,6 +258,7 @@ pub mod panic_count { LOCAL_PANIC_COUNT.with(|c| c.get()) } + #[inline] pub fn is_zero() -> bool { if GLOBAL_PANIC_COUNT.load(Ordering::Relaxed) == 0 { // Fast path: if `GLOBAL_PANIC_COUNT` is zero, all threads @@ -265,9 +266,17 @@ pub mod panic_count { // equal to zero, so TLS access can be avoided. true } else { - LOCAL_PANIC_COUNT.with(|c| c.get() == 0) + is_zero_slow_path() } } + + // Slow path is in a separate function to reduce the amount of code + // inlined from `is_zero`. + #[inline(never)] + #[cold] + fn is_zero_slow_path() -> bool { + LOCAL_PANIC_COUNT.with(|c| c.get() == 0) + } } #[cfg(test)] @@ -350,6 +359,7 @@ pub unsafe fn r#try R>(f: F) -> Result> } /// Determines whether the current thread is unwinding because of panic. +#[inline] pub fn panicking() -> bool { !panic_count::is_zero() } From 09af1845d7b43c0a17c1b98f6be92c2c0fbc202b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 24 Jun 2020 14:16:41 -0700 Subject: [PATCH 18/36] review comments: clean up code * deduplicate logic * fix typos * remove unnecessary state --- src/librustc_typeck/check/method/mod.rs | 3 +- src/librustc_typeck/check/method/probe.rs | 4 +- src/librustc_typeck/check/op.rs | 502 +++++++++------------- src/test/ui/issues/issue-5239-1.stderr | 2 +- 4 files changed, 214 insertions(+), 297 deletions(-) diff --git a/src/librustc_typeck/check/method/mod.rs b/src/librustc_typeck/check/method/mod.rs index ac3fa15417e9c..ad980fd2f1215 100644 --- a/src/librustc_typeck/check/method/mod.rs +++ b/src/librustc_typeck/check/method/mod.rs @@ -296,8 +296,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { opt_input_types: Option<&[Ty<'tcx>]>, ) -> Option>> { debug!( - "lookup_in_trait_adjusted(self_ty={:?}, \ - m_name={}, trait_def_id={:?})", + "lookup_in_trait_adjusted(self_ty={:?}, m_name={}, trait_def_id={:?})", self_ty, m_name, trait_def_id ); diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs index 93bcd5cf29149..093f76be00865 100644 --- a/src/librustc_typeck/check/method/probe.rs +++ b/src/librustc_typeck/check/method/probe.rs @@ -379,8 +379,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.tcx.sess, span, E0699, - "the type of this value must be known \ - to call a method on a raw pointer on it" + "the type of this value must be known to call a method on a raw pointer on \ + it" ) .emit(); } else { diff --git a/src/librustc_typeck/check/op.rs b/src/librustc_typeck/check/op.rs index 7acf884318573..0184c00c475b5 100644 --- a/src/librustc_typeck/check/op.rs +++ b/src/librustc_typeck/check/op.rs @@ -251,303 +251,222 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { method.sig.output() } + // error types are considered "builtin" + Err(()) if lhs_ty.references_error() || rhs_ty.references_error() => { + self.tcx.ty_error() + } Err(()) => { - // error types are considered "builtin" - if !lhs_ty.references_error() && !rhs_ty.references_error() { - let source_map = self.tcx.sess.source_map(); - - let note = |err: &mut DiagnosticBuilder<'_>, missing_trait| { - err.note(&format!( - "the trait `{}` is not implemented for `{}`", - missing_trait, lhs_ty - )); - }; - match is_assign { - IsAssign::Yes => { - let mut err = struct_span_err!( - self.tcx.sess, - expr.span, - E0368, - "binary assignment operation `{}=` cannot be applied to type `{}`", - op.node.as_str(), + let source_map = self.tcx.sess.source_map(); + let (mut err, missing_trait, use_output, involves_fn) = match is_assign { + IsAssign::Yes => { + let mut err = struct_span_err!( + self.tcx.sess, + expr.span, + E0368, + "binary assignment operation `{}=` cannot be applied to type `{}`", + op.node.as_str(), + lhs_ty, + ); + err.span_label( + lhs_expr.span, + format!("cannot use `{}=` on type `{}`", op.node.as_str(), lhs_ty), + ); + let missing_trait = match op.node { + hir::BinOpKind::Add => Some("std::ops::AddAssign"), + hir::BinOpKind::Sub => Some("std::ops::SubAssign"), + hir::BinOpKind::Mul => Some("std::ops::MulAssign"), + hir::BinOpKind::Div => Some("std::ops::DivAssign"), + hir::BinOpKind::Rem => Some("std::ops::RemAssign"), + hir::BinOpKind::BitAnd => Some("std::ops::BitAndAssign"), + hir::BinOpKind::BitXor => Some("std::ops::BitXorAssign"), + hir::BinOpKind::BitOr => Some("std::ops::BitOrAssign"), + hir::BinOpKind::Shl => Some("std::ops::ShlAssign"), + hir::BinOpKind::Shr => Some("std::ops::ShrAssign"), + _ => None, + }; + (err, missing_trait, false, false) + } + IsAssign::No => { + let (message, missing_trait, use_output) = match op.node { + hir::BinOpKind::Add => ( + format!("cannot add `{}` to `{}`", rhs_ty, lhs_ty), + Some("std::ops::Add"), + true, + ), + hir::BinOpKind::Sub => ( + format!("cannot subtract `{}` from `{}`", rhs_ty, lhs_ty), + Some("std::ops::Sub"), + true, + ), + hir::BinOpKind::Mul => ( + format!("cannot multiply `{}` to `{}`", rhs_ty, lhs_ty), + Some("std::ops::Mul"), + true, + ), + hir::BinOpKind::Div => ( + format!("cannot divide `{}` by `{}`", lhs_ty, rhs_ty), + Some("std::ops::Div"), + true, + ), + hir::BinOpKind::Rem => ( + format!("cannot mod `{}` by `{}`", lhs_ty, rhs_ty), + Some("std::ops::Rem"), + true, + ), + hir::BinOpKind::BitAnd => ( + format!("no implementation for `{} & {}`", lhs_ty, rhs_ty), + Some("std::ops::BitAnd"), + true, + ), + hir::BinOpKind::BitXor => ( + format!("no implementation for `{} ^ {}`", lhs_ty, rhs_ty), + Some("std::ops::BitXor"), + true, + ), + hir::BinOpKind::BitOr => ( + format!("no implementation for `{} | {}`", lhs_ty, rhs_ty), + Some("std::ops::BitOr"), + true, + ), + hir::BinOpKind::Shl => ( + format!("no implementation for `{} << {}`", lhs_ty, rhs_ty), + Some("std::ops::Shl"), + true, + ), + hir::BinOpKind::Shr => ( + format!("no implementation for `{} >> {}`", lhs_ty, rhs_ty), + Some("std::ops::Shr"), + true, + ), + hir::BinOpKind::Eq | hir::BinOpKind::Ne => ( + format!( + "binary operation `{}` cannot be applied to type `{}`", + op.node.as_str(), + lhs_ty + ), + Some("std::cmp::PartialEq"), + false, + ), + hir::BinOpKind::Lt + | hir::BinOpKind::Le + | hir::BinOpKind::Gt + | hir::BinOpKind::Ge => ( + format!( + "binary operation `{}` cannot be applied to type `{}`", + op.node.as_str(), + lhs_ty + ), + Some("std::cmp::PartialOrd"), + false, + ), + _ => ( + format!( + "binary operation `{}` cannot be applied to type `{}`", + op.node.as_str(), + lhs_ty + ), + None, + false, + ), + }; + let mut err = + struct_span_err!(self.tcx.sess, op.span, E0369, "{}", message.as_str()); + let mut involves_fn = false; + if !lhs_expr.span.eq(&rhs_expr.span) { + involves_fn |= self.add_type_neq_err_label( + &mut err, + lhs_expr.span, lhs_ty, + rhs_ty, + op, + is_assign, ); - err.span_label( - lhs_expr.span, - format!("cannot use `{}=` on type `{}`", op.node.as_str(), lhs_ty), + involves_fn |= self.add_type_neq_err_label( + &mut err, + rhs_expr.span, + rhs_ty, + lhs_ty, + op, + is_assign, ); - let mut suggested_deref = false; - if let Ref(_, rty, _) = lhs_ty.kind { - if { - self.infcx.type_is_copy_modulo_regions( - self.param_env, - rty, - lhs_expr.span, - ) && self - .lookup_op_method(rty, &[rhs_ty], Op::Binary(op, is_assign)) - .is_ok() - } { - if let Ok(lstring) = source_map.span_to_snippet(lhs_expr.span) { - let msg = &format!( - "`{}=` can be used on '{}', you can dereference `{}`", - op.node.as_str(), - rty.peel_refs(), - lstring, - ); - err.span_suggestion_verbose( - lhs_expr.span.shrink_to_lo(), - msg, - "*".to_string(), - rustc_errors::Applicability::MachineApplicable, - ); - suggested_deref = true; - } - } - } - let missing_trait = match op.node { - hir::BinOpKind::Add => Some("std::ops::AddAssign"), - hir::BinOpKind::Sub => Some("std::ops::SubAssign"), - hir::BinOpKind::Mul => Some("std::ops::MulAssign"), - hir::BinOpKind::Div => Some("std::ops::DivAssign"), - hir::BinOpKind::Rem => Some("std::ops::RemAssign"), - hir::BinOpKind::BitAnd => Some("std::ops::BitAndAssign"), - hir::BinOpKind::BitXor => Some("std::ops::BitXorAssign"), - hir::BinOpKind::BitOr => Some("std::ops::BitOrAssign"), - hir::BinOpKind::Shl => Some("std::ops::ShlAssign"), - hir::BinOpKind::Shr => Some("std::ops::ShrAssign"), - _ => None, - }; - if let Some(missing_trait) = missing_trait { - let mut visitor = TypeParamVisitor(vec![]); - visitor.visit_ty(lhs_ty); - - let mut sugg = false; - if op.node == hir::BinOpKind::Add - && self.check_str_addition( - lhs_expr, rhs_expr, lhs_ty, rhs_ty, &mut err, true, op, - ) - { - // This has nothing here because it means we did string - // concatenation (e.g., "Hello " += "World!"). This means - // we don't want the note in the else clause to be emitted - sugg = true; - } else if let [ty] = &visitor.0[..] { - if let ty::Param(p) = ty.kind { - // FIXME: This *guesses* that constraining the type param - // will make the operation available, but this is only true - // when the corresponding trait has a blanked - // implementation, like the following: - // `impl<'a> PartialEq for &'a [T] where T: PartialEq {}` - // The correct thing to do would be to verify this - // projection would hold. - if *ty != lhs_ty { - note(&mut err, missing_trait); - } - suggest_constraining_param( - self.tcx, - self.body_id, - &mut err, - ty, - rhs_ty, - missing_trait, - p, - false, - ); - sugg = true; - } - } - if !sugg && !suggested_deref { - suggest_impl_missing(&mut err, lhs_ty, &missing_trait); - } - } - err.emit(); } - IsAssign::No => { - let (message, missing_trait, use_output) = match op.node { - hir::BinOpKind::Add => ( - format!("cannot add `{}` to `{}`", rhs_ty, lhs_ty), - Some("std::ops::Add"), - true, - ), - hir::BinOpKind::Sub => ( - format!("cannot subtract `{}` from `{}`", rhs_ty, lhs_ty), - Some("std::ops::Sub"), - true, - ), - hir::BinOpKind::Mul => ( - format!("cannot multiply `{}` to `{}`", rhs_ty, lhs_ty), - Some("std::ops::Mul"), - true, - ), - hir::BinOpKind::Div => ( - format!("cannot divide `{}` by `{}`", lhs_ty, rhs_ty), - Some("std::ops::Div"), - true, - ), - hir::BinOpKind::Rem => ( - format!("cannot mod `{}` by `{}`", lhs_ty, rhs_ty), - Some("std::ops::Rem"), - true, - ), - hir::BinOpKind::BitAnd => ( - format!("no implementation for `{} & {}`", lhs_ty, rhs_ty), - Some("std::ops::BitAnd"), - true, - ), - hir::BinOpKind::BitXor => ( - format!("no implementation for `{} ^ {}`", lhs_ty, rhs_ty), - Some("std::ops::BitXor"), - true, - ), - hir::BinOpKind::BitOr => ( - format!("no implementation for `{} | {}`", lhs_ty, rhs_ty), - Some("std::ops::BitOr"), - true, - ), - hir::BinOpKind::Shl => ( - format!("no implementation for `{} << {}`", lhs_ty, rhs_ty), - Some("std::ops::Shl"), - true, - ), - hir::BinOpKind::Shr => ( - format!("no implementation for `{} >> {}`", lhs_ty, rhs_ty), - Some("std::ops::Shr"), - true, - ), - hir::BinOpKind::Eq | hir::BinOpKind::Ne => ( - format!( - "binary operation `{}` cannot be applied to type `{}`", - op.node.as_str(), - lhs_ty - ), - Some("std::cmp::PartialEq"), - false, - ), - hir::BinOpKind::Lt - | hir::BinOpKind::Le - | hir::BinOpKind::Gt - | hir::BinOpKind::Ge => ( - format!( - "binary operation `{}` cannot be applied to type `{}`", - op.node.as_str(), - lhs_ty - ), - Some("std::cmp::PartialOrd"), - false, - ), - _ => ( - format!( - "binary operation `{}` cannot be applied to type `{}`", - op.node.as_str(), - lhs_ty - ), - None, - false, - ), - }; - let mut err = struct_span_err!( - self.tcx.sess, - op.span, - E0369, - "{}", - message.as_str() + (err, missing_trait, use_output, involves_fn) + } + }; + let mut suggested_deref = false; + if let Ref(_, rty, _) = lhs_ty.kind { + if { + self.infcx.type_is_copy_modulo_regions(self.param_env, rty, lhs_expr.span) + && self + .lookup_op_method(rty, &[rhs_ty], Op::Binary(op, is_assign)) + .is_ok() + } { + if let Ok(lstring) = source_map.span_to_snippet(lhs_expr.span) { + let msg = &format!( + "`{}{}` can be used on `{}`, you can dereference `{}`", + op.node.as_str(), + match is_assign { + IsAssign::Yes => "=", + IsAssign::No => "", + }, + rty.peel_refs(), + lstring, ); - - let mut involves_fn = false; - if !lhs_expr.span.eq(&rhs_expr.span) { - involves_fn |= self.add_type_neq_err_label( - &mut err, - lhs_expr.span, - lhs_ty, - rhs_ty, - op, - is_assign, - ); - involves_fn |= self.add_type_neq_err_label( - &mut err, - rhs_expr.span, - rhs_ty, - lhs_ty, - op, - is_assign, - ); - } - - let mut suggested_deref = false; - if let Ref(_, rty, _) = lhs_ty.kind { - if { - self.infcx.type_is_copy_modulo_regions( - self.param_env, - rty, - lhs_expr.span, - ) && self - .lookup_op_method(rty, &[rhs_ty], Op::Binary(op, is_assign)) - .is_ok() - } { - if let Ok(lstring) = source_map.span_to_snippet(lhs_expr.span) { - err.span_suggestion_verbose( - lhs_expr.span.shrink_to_lo(), - &format!( - "`{}` can be used on `{}`, you can dereference \ - `{}`", - op.node.as_str(), - rty.peel_refs(), - lstring, - ), - "*".to_string(), - Applicability::MachineApplicable, - ); - suggested_deref = true; - } - } - } - if let Some(missing_trait) = missing_trait { - let mut visitor = TypeParamVisitor(vec![]); - visitor.visit_ty(lhs_ty); - - let mut sugg = false; - if op.node == hir::BinOpKind::Add - && self.check_str_addition( - lhs_expr, rhs_expr, lhs_ty, rhs_ty, &mut err, false, op, - ) - { - // This has nothing here because it means we did string - // concatenation (e.g., "Hello " + "World!"). This means - // we don't want the note in the else clause to be emitted - sugg = true; - } else if let [ty] = &visitor.0[..] { - if let ty::Param(p) = ty.kind { - // FIXME: This *guesses* that constraining the type param - // will make the operation available, but this is only true - // when the corresponding trait has a blanked - // implementation, like the following: - // `impl<'a> PartialEq for &'a [T] where T: PartialEq {}` - // The correct thing to do would be to verify this - // projection would hold. - if *ty != lhs_ty { - note(&mut err, missing_trait); - } - suggest_constraining_param( - self.tcx, - self.body_id, - &mut err, - ty, - rhs_ty, - missing_trait, - p, - use_output, - ); - sugg = true; - } - } - if !sugg && !suggested_deref && !involves_fn { - suggest_impl_missing(&mut err, lhs_ty, &missing_trait); - } + err.span_suggestion_verbose( + lhs_expr.span.shrink_to_lo(), + msg, + "*".to_string(), + rustc_errors::Applicability::MachineApplicable, + ); + suggested_deref = true; + } + } + } + if let Some(missing_trait) = missing_trait { + let mut visitor = TypeParamVisitor(vec![]); + visitor.visit_ty(lhs_ty); + + if op.node == hir::BinOpKind::Add + && self.check_str_addition( + lhs_expr, rhs_expr, lhs_ty, rhs_ty, &mut err, is_assign, op, + ) + { + // This has nothing here because it means we did string + // concatenation (e.g., "Hello " + "World!"). This means + // we don't want the note in the else clause to be emitted + } else if let [ty] = &visitor.0[..] { + if let ty::Param(p) = ty.kind { + // FIXME: This *guesses* that constraining the type param + // will make the operation available, but this is only true + // when the corresponding trait has a blanket + // implementation, like the following: + // `impl<'a> PartialEq for &'a [T] where T: PartialEq {}` + // The correct thing to do would be to verify this + // projection would hold. + if *ty != lhs_ty { + err.note(&format!( + "the trait `{}` is not implemented for `{}`", + missing_trait, lhs_ty + )); } - err.emit(); + suggest_constraining_param( + self.tcx, + self.body_id, + &mut err, + ty, + rhs_ty, + missing_trait, + p, + use_output, + ); + } else { + bug!("type param visitor stored a non type param: {:?}", ty.kind); } + } else if !suggested_deref && !involves_fn { + suggest_impl_missing(&mut err, lhs_ty, &missing_trait); } } + err.emit(); self.tcx.ty_error() } }; @@ -621,7 +540,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { lhs_ty: Ty<'tcx>, rhs_ty: Ty<'tcx>, err: &mut rustc_errors::DiagnosticBuilder<'_>, - is_assign: bool, + is_assign: IsAssign, op: hir::BinOp, ) -> bool { let source_map = self.tcx.sess.source_map(); @@ -644,7 +563,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { &format!("{:?}", rhs_ty) == "&&str" ) => { - if !is_assign { // Do not supply this message if `&str += &str` + if let IsAssign::No = is_assign { // Do not supply this message if `&str += &str` err.span_label( op.span, "`+` cannot be used to concatenate two `&str` strings", @@ -685,7 +604,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { source_map.span_to_snippet(rhs_expr.span), is_assign, ) { - (Ok(l), Ok(r), false) => { + (Ok(l), Ok(r), IsAssign::No) => { let to_string = if l.starts_with('&') { // let a = String::new(); let b = String::new(); // let _ = &a + b; @@ -738,8 +657,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { err.span_label( ex.span, format!( - "cannot apply unary \ - operator `{}`", + "cannot apply unary operator `{}`", op.as_str() ), ); diff --git a/src/test/ui/issues/issue-5239-1.stderr b/src/test/ui/issues/issue-5239-1.stderr index d0c71e73463c1..078a7ef2173bd 100644 --- a/src/test/ui/issues/issue-5239-1.stderr +++ b/src/test/ui/issues/issue-5239-1.stderr @@ -6,7 +6,7 @@ LL | let x = |ref x: isize| { x += 1; }; | | | cannot use `+=` on type `&isize` | -help: `+=` can be used on 'isize', you can dereference `x` +help: `+=` can be used on `isize`, you can dereference `x` | LL | let x = |ref x: isize| { *x += 1; }; | ^ From a5a831f51169767c27ddd7edb30269d01f00bf29 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Wed, 17 Jun 2020 16:04:25 -0700 Subject: [PATCH 19/36] Emit line info for generator variants --- .../debuginfo/metadata.rs | 66 ++++++++++++++++--- src/librustc_middle/mir/query.rs | 4 ++ src/librustc_mir/transform/generator.rs | 22 ++++++- ...ustc.main-{{closure}}.generator_drop.0.mir | 2 +- ...tc.main-{{closure}}.generator_resume.0.mir | 2 +- 5 files changed, 83 insertions(+), 13 deletions(-) diff --git a/src/librustc_codegen_llvm/debuginfo/metadata.rs b/src/librustc_codegen_llvm/debuginfo/metadata.rs index 8a957a729fb68..ee8ab7c0e4df3 100644 --- a/src/librustc_codegen_llvm/debuginfo/metadata.rs +++ b/src/librustc_codegen_llvm/debuginfo/metadata.rs @@ -392,6 +392,7 @@ fn vec_slice_metadata( align: pointer_align, flags: DIFlags::FlagZero, discriminant: None, + source_info: None, }, MemberDescription { name: "length".to_owned(), @@ -401,6 +402,7 @@ fn vec_slice_metadata( align: usize_align, flags: DIFlags::FlagZero, discriminant: None, + source_info: None, }, ]; @@ -508,6 +510,7 @@ fn trait_pointer_metadata( align: data_ptr_field.align.abi, flags: DIFlags::FlagArtificial, discriminant: None, + source_info: None, }, MemberDescription { name: "vtable".to_owned(), @@ -517,6 +520,7 @@ fn trait_pointer_metadata( align: vtable_field.align.abi, flags: DIFlags::FlagArtificial, discriminant: None, + source_info: None, }, ]; @@ -1026,6 +1030,12 @@ impl MetadataCreationResult<'ll> { } } +#[derive(Debug)] +struct SourceInfo<'ll> { + file: &'ll DIFile, + line: u32, +} + /// Description of a type member, which can either be a regular field (as in /// structs or tuples) or an enum variant. #[derive(Debug)] @@ -1037,6 +1047,7 @@ struct MemberDescription<'ll> { align: Align, flags: DIFlags, discriminant: Option, + source_info: Option>, } impl<'ll> MemberDescription<'ll> { @@ -1045,14 +1056,18 @@ impl<'ll> MemberDescription<'ll> { cx: &CodegenCx<'ll, '_>, composite_type_metadata: &'ll DIScope, ) -> &'ll DIType { + let (file, line) = self + .source_info + .map(|info| (info.file, info.line)) + .unwrap_or_else(|| (unknown_file_metadata(cx), UNKNOWN_LINE_NUMBER)); unsafe { llvm::LLVMRustDIBuilderCreateVariantMemberType( DIB(cx), composite_type_metadata, self.name.as_ptr().cast(), self.name.len(), - unknown_file_metadata(cx), - UNKNOWN_LINE_NUMBER, + file, + line, self.size.bits(), self.align.bits() as u32, self.offset.bits(), @@ -1124,6 +1139,7 @@ impl<'tcx> StructMemberDescriptionFactory<'tcx> { align: field.align.abi, flags: DIFlags::FlagZero, discriminant: None, + source_info: None, } }) .collect() @@ -1185,6 +1201,7 @@ impl<'tcx> TupleMemberDescriptionFactory<'tcx> { align, flags: DIFlags::FlagZero, discriminant: None, + source_info: None, } }) .collect() @@ -1244,6 +1261,7 @@ impl<'tcx> UnionMemberDescriptionFactory<'tcx> { align: field.align.abi, flags: DIFlags::FlagZero, discriminant: None, + source_info: None, } }) .collect() @@ -1351,10 +1369,11 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { let variant_info_for = |index: VariantIdx| match self.enum_type.kind { ty::Adt(adt, _) => VariantInfo::Adt(&adt.variants[index]), - ty::Generator(_, substs, _) => { + ty::Generator(def_id, substs, _) => { let (generator_layout, generator_saved_local_names) = generator_variant_info_data.as_ref().unwrap(); VariantInfo::Generator { + def_id, substs, generator_layout: *generator_layout, generator_saved_local_names, @@ -1406,6 +1425,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { align: self.layout.align.abi, flags: DIFlags::FlagZero, discriminant: None, + source_info: variant_info.source_info(cx), }] } Variants::Multiple { @@ -1462,6 +1482,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { self.layout.ty.discriminant_for_variant(cx.tcx, i).unwrap().val as u64, ), + source_info: variant_info.source_info(cx), } }) .collect() @@ -1527,7 +1548,8 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { self.layout.fields.offset(tag_field), self.layout.field(cx, tag_field).size, ); - variant_info_for(*niche_variants.start()).map_struct_name(|variant_name| { + let variant_info = variant_info_for(*niche_variants.start()); + variant_info.map_struct_name(|variant_name| { name.push_str(variant_name); }); @@ -1540,6 +1562,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { align: variant.align.abi, flags: DIFlags::FlagZero, discriminant: None, + source_info: variant_info.source_info(cx), }] } else { variants @@ -1589,6 +1612,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { align: self.layout.align.abi, flags: DIFlags::FlagZero, discriminant: niche_value, + source_info: variant_info.source_info(cx), } }) .collect() @@ -1631,6 +1655,7 @@ impl VariantMemberDescriptionFactory<'ll, 'tcx> { align, flags: DIFlags::FlagZero, discriminant: None, + source_info: None, } }) .collect() @@ -1651,6 +1676,7 @@ enum EnumTagInfo<'ll> { enum VariantInfo<'a, 'tcx> { Adt(&'tcx ty::VariantDef), Generator { + def_id: DefId, substs: SubstsRef<'tcx>, generator_layout: &'tcx GeneratorLayout<'tcx>, generator_saved_local_names: &'a IndexVec>, @@ -1699,6 +1725,24 @@ impl<'tcx> VariantInfo<'_, 'tcx> { }; field_name.map(|name| name.to_string()).unwrap_or_else(|| format!("__{}", i)) } + + fn source_info(&self, cx: &CodegenCx<'ll, 'tcx>) -> Option> { + match self { + VariantInfo::Generator { def_id, variant_index, .. } => { + let span = + cx.tcx.generator_layout(*def_id).variant_source_info[*variant_index].span; + if !span.is_dummy() { + let loc = cx.lookup_debug_loc(span.lo()); + return Some(SourceInfo { + file: file_metadata(cx, &loc.file, def_id.krate), + line: loc.line.unwrap_or(UNKNOWN_LINE_NUMBER), + }); + } + } + _ => {} + } + None + } } /// Returns a tuple of (1) `type_metadata_stub` of the variant, (2) a @@ -1778,7 +1822,8 @@ fn prepare_enum_metadata( span: Span, outer_field_tys: Vec>, ) -> RecursiveTypeDescription<'ll, 'tcx> { - let enum_name = compute_debuginfo_type_name(cx.tcx, enum_type, false); + let tcx = cx.tcx; + let enum_name = compute_debuginfo_type_name(tcx, enum_type, false); let containing_scope = get_namespace_for_item(cx, enum_def_id); // FIXME: This should emit actual file metadata for the enum, but we @@ -1792,7 +1837,7 @@ fn prepare_enum_metadata( let discriminant_type_metadata = |discr: Primitive| { let enumerators_metadata: Vec<_> = match enum_type.kind { ty::Adt(def, _) => def - .discriminants(cx.tcx) + .discriminants(tcx) .zip(&def.variants) .map(|((_, discr), v)| { let name = v.ident.as_str(); @@ -1815,15 +1860,16 @@ fn prepare_enum_metadata( .collect(), ty::Generator(_, substs, _) => substs .as_generator() - .variant_range(enum_def_id, cx.tcx) + .variant_range(enum_def_id, tcx) .map(|variant_index| { + debug_assert_eq!(tcx.types.u32, substs.as_generator().discr_ty(tcx)); let name = substs.as_generator().variant_name(variant_index); unsafe { Some(llvm::LLVMRustDIBuilderCreateEnumerator( DIB(cx), name.as_ptr().cast(), name.len(), - // Generators use u32 as discriminant type. + // Generators use u32 as discriminant type, verified above. variant_index.as_u32().into(), true, // IsUnsigned )) @@ -1841,12 +1887,12 @@ fn prepare_enum_metadata( None => { let (discriminant_size, discriminant_align) = (discr.size(cx), discr.align(cx)); let discriminant_base_type_metadata = - type_metadata(cx, discr.to_ty(cx.tcx), rustc_span::DUMMY_SP); + type_metadata(cx, discr.to_ty(tcx), rustc_span::DUMMY_SP); let item_name; let discriminant_name = match enum_type.kind { ty::Adt(..) => { - item_name = cx.tcx.item_name(enum_def_id).as_str(); + item_name = tcx.item_name(enum_def_id).as_str(); &*item_name } ty::Generator(..) => enum_name.as_str(), diff --git a/src/librustc_middle/mir/query.rs b/src/librustc_middle/mir/query.rs index 1aae97cc2a894..b77b069befd00 100644 --- a/src/librustc_middle/mir/query.rs +++ b/src/librustc_middle/mir/query.rs @@ -67,6 +67,10 @@ pub struct GeneratorLayout<'tcx> { /// be stored in multiple variants. pub variant_fields: IndexVec>, + /// The source that led to each variant being created (usually, a yield or + /// await). + pub variant_source_info: IndexVec, + /// Which saved locals are storage-live at the same time. Locals that do not /// have conflicts with each other are allowed to overlap in the computed /// layout. diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index 59be8dc224dee..523d3c9af3f68 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -422,6 +422,9 @@ struct LivenessInfo { /// The set of saved locals live at each suspension point. live_locals_at_suspension_points: Vec>, + /// Parallel vec to the above with SourceInfo for each yield terminator. + source_info_at_suspension_points: Vec, + /// For every saved local, the set of other saved locals that are /// storage-live at the same time as this local. We cannot overlap locals in /// the layout which have conflicting storage. @@ -473,6 +476,7 @@ fn locals_live_across_suspend_points( let mut storage_liveness_map = IndexVec::from_elem(None, body.basic_blocks()); let mut live_locals_at_suspension_points = Vec::new(); + let mut source_info_at_suspension_points = Vec::new(); let mut live_locals_at_any_suspension_point = BitSet::new_empty(body.local_decls.len()); for (block, data) in body.basic_blocks().iter_enumerated() { @@ -518,6 +522,7 @@ fn locals_live_across_suspend_points( live_locals_at_any_suspension_point.union(&live_locals); live_locals_at_suspension_points.push(live_locals); + source_info_at_suspension_points.push(data.terminator().source_info); } } @@ -541,6 +546,7 @@ fn locals_live_across_suspend_points( LivenessInfo { saved_locals, live_locals_at_suspension_points, + source_info_at_suspension_points, storage_conflicts, storage_liveness: storage_liveness_map, } @@ -754,6 +760,7 @@ fn compute_layout<'tcx>( let LivenessInfo { saved_locals, live_locals_at_suspension_points, + source_info_at_suspension_points, storage_conflicts, storage_liveness, } = liveness; @@ -768,7 +775,18 @@ fn compute_layout<'tcx>( } // Leave empty variants for the UNRESUMED, RETURNED, and POISONED states. + // In debuginfo, these will correspond to the beginning (UNRESUMED) or end + // (RETURNED, POISONED) of the function. const RESERVED_VARIANTS: usize = 3; + let body_span = body.source_scopes[OUTERMOST_SOURCE_SCOPE].span; + let mut variant_source_info: IndexVec = [ + SourceInfo::outermost(body_span.shrink_to_lo()), + SourceInfo::outermost(body_span.shrink_to_hi()), + SourceInfo::outermost(body_span.shrink_to_hi()), + ] + .iter() + .copied() + .collect(); // Build the generator variant field list. // Create a map from local indices to generator struct indices. @@ -787,11 +805,13 @@ fn compute_layout<'tcx>( remap.entry(locals[saved_local]).or_insert((tys[saved_local], variant_index, idx)); } variant_fields.push(fields); + variant_source_info.push(source_info_at_suspension_points[suspension_point_idx]); } debug!("generator variant_fields = {:?}", variant_fields); debug!("generator storage_conflicts = {:#?}", storage_conflicts); - let layout = GeneratorLayout { field_tys: tys, variant_fields, storage_conflicts }; + let layout = + GeneratorLayout { field_tys: tys, variant_fields, variant_source_info, storage_conflicts }; (remap, layout, storage_liveness) } diff --git a/src/test/mir-opt/generator-drop-cleanup/rustc.main-{{closure}}.generator_drop.0.mir b/src/test/mir-opt/generator-drop-cleanup/rustc.main-{{closure}}.generator_drop.0.mir index 3e7083ff62ecd..b34d42155cc6c 100644 --- a/src/test/mir-opt/generator-drop-cleanup/rustc.main-{{closure}}.generator_drop.0.mir +++ b/src/test/mir-opt/generator-drop-cleanup/rustc.main-{{closure}}.generator_drop.0.mir @@ -1,5 +1,5 @@ // MIR for `main::{{closure}}#0` 0 generator_drop -// generator_layout = GeneratorLayout { field_tys: [std::string::String], variant_fields: [[], [], [], [_0]], storage_conflicts: BitMatrix { num_rows: 1, num_columns: 1, words: [1], marker: PhantomData } } +// generator_layout = GeneratorLayout { field_tys: [std::string::String], variant_fields: [[], [], [], [_0]], variant_source_info: [SourceInfo { span: $DIR/generator-drop-cleanup.rs:10:15: 10:15 (#0), scope: scope[0] }, SourceInfo { span: $DIR/generator-drop-cleanup.rs:13:6: 13:6 (#0), scope: scope[0] }, SourceInfo { span: $DIR/generator-drop-cleanup.rs:13:6: 13:6 (#0), scope: scope[0] }, SourceInfo { span: $DIR/generator-drop-cleanup.rs:12:9: 12:14 (#0), scope: scope[1] }], storage_conflicts: BitMatrix { num_rows: 1, num_columns: 1, words: [1], marker: PhantomData } } fn main::{{closure}}#0(_1: *mut [generator@$DIR/generator-drop-cleanup.rs:10:15: 13:6 {std::string::String, ()}]) -> () { let mut _0: (); // return place in scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6 diff --git a/src/test/mir-opt/generator-tiny/rustc.main-{{closure}}.generator_resume.0.mir b/src/test/mir-opt/generator-tiny/rustc.main-{{closure}}.generator_resume.0.mir index c73dea5f8fde6..2398e9501452e 100644 --- a/src/test/mir-opt/generator-tiny/rustc.main-{{closure}}.generator_resume.0.mir +++ b/src/test/mir-opt/generator-tiny/rustc.main-{{closure}}.generator_resume.0.mir @@ -1,5 +1,5 @@ // MIR for `main::{{closure}}#0` 0 generator_resume -// generator_layout = GeneratorLayout { field_tys: [HasDrop], variant_fields: [[], [], [], [_0]], storage_conflicts: BitMatrix { num_rows: 1, num_columns: 1, words: [1], marker: PhantomData } } +// generator_layout = GeneratorLayout { field_tys: [HasDrop], variant_fields: [[], [], [], [_0]], variant_source_info: [SourceInfo { span: $DIR/generator-tiny.rs:19:16: 19:16 (#0), scope: scope[0] }, SourceInfo { span: $DIR/generator-tiny.rs:25:6: 25:6 (#0), scope: scope[0] }, SourceInfo { span: $DIR/generator-tiny.rs:25:6: 25:6 (#0), scope: scope[0] }, SourceInfo { span: $DIR/generator-tiny.rs:22:13: 22:18 (#0), scope: scope[1] }], storage_conflicts: BitMatrix { num_rows: 1, num_columns: 1, words: [1], marker: PhantomData } } fn main::{{closure}}#0(_1: std::pin::Pin<&mut [generator@$DIR/generator-tiny.rs:19:16: 25:6 {u8, HasDrop, ()}]>, _2: u8) -> std::ops::GeneratorState<(), ()> { debug _x => _10; // in scope 0 at $DIR/generator-tiny.rs:19:17: 19:19 From eb726c0fcec3f9d12a6a9990c5c041478e1e7187 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Wed, 17 Jun 2020 19:39:23 -0700 Subject: [PATCH 20/36] Add Artificial flag to generator variants --- .../debuginfo/metadata.rs | 67 +++++++++++++++---- 1 file changed, 53 insertions(+), 14 deletions(-) diff --git a/src/librustc_codegen_llvm/debuginfo/metadata.rs b/src/librustc_codegen_llvm/debuginfo/metadata.rs index ee8ab7c0e4df3..f26c45e90110a 100644 --- a/src/librustc_codegen_llvm/debuginfo/metadata.rs +++ b/src/librustc_codegen_llvm/debuginfo/metadata.rs @@ -863,7 +863,7 @@ fn foreign_type_metadata( debug!("foreign_type_metadata: {:?}", t); let name = compute_debuginfo_type_name(cx.tcx, t, false); - create_struct_stub(cx, t, &name, unique_type_id, NO_SCOPE_METADATA) + create_struct_stub(cx, t, &name, unique_type_id, NO_SCOPE_METADATA, DIFlags::FlagZero) } fn pointer_type_metadata( @@ -1161,8 +1161,14 @@ fn prepare_struct_metadata( let containing_scope = get_namespace_for_item(cx, struct_def_id); - let struct_metadata_stub = - create_struct_stub(cx, struct_type, &struct_name, unique_type_id, Some(containing_scope)); + let struct_metadata_stub = create_struct_stub( + cx, + struct_type, + &struct_name, + unique_type_id, + Some(containing_scope), + DIFlags::FlagZero, + ); create_and_register_recursive_type_forward_declaration( cx, @@ -1218,8 +1224,14 @@ fn prepare_tuple_metadata( ) -> RecursiveTypeDescription<'ll, 'tcx> { let tuple_name = compute_debuginfo_type_name(cx.tcx, tuple_type, false); - let struct_stub = - create_struct_stub(cx, tuple_type, &tuple_name[..], unique_type_id, containing_scope); + let struct_stub = create_struct_stub( + cx, + tuple_type, + &tuple_name[..], + unique_type_id, + containing_scope, + DIFlags::FlagZero, + ); create_and_register_recursive_type_forward_declaration( cx, @@ -1390,6 +1402,10 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { } else { type_metadata(cx, self.enum_type, self.span) }; + let flags = match self.enum_type.kind { + ty::Generator(..) => DIFlags::FlagArtificial, + _ => DIFlags::FlagZero, + }; match self.layout.variants { Variants::Single { index } => { @@ -1423,7 +1439,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { offset: Size::ZERO, size: self.layout.size, align: self.layout.align.abi, - flags: DIFlags::FlagZero, + flags, discriminant: None, source_info: variant_info.source_info(cx), }] @@ -1477,7 +1493,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { offset: Size::ZERO, size: self.layout.size, align: self.layout.align.abi, - flags: DIFlags::FlagZero, + flags, discriminant: Some( self.layout.ty.discriminant_for_variant(cx.tcx, i).unwrap().val as u64, @@ -1560,7 +1576,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { offset: Size::ZERO, size: variant.size, align: variant.align.abi, - flags: DIFlags::FlagZero, + flags, discriminant: None, source_info: variant_info.source_info(cx), }] @@ -1610,7 +1626,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { offset: Size::ZERO, size: self.layout.size, align: self.layout.align.abi, - flags: DIFlags::FlagZero, + flags, discriminant: niche_value, source_info: variant_info.source_info(cx), } @@ -1743,6 +1759,14 @@ impl<'tcx> VariantInfo<'_, 'tcx> { } None } + + #[allow(dead_code)] + fn is_artificial(&self) -> bool { + match self { + VariantInfo::Generator { .. } => true, + VariantInfo::Adt(..) => false, + } + } } /// Returns a tuple of (1) `type_metadata_stub` of the variant, (2) a @@ -1762,7 +1786,15 @@ fn describe_enum_variant( .type_map .borrow_mut() .get_unique_type_id_of_enum_variant(cx, layout.ty, &variant_name); - create_struct_stub(cx, layout.ty, &variant_name, unique_type_id, Some(containing_scope)) + create_struct_stub( + cx, + layout.ty, + &variant_name, + unique_type_id, + Some(containing_scope), + // FIXME(tmandry): This doesn't seem to have any effect. + if variant.is_artificial() { DIFlags::FlagArtificial } else { DIFlags::FlagZero }, + ) }); // Build an array of (field name, field type) pairs to be captured in the factory closure. @@ -1824,6 +1856,11 @@ fn prepare_enum_metadata( ) -> RecursiveTypeDescription<'ll, 'tcx> { let tcx = cx.tcx; let enum_name = compute_debuginfo_type_name(tcx, enum_type, false); + // FIXME(tmandry): This doesn't seem to have any effect. + let enum_flags = match enum_type.kind { + ty::Generator(..) => DIFlags::FlagArtificial, + _ => DIFlags::FlagZero, + }; let containing_scope = get_namespace_for_item(cx, enum_def_id); // FIXME: This should emit actual file metadata for the enum, but we @@ -1958,7 +1995,7 @@ fn prepare_enum_metadata( UNKNOWN_LINE_NUMBER, layout.size.bits(), layout.align.abi.bits() as u32, - DIFlags::FlagZero, + enum_flags, None, 0, // RuntimeLang unique_type_id_str.as_ptr().cast(), @@ -2079,7 +2116,7 @@ fn prepare_enum_metadata( UNKNOWN_LINE_NUMBER, layout.size.bits(), layout.align.abi.bits() as u32, - DIFlags::FlagZero, + enum_flags, discriminator_metadata, empty_array, variant_part_unique_type_id_str.as_ptr().cast(), @@ -2105,7 +2142,7 @@ fn prepare_enum_metadata( UNKNOWN_LINE_NUMBER, layout.size.bits(), layout.align.abi.bits() as u32, - DIFlags::FlagZero, + enum_flags, None, type_array, 0, @@ -2156,6 +2193,7 @@ fn composite_type_metadata( composite_type_name, composite_type_unique_id, containing_scope, + DIFlags::FlagZero, ); // ... and immediately create and add the member descriptions. set_members_of_composite_type(cx, composite_type, composite_type_metadata, member_descriptions); @@ -2257,6 +2295,7 @@ fn create_struct_stub( struct_type_name: &str, unique_type_id: UniqueTypeId, containing_scope: Option<&'ll DIScope>, + flags: DIFlags, ) -> &'ll DICompositeType { let (struct_size, struct_align) = cx.size_and_align_of(struct_type); @@ -2278,7 +2317,7 @@ fn create_struct_stub( UNKNOWN_LINE_NUMBER, struct_size.bits(), struct_align.bits() as u32, - DIFlags::FlagZero, + flags, None, empty_array, 0, From 367858aedc1e1dbee5560676c00618a386ed8e37 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Thu, 18 Jun 2020 16:17:25 -0700 Subject: [PATCH 21/36] Add test for generator debuginfo --- src/test/codegen/generator-debug.rs | 94 +++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 src/test/codegen/generator-debug.rs diff --git a/src/test/codegen/generator-debug.rs b/src/test/codegen/generator-debug.rs new file mode 100644 index 0000000000000..3007099393ffe --- /dev/null +++ b/src/test/codegen/generator-debug.rs @@ -0,0 +1,94 @@ +// Verify debuginfo for generators: +// - Each variant points to the file and line of its yield point +// - The generator types and variants are marked artificial +// - Captured vars from the source are not marked artificial +// +// ignore-tidy-linelength +// compile-flags: -C debuginfo=2 --edition=2018 + +#![feature(generators, generator_trait)] +use std::ops::Generator; + +fn generator_test() -> impl Generator { + || { + yield 0; + let s = String::from("foo"); + yield 1; + } +} + +async fn foo() {} +async fn async_fn_test() { + foo().await; + let s = String::from("foo"); + foo().await; +} + +// FIXME: We need "checksum" to prevent matching with the wrong (duplicate) file +// metadata, even when -C codegen-units=1. +// CHECK: [[FILE:!.*]] = !DIFile(filename: "{{.*}}/generator-debug.rs", {{.*}}, checksum: + +// CHECK: [[GEN:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "generator-0", scope: [[FN:![0-9]*]], +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: [[FN]] = !DINamespace(name: "generator_test" +// CHECK: [[VARIANT:!.*]] = !DICompositeType(tag: DW_TAG_variant_part, scope: [[FN]], +// CHECK-SAME: flags: DIFlagArtificial +// CHECK-SAME: discriminator: [[DISC:![0-9]*]] +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "0", scope: [[VARIANT]], +// CHECK-SAME: file: [[FILE]], line: 13, +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DICompositeType(tag: DW_TAG_structure_type, name: "Unresumed", scope: [[GEN]], +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "1", scope: [[VARIANT]], +// CHECK-SAME: file: [[FILE]], line: 17, +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "2", scope: [[VARIANT]], +// CHECK-SAME: file: [[FILE]], line: 17, +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "3", scope: [[VARIANT]], +// CHECK-SAME: file: [[FILE]], line: 14, +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "4", scope: [[VARIANT]], +// CHECK-SAME: file: [[FILE]], line: 16, +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: [[S1:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "Suspend1", scope: [[GEN]], +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "s", scope: [[S1]] +// CHECK-NOT: flags: DIFlagArtificial +// CHECK-SAME: ) +// CHECK: [[DISC]] = !DIDerivedType(tag: DW_TAG_member, name: "__state", scope: [[FN]], +// CHECK-SAME: flags: DIFlagArtificial + +// CHECK: [[GEN:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "generator-0", scope: [[FN:![0-9]*]], +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: [[FN]] = !DINamespace(name: "async_fn_test" +// CHECK: [[VARIANT:!.*]] = !DICompositeType(tag: DW_TAG_variant_part, scope: [[FN]], +// CHECK-SAME: flags: DIFlagArtificial +// CHECK-SAME: discriminator: [[DISC:![0-9]*]] +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "0", scope: [[VARIANT]], +// CHECK-SAME: file: [[FILE]], line: 21, +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "1", scope: [[VARIANT]], +// CHECK-SAME: file: [[FILE]], line: 25, +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "2", scope: [[VARIANT]], +// CHECK-SAME: file: [[FILE]], line: 25, +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "3", scope: [[VARIANT]], +// CHECK-SAME: file: [[FILE]], line: 22, +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "4", scope: [[VARIANT]], +// CHECK-SAME: file: [[FILE]], line: 24, +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: [[S1:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "Suspend1", scope: [[GEN]], +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "s", scope: [[S1]] +// CHECK-NOT: flags: DIFlagArtificial +// CHECK-SAME: ) +// CHECK: [[DISC]] = !DIDerivedType(tag: DW_TAG_member, name: "__state", scope: [[FN]], +// CHECK-SAME: flags: DIFlagArtificial + +fn main() { + let _dummy = generator_test(); + let _dummy = async_fn_test(); +} From 242a5cd4c673c02d817fb5cbadd5bd3ff08f10eb Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Fri, 19 Jun 2020 18:37:52 -0700 Subject: [PATCH 22/36] Allow calling GeneratorSubsts::variant_name() without substs --- src/librustc_codegen_llvm/debuginfo/metadata.rs | 14 ++++++-------- src/librustc_codegen_llvm/type_of.rs | 4 ++-- src/librustc_middle/ty/sty.rs | 2 +- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/librustc_codegen_llvm/debuginfo/metadata.rs b/src/librustc_codegen_llvm/debuginfo/metadata.rs index f26c45e90110a..0338aff509658 100644 --- a/src/librustc_codegen_llvm/debuginfo/metadata.rs +++ b/src/librustc_codegen_llvm/debuginfo/metadata.rs @@ -33,9 +33,9 @@ use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc_middle::mir::interpret::truncate; use rustc_middle::mir::{self, Field, GeneratorLayout}; use rustc_middle::ty::layout::{self, IntegerExt, PrimitiveExt, TyAndLayout}; -use rustc_middle::ty::subst::{GenericArgKind, SubstsRef}; +use rustc_middle::ty::subst::GenericArgKind; use rustc_middle::ty::Instance; -use rustc_middle::ty::{self, AdtKind, ParamEnv, Ty, TyCtxt}; +use rustc_middle::ty::{self, AdtKind, GeneratorSubsts, ParamEnv, Ty, TyCtxt}; use rustc_middle::{bug, span_bug}; use rustc_session::config::{self, DebugInfo}; use rustc_span::symbol::{Interner, Symbol}; @@ -1381,12 +1381,11 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> { let variant_info_for = |index: VariantIdx| match self.enum_type.kind { ty::Adt(adt, _) => VariantInfo::Adt(&adt.variants[index]), - ty::Generator(def_id, substs, _) => { + ty::Generator(def_id, _, _) => { let (generator_layout, generator_saved_local_names) = generator_variant_info_data.as_ref().unwrap(); VariantInfo::Generator { def_id, - substs, generator_layout: *generator_layout, generator_saved_local_names, variant_index: index, @@ -1693,7 +1692,6 @@ enum VariantInfo<'a, 'tcx> { Adt(&'tcx ty::VariantDef), Generator { def_id: DefId, - substs: SubstsRef<'tcx>, generator_layout: &'tcx GeneratorLayout<'tcx>, generator_saved_local_names: &'a IndexVec>, variant_index: VariantIdx, @@ -1704,8 +1702,8 @@ impl<'tcx> VariantInfo<'_, 'tcx> { fn map_struct_name(&self, f: impl FnOnce(&str) -> R) -> R { match self { VariantInfo::Adt(variant) => f(&variant.ident.as_str()), - VariantInfo::Generator { substs, variant_index, .. } => { - f(&substs.as_generator().variant_name(*variant_index)) + VariantInfo::Generator { variant_index, .. } => { + f(&GeneratorSubsts::variant_name(*variant_index)) } } } @@ -1900,7 +1898,7 @@ fn prepare_enum_metadata( .variant_range(enum_def_id, tcx) .map(|variant_index| { debug_assert_eq!(tcx.types.u32, substs.as_generator().discr_ty(tcx)); - let name = substs.as_generator().variant_name(variant_index); + let name = GeneratorSubsts::variant_name(variant_index); unsafe { Some(llvm::LLVMRustDIBuilderCreateEnumerator( DIB(cx), diff --git a/src/librustc_codegen_llvm/type_of.rs b/src/librustc_codegen_llvm/type_of.rs index 77009aca6d32e..5a0da6be5980e 100644 --- a/src/librustc_codegen_llvm/type_of.rs +++ b/src/librustc_codegen_llvm/type_of.rs @@ -70,10 +70,10 @@ fn uncached_llvm_type<'a, 'tcx>( write!(&mut name, "::{}", def.variants[index].ident).unwrap(); } } - if let (&ty::Generator(_, substs, _), &Variants::Single { index }) + if let (&ty::Generator(_, _, _), &Variants::Single { index }) = (&layout.ty.kind, &layout.variants) { - write!(&mut name, "::{}", substs.as_generator().variant_name(index)).unwrap(); + write!(&mut name, "::{}", ty::GeneratorSubsts::variant_name(index)).unwrap(); } Some(name) } diff --git a/src/librustc_middle/ty/sty.rs b/src/librustc_middle/ty/sty.rs index 1d680c3563675..79fa06790bd4a 100644 --- a/src/librustc_middle/ty/sty.rs +++ b/src/librustc_middle/ty/sty.rs @@ -523,7 +523,7 @@ impl<'tcx> GeneratorSubsts<'tcx> { /// Calls `f` with a reference to the name of the enumerator for the given /// variant `v`. #[inline] - pub fn variant_name(self, v: VariantIdx) -> Cow<'static, str> { + pub fn variant_name(v: VariantIdx) -> Cow<'static, str> { match v.as_usize() { Self::UNRESUMED => Cow::from(Self::UNRESUMED_NAME), Self::RETURNED => Cow::from(Self::RETURNED_NAME), From 547d86307c6acd10520af96f8e2974522c50ac1e Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Fri, 19 Jun 2020 20:19:19 -0700 Subject: [PATCH 23/36] Improve GeneratorLayout debug output --- src/librustc_index/bit_set.rs | 18 +++++- src/librustc_middle/mir/query.rs | 60 ++++++++++++++++++- src/librustc_middle/ty/sty.rs | 1 - src/librustc_mir/util/pretty.rs | 2 +- ...ustc.main-{{closure}}.generator_drop.0.mir | 15 ++++- ...tc.main-{{closure}}.generator_resume.0.mir | 15 ++++- 6 files changed, 105 insertions(+), 6 deletions(-) diff --git a/src/librustc_index/bit_set.rs b/src/librustc_index/bit_set.rs index 46c38840516e2..cb8b30830c5de 100644 --- a/src/librustc_index/bit_set.rs +++ b/src/librustc_index/bit_set.rs @@ -700,7 +700,7 @@ impl GrowableBitSet { /// /// All operations that involve a row and/or column index will panic if the /// index exceeds the relevant bound. -#[derive(Clone, Debug, Eq, PartialEq, RustcDecodable, RustcEncodable)] +#[derive(Clone, Eq, PartialEq, RustcDecodable, RustcEncodable)] pub struct BitMatrix { num_rows: usize, num_columns: usize, @@ -876,6 +876,22 @@ impl BitMatrix { } } +impl fmt::Debug for BitMatrix { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + /// Forces its contents to print in regular mode instead of alternate mode. + struct OneLinePrinter(T); + impl fmt::Debug for OneLinePrinter { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(fmt, "{:?}", self.0) + } + } + + write!(fmt, "BitMatrix({}x{}) ", self.num_rows, self.num_columns)?; + let items = self.rows().flat_map(|r| self.iter(r).map(move |c| (r, c))); + fmt.debug_set().entries(items.map(OneLinePrinter)).finish() + } +} + /// A fixed-column-size, variable-row-size 2D bit matrix with a moderately /// sparse representation. /// diff --git a/src/librustc_middle/mir/query.rs b/src/librustc_middle/mir/query.rs index b77b069befd00..9ad79230a4f6d 100644 --- a/src/librustc_middle/mir/query.rs +++ b/src/librustc_middle/mir/query.rs @@ -10,6 +10,8 @@ use rustc_index::vec::IndexVec; use rustc_span::{Span, Symbol}; use rustc_target::abi::VariantIdx; use smallvec::SmallVec; +use std::cell::Cell; +use std::fmt::{self, Debug}; use super::{Field, SourceInfo}; @@ -58,7 +60,7 @@ rustc_index::newtype_index! { } /// The layout of generator state. -#[derive(Clone, Debug, RustcEncodable, RustcDecodable, HashStable, TypeFoldable)] +#[derive(Clone, RustcEncodable, RustcDecodable, HashStable, TypeFoldable)] pub struct GeneratorLayout<'tcx> { /// The type of every local stored inside the generator. pub field_tys: IndexVec>, @@ -77,6 +79,62 @@ pub struct GeneratorLayout<'tcx> { pub storage_conflicts: BitMatrix, } +impl Debug for GeneratorLayout<'_> { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + /// Prints an iterator of (key, value) tuples as a map. + struct MapPrinter<'a, K, V>(Cell + 'a>>>); + impl<'a, K, V> MapPrinter<'a, K, V> { + fn new(iter: impl Iterator + 'a) -> Self { + Self(Cell::new(Some(Box::new(iter)))) + } + } + impl<'a, K: Debug, V: Debug> Debug for MapPrinter<'a, K, V> { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt.debug_map().entries(self.0.take().unwrap()).finish() + } + } + + /// Prints the generator variant name. + struct GenVariantPrinter(VariantIdx); + impl From for GenVariantPrinter { + fn from(idx: VariantIdx) -> Self { + GenVariantPrinter(idx) + } + } + impl Debug for GenVariantPrinter { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + let variant_name = ty::GeneratorSubsts::variant_name(self.0); + if fmt.alternate() { + write!(fmt, "{:9}({:?})", variant_name, self.0) + } else { + write!(fmt, "{}", variant_name) + } + } + } + + /// Forces its contents to print in regular mode instead of alternate mode. + struct OneLinePrinter(T); + impl Debug for OneLinePrinter { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(fmt, "{:?}", self.0) + } + } + + fmt.debug_struct("GeneratorLayout") + .field("field_tys", &MapPrinter::new(self.field_tys.iter_enumerated())) + .field( + "variant_fields", + &MapPrinter::new( + self.variant_fields + .iter_enumerated() + .map(|(k, v)| (GenVariantPrinter(k), OneLinePrinter(v))), + ), + ) + .field("storage_conflicts", &self.storage_conflicts) + .finish() + } +} + #[derive(Debug, RustcEncodable, RustcDecodable, HashStable)] pub struct BorrowCheckResult<'tcx> { /// All the opaque types that are restricted to concrete types diff --git a/src/librustc_middle/ty/sty.rs b/src/librustc_middle/ty/sty.rs index 79fa06790bd4a..8f86d2ef522d3 100644 --- a/src/librustc_middle/ty/sty.rs +++ b/src/librustc_middle/ty/sty.rs @@ -522,7 +522,6 @@ impl<'tcx> GeneratorSubsts<'tcx> { /// Calls `f` with a reference to the name of the enumerator for the given /// variant `v`. - #[inline] pub fn variant_name(v: VariantIdx) -> Cow<'static, str> { match v.as_usize() { Self::UNRESUMED => Cow::from(Self::UNRESUMED_NAME), diff --git a/src/librustc_mir/util/pretty.rs b/src/librustc_mir/util/pretty.rs index 02614044063fc..db45481e4fd25 100644 --- a/src/librustc_mir/util/pretty.rs +++ b/src/librustc_mir/util/pretty.rs @@ -131,7 +131,7 @@ fn dump_matched_mir_node<'tcx, F>( } writeln!(file, " {} {}", disambiguator, pass_name)?; if let Some(ref layout) = body.generator_layout { - writeln!(file, "// generator_layout = {:?}", layout)?; + writeln!(file, "/* generator_layout = {:#?} */", layout)?; } writeln!(file)?; extra_data(PassWhere::BeforeCFG, &mut file)?; diff --git a/src/test/mir-opt/generator-drop-cleanup/rustc.main-{{closure}}.generator_drop.0.mir b/src/test/mir-opt/generator-drop-cleanup/rustc.main-{{closure}}.generator_drop.0.mir index b34d42155cc6c..3c77995eea893 100644 --- a/src/test/mir-opt/generator-drop-cleanup/rustc.main-{{closure}}.generator_drop.0.mir +++ b/src/test/mir-opt/generator-drop-cleanup/rustc.main-{{closure}}.generator_drop.0.mir @@ -1,5 +1,18 @@ // MIR for `main::{{closure}}#0` 0 generator_drop -// generator_layout = GeneratorLayout { field_tys: [std::string::String], variant_fields: [[], [], [], [_0]], variant_source_info: [SourceInfo { span: $DIR/generator-drop-cleanup.rs:10:15: 10:15 (#0), scope: scope[0] }, SourceInfo { span: $DIR/generator-drop-cleanup.rs:13:6: 13:6 (#0), scope: scope[0] }, SourceInfo { span: $DIR/generator-drop-cleanup.rs:13:6: 13:6 (#0), scope: scope[0] }, SourceInfo { span: $DIR/generator-drop-cleanup.rs:12:9: 12:14 (#0), scope: scope[1] }], storage_conflicts: BitMatrix { num_rows: 1, num_columns: 1, words: [1], marker: PhantomData } } +/* generator_layout = GeneratorLayout { + field_tys: { + _0: std::string::String, + }, + variant_fields: { + Unresumed(0): [], + Returned (1): [], + Panicked (2): [], + Suspend0 (3): [_0], + }, + storage_conflicts: BitMatrix(1x1) { + (_0, _0), + }, +} */ fn main::{{closure}}#0(_1: *mut [generator@$DIR/generator-drop-cleanup.rs:10:15: 13:6 {std::string::String, ()}]) -> () { let mut _0: (); // return place in scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6 diff --git a/src/test/mir-opt/generator-tiny/rustc.main-{{closure}}.generator_resume.0.mir b/src/test/mir-opt/generator-tiny/rustc.main-{{closure}}.generator_resume.0.mir index 2398e9501452e..bd6db11a7e73e 100644 --- a/src/test/mir-opt/generator-tiny/rustc.main-{{closure}}.generator_resume.0.mir +++ b/src/test/mir-opt/generator-tiny/rustc.main-{{closure}}.generator_resume.0.mir @@ -1,5 +1,18 @@ // MIR for `main::{{closure}}#0` 0 generator_resume -// generator_layout = GeneratorLayout { field_tys: [HasDrop], variant_fields: [[], [], [], [_0]], variant_source_info: [SourceInfo { span: $DIR/generator-tiny.rs:19:16: 19:16 (#0), scope: scope[0] }, SourceInfo { span: $DIR/generator-tiny.rs:25:6: 25:6 (#0), scope: scope[0] }, SourceInfo { span: $DIR/generator-tiny.rs:25:6: 25:6 (#0), scope: scope[0] }, SourceInfo { span: $DIR/generator-tiny.rs:22:13: 22:18 (#0), scope: scope[1] }], storage_conflicts: BitMatrix { num_rows: 1, num_columns: 1, words: [1], marker: PhantomData } } +/* generator_layout = GeneratorLayout { + field_tys: { + _0: HasDrop, + }, + variant_fields: { + Unresumed(0): [], + Returned (1): [], + Panicked (2): [], + Suspend0 (3): [_0], + }, + storage_conflicts: BitMatrix(1x1) { + (_0, _0), + }, +} */ fn main::{{closure}}#0(_1: std::pin::Pin<&mut [generator@$DIR/generator-tiny.rs:19:16: 25:6 {u8, HasDrop, ()}]>, _2: u8) -> std::ops::GeneratorState<(), ()> { debug _x => _10; // in scope 0 at $DIR/generator-tiny.rs:19:17: 19:19 From 477ecc51caea03c22eb6c7dedcf5656593b71fb0 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Tue, 23 Jun 2020 16:21:53 -0700 Subject: [PATCH 24/36] Generalize generator-debug test a bit Don't be so reliant on particular line ordering (though FileCheck makes this hard in general, IMO). Also disable for MSVC. --- src/test/codegen/generator-debug.rs | 40 ++++++++++++++--------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/src/test/codegen/generator-debug.rs b/src/test/codegen/generator-debug.rs index 3007099393ffe..fa16a460dde4d 100644 --- a/src/test/codegen/generator-debug.rs +++ b/src/test/codegen/generator-debug.rs @@ -5,6 +5,7 @@ // // ignore-tidy-linelength // compile-flags: -C debuginfo=2 --edition=2018 +// ignore-msvc #![feature(generators, generator_trait)] use std::ops::Generator; @@ -26,66 +27,63 @@ async fn async_fn_test() { // FIXME: We need "checksum" to prevent matching with the wrong (duplicate) file // metadata, even when -C codegen-units=1. -// CHECK: [[FILE:!.*]] = !DIFile(filename: "{{.*}}/generator-debug.rs", {{.*}}, checksum: +// CHECK-DAG: [[FILE:!.*]] = !DIFile(filename: "{{.*}}generator-debug.rs", {{.*}}, checksum: -// CHECK: [[GEN:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "generator-0", scope: [[FN:![0-9]*]], -// CHECK-SAME: flags: DIFlagArtificial -// CHECK: [[FN]] = !DINamespace(name: "generator_test" -// CHECK: [[VARIANT:!.*]] = !DICompositeType(tag: DW_TAG_variant_part, scope: [[FN]], +// CHECK-DAG: [[GEN_FN:!.*]] = !DINamespace(name: "generator_test" +// CHECK-DAG: [[GEN:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "generator-0", scope: [[GEN_FN]], {{.*}}flags: DIFlagArtificial +// CHECK: [[VARIANT:!.*]] = !DICompositeType(tag: DW_TAG_variant_part, scope: [[GEN_FN]], // CHECK-SAME: flags: DIFlagArtificial // CHECK-SAME: discriminator: [[DISC:![0-9]*]] // CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "0", scope: [[VARIANT]], -// CHECK-SAME: file: [[FILE]], line: 13, +// CHECK-SAME: file: [[FILE]], line: 14, // CHECK-SAME: flags: DIFlagArtificial // CHECK: {{!.*}} = !DICompositeType(tag: DW_TAG_structure_type, name: "Unresumed", scope: [[GEN]], // CHECK-SAME: flags: DIFlagArtificial // CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "1", scope: [[VARIANT]], -// CHECK-SAME: file: [[FILE]], line: 17, +// CHECK-SAME: file: [[FILE]], line: 18, // CHECK-SAME: flags: DIFlagArtificial // CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "2", scope: [[VARIANT]], -// CHECK-SAME: file: [[FILE]], line: 17, +// CHECK-SAME: file: [[FILE]], line: 18, // CHECK-SAME: flags: DIFlagArtificial // CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "3", scope: [[VARIANT]], -// CHECK-SAME: file: [[FILE]], line: 14, +// CHECK-SAME: file: [[FILE]], line: 15, // CHECK-SAME: flags: DIFlagArtificial // CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "4", scope: [[VARIANT]], -// CHECK-SAME: file: [[FILE]], line: 16, +// CHECK-SAME: file: [[FILE]], line: 17, // CHECK-SAME: flags: DIFlagArtificial // CHECK: [[S1:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "Suspend1", scope: [[GEN]], // CHECK-SAME: flags: DIFlagArtificial // CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "s", scope: [[S1]] // CHECK-NOT: flags: DIFlagArtificial // CHECK-SAME: ) -// CHECK: [[DISC]] = !DIDerivedType(tag: DW_TAG_member, name: "__state", scope: [[FN]], +// CHECK: [[DISC]] = !DIDerivedType(tag: DW_TAG_member, name: "__state", scope: [[GEN_FN]], // CHECK-SAME: flags: DIFlagArtificial -// CHECK: [[GEN:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "generator-0", scope: [[FN:![0-9]*]], -// CHECK-SAME: flags: DIFlagArtificial -// CHECK: [[FN]] = !DINamespace(name: "async_fn_test" -// CHECK: [[VARIANT:!.*]] = !DICompositeType(tag: DW_TAG_variant_part, scope: [[FN]], +// CHECK-DAG: [[GEN:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "generator-0", scope: [[ASYNC_FN:![0-9]*]], {{.*}}flags: DIFlagArtificial +// CHECK: [[VARIANT:!.*]] = !DICompositeType(tag: DW_TAG_variant_part, scope: [[ASYNC_FN]], // CHECK-SAME: flags: DIFlagArtificial // CHECK-SAME: discriminator: [[DISC:![0-9]*]] // CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "0", scope: [[VARIANT]], -// CHECK-SAME: file: [[FILE]], line: 21, +// CHECK-SAME: file: [[FILE]], line: 22, // CHECK-SAME: flags: DIFlagArtificial // CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "1", scope: [[VARIANT]], -// CHECK-SAME: file: [[FILE]], line: 25, +// CHECK-SAME: file: [[FILE]], line: 26, // CHECK-SAME: flags: DIFlagArtificial // CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "2", scope: [[VARIANT]], -// CHECK-SAME: file: [[FILE]], line: 25, +// CHECK-SAME: file: [[FILE]], line: 26, // CHECK-SAME: flags: DIFlagArtificial // CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "3", scope: [[VARIANT]], -// CHECK-SAME: file: [[FILE]], line: 22, +// CHECK-SAME: file: [[FILE]], line: 23, // CHECK-SAME: flags: DIFlagArtificial // CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "4", scope: [[VARIANT]], -// CHECK-SAME: file: [[FILE]], line: 24, +// CHECK-SAME: file: [[FILE]], line: 25, // CHECK-SAME: flags: DIFlagArtificial // CHECK: [[S1:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "Suspend1", scope: [[GEN]], // CHECK-SAME: flags: DIFlagArtificial // CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "s", scope: [[S1]] // CHECK-NOT: flags: DIFlagArtificial // CHECK-SAME: ) -// CHECK: [[DISC]] = !DIDerivedType(tag: DW_TAG_member, name: "__state", scope: [[FN]], +// CHECK: [[DISC]] = !DIDerivedType(tag: DW_TAG_member, name: "__state", scope: [[ASYNC_FN]], // CHECK-SAME: flags: DIFlagArtificial fn main() { From 2d652d9d735f10402803318351f7a8ee91fc4c47 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Tue, 23 Jun 2020 16:23:01 -0700 Subject: [PATCH 25/36] Add generator-debug test for MSVC ..which doesn't use variant types. --- .../debuginfo/metadata.rs | 18 ++-- src/test/codegen/generator-debug-msvc.rs | 88 +++++++++++++++++++ 2 files changed, 98 insertions(+), 8 deletions(-) create mode 100644 src/test/codegen/generator-debug-msvc.rs diff --git a/src/librustc_codegen_llvm/debuginfo/metadata.rs b/src/librustc_codegen_llvm/debuginfo/metadata.rs index 0338aff509658..33351c06d27ee 100644 --- a/src/librustc_codegen_llvm/debuginfo/metadata.rs +++ b/src/librustc_codegen_llvm/debuginfo/metadata.rs @@ -1652,23 +1652,25 @@ impl VariantMemberDescriptionFactory<'ll, 'tcx> { .iter() .enumerate() .map(|(i, &(ref name, ty))| { + // Discriminant is always the first field of our variant + // when using the enum fallback. + let is_artificial_discr = use_enum_fallback(cx) && i == 0; let (size, align) = cx.size_and_align_of(ty); MemberDescription { name: name.to_string(), - type_metadata: if use_enum_fallback(cx) { - match self.tag_type_metadata { - // Discriminant is always the first field of our variant - // when using the enum fallback. - Some(metadata) if i == 0 => metadata, - _ => type_metadata(cx, ty, self.span), - } + type_metadata: if is_artificial_discr { + self.tag_type_metadata.unwrap_or_else(|| type_metadata(cx, ty, self.span)) } else { type_metadata(cx, ty, self.span) }, offset: self.offsets[i], size, align, - flags: DIFlags::FlagZero, + flags: if is_artificial_discr { + DIFlags::FlagArtificial + } else { + DIFlags::FlagZero + }, discriminant: None, source_info: None, } diff --git a/src/test/codegen/generator-debug-msvc.rs b/src/test/codegen/generator-debug-msvc.rs new file mode 100644 index 0000000000000..ebd0c7ba544b0 --- /dev/null +++ b/src/test/codegen/generator-debug-msvc.rs @@ -0,0 +1,88 @@ +// Verify debuginfo for generators: +// - Each variant points to the file and line of its yield point +// - The generator types and variants are marked artificial +// - Captured vars from the source are not marked artificial +// +// ignore-tidy-linelength +// compile-flags: -C debuginfo=2 --edition=2018 +// only-msvc + +#![feature(generators, generator_trait)] +use std::ops::Generator; + +fn generator_test() -> impl Generator { + || { + yield 0; + let s = String::from("foo"); + yield 1; + } +} + +async fn foo() {} +async fn async_fn_test() { + foo().await; + let s = String::from("foo"); + foo().await; +} + +// FIXME: We need "checksum" to prevent matching with the wrong (duplicate) file +// metadata, even when -C codegen-units=1. +// CHECK-DAG: [[FILE:!.*]] = !DIFile(filename: "{{.*}}generator-debug-msvc.rs", {{.*}}, checksum: + +// CHECK-DAG: [[GEN_FN:!.*]] = !DINamespace(name: "generator_test" +// CHECK-DAG: [[GEN:!.*]] = !DICompositeType(tag: DW_TAG_union_type, name: "generator-0", scope: [[GEN_FN]], +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], +// For brevity, we just check the struct name and members of the last variant. +// CHECK-SAME: file: [[FILE]], line: 14, +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], +// CHECK-SAME: file: [[FILE]], line: 18, +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], +// CHECK-SAME: file: [[FILE]], line: 18, +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], +// CHECK-SAME: file: [[FILE]], line: 15, +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], +// CHECK-SAME: file: [[FILE]], line: 17, +// CHECK-SAME: baseType: [[VARIANT:![0-9]*]] +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: [[S1:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "Suspend1", scope: [[GEN_FN]], +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "RUST$ENUM$DISR", scope: [[S1]], +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "s", scope: [[S1]] +// CHECK-NOT: flags: DIFlagArtificial +// CHECK-SAME: ) + +// CHECK-DAG: [[GEN:!.*]] = !DICompositeType(tag: DW_TAG_union_type, name: "generator-0", scope: [[ASYNC_FN:![0-9]*]], {{.*}}flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], +// For brevity, we just check the struct name and members of the last variant. +// CHECK-SAME: file: [[FILE]], line: 22, +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], +// CHECK-SAME: file: [[FILE]], line: 26, +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], +// CHECK-SAME: file: [[FILE]], line: 26, +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], +// CHECK-SAME: file: [[FILE]], line: 23, +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], +// CHECK-SAME: file: [[FILE]], line: 25, +// CHECK-SAME: baseType: [[VARIANT:![0-9]*]] +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: [[S1:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "Suspend1", scope: [[ASYNC_FN]], +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "RUST$ENUM$DISR", scope: [[S1]], +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "s", scope: [[S1]] +// CHECK-NOT: flags: DIFlagArtificial +// CHECK-SAME: ) + +fn main() { + let _dummy = generator_test(); + let _dummy = async_fn_test(); +} From fe3df646fe415123d0a6bd09df698dc69074263a Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Tue, 23 Jun 2020 20:19:26 -0700 Subject: [PATCH 26/36] Give up on checking filename --- src/test/codegen/generator-debug-msvc.rs | 6 ++---- src/test/codegen/generator-debug.rs | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/test/codegen/generator-debug-msvc.rs b/src/test/codegen/generator-debug-msvc.rs index ebd0c7ba544b0..705fa5187292c 100644 --- a/src/test/codegen/generator-debug-msvc.rs +++ b/src/test/codegen/generator-debug-msvc.rs @@ -25,15 +25,13 @@ async fn async_fn_test() { foo().await; } -// FIXME: We need "checksum" to prevent matching with the wrong (duplicate) file -// metadata, even when -C codegen-units=1. -// CHECK-DAG: [[FILE:!.*]] = !DIFile(filename: "{{.*}}generator-debug-msvc.rs", {{.*}}, checksum: +// FIXME: No way to reliably check the filename. // CHECK-DAG: [[GEN_FN:!.*]] = !DINamespace(name: "generator_test" // CHECK-DAG: [[GEN:!.*]] = !DICompositeType(tag: DW_TAG_union_type, name: "generator-0", scope: [[GEN_FN]], // CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], // For brevity, we just check the struct name and members of the last variant. -// CHECK-SAME: file: [[FILE]], line: 14, +// CHECK-SAME: file: [[FILE:![0-9]*]], line: 14, // CHECK-SAME: flags: DIFlagArtificial // CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], // CHECK-SAME: file: [[FILE]], line: 18, diff --git a/src/test/codegen/generator-debug.rs b/src/test/codegen/generator-debug.rs index fa16a460dde4d..4de57c644e8ca 100644 --- a/src/test/codegen/generator-debug.rs +++ b/src/test/codegen/generator-debug.rs @@ -25,9 +25,7 @@ async fn async_fn_test() { foo().await; } -// FIXME: We need "checksum" to prevent matching with the wrong (duplicate) file -// metadata, even when -C codegen-units=1. -// CHECK-DAG: [[FILE:!.*]] = !DIFile(filename: "{{.*}}generator-debug.rs", {{.*}}, checksum: +// FIXME: No way to reliably check the filename. // CHECK-DAG: [[GEN_FN:!.*]] = !DINamespace(name: "generator_test" // CHECK-DAG: [[GEN:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "generator-0", scope: [[GEN_FN]], {{.*}}flags: DIFlagArtificial @@ -35,7 +33,7 @@ async fn async_fn_test() { // CHECK-SAME: flags: DIFlagArtificial // CHECK-SAME: discriminator: [[DISC:![0-9]*]] // CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "0", scope: [[VARIANT]], -// CHECK-SAME: file: [[FILE]], line: 14, +// CHECK-SAME: file: [[FILE:![0-9]*]], line: 14, // CHECK-SAME: flags: DIFlagArtificial // CHECK: {{!.*}} = !DICompositeType(tag: DW_TAG_structure_type, name: "Unresumed", scope: [[GEN]], // CHECK-SAME: flags: DIFlagArtificial From 887fbd9d3411ea7def2cc7a508d74bea6b7f19bb Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Wed, 24 Jun 2020 14:17:31 -0700 Subject: [PATCH 27/36] Split out async fn and generator test This keeps FileCheck from tripping over unimportant differences in codegen. --- src/test/codegen/async-fn-debug-msvc.rs | 48 ++++++++++++++++++++++ src/test/codegen/async-fn-debug.rs | 51 ++++++++++++++++++++++++ src/test/codegen/generator-debug-msvc.rs | 40 ++----------------- src/test/codegen/generator-debug.rs | 35 ---------------- 4 files changed, 102 insertions(+), 72 deletions(-) create mode 100644 src/test/codegen/async-fn-debug-msvc.rs create mode 100644 src/test/codegen/async-fn-debug.rs diff --git a/src/test/codegen/async-fn-debug-msvc.rs b/src/test/codegen/async-fn-debug-msvc.rs new file mode 100644 index 0000000000000..4e145b81ecbf7 --- /dev/null +++ b/src/test/codegen/async-fn-debug-msvc.rs @@ -0,0 +1,48 @@ +// Verify debuginfo for generators: +// - Each variant points to the file and line of its yield point +// - The generator types and variants are marked artificial +// - Captured vars from the source are not marked artificial +// +// ignore-tidy-linelength +// compile-flags: -C debuginfo=2 --edition=2018 +// only-msvc + +async fn foo() {} +async fn async_fn_test() { + foo().await; + let s = String::from("foo"); + foo().await; +} + +// FIXME: No way to reliably check the filename. + +// CHECK-DAG: [[ASYNC_FN:!.*]] = !DINamespace(name: "async_fn_test" +// CHECK-DAG: [[GEN:!.*]] = !DICompositeType(tag: DW_TAG_union_type, name: "generator-0", scope: [[ASYNC_FN]], {{.*}}flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], +// For brevity, we only check the struct name and members of the last variant. +// CHECK-SAME: file: [[FILE:![0-9]*]], line: 11, +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], +// CHECK-SAME: file: [[FILE]], line: 15, +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], +// CHECK-SAME: file: [[FILE]], line: 15, +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], +// CHECK-SAME: file: [[FILE]], line: 12, +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], +// CHECK-SAME: file: [[FILE]], line: 14, +// CHECK-SAME: baseType: [[VARIANT:![0-9]*]] +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: [[S1:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "Suspend1", scope: [[ASYNC_FN]], +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "RUST$ENUM$DISR", scope: [[S1]], +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "s", scope: [[S1]] +// CHECK-NOT: flags: DIFlagArtificial +// CHECK-SAME: ) + +fn main() { + let _dummy = async_fn_test(); +} diff --git a/src/test/codegen/async-fn-debug.rs b/src/test/codegen/async-fn-debug.rs new file mode 100644 index 0000000000000..8fa4be1ae86d8 --- /dev/null +++ b/src/test/codegen/async-fn-debug.rs @@ -0,0 +1,51 @@ +// Verify debuginfo for async fn: +// - Each variant points to the file and line of its yield point +// - The generator types and variants are marked artificial +// - Captured vars from the source are not marked artificial +// +// ignore-tidy-linelength +// compile-flags: -C debuginfo=2 --edition=2018 +// ignore-msvc + +async fn foo() {} +async fn async_fn_test() { + foo().await; + let s = String::from("foo"); + foo().await; +} + +// FIXME: No way to reliably check the filename. + +// CHECK-DAG: [[ASYNC_FN:!.*]] = !DINamespace(name: "async_fn_test" +// CHECK-DAG: [[GEN:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "generator-0", scope: [[ASYNC_FN]], {{.*}}flags: DIFlagArtificial +// CHECK: [[VARIANT:!.*]] = !DICompositeType(tag: DW_TAG_variant_part, scope: [[ASYNC_FN]], +// CHECK-SAME: flags: DIFlagArtificial +// CHECK-SAME: discriminator: [[DISC:![0-9]*]] +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "0", scope: [[VARIANT]], +// CHECK-SAME: file: [[FILE:![0-9]*]], line: 11, +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DICompositeType(tag: DW_TAG_structure_type, name: "Unresumed", scope: [[GEN]], +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "1", scope: [[VARIANT]], +// CHECK-SAME: file: [[FILE]], line: 15, +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "2", scope: [[VARIANT]], +// CHECK-SAME: file: [[FILE]], line: 15, +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "3", scope: [[VARIANT]], +// CHECK-SAME: file: [[FILE]], line: 12, +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "4", scope: [[VARIANT]], +// CHECK-SAME: file: [[FILE]], line: 14, +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: [[S1:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "Suspend1", scope: [[GEN]], +// CHECK-SAME: flags: DIFlagArtificial +// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "s", scope: [[S1]] +// CHECK-NOT: flags: DIFlagArtificial +// CHECK-SAME: ) +// CHECK: [[DISC]] = !DIDerivedType(tag: DW_TAG_member, name: "__state", scope: [[ASYNC_FN]], +// CHECK-SAME: flags: DIFlagArtificial + +fn main() { + let _dummy = async_fn_test(); +} diff --git a/src/test/codegen/generator-debug-msvc.rs b/src/test/codegen/generator-debug-msvc.rs index 705fa5187292c..82a1568ea9584 100644 --- a/src/test/codegen/generator-debug-msvc.rs +++ b/src/test/codegen/generator-debug-msvc.rs @@ -4,7 +4,7 @@ // - Captured vars from the source are not marked artificial // // ignore-tidy-linelength -// compile-flags: -C debuginfo=2 --edition=2018 +// compile-flags: -C debuginfo=2 // only-msvc #![feature(generators, generator_trait)] @@ -18,19 +18,12 @@ fn generator_test() -> impl Generator { } } -async fn foo() {} -async fn async_fn_test() { - foo().await; - let s = String::from("foo"); - foo().await; -} - // FIXME: No way to reliably check the filename. // CHECK-DAG: [[GEN_FN:!.*]] = !DINamespace(name: "generator_test" -// CHECK-DAG: [[GEN:!.*]] = !DICompositeType(tag: DW_TAG_union_type, name: "generator-0", scope: [[GEN_FN]], +// CHECK-DAG: [[GEN:!.*]] = !DICompositeType(tag: DW_TAG_union_type, name: "generator-0", scope: [[GEN_FN]], {{.*}}flags: DIFlagArtificial // CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], -// For brevity, we just check the struct name and members of the last variant. +// For brevity, we only check the struct name and members of the last variant. // CHECK-SAME: file: [[FILE:![0-9]*]], line: 14, // CHECK-SAME: flags: DIFlagArtificial // CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], @@ -54,33 +47,6 @@ async fn async_fn_test() { // CHECK-NOT: flags: DIFlagArtificial // CHECK-SAME: ) -// CHECK-DAG: [[GEN:!.*]] = !DICompositeType(tag: DW_TAG_union_type, name: "generator-0", scope: [[ASYNC_FN:![0-9]*]], {{.*}}flags: DIFlagArtificial -// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], -// For brevity, we just check the struct name and members of the last variant. -// CHECK-SAME: file: [[FILE]], line: 22, -// CHECK-SAME: flags: DIFlagArtificial -// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], -// CHECK-SAME: file: [[FILE]], line: 26, -// CHECK-SAME: flags: DIFlagArtificial -// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], -// CHECK-SAME: file: [[FILE]], line: 26, -// CHECK-SAME: flags: DIFlagArtificial -// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], -// CHECK-SAME: file: [[FILE]], line: 23, -// CHECK-SAME: flags: DIFlagArtificial -// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, scope: [[GEN]], -// CHECK-SAME: file: [[FILE]], line: 25, -// CHECK-SAME: baseType: [[VARIANT:![0-9]*]] -// CHECK-SAME: flags: DIFlagArtificial -// CHECK: [[S1:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "Suspend1", scope: [[ASYNC_FN]], -// CHECK-SAME: flags: DIFlagArtificial -// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "RUST$ENUM$DISR", scope: [[S1]], -// CHECK-SAME: flags: DIFlagArtificial -// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "s", scope: [[S1]] -// CHECK-NOT: flags: DIFlagArtificial -// CHECK-SAME: ) - fn main() { let _dummy = generator_test(); - let _dummy = async_fn_test(); } diff --git a/src/test/codegen/generator-debug.rs b/src/test/codegen/generator-debug.rs index 4de57c644e8ca..5c7c64148189a 100644 --- a/src/test/codegen/generator-debug.rs +++ b/src/test/codegen/generator-debug.rs @@ -18,13 +18,6 @@ fn generator_test() -> impl Generator { } } -async fn foo() {} -async fn async_fn_test() { - foo().await; - let s = String::from("foo"); - foo().await; -} - // FIXME: No way to reliably check the filename. // CHECK-DAG: [[GEN_FN:!.*]] = !DINamespace(name: "generator_test" @@ -57,34 +50,6 @@ async fn async_fn_test() { // CHECK: [[DISC]] = !DIDerivedType(tag: DW_TAG_member, name: "__state", scope: [[GEN_FN]], // CHECK-SAME: flags: DIFlagArtificial -// CHECK-DAG: [[GEN:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "generator-0", scope: [[ASYNC_FN:![0-9]*]], {{.*}}flags: DIFlagArtificial -// CHECK: [[VARIANT:!.*]] = !DICompositeType(tag: DW_TAG_variant_part, scope: [[ASYNC_FN]], -// CHECK-SAME: flags: DIFlagArtificial -// CHECK-SAME: discriminator: [[DISC:![0-9]*]] -// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "0", scope: [[VARIANT]], -// CHECK-SAME: file: [[FILE]], line: 22, -// CHECK-SAME: flags: DIFlagArtificial -// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "1", scope: [[VARIANT]], -// CHECK-SAME: file: [[FILE]], line: 26, -// CHECK-SAME: flags: DIFlagArtificial -// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "2", scope: [[VARIANT]], -// CHECK-SAME: file: [[FILE]], line: 26, -// CHECK-SAME: flags: DIFlagArtificial -// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "3", scope: [[VARIANT]], -// CHECK-SAME: file: [[FILE]], line: 23, -// CHECK-SAME: flags: DIFlagArtificial -// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "4", scope: [[VARIANT]], -// CHECK-SAME: file: [[FILE]], line: 25, -// CHECK-SAME: flags: DIFlagArtificial -// CHECK: [[S1:!.*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "Suspend1", scope: [[GEN]], -// CHECK-SAME: flags: DIFlagArtificial -// CHECK: {{!.*}} = !DIDerivedType(tag: DW_TAG_member, name: "s", scope: [[S1]] -// CHECK-NOT: flags: DIFlagArtificial -// CHECK-SAME: ) -// CHECK: [[DISC]] = !DIDerivedType(tag: DW_TAG_member, name: "__state", scope: [[ASYNC_FN]], -// CHECK-SAME: flags: DIFlagArtificial - fn main() { let _dummy = generator_test(); - let _dummy = async_fn_test(); } From 8f40dae93b3a66a9cdd0f940244da7f602618fba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 24 Jun 2020 16:17:04 -0700 Subject: [PATCH 28/36] Suggest type param trait bound for binop only when appropriate Verify that the binop trait *is* implemented for the types *if* all the involved type parameters are replaced with fresh inferred types. When this is the case, it means that the type parameter was indeed missing a trait bound. If this is not the case, provide a generic `note` refering to the type that doesn't implement the expected trait. --- src/librustc_typeck/check/op.rs | 67 +++++++++++++------ src/test/ui/issues/issue-35668.stderr | 1 - src/test/ui/suggestions/invalid-bin-op.rs | 7 ++ src/test/ui/suggestions/invalid-bin-op.stderr | 13 ++++ .../missing-trait-bound-for-op.fixed | 12 +--- .../suggestions/missing-trait-bound-for-op.rs | 12 +--- .../missing-trait-bound-for-op.stderr | 15 ++--- .../trait-resolution-in-overloaded-op.stderr | 1 - 8 files changed, 78 insertions(+), 50 deletions(-) create mode 100644 src/test/ui/suggestions/invalid-bin-op.rs create mode 100644 src/test/ui/suggestions/invalid-bin-op.stderr diff --git a/src/librustc_typeck/check/op.rs b/src/librustc_typeck/check/op.rs index 0184c00c475b5..e333b706e745d 100644 --- a/src/librustc_typeck/check/op.rs +++ b/src/librustc_typeck/check/op.rs @@ -8,6 +8,7 @@ use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKi use rustc_middle::ty::adjustment::{ Adjust, Adjustment, AllowTwoPhase, AutoBorrow, AutoBorrowMutability, }; +use rustc_middle::ty::fold::TypeFolder; use rustc_middle::ty::TyKind::{Adt, Array, Char, FnDef, Never, Ref, Str, Tuple, Uint}; use rustc_middle::ty::{ self, suggest_constraining_type_param, Ty, TyCtxt, TypeFoldable, TypeVisitor, @@ -436,29 +437,36 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // we don't want the note in the else clause to be emitted } else if let [ty] = &visitor.0[..] { if let ty::Param(p) = ty.kind { - // FIXME: This *guesses* that constraining the type param - // will make the operation available, but this is only true - // when the corresponding trait has a blanket - // implementation, like the following: - // `impl<'a> PartialEq for &'a [T] where T: PartialEq {}` - // The correct thing to do would be to verify this - // projection would hold. - if *ty != lhs_ty { + // Check if the method would be found if the type param wasn't + // involved. If so, it means that adding a trait bound to the param is + // enough. Otherwise we do not give the suggestion. + let mut eraser = TypeParamEraser(&self, expr.span); + let needs_bound = self + .lookup_op_method( + eraser.fold_ty(lhs_ty), + &[eraser.fold_ty(rhs_ty)], + Op::Binary(op, is_assign), + ) + .is_ok(); + if needs_bound { + suggest_constraining_param( + self.tcx, + self.body_id, + &mut err, + ty, + rhs_ty, + missing_trait, + p, + use_output, + ); + } else if *ty != lhs_ty { + // When we know that a missing bound is responsible, we don't show + // this note as it is redundant. err.note(&format!( "the trait `{}` is not implemented for `{}`", missing_trait, lhs_ty )); } - suggest_constraining_param( - self.tcx, - self.body_id, - &mut err, - ty, - rhs_ty, - missing_trait, - p, - use_output, - ); } else { bug!("type param visitor stored a non type param: {:?}", ty.kind); } @@ -656,10 +664,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); err.span_label( ex.span, - format!( - "cannot apply unary operator `{}`", - op.as_str() - ), + format!("cannot apply unary operator `{}`", op.as_str()), ); match actual.kind { Uint(_) if op == hir::UnOp::UnNeg => { @@ -954,3 +959,21 @@ impl<'tcx> TypeVisitor<'tcx> for TypeParamVisitor<'tcx> { ty.super_visit_with(self) } } + +struct TypeParamEraser<'a, 'tcx>(&'a FnCtxt<'a, 'tcx>, Span); + +impl TypeFolder<'tcx> for TypeParamEraser<'_, 'tcx> { + fn tcx(&self) -> TyCtxt<'tcx> { + self.0.tcx + } + + fn fold_ty(&mut self, ty: Ty<'tcx>) -> Ty<'tcx> { + match ty.kind { + ty::Param(_) => self.0.next_ty_var(TypeVariableOrigin { + kind: TypeVariableOriginKind::MiscVariable, + span: self.1, + }), + _ => ty.super_fold_with(self), + } + } +} diff --git a/src/test/ui/issues/issue-35668.stderr b/src/test/ui/issues/issue-35668.stderr index c8f1f88a4708e..600cacc23aef5 100644 --- a/src/test/ui/issues/issue-35668.stderr +++ b/src/test/ui/issues/issue-35668.stderr @@ -6,7 +6,6 @@ LL | a.iter().map(|a| a*a) | | | &T | - = note: the trait `std::ops::Mul` is not implemented for `&T` help: consider restricting type parameter `T` | LL | fn func<'a, T: std::ops::Mul>(a: &'a [T]) -> impl Iterator { diff --git a/src/test/ui/suggestions/invalid-bin-op.rs b/src/test/ui/suggestions/invalid-bin-op.rs new file mode 100644 index 0000000000000..bea1b91558646 --- /dev/null +++ b/src/test/ui/suggestions/invalid-bin-op.rs @@ -0,0 +1,7 @@ +pub fn foo(s: S, t: S) { + let _ = s == t; //~ ERROR binary operation `==` cannot be applied to type `S` +} + +struct S(T); + +fn main() {} diff --git a/src/test/ui/suggestions/invalid-bin-op.stderr b/src/test/ui/suggestions/invalid-bin-op.stderr new file mode 100644 index 0000000000000..7668eddf6070a --- /dev/null +++ b/src/test/ui/suggestions/invalid-bin-op.stderr @@ -0,0 +1,13 @@ +error[E0369]: binary operation `==` cannot be applied to type `S` + --> $DIR/invalid-bin-op.rs:2:15 + | +LL | let _ = s == t; + | - ^^ - S + | | + | S + | + = note: the trait `std::cmp::PartialEq` is not implemented for `S` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0369`. diff --git a/src/test/ui/suggestions/missing-trait-bound-for-op.fixed b/src/test/ui/suggestions/missing-trait-bound-for-op.fixed index 02886bb845cf7..6b24375e41503 100644 --- a/src/test/ui/suggestions/missing-trait-bound-for-op.fixed +++ b/src/test/ui/suggestions/missing-trait-bound-for-op.fixed @@ -1,13 +1,7 @@ // run-rustfix -pub fn strip_prefix<'a, T: std::cmp::PartialEq>(s: &'a [T], prefix: &[T]) -> Option<&'a [T]> { - let n = prefix.len(); - if n <= s.len() { - let (head, tail) = s.split_at(n); - if head == prefix { //~ ERROR binary operation `==` cannot be applied to type `&[T]` - return Some(tail); - } - } - None +pub fn foo(s: &[T], t: &[T]) { + let _ = s == t; //~ ERROR binary operation `==` cannot be applied to type `&[T]` } + fn main() {} diff --git a/src/test/ui/suggestions/missing-trait-bound-for-op.rs b/src/test/ui/suggestions/missing-trait-bound-for-op.rs index aa4ef467360d9..df47be070c9ea 100644 --- a/src/test/ui/suggestions/missing-trait-bound-for-op.rs +++ b/src/test/ui/suggestions/missing-trait-bound-for-op.rs @@ -1,13 +1,7 @@ // run-rustfix -pub fn strip_prefix<'a, T>(s: &'a [T], prefix: &[T]) -> Option<&'a [T]> { - let n = prefix.len(); - if n <= s.len() { - let (head, tail) = s.split_at(n); - if head == prefix { //~ ERROR binary operation `==` cannot be applied to type `&[T]` - return Some(tail); - } - } - None +pub fn foo(s: &[T], t: &[T]) { + let _ = s == t; //~ ERROR binary operation `==` cannot be applied to type `&[T]` } + fn main() {} diff --git a/src/test/ui/suggestions/missing-trait-bound-for-op.stderr b/src/test/ui/suggestions/missing-trait-bound-for-op.stderr index dab4e575be1fa..0e0d397d6fc15 100644 --- a/src/test/ui/suggestions/missing-trait-bound-for-op.stderr +++ b/src/test/ui/suggestions/missing-trait-bound-for-op.stderr @@ -1,16 +1,15 @@ error[E0369]: binary operation `==` cannot be applied to type `&[T]` - --> $DIR/missing-trait-bound-for-op.rs:7:17 + --> $DIR/missing-trait-bound-for-op.rs:4:15 | -LL | if head == prefix { - | ---- ^^ ------ &[T] - | | - | &[T] +LL | let _ = s == t; + | - ^^ - &[T] + | | + | &[T] | - = note: the trait `std::cmp::PartialEq` is not implemented for `&[T]` help: consider restricting type parameter `T` | -LL | pub fn strip_prefix<'a, T: std::cmp::PartialEq>(s: &'a [T], prefix: &[T]) -> Option<&'a [T]> { - | ^^^^^^^^^^^^^^^^^^^^^ +LL | pub fn foo(s: &[T], t: &[T]) { + | ^^^^^^^^^^^^^^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/traits/trait-resolution-in-overloaded-op.stderr b/src/test/ui/traits/trait-resolution-in-overloaded-op.stderr index 1c424ce7da669..507d53dc07c4c 100644 --- a/src/test/ui/traits/trait-resolution-in-overloaded-op.stderr +++ b/src/test/ui/traits/trait-resolution-in-overloaded-op.stderr @@ -6,7 +6,6 @@ LL | a * b | | | &T | - = note: the trait `std::ops::Mul` is not implemented for `&T` help: consider further restricting this bound | LL | fn foo + std::ops::Mul>(a: &T, b: f64) -> f64 { From 520461f1fb2730f8edb17922f3bcc74fccdc52d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 19 Jun 2020 22:45:09 -0700 Subject: [PATCH 29/36] Provide suggestions for some moved value errors When encountering an used moved value where the previous move happened in a `match` or `if let` pattern, suggest using `ref`. Fix #63988. When encountering a `&mut` value that is used in multiple iterations of a loop, suggest reborrowing it with `&mut *`. Fix #62112. --- .../diagnostics/conflict_errors.rs | 39 +++++++++++++++++-- .../diagnostics/explain_borrow.rs | 2 +- .../borrow_check/diagnostics/mod.rs | 20 +++++++--- src/test/ui/borrowck/issue-41962.stderr | 4 ++ src/test/ui/borrowck/move-in-pattern-mut.rs | 23 +++++++++++ .../ui/borrowck/move-in-pattern-mut.stderr | 33 ++++++++++++++++ src/test/ui/borrowck/move-in-pattern.fixed | 24 ++++++++++++ src/test/ui/borrowck/move-in-pattern.rs | 24 ++++++++++++ src/test/ui/borrowck/move-in-pattern.stderr | 33 ++++++++++++++++ .../ui/borrowck/mut-borrow-in-loop-2.fixed | 35 +++++++++++++++++ src/test/ui/borrowck/mut-borrow-in-loop-2.rs | 35 +++++++++++++++++ .../ui/borrowck/mut-borrow-in-loop-2.stderr | 17 ++++++++ ...sed-on-type-cyclic-types-issue-4821.stderr | 4 ++ src/test/ui/nll/issue-53807.stderr | 4 ++ ...can-live-while-the-other-survives-1.stderr | 8 ++++ ...orrowck-pat-by-move-and-ref-inverse.stderr | 16 ++++++++ src/test/ui/ref-suggestion.stderr | 4 ++ 17 files changed, 315 insertions(+), 10 deletions(-) create mode 100644 src/test/ui/borrowck/move-in-pattern-mut.rs create mode 100644 src/test/ui/borrowck/move-in-pattern-mut.stderr create mode 100644 src/test/ui/borrowck/move-in-pattern.fixed create mode 100644 src/test/ui/borrowck/move-in-pattern.rs create mode 100644 src/test/ui/borrowck/move-in-pattern.stderr create mode 100644 src/test/ui/borrowck/mut-borrow-in-loop-2.fixed create mode 100644 src/test/ui/borrowck/mut-borrow-in-loop-2.rs create mode 100644 src/test/ui/borrowck/mut-borrow-in-loop-2.stderr diff --git a/src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs b/src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs index 8d7944004c75e..60a1fe0b19870 100644 --- a/src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs @@ -156,6 +156,20 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { format!("variable moved due to use{}", move_spans.describe()), ); } + if let UseSpans::PatUse(span) = move_spans { + err.span_suggestion_verbose( + span.shrink_to_lo(), + &format!( + "borrow this field in the pattern to avoid moving {}", + self.describe_place(moved_place.as_ref()) + .map(|n| format!("`{}`", n)) + .unwrap_or_else(|| "the value".to_string()) + ), + "ref ".to_string(), + Applicability::MachineApplicable, + ); + } + if Some(DesugaringKind::ForLoop) == move_span.desugaring_kind() { let sess = self.infcx.tcx.sess; if let Ok(snippet) = sess.source_map().span_to_snippet(move_span) { @@ -198,11 +212,28 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { _ => true, }; - if needs_note { - let mpi = self.move_data.moves[move_out_indices[0]].path; - let place = &self.move_data.move_paths[mpi].place; + let mpi = self.move_data.moves[move_out_indices[0]].path; + let place = &self.move_data.move_paths[mpi].place; + let ty = place.ty(self.body, self.infcx.tcx).ty; + + if is_loop_move { + if let ty::Ref(_, _, hir::Mutability::Mut) = ty.kind { + // We have a `&mut` ref, we need to reborrow on each iteration (#62112). + err.span_suggestion_verbose( + span.shrink_to_lo(), + &format!( + "consider creating a fresh reborrow of {} here", + self.describe_place(moved_place) + .map(|n| format!("`{}`", n)) + .unwrap_or_else(|| "the mutable reference".to_string()), + ), + "&mut *".to_string(), + Applicability::MachineApplicable, + ); + } + } - let ty = place.ty(self.body, self.infcx.tcx).ty; + if needs_note { let opt_name = self.describe_place_with_options(place.as_ref(), IncludingDowncast(true)); let note_msg = match opt_name { diff --git a/src/librustc_mir/borrow_check/diagnostics/explain_borrow.rs b/src/librustc_mir/borrow_check/diagnostics/explain_borrow.rs index 5253acbba7f1c..849fd63998db4 100644 --- a/src/librustc_mir/borrow_check/diagnostics/explain_borrow.rs +++ b/src/librustc_mir/borrow_check/diagnostics/explain_borrow.rs @@ -509,7 +509,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // Used in a closure. (LaterUseKind::ClosureCapture, var_span) } - UseSpans::OtherUse(span) => { + UseSpans::PatUse(span) | UseSpans::OtherUse(span) => { let block = &self.body.basic_blocks()[location.block]; let kind = if let Some(&Statement { diff --git a/src/librustc_mir/borrow_check/diagnostics/mod.rs b/src/librustc_mir/borrow_check/diagnostics/mod.rs index ca8e54ea28649..388076a9d60af 100644 --- a/src/librustc_mir/borrow_check/diagnostics/mod.rs +++ b/src/librustc_mir/borrow_check/diagnostics/mod.rs @@ -542,20 +542,26 @@ pub(super) enum UseSpans { // The span of the first use of the captured variable inside the closure. var_span: Span, }, - // This access has a single span associated to it: common case. + /// This access is caused by a `match` or `if let` pattern. + PatUse(Span), + /// This access has a single span associated to it: common case. OtherUse(Span), } impl UseSpans { pub(super) fn args_or_use(self) -> Span { match self { - UseSpans::ClosureUse { args_span: span, .. } | UseSpans::OtherUse(span) => span, + UseSpans::ClosureUse { args_span: span, .. } + | UseSpans::PatUse(span) + | UseSpans::OtherUse(span) => span, } } pub(super) fn var_or_use(self) -> Span { match self { - UseSpans::ClosureUse { var_span: span, .. } | UseSpans::OtherUse(span) => span, + UseSpans::ClosureUse { var_span: span, .. } + | UseSpans::PatUse(span) + | UseSpans::OtherUse(span) => span, } } @@ -624,7 +630,7 @@ impl UseSpans { { match self { closure @ UseSpans::ClosureUse { .. } => closure, - UseSpans::OtherUse(_) => if_other(), + UseSpans::PatUse(_) | UseSpans::OtherUse(_) => if_other(), } } } @@ -741,7 +747,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } - OtherUse(stmt.source_info.span) + if moved_place.projection.iter().any(|p| matches!(p, ProjectionElem::Downcast(..))) { + PatUse(stmt.source_info.span) + } else { + OtherUse(stmt.source_info.span) + } } /// Finds the span of arguments of a closure (within `maybe_closure_span`) diff --git a/src/test/ui/borrowck/issue-41962.stderr b/src/test/ui/borrowck/issue-41962.stderr index 422d1605aa46b..604143b4e7efd 100644 --- a/src/test/ui/borrowck/issue-41962.stderr +++ b/src/test/ui/borrowck/issue-41962.stderr @@ -5,6 +5,10 @@ LL | if let Some(thing) = maybe { | ^^^^^ value moved here, in previous iteration of loop | = note: move occurs because value has type `std::vec::Vec`, which does not implement the `Copy` trait +help: borrow this field in the pattern to avoid moving `maybe.0` + | +LL | if let Some(ref thing) = maybe { + | ^^^ error: aborting due to previous error diff --git a/src/test/ui/borrowck/move-in-pattern-mut.rs b/src/test/ui/borrowck/move-in-pattern-mut.rs new file mode 100644 index 0000000000000..175eb3b7a04d1 --- /dev/null +++ b/src/test/ui/borrowck/move-in-pattern-mut.rs @@ -0,0 +1,23 @@ +// Issue #63988 +#[derive(Debug)] +struct S; +fn foo(_: Option) {} + +enum E { + V { + s: S, + } +} +fn bar(_: E) {} + +fn main() { + let s = Some(S); + if let Some(mut x) = s { + x = S; + } + foo(s); //~ ERROR use of moved value: `s` + let mut e = E::V { s: S }; + let E::V { s: mut x } = e; + x = S; + bar(e); //~ ERROR use of moved value: `e` +} diff --git a/src/test/ui/borrowck/move-in-pattern-mut.stderr b/src/test/ui/borrowck/move-in-pattern-mut.stderr new file mode 100644 index 0000000000000..391638444c3bd --- /dev/null +++ b/src/test/ui/borrowck/move-in-pattern-mut.stderr @@ -0,0 +1,33 @@ +error[E0382]: use of moved value: `s` + --> $DIR/move-in-pattern-mut.rs:18:9 + | +LL | if let Some(mut x) = s { + | ----- value moved here +... +LL | foo(s); + | ^ value used here after partial move + | + = note: move occurs because value has type `S`, which does not implement the `Copy` trait +help: borrow this field in the pattern to avoid moving `s.0` + | +LL | if let Some(ref mut x) = s { + | ^^^ + +error[E0382]: use of moved value: `e` + --> $DIR/move-in-pattern-mut.rs:22:9 + | +LL | let E::V { s: mut x } = e; + | ----- value moved here +LL | x = S; +LL | bar(e); + | ^ value used here after partial move + | + = note: move occurs because value has type `S`, which does not implement the `Copy` trait +help: borrow this field in the pattern to avoid moving `e.s` + | +LL | let E::V { s: ref mut x } = e; + | ^^^ + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0382`. diff --git a/src/test/ui/borrowck/move-in-pattern.fixed b/src/test/ui/borrowck/move-in-pattern.fixed new file mode 100644 index 0000000000000..f55fdcc5f90e8 --- /dev/null +++ b/src/test/ui/borrowck/move-in-pattern.fixed @@ -0,0 +1,24 @@ +// run-rustfix +// Issue #63988 +#[derive(Debug)] +struct S; +fn foo(_: Option) {} + +enum E { + V { + s: S, + } +} +fn bar(_: E) {} + +fn main() { + let s = Some(S); + if let Some(ref x) = s { + let _ = x; + } + foo(s); //~ ERROR use of moved value: `s` + let e = E::V { s: S }; + let E::V { s: ref x } = e; + let _ = x; + bar(e); //~ ERROR use of moved value: `e` +} diff --git a/src/test/ui/borrowck/move-in-pattern.rs b/src/test/ui/borrowck/move-in-pattern.rs new file mode 100644 index 0000000000000..7ad04b9490c25 --- /dev/null +++ b/src/test/ui/borrowck/move-in-pattern.rs @@ -0,0 +1,24 @@ +// run-rustfix +// Issue #63988 +#[derive(Debug)] +struct S; +fn foo(_: Option) {} + +enum E { + V { + s: S, + } +} +fn bar(_: E) {} + +fn main() { + let s = Some(S); + if let Some(x) = s { + let _ = x; + } + foo(s); //~ ERROR use of moved value: `s` + let e = E::V { s: S }; + let E::V { s: x } = e; + let _ = x; + bar(e); //~ ERROR use of moved value: `e` +} diff --git a/src/test/ui/borrowck/move-in-pattern.stderr b/src/test/ui/borrowck/move-in-pattern.stderr new file mode 100644 index 0000000000000..c5cb24455eb61 --- /dev/null +++ b/src/test/ui/borrowck/move-in-pattern.stderr @@ -0,0 +1,33 @@ +error[E0382]: use of moved value: `s` + --> $DIR/move-in-pattern.rs:19:9 + | +LL | if let Some(x) = s { + | - value moved here +... +LL | foo(s); + | ^ value used here after partial move + | + = note: move occurs because value has type `S`, which does not implement the `Copy` trait +help: borrow this field in the pattern to avoid moving `s.0` + | +LL | if let Some(ref x) = s { + | ^^^ + +error[E0382]: use of moved value: `e` + --> $DIR/move-in-pattern.rs:23:9 + | +LL | let E::V { s: x } = e; + | - value moved here +LL | let _ = x; +LL | bar(e); + | ^ value used here after partial move + | + = note: move occurs because value has type `S`, which does not implement the `Copy` trait +help: borrow this field in the pattern to avoid moving `e.s` + | +LL | let E::V { s: ref x } = e; + | ^^^ + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0382`. diff --git a/src/test/ui/borrowck/mut-borrow-in-loop-2.fixed b/src/test/ui/borrowck/mut-borrow-in-loop-2.fixed new file mode 100644 index 0000000000000..ceeba30a90f29 --- /dev/null +++ b/src/test/ui/borrowck/mut-borrow-in-loop-2.fixed @@ -0,0 +1,35 @@ +// run-rustfix +#![allow(dead_code)] + +struct Events(R); + +struct Other; + +pub trait Trait { + fn handle(value: T) -> Self; +} + +// Blanket impl. (If you comment this out, compiler figures out that it +// is passing an `&mut` to a method that must be expecting an `&mut`, +// and injects an auto-reborrow.) +impl Trait for T where T: From { + fn handle(_: U) -> Self { unimplemented!() } +} + +impl<'a, R> Trait<&'a mut Events> for Other { + fn handle(_: &'a mut Events) -> Self { unimplemented!() } +} + +fn this_compiles<'a, R>(value: &'a mut Events) { + for _ in 0..3 { + Other::handle(&mut *value); + } +} + +fn this_does_not<'a, R>(value: &'a mut Events) { + for _ in 0..3 { + Other::handle(&mut *value); //~ ERROR use of moved value: `value` + } +} + +fn main() {} diff --git a/src/test/ui/borrowck/mut-borrow-in-loop-2.rs b/src/test/ui/borrowck/mut-borrow-in-loop-2.rs new file mode 100644 index 0000000000000..d13fb7e567939 --- /dev/null +++ b/src/test/ui/borrowck/mut-borrow-in-loop-2.rs @@ -0,0 +1,35 @@ +// run-rustfix +#![allow(dead_code)] + +struct Events(R); + +struct Other; + +pub trait Trait { + fn handle(value: T) -> Self; +} + +// Blanket impl. (If you comment this out, compiler figures out that it +// is passing an `&mut` to a method that must be expecting an `&mut`, +// and injects an auto-reborrow.) +impl Trait for T where T: From { + fn handle(_: U) -> Self { unimplemented!() } +} + +impl<'a, R> Trait<&'a mut Events> for Other { + fn handle(_: &'a mut Events) -> Self { unimplemented!() } +} + +fn this_compiles<'a, R>(value: &'a mut Events) { + for _ in 0..3 { + Other::handle(&mut *value); + } +} + +fn this_does_not<'a, R>(value: &'a mut Events) { + for _ in 0..3 { + Other::handle(value); //~ ERROR use of moved value: `value` + } +} + +fn main() {} diff --git a/src/test/ui/borrowck/mut-borrow-in-loop-2.stderr b/src/test/ui/borrowck/mut-borrow-in-loop-2.stderr new file mode 100644 index 0000000000000..fa1b741394acb --- /dev/null +++ b/src/test/ui/borrowck/mut-borrow-in-loop-2.stderr @@ -0,0 +1,17 @@ +error[E0382]: use of moved value: `value` + --> $DIR/mut-borrow-in-loop-2.rs:31:23 + | +LL | fn this_does_not<'a, R>(value: &'a mut Events) { + | ----- move occurs because `value` has type `&mut Events`, which does not implement the `Copy` trait +LL | for _ in 0..3 { +LL | Other::handle(value); + | ^^^^^ value moved here, in previous iteration of loop + | +help: consider creating a fresh reborrow of `value` here + | +LL | Other::handle(&mut *value); + | ^^^^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0382`. diff --git a/src/test/ui/moves/moves-based-on-type-cyclic-types-issue-4821.stderr b/src/test/ui/moves/moves-based-on-type-cyclic-types-issue-4821.stderr index fb8562d00ead1..952985fcddee6 100644 --- a/src/test/ui/moves/moves-based-on-type-cyclic-types-issue-4821.stderr +++ b/src/test/ui/moves/moves-based-on-type-cyclic-types-issue-4821.stderr @@ -8,6 +8,10 @@ LL | consume(node) + r | ^^^^ value used here after partial move | = note: move occurs because value has type `std::boxed::Box`, which does not implement the `Copy` trait +help: borrow this field in the pattern to avoid moving `node.next.0` + | +LL | Some(ref right) => consume(right), + | ^^^ error: aborting due to previous error diff --git a/src/test/ui/nll/issue-53807.stderr b/src/test/ui/nll/issue-53807.stderr index 2b15da3710e62..4f36a4ccab28f 100644 --- a/src/test/ui/nll/issue-53807.stderr +++ b/src/test/ui/nll/issue-53807.stderr @@ -5,6 +5,10 @@ LL | if let Some(thing) = maybe { | ^^^^^ value moved here, in previous iteration of loop | = note: move occurs because value has type `std::vec::Vec`, which does not implement the `Copy` trait +help: borrow this field in the pattern to avoid moving `maybe.0` + | +LL | if let Some(ref thing) = maybe { + | ^^^ error: aborting due to previous error diff --git a/src/test/ui/pattern/bindings-after-at/bind-by-move-neither-can-live-while-the-other-survives-1.stderr b/src/test/ui/pattern/bindings-after-at/bind-by-move-neither-can-live-while-the-other-survives-1.stderr index f2186b9298e68..8a6ea8e91a25a 100644 --- a/src/test/ui/pattern/bindings-after-at/bind-by-move-neither-can-live-while-the-other-survives-1.stderr +++ b/src/test/ui/pattern/bindings-after-at/bind-by-move-neither-can-live-while-the-other-survives-1.stderr @@ -46,6 +46,10 @@ LL | Some(_z @ ref _y) => {} | value moved here | = note: move occurs because value has type `X`, which does not implement the `Copy` trait +help: borrow this field in the pattern to avoid moving `x.0` + | +LL | Some(ref _z @ ref _y) => {} + | ^^^ error[E0382]: borrow of moved value --> $DIR/bind-by-move-neither-can-live-while-the-other-survives-1.rs:35:19 @@ -57,6 +61,10 @@ LL | Some(_z @ ref mut _y) => {} | value moved here | = note: move occurs because value has type `X`, which does not implement the `Copy` trait +help: borrow this field in the pattern to avoid moving `x.0` + | +LL | Some(ref _z @ ref mut _y) => {} + | ^^^ error: aborting due to 6 previous errors diff --git a/src/test/ui/pattern/bindings-after-at/borrowck-pat-by-move-and-ref-inverse.stderr b/src/test/ui/pattern/bindings-after-at/borrowck-pat-by-move-and-ref-inverse.stderr index f819e671436ec..5058998f2a7c1 100644 --- a/src/test/ui/pattern/bindings-after-at/borrowck-pat-by-move-and-ref-inverse.stderr +++ b/src/test/ui/pattern/bindings-after-at/borrowck-pat-by-move-and-ref-inverse.stderr @@ -357,6 +357,10 @@ LL | a @ Some((mut b @ ref mut c, d @ ref e)) => {} | value moved here | = note: move occurs because value has type `main::U`, which does not implement the `Copy` trait +help: borrow this field in the pattern to avoid moving the value + | +LL | a @ Some((ref mut b @ ref mut c, d @ ref e)) => {} + | ^^^ error[E0382]: use of moved value --> $DIR/borrowck-pat-by-move-and-ref-inverse.rs:61:38 @@ -379,6 +383,10 @@ LL | a @ Some((mut b @ ref mut c, d @ ref e)) => {} | value moved here | = note: move occurs because value has type `main::U`, which does not implement the `Copy` trait +help: borrow this field in the pattern to avoid moving the value + | +LL | a @ Some((mut b @ ref mut c, ref d @ ref e)) => {} + | ^^^ error[E0382]: borrow of moved value --> $DIR/borrowck-pat-by-move-and-ref-inverse.rs:71:30 @@ -412,6 +420,10 @@ LL | a @ Some((mut b @ ref mut c, d @ ref e)) => {} | value moved here | = note: move occurs because value has type `main::U`, which does not implement the `Copy` trait +help: borrow this field in the pattern to avoid moving the value + | +LL | a @ Some((ref mut b @ ref mut c, d @ ref e)) => {} + | ^^^ error[E0382]: use of moved value --> $DIR/borrowck-pat-by-move-and-ref-inverse.rs:83:38 @@ -434,6 +446,10 @@ LL | a @ Some((mut b @ ref mut c, d @ ref e)) => {} | value moved here | = note: move occurs because value has type `main::U`, which does not implement the `Copy` trait +help: borrow this field in the pattern to avoid moving the value + | +LL | a @ Some((mut b @ ref mut c, ref d @ ref e)) => {} + | ^^^ error[E0382]: borrow of moved value --> $DIR/borrowck-pat-by-move-and-ref-inverse.rs:93:30 diff --git a/src/test/ui/ref-suggestion.stderr b/src/test/ui/ref-suggestion.stderr index 9ff8e21bb58bd..97d2c174d9adb 100644 --- a/src/test/ui/ref-suggestion.stderr +++ b/src/test/ui/ref-suggestion.stderr @@ -28,6 +28,10 @@ LL | x; | ^ value used here after partial move | = note: move occurs because value has type `std::vec::Vec`, which does not implement the `Copy` trait +help: borrow this field in the pattern to avoid moving `x.0.0` + | +LL | (Some(ref y), ()) => {}, + | ^^^ error: aborting due to 3 previous errors From 3d09017477e562d44cb90fca1a4a38c75f664f2d Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Thu, 25 Jun 2020 10:05:30 +0200 Subject: [PATCH 30/36] Add a compile fail example, binding -> variable, apply suggestions --- src/libstd/keyword_docs.rs | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/src/libstd/keyword_docs.rs b/src/libstd/keyword_docs.rs index 7d54f9ed2d20f..32fa0da3e2925 100644 --- a/src/libstd/keyword_docs.rs +++ b/src/libstd/keyword_docs.rs @@ -965,20 +965,21 @@ mod move_keyword {} #[doc(keyword = "mut")] // -/// A mutable binding, reference, or pointer. +/// A mutable variable, reference, or pointer. /// -/// `mut` can be used in several situations. The first is mutable bindings, +/// `mut` can be used in several situations. The first is mutable variables, /// which can be used anywhere you can bind a value to a variable name. Some /// examples: /// -/// ``` -/// // A mutable binding in the parameter list of a function. +/// ```rust +/// // A mutable variable in the parameter list of a function. /// fn foo(mut x: u8, y: u8) -> u8 { /// x += y; /// x /// } /// -/// // A mutable binding for a variable. +/// // Modifying a mutable variable. +/// # #[allow(unused_assignments)] /// let mut a = 5; /// a = 6; /// @@ -986,17 +987,17 @@ mod move_keyword {} /// assert_eq!(a, 6); /// ``` /// -/// The second is references. They can be created from `mut` bindings and must -/// be unique: no other binding can have a mutable reference, nor a simple -/// reference. +/// The second is mutable references. They can be created from `mut` variables +/// and must be unique: no other variables can have a mutable reference, nor a +/// shared reference. /// -/// ``` +/// ```rust /// // Taking a mutable reference. /// fn push_two(v: &mut Vec) { /// v.push(2); /// } /// -/// // You cannot take a mutable reference to a non-mutable variable. +/// // A mutable reference cannot be taken to a non-mutable variable. /// let mut v = vec![0, 1]; /// // Passing a mutable reference. /// push_two(&mut v); @@ -1004,10 +1005,18 @@ mod move_keyword {} /// assert_eq!(v, vec![0, 1, 2]); /// ``` /// -/// Mutable pointers work much like mutable references, with the added -/// possibility of being nul. The syntax is `*mut Type`. +/// ```rust,compile_fail,E0502 +/// let mut v = vec![0, 1]; +/// let mut_ref_v = &mut v; +/// ##[allow(unused)] +/// let ref_v = &v; +/// mut_ref_v.push(2); +/// ``` +/// +/// Mutable raw pointers work much like mutable references, with the added +/// possibility of not pointing to a valid object. The syntax is `*mut Type`. /// -/// You can find more information on mutable references and pointers in the +/// More information on mutable references and pointers can be found in``` /// [Reference]. /// /// [Reference]: ../reference/types/pointer.html#mutable-references-mut From 2e3f51775a6427f34878c2041be8e0c8d656ea3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Miku=C5=82a?= Date: Thu, 28 May 2020 19:54:08 +0200 Subject: [PATCH 31/36] Add unstable rustc option to control self-contained linkage mode --- src/librustc_codegen_ssa/back/link.rs | 54 ++++++++++++++++++--------- src/librustc_session/filesearch.rs | 4 +- src/librustc_session/options.rs | 3 ++ 3 files changed, 42 insertions(+), 19 deletions(-) diff --git a/src/librustc_codegen_ssa/back/link.rs b/src/librustc_codegen_ssa/back/link.rs index 6c995be913c9e..ae1e4da753331 100644 --- a/src/librustc_codegen_ssa/back/link.rs +++ b/src/librustc_codegen_ssa/back/link.rs @@ -140,7 +140,12 @@ pub fn link_binary<'a, B: ArchiveBuilder<'a>>( // The third parameter is for env vars, used on windows to set up the // path for MSVC to find its DLLs, and gcc to find its bundled // toolchain -fn get_linker(sess: &Session, linker: &Path, flavor: LinkerFlavor) -> Command { +fn get_linker( + sess: &Session, + linker: &Path, + flavor: LinkerFlavor, + self_contained: bool, +) -> Command { let msvc_tool = windows_registry::find_tool(&sess.opts.target_triple.triple(), "link.exe"); // If our linker looks like a batch script on Windows then to execute this @@ -199,7 +204,7 @@ fn get_linker(sess: &Session, linker: &Path, flavor: LinkerFlavor) -> Command { // The compiler's sysroot often has some bundled tools, so add it to the // PATH for the child. - let mut new_path = sess.host_filesearch(PathKind::All).get_tools_search_paths(); + let mut new_path = sess.host_filesearch(PathKind::All).get_tools_search_paths(self_contained); let mut msvc_changed_path = false; if sess.target.target.options.is_like_msvc { if let Some(ref tool) = msvc_tool { @@ -563,7 +568,7 @@ fn link_natively<'a, B: ArchiveBuilder<'a>>( .iter() .copied() .flatten() - .map(|obj| get_object_file_path(sess, obj).into_os_string()) + .map(|obj| get_object_file_path(sess, obj, fallback).into_os_string()) .collect::>() }; let pre_objects_static_pie = get_objects(pre_objects, LinkOutputKind::StaticPicExe); @@ -1066,9 +1071,11 @@ fn get_crt_libs_path(sess: &Session) -> Option { } } -fn get_object_file_path(sess: &Session, name: &str) -> PathBuf { +fn get_object_file_path(sess: &Session, name: &str, self_contained: bool) -> PathBuf { // prefer system {,dll}crt2.o libs, see get_crt_libs_path comment for more details - if sess.target.target.llvm_target.contains("windows-gnu") { + if sess.opts.debugging_opts.link_self_contained.is_none() + && sess.target.target.llvm_target.contains("windows-gnu") + { if let Some(compiler_libs_path) = get_crt_libs_path(sess) { let file_path = compiler_libs_path.join(name); if file_path.exists() { @@ -1081,9 +1088,12 @@ fn get_object_file_path(sess: &Session, name: &str) -> PathBuf { if file_path.exists() { return file_path; } - let file_path = fs.get_selfcontained_lib_path().join(name); - if file_path.exists() { - return file_path; + // Special directory with objects used only in self-contained linkage mode + if self_contained { + let file_path = fs.get_selfcontained_lib_path().join(name); + if file_path.exists() { + return file_path; + } } for search_path in fs.search_paths() { let file_path = search_path.dir.join(name); @@ -1268,6 +1278,10 @@ fn link_output_kind(sess: &Session, crate_type: CrateType) -> LinkOutputKind { /// Whether we link to our own CRT objects instead of relying on gcc to pull them. /// We only provide such support for a very limited number of targets. fn crt_objects_fallback(sess: &Session, crate_type: CrateType) -> bool { + if let Some(self_contained) = sess.opts.debugging_opts.link_self_contained { + return self_contained; + } + match sess.target.target.options.crt_objects_fallback { // FIXME: Find a better heuristic for "native musl toolchain is available", // based on host and linker path, for example. @@ -1292,7 +1306,7 @@ fn add_pre_link_objects( let opts = &sess.target.target.options; let objects = if fallback { &opts.pre_link_objects_fallback } else { &opts.pre_link_objects }; for obj in objects.get(&link_output_kind).iter().copied().flatten() { - cmd.add_object(&get_object_file_path(sess, obj)); + cmd.add_object(&get_object_file_path(sess, obj, fallback)); } } @@ -1306,7 +1320,7 @@ fn add_post_link_objects( let opts = &sess.target.target.options; let objects = if fallback { &opts.post_link_objects_fallback } else { &opts.post_link_objects }; for obj in objects.get(&link_output_kind).iter().copied().flatten() { - cmd.add_object(&get_object_file_path(sess, obj)); + cmd.add_object(&get_object_file_path(sess, obj, fallback)); } } @@ -1468,9 +1482,12 @@ fn link_local_crate_native_libs_and_dependent_crate_libs<'a, B: ArchiveBuilder<' } /// Add sysroot and other globally set directories to the directory search list. -fn add_library_search_dirs(cmd: &mut dyn Linker, sess: &Session) { +fn add_library_search_dirs(cmd: &mut dyn Linker, sess: &Session, self_contained: bool) { // Prefer system mingw-w64 libs, see get_crt_libs_path comment for more details. - if cfg!(windows) && sess.target.target.llvm_target.contains("windows-gnu") { + if sess.opts.debugging_opts.link_self_contained.is_none() + && cfg!(windows) + && sess.target.target.llvm_target.contains("windows-gnu") + { if let Some(compiler_libs_path) = get_crt_libs_path(sess) { cmd.include_path(&compiler_libs_path); } @@ -1481,8 +1498,11 @@ fn add_library_search_dirs(cmd: &mut dyn Linker, sess: &Session) { let lib_path = sess.target_filesearch(PathKind::All).get_lib_path(); cmd.include_path(&fix_windows_verbatim_for_gcc(&lib_path)); - let lib_path = sess.target_filesearch(PathKind::All).get_selfcontained_lib_path(); - cmd.include_path(&fix_windows_verbatim_for_gcc(&lib_path)); + // Special directory with libraries used only in self-contained linkage mode + if self_contained { + let lib_path = sess.target_filesearch(PathKind::All).get_selfcontained_lib_path(); + cmd.include_path(&fix_windows_verbatim_for_gcc(&lib_path)); + } } /// Add options making relocation sections in the produced ELF files read-only @@ -1545,13 +1565,13 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>( codegen_results: &CodegenResults, target_cpu: &str, ) -> Command { - let base_cmd = get_linker(sess, path, flavor); + let crt_objects_fallback = crt_objects_fallback(sess, crate_type); + let base_cmd = get_linker(sess, path, flavor, crt_objects_fallback); // FIXME: Move `/LIBPATH` addition for uwp targets from the linker construction // to the linker args construction. assert!(base_cmd.get_args().is_empty() || sess.target.target.target_vendor == "uwp"); let cmd = &mut *codegen_results.linker_info.to_linker(base_cmd, &sess, flavor, target_cpu); let link_output_kind = link_output_kind(sess, crate_type); - let crt_objects_fallback = crt_objects_fallback(sess, crate_type); // NO-OPT-OUT, OBJECT-FILES-MAYBE, CUSTOMIZATION-POINT add_pre_link_args(cmd, sess, flavor); @@ -1597,7 +1617,7 @@ fn linker_with_args<'a, B: ArchiveBuilder<'a>>( // NO-OPT-OUT, OBJECT-FILES-NO, AUDIT-ORDER // FIXME: Order-dependent, at least relatively to other args adding searh directories. - add_library_search_dirs(cmd, sess); + add_library_search_dirs(cmd, sess, crt_objects_fallback); // OBJECT-FILES-YES add_local_crate_regular_objects(cmd, codegen_results); diff --git a/src/librustc_session/filesearch.rs b/src/librustc_session/filesearch.rs index 5586b82b0edc0..675a880cef47c 100644 --- a/src/librustc_session/filesearch.rs +++ b/src/librustc_session/filesearch.rs @@ -92,13 +92,13 @@ impl<'a> FileSearch<'a> { } // Returns a list of directories where target-specific tool binaries are located. - pub fn get_tools_search_paths(&self) -> Vec { + pub fn get_tools_search_paths(&self, self_contained: bool) -> Vec { let mut p = PathBuf::from(self.sysroot); p.push(find_libdir(self.sysroot).as_ref()); p.push(RUST_LIB_DIR); p.push(&self.triple); p.push("bin"); - vec![p.clone(), p.join("self-contained")] + if self_contained { vec![p.clone(), p.join("self-contained")] } else { vec![p.clone()] } } } diff --git a/src/librustc_session/options.rs b/src/librustc_session/options.rs index 6c6f27502b614..055ae2fa2b5e5 100644 --- a/src/librustc_session/options.rs +++ b/src/librustc_session/options.rs @@ -888,6 +888,9 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, "keep hygiene data after analysis (default: no)"), link_native_libraries: bool = (true, parse_bool, [UNTRACKED], "link native libraries in the linker invocation (default: yes)"), + link_self_contained: Option = (None, parse_opt_bool, [TRACKED], + "control whether to link Rust provided C objects/libraries or rely + on C toolchain installed in the system"), link_only: bool = (false, parse_bool, [TRACKED], "link the `.rlink` file generated by `-Z no-link` (default: no)"), llvm_time_trace: bool = (false, parse_bool, [UNTRACKED], From 54293c1f157ee83d8e9949a7908e093ae0d5817a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Miku=C5=82a?= Date: Mon, 22 Jun 2020 19:56:56 +0200 Subject: [PATCH 32/36] Rename get_self_contained_lib_path --- src/librustc_codegen_ssa/back/link.rs | 4 ++-- src/librustc_session/filesearch.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_codegen_ssa/back/link.rs b/src/librustc_codegen_ssa/back/link.rs index ae1e4da753331..39f54b83a23bc 100644 --- a/src/librustc_codegen_ssa/back/link.rs +++ b/src/librustc_codegen_ssa/back/link.rs @@ -1090,7 +1090,7 @@ fn get_object_file_path(sess: &Session, name: &str, self_contained: bool) -> Pat } // Special directory with objects used only in self-contained linkage mode if self_contained { - let file_path = fs.get_selfcontained_lib_path().join(name); + let file_path = fs.get_self_contained_lib_path().join(name); if file_path.exists() { return file_path; } @@ -1500,7 +1500,7 @@ fn add_library_search_dirs(cmd: &mut dyn Linker, sess: &Session, self_contained: // Special directory with libraries used only in self-contained linkage mode if self_contained { - let lib_path = sess.target_filesearch(PathKind::All).get_selfcontained_lib_path(); + let lib_path = sess.target_filesearch(PathKind::All).get_self_contained_lib_path(); cmd.include_path(&fix_windows_verbatim_for_gcc(&lib_path)); } } diff --git a/src/librustc_session/filesearch.rs b/src/librustc_session/filesearch.rs index 675a880cef47c..27396c524f4e6 100644 --- a/src/librustc_session/filesearch.rs +++ b/src/librustc_session/filesearch.rs @@ -41,7 +41,7 @@ impl<'a> FileSearch<'a> { make_target_lib_path(self.sysroot, self.triple) } - pub fn get_selfcontained_lib_path(&self) -> PathBuf { + pub fn get_self_contained_lib_path(&self) -> PathBuf { self.get_lib_path().join("self-contained") } From f27dcd7ee05e948c92fb7e742d6b26c34e20b39c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Miku=C5=82a?= Date: Thu, 25 Jun 2020 11:15:09 +0200 Subject: [PATCH 33/36] Rename remaining `fallback` to `self_contained` --- src/librustc_codegen_ssa/back/link.rs | 32 +++++++++++++++++---------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/src/librustc_codegen_ssa/back/link.rs b/src/librustc_codegen_ssa/back/link.rs index 39f54b83a23bc..a34029410784a 100644 --- a/src/librustc_codegen_ssa/back/link.rs +++ b/src/librustc_codegen_ssa/back/link.rs @@ -556,19 +556,25 @@ fn link_natively<'a, B: ArchiveBuilder<'a>>( "Linker does not support -static-pie command line option. Retrying with -static instead." ); // Mirror `add_(pre,post)_link_objects` to replace CRT objects. - let fallback = crt_objects_fallback(sess, crate_type); + let self_contained = crt_objects_fallback(sess, crate_type); let opts = &sess.target.target.options; - let pre_objects = - if fallback { &opts.pre_link_objects_fallback } else { &opts.pre_link_objects }; - let post_objects = - if fallback { &opts.post_link_objects_fallback } else { &opts.post_link_objects }; + let pre_objects = if self_contained { + &opts.pre_link_objects_fallback + } else { + &opts.pre_link_objects + }; + let post_objects = if self_contained { + &opts.post_link_objects_fallback + } else { + &opts.post_link_objects + }; let get_objects = |objects: &CrtObjects, kind| { objects .get(&kind) .iter() .copied() .flatten() - .map(|obj| get_object_file_path(sess, obj, fallback).into_os_string()) + .map(|obj| get_object_file_path(sess, obj, self_contained).into_os_string()) .collect::>() }; let pre_objects_static_pie = get_objects(pre_objects, LinkOutputKind::StaticPicExe); @@ -1301,12 +1307,13 @@ fn add_pre_link_objects( cmd: &mut dyn Linker, sess: &Session, link_output_kind: LinkOutputKind, - fallback: bool, + self_contained: bool, ) { let opts = &sess.target.target.options; - let objects = if fallback { &opts.pre_link_objects_fallback } else { &opts.pre_link_objects }; + let objects = + if self_contained { &opts.pre_link_objects_fallback } else { &opts.pre_link_objects }; for obj in objects.get(&link_output_kind).iter().copied().flatten() { - cmd.add_object(&get_object_file_path(sess, obj, fallback)); + cmd.add_object(&get_object_file_path(sess, obj, self_contained)); } } @@ -1315,12 +1322,13 @@ fn add_post_link_objects( cmd: &mut dyn Linker, sess: &Session, link_output_kind: LinkOutputKind, - fallback: bool, + self_contained: bool, ) { let opts = &sess.target.target.options; - let objects = if fallback { &opts.post_link_objects_fallback } else { &opts.post_link_objects }; + let objects = + if self_contained { &opts.post_link_objects_fallback } else { &opts.post_link_objects }; for obj in objects.get(&link_output_kind).iter().copied().flatten() { - cmd.add_object(&get_object_file_path(sess, obj, fallback)); + cmd.add_object(&get_object_file_path(sess, obj, self_contained)); } } From eb6d9a49a803ee84c9f9aad7ca5657b4ded76941 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 21 Jun 2020 14:07:59 +0200 Subject: [PATCH 34/36] Add E0766 error for unterminated double quote byte string --- src/librustc_error_codes/error_codes.rs | 1 + src/librustc_error_codes/error_codes/E0766.md | 13 +++++++++++++ src/librustc_parse/lexer/mod.rs | 15 +++++++++------ 3 files changed, 23 insertions(+), 6 deletions(-) create mode 100644 src/librustc_error_codes/error_codes/E0766.md diff --git a/src/librustc_error_codes/error_codes.rs b/src/librustc_error_codes/error_codes.rs index 6a5e23adafa53..162585360fbda 100644 --- a/src/librustc_error_codes/error_codes.rs +++ b/src/librustc_error_codes/error_codes.rs @@ -446,6 +446,7 @@ E0762: include_str!("./error_codes/E0762.md"), E0763: include_str!("./error_codes/E0763.md"), E0764: include_str!("./error_codes/E0764.md"), E0765: include_str!("./error_codes/E0765.md"), +E0766: include_str!("./error_codes/E0766.md"), ; // E0006, // merged with E0005 // E0008, // cannot bind by-move into a pattern guard diff --git a/src/librustc_error_codes/error_codes/E0766.md b/src/librustc_error_codes/error_codes/E0766.md new file mode 100644 index 0000000000000..4e775df2cac4d --- /dev/null +++ b/src/librustc_error_codes/error_codes/E0766.md @@ -0,0 +1,13 @@ +A double quote byte string (`b"`) was not terminated. + +Erroneous code example: + +```compile_fail,E0766 +let s = b"; // error! +``` + +To fix this error, add the missing double quote at the end of the string: + +``` +let s = b""; // ok! +``` diff --git a/src/librustc_parse/lexer/mod.rs b/src/librustc_parse/lexer/mod.rs index 8e74c3847bc90..5050f03bea9b2 100644 --- a/src/librustc_parse/lexer/mod.rs +++ b/src/librustc_parse/lexer/mod.rs @@ -367,12 +367,15 @@ impl<'a> StringReader<'a> { } rustc_lexer::LiteralKind::ByteStr { terminated } => { if !terminated { - self.fatal_span_( - start + BytePos(1), - suffix_start, - "unterminated double quote byte string", - ) - .raise() + self.sess + .span_diagnostic + .struct_span_fatal_with_code( + self.mk_sp(start + BytePos(1), suffix_start), + "unterminated double quote byte string", + error_code!(E0766), + ) + .emit(); + FatalError.raise(); } (token::ByteStr, Mode::ByteStr, 2, 1) // b" " } From 33302fa7d225a2d1152f0fe9a52e55fbd85d3017 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 21 Jun 2020 14:08:06 +0200 Subject: [PATCH 35/36] Update UI test --- src/test/ui/parser/byte-string-literals.stderr | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/ui/parser/byte-string-literals.stderr b/src/test/ui/parser/byte-string-literals.stderr index ca964cd4b8f21..9be9064414796 100644 --- a/src/test/ui/parser/byte-string-literals.stderr +++ b/src/test/ui/parser/byte-string-literals.stderr @@ -22,7 +22,7 @@ error: byte constant must be ASCII. Use a \xHH escape for a non-ASCII byte LL | b"é"; | ^ -error: unterminated double quote byte string +error[E0766]: unterminated double quote byte string --> $DIR/byte-string-literals.rs:7:6 | LL | b"a @@ -32,3 +32,4 @@ LL | | } error: aborting due to 5 previous errors +For more information about this error, try `rustc --explain E0766`. From 25e864e19812e079a9e69134fd1bbbfee444c0a2 Mon Sep 17 00:00:00 2001 From: Charles Lew Date: Tue, 23 Jun 2020 19:45:13 +0800 Subject: [PATCH 36/36] Implement mixed script confusable lint. --- Cargo.lock | 8 +- src/librustc_lint/Cargo.toml | 2 +- src/librustc_lint/non_ascii_idents.rs | 127 +++++++++++++++++- .../lint-mixed-script-confusables-2.rs | 20 +++ .../lint-mixed-script-confusables.rs | 15 +++ .../lint-mixed-script-confusables.stderr | 34 +++++ src/test/ui/utf8_idents.rs | 2 + src/test/ui/utf8_idents.stderr | 10 +- 8 files changed, 206 insertions(+), 12 deletions(-) create mode 100644 src/test/ui/lint/rfc-2457-non-ascii-idents/lint-mixed-script-confusables-2.rs create mode 100644 src/test/ui/lint/rfc-2457-non-ascii-idents/lint-mixed-script-confusables.rs create mode 100644 src/test/ui/lint/rfc-2457-non-ascii-idents/lint-mixed-script-confusables.stderr diff --git a/Cargo.lock b/Cargo.lock index 0c1c533f39579..e05439d71e7e4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5405,15 +5405,15 @@ dependencies = [ [[package]] name = "unicode-script" -version = "0.4.0" +version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5b2c5c29e805da6817f5af6a627d65adb045cebf05cccd5a3493d6109454391c" +checksum = "58b33414ea8db4b7ea0343548dbdc31d27aef06beacf7044a87e564d9b0feb7d" [[package]] name = "unicode-security" -version = "0.0.3" +version = "0.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a5f9011bbed9c13372bc8df618b55a38138445199caf3b61d432c6859c36dee0" +checksum = "5d87c28edc5b263377e448d6cdcb935c06b95413d8013ba6fae470558ccab18f" dependencies = [ "unicode-normalization", "unicode-script", diff --git a/src/librustc_lint/Cargo.toml b/src/librustc_lint/Cargo.toml index ada6f2a9381dc..58c15257326ae 100644 --- a/src/librustc_lint/Cargo.toml +++ b/src/librustc_lint/Cargo.toml @@ -10,7 +10,7 @@ path = "lib.rs" [dependencies] log = "0.4" -unicode-security = "0.0.3" +unicode-security = "0.0.5" rustc_middle = { path = "../librustc_middle" } rustc_ast_pretty = { path = "../librustc_ast_pretty" } rustc_attr = { path = "../librustc_attr" } diff --git a/src/librustc_lint/non_ascii_idents.rs b/src/librustc_lint/non_ascii_idents.rs index 90bd7ad4acf71..30dbd069c29bd 100644 --- a/src/librustc_lint/non_ascii_idents.rs +++ b/src/librustc_lint/non_ascii_idents.rs @@ -24,12 +24,20 @@ declare_lint! { crate_level_only } -declare_lint_pass!(NonAsciiIdents => [NON_ASCII_IDENTS, UNCOMMON_CODEPOINTS, CONFUSABLE_IDENTS]); +declare_lint! { + pub MIXED_SCRIPT_CONFUSABLES, + Warn, + "detects Unicode scripts whose mixed script confusables codepoints are solely used", + crate_level_only +} + +declare_lint_pass!(NonAsciiIdents => [NON_ASCII_IDENTS, UNCOMMON_CODEPOINTS, CONFUSABLE_IDENTS, MIXED_SCRIPT_CONFUSABLES]); impl EarlyLintPass for NonAsciiIdents { fn check_crate(&mut self, cx: &EarlyContext<'_>, _: &ast::Crate) { use rustc_session::lint::Level; use rustc_span::Span; + use std::collections::BTreeMap; use unicode_security::GeneralSecurityProfile; use utils::CowBoxSymStr; @@ -37,8 +45,14 @@ impl EarlyLintPass for NonAsciiIdents { let check_uncommon_codepoints = cx.builder.lint_level(UNCOMMON_CODEPOINTS).0 != Level::Allow; let check_confusable_idents = cx.builder.lint_level(CONFUSABLE_IDENTS).0 != Level::Allow; + let check_mixed_script_confusables = + cx.builder.lint_level(MIXED_SCRIPT_CONFUSABLES).0 != Level::Allow; - if !check_non_ascii_idents && !check_uncommon_codepoints && !check_confusable_idents { + if !check_non_ascii_idents + && !check_uncommon_codepoints + && !check_confusable_idents + && !check_mixed_script_confusables + { return; } @@ -107,6 +121,115 @@ impl EarlyLintPass for NonAsciiIdents { .or_insert((symbol_str, sp, is_ascii)); } } + + if has_non_ascii_idents && check_mixed_script_confusables { + use unicode_security::is_potential_mixed_script_confusable_char; + use unicode_security::mixed_script::AugmentedScriptSet; + + #[derive(Clone)] + enum ScriptSetUsage { + Suspicious(Vec, Span), + Verified, + } + + let mut script_states: FxHashMap = + FxHashMap::default(); + let latin_augmented_script_set = AugmentedScriptSet::for_char('A'); + script_states.insert(latin_augmented_script_set, ScriptSetUsage::Verified); + + let mut has_suspicous = false; + for (symbol, &sp) in symbols.iter() { + let symbol_str = symbol.as_str(); + for ch in symbol_str.chars() { + if ch.is_ascii() { + // all ascii characters are covered by exception. + continue; + } + if !GeneralSecurityProfile::identifier_allowed(ch) { + // this character is covered by `uncommon_codepoints` lint. + continue; + } + let augmented_script_set = AugmentedScriptSet::for_char(ch); + script_states + .entry(augmented_script_set) + .and_modify(|existing_state| { + if let ScriptSetUsage::Suspicious(ch_list, _) = existing_state { + if is_potential_mixed_script_confusable_char(ch) { + ch_list.push(ch); + } else { + *existing_state = ScriptSetUsage::Verified; + } + } + }) + .or_insert_with(|| { + if !is_potential_mixed_script_confusable_char(ch) { + ScriptSetUsage::Verified + } else { + has_suspicous = true; + ScriptSetUsage::Suspicious(vec![ch], sp) + } + }); + } + } + + if has_suspicous { + let verified_augmented_script_sets = script_states + .iter() + .flat_map(|(k, v)| match v { + ScriptSetUsage::Verified => Some(*k), + _ => None, + }) + .collect::>(); + + // we're sorting the output here. + let mut lint_reports: BTreeMap<(Span, Vec), AugmentedScriptSet> = + BTreeMap::new(); + + 'outerloop: for (augment_script_set, usage) in script_states { + let (mut ch_list, sp) = match usage { + ScriptSetUsage::Verified => continue, + ScriptSetUsage::Suspicious(ch_list, sp) => (ch_list, sp), + }; + + if augment_script_set.is_all() { + continue; + } + + for existing in verified_augmented_script_sets.iter() { + if existing.is_all() { + continue; + } + let mut intersect = *existing; + intersect.intersect_with(augment_script_set); + if !intersect.is_empty() && !intersect.is_all() { + continue 'outerloop; + } + } + + ch_list.sort(); + ch_list.dedup(); + lint_reports.insert((sp, ch_list), augment_script_set); + } + + for ((sp, ch_list), script_set) in lint_reports { + cx.struct_span_lint(MIXED_SCRIPT_CONFUSABLES, sp, |lint| { + let message = format!( + "The usage of Script Group `{}` in this crate consists solely of mixed script confusables", + script_set); + let mut note = "The usage includes ".to_string(); + for (idx, ch) in ch_list.into_iter().enumerate() { + if idx != 0 { + note += ", "; + } + let char_info = format!("'{}' (U+{:04X})", ch, ch as u32); + note += &char_info; + } + note += "."; + lint.build(&message).note(¬e).note("Please recheck to make sure their usages are indeed what you want.").emit() + }); + } + } + } } } diff --git a/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-mixed-script-confusables-2.rs b/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-mixed-script-confusables-2.rs new file mode 100644 index 0000000000000..a5b45466da5ca --- /dev/null +++ b/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-mixed-script-confusables-2.rs @@ -0,0 +1,20 @@ +// check-pass +#![feature(non_ascii_idents)] +#![deny(mixed_script_confusables)] + +struct ΑctuallyNotLatin; + +fn main() { + let λ = 42; // this usage of Greek confirms that Greek is used intentionally. +} + +mod роре { + const エ: &'static str = "アイウ"; + + // this usage of Katakana confirms that Katakana is used intentionally. + fn ニャン() { + let д: usize = 100; // this usage of Cyrillic confirms that Cyrillic is used intentionally. + + println!("meow!"); + } +} diff --git a/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-mixed-script-confusables.rs b/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-mixed-script-confusables.rs new file mode 100644 index 0000000000000..4637b03f250de --- /dev/null +++ b/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-mixed-script-confusables.rs @@ -0,0 +1,15 @@ +#![feature(non_ascii_idents)] +#![deny(mixed_script_confusables)] + +struct ΑctuallyNotLatin; +//~^ ERROR The usage of Script Group `Greek` in this crate consists solely of + +fn main() { + let v = ΑctuallyNotLatin; +} + +mod роре { +//~^ ERROR The usage of Script Group `Cyrillic` in this crate consists solely of + const エ: &'static str = "アイウ"; + //~^ ERROR The usage of Script Group `Japanese, Katakana` in this crate consists solely of +} diff --git a/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-mixed-script-confusables.stderr b/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-mixed-script-confusables.stderr new file mode 100644 index 0000000000000..6f75a1ece3766 --- /dev/null +++ b/src/test/ui/lint/rfc-2457-non-ascii-idents/lint-mixed-script-confusables.stderr @@ -0,0 +1,34 @@ +error: The usage of Script Group `Greek` in this crate consists solely of mixed script confusables + --> $DIR/lint-mixed-script-confusables.rs:4:8 + | +LL | struct ΑctuallyNotLatin; + | ^^^^^^^^^^^^^^^^ + | +note: the lint level is defined here + --> $DIR/lint-mixed-script-confusables.rs:2:9 + | +LL | #![deny(mixed_script_confusables)] + | ^^^^^^^^^^^^^^^^^^^^^^^^ + = note: The usage includes 'Α' (U+0391). + = note: Please recheck to make sure their usages are indeed what you want. + +error: The usage of Script Group `Cyrillic` in this crate consists solely of mixed script confusables + --> $DIR/lint-mixed-script-confusables.rs:11:5 + | +LL | mod роре { + | ^^^^ + | + = note: The usage includes 'е' (U+0435), 'о' (U+043E), 'р' (U+0440). + = note: Please recheck to make sure their usages are indeed what you want. + +error: The usage of Script Group `Japanese, Katakana` in this crate consists solely of mixed script confusables + --> $DIR/lint-mixed-script-confusables.rs:13:11 + | +LL | const エ: &'static str = "アイウ"; + | ^^ + | + = note: The usage includes 'エ' (U+30A8). + = note: Please recheck to make sure their usages are indeed what you want. + +error: aborting due to 3 previous errors + diff --git a/src/test/ui/utf8_idents.rs b/src/test/ui/utf8_idents.rs index f59d5502aae30..6c54086cc2009 100644 --- a/src/test/ui/utf8_idents.rs +++ b/src/test/ui/utf8_idents.rs @@ -1,3 +1,5 @@ +#![allow(mixed_script_confusables)] + fn foo< 'β, //~ ERROR non-ascii idents are not fully supported γ //~ ERROR non-ascii idents are not fully supported diff --git a/src/test/ui/utf8_idents.stderr b/src/test/ui/utf8_idents.stderr index 877412df8fa1c..2fc0b1c39effb 100644 --- a/src/test/ui/utf8_idents.stderr +++ b/src/test/ui/utf8_idents.stderr @@ -1,5 +1,5 @@ error[E0658]: non-ascii idents are not fully supported - --> $DIR/utf8_idents.rs:2:5 + --> $DIR/utf8_idents.rs:4:5 | LL | 'β, | ^^ @@ -8,7 +8,7 @@ LL | 'β, = help: add `#![feature(non_ascii_idents)]` to the crate attributes to enable error[E0658]: non-ascii idents are not fully supported - --> $DIR/utf8_idents.rs:3:5 + --> $DIR/utf8_idents.rs:5:5 | LL | γ | ^ @@ -17,7 +17,7 @@ LL | γ = help: add `#![feature(non_ascii_idents)]` to the crate attributes to enable error[E0658]: non-ascii idents are not fully supported - --> $DIR/utf8_idents.rs:8:5 + --> $DIR/utf8_idents.rs:10:5 | LL | δ: usize | ^ @@ -26,7 +26,7 @@ LL | δ: usize = help: add `#![feature(non_ascii_idents)]` to the crate attributes to enable error[E0658]: non-ascii idents are not fully supported - --> $DIR/utf8_idents.rs:12:9 + --> $DIR/utf8_idents.rs:14:9 | LL | let α = 0.00001f64; | ^ @@ -35,7 +35,7 @@ LL | let α = 0.00001f64; = help: add `#![feature(non_ascii_idents)]` to the crate attributes to enable warning: type parameter `γ` should have an upper camel case name - --> $DIR/utf8_idents.rs:3:5 + --> $DIR/utf8_idents.rs:5:5 | LL | γ | ^ help: convert the identifier to upper camel case: `Γ`