From 95c4899e55e7aab68f06e67660257d73e6a46eda Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Sun, 7 Jun 2020 23:36:07 +0200 Subject: [PATCH 01/21] Added an example where explicitly dropping a lock is necessary/a good idea. --- src/libstd/sync/mutex.rs | 61 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/src/libstd/sync/mutex.rs b/src/libstd/sync/mutex.rs index 797b22fdd1279..6077e1a402965 100644 --- a/src/libstd/sync/mutex.rs +++ b/src/libstd/sync/mutex.rs @@ -107,6 +107,67 @@ use crate::sys_common::poison::{self, LockResult, TryLockError, TryLockResult}; /// /// *guard += 1; /// ``` +/// +/// It is sometimes a good idea (or even necessary) to manually drop the mutex +/// to unlock it as soon as possible. If you need the resource until the end of +/// the scope, this is not needed. +/// +/// ``` +/// use std::sync::{Arc, Mutex}; +/// use std::thread; +/// +/// const N: usize = 3; +/// +/// // Some data to work with in multiple threads. +/// let data_mutex = Arc::new(Mutex::new([1, 2, 3, 4])); +/// // The result of all the work across all threads. +/// let res_mutex = Arc::new(Mutex::new(0)); +/// +/// // Threads other than the main thread. +/// let mut threads = Vec::with_capacity(N); +/// (0..N).for_each(|_| { +/// // Getting clones for the mutexes. +/// let data_mutex_clone = Arc::clone(&data_mutex); +/// let res_mutex_clone = Arc::clone(&res_mutex); +/// +/// threads.push(thread::spawn(move || { +/// let data = *data_mutex_clone.lock().unwrap(); +/// // This is the result of some important and long-ish work. +/// let result = data.iter().fold(0, |acc, x| acc + x * 2); +/// // We drop the `data` explicitely because it's not necessary anymore +/// // and the thread still has work to do. This allow other threads to +/// // start working on the data immediately, without waiting +/// // for the rest of the unrelated work to be done here. +/// std::mem::drop(data); +/// *res_mutex_clone.lock().unwrap() += result; +/// })); +/// }); +/// +/// let data = *data_mutex.lock().unwrap(); +/// // This is the result of some important and long-ish work. +/// let result = data.iter().fold(0, |acc, x| acc + x * 2); +/// // We drop the `data` explicitely because it's not necessary anymore +/// // and the thread still has work to do. This allow other threads to +/// // start working on the data immediately, without waiting +/// // for the rest of the unrelated work to be done here. +/// // +/// // It's even more important here because we `.join` the threads after that. +/// // If we had not dropped the lock, a thread could be waiting forever for +/// // it, causing a deadlock. +/// std::mem::drop(data); +/// // Here the lock is not assigned to a variable and so, even if the scope +/// // does not end after this line, the mutex is still released: +/// // there is no deadlock. +/// *res_mutex.lock().unwrap() += result; +/// +/// threads.into_iter().for_each(|thread| { +/// thread +/// .join() +/// .expect("The thread creating or execution failed !") +/// }); +/// +/// assert_eq!(*res_mutex.lock().unwrap(), 80); +/// ``` #[stable(feature = "rust1", since = "1.0.0")] #[cfg_attr(not(test), rustc_diagnostic_item = "mutex_type")] pub struct Mutex { From 9c8f881ccd050f06387612e4b8aa18111c51a63b Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Sun, 7 Jun 2020 23:42:55 +0200 Subject: [PATCH 02/21] Improved the example to work with mutable data, providing a reason for the mutex holding it --- src/libstd/sync/mutex.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libstd/sync/mutex.rs b/src/libstd/sync/mutex.rs index 6077e1a402965..633496154aefe 100644 --- a/src/libstd/sync/mutex.rs +++ b/src/libstd/sync/mutex.rs @@ -119,7 +119,7 @@ use crate::sys_common::poison::{self, LockResult, TryLockError, TryLockResult}; /// const N: usize = 3; /// /// // Some data to work with in multiple threads. -/// let data_mutex = Arc::new(Mutex::new([1, 2, 3, 4])); +/// let data_mutex = Arc::new(Mutex::new(vec![1, 2, 3, 4])); /// // The result of all the work across all threads. /// let res_mutex = Arc::new(Mutex::new(0)); /// @@ -131,9 +131,10 @@ use crate::sys_common::poison::{self, LockResult, TryLockError, TryLockResult}; /// let res_mutex_clone = Arc::clone(&res_mutex); /// /// threads.push(thread::spawn(move || { -/// let data = *data_mutex_clone.lock().unwrap(); +/// let mut data = data_mutex_clone.lock().unwrap(); /// // This is the result of some important and long-ish work. /// let result = data.iter().fold(0, |acc, x| acc + x * 2); +/// data.push(result); /// // We drop the `data` explicitely because it's not necessary anymore /// // and the thread still has work to do. This allow other threads to /// // start working on the data immediately, without waiting @@ -143,9 +144,10 @@ use crate::sys_common::poison::{self, LockResult, TryLockError, TryLockResult}; /// })); /// }); /// -/// let data = *data_mutex.lock().unwrap(); +/// let mut data = data_mutex.lock().unwrap(); /// // This is the result of some important and long-ish work. /// let result = data.iter().fold(0, |acc, x| acc + x * 2); +/// data.push(result); /// // We drop the `data` explicitely because it's not necessary anymore /// // and the thread still has work to do. This allow other threads to /// // start working on the data immediately, without waiting @@ -166,7 +168,7 @@ use crate::sys_common::poison::{self, LockResult, TryLockError, TryLockResult}; /// .expect("The thread creating or execution failed !") /// }); /// -/// assert_eq!(*res_mutex.lock().unwrap(), 80); +/// assert_eq!(*res_mutex.lock().unwrap(), 800); /// ``` #[stable(feature = "rust1", since = "1.0.0")] #[cfg_attr(not(test), rustc_diagnostic_item = "mutex_type")] From fdef1a5915332a3f9012b41f4176cf2a16025257 Mon Sep 17 00:00:00 2001 From: Poliorcetics Date: Mon, 8 Jun 2020 16:29:47 +0200 Subject: [PATCH 03/21] Simply use drop instead of std::mem::drop Co-authored-by: LeSeulArtichaut --- src/libstd/sync/mutex.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libstd/sync/mutex.rs b/src/libstd/sync/mutex.rs index 633496154aefe..c2c86fae654cf 100644 --- a/src/libstd/sync/mutex.rs +++ b/src/libstd/sync/mutex.rs @@ -139,7 +139,7 @@ use crate::sys_common::poison::{self, LockResult, TryLockError, TryLockResult}; /// // and the thread still has work to do. This allow other threads to /// // start working on the data immediately, without waiting /// // for the rest of the unrelated work to be done here. -/// std::mem::drop(data); +/// drop(data); /// *res_mutex_clone.lock().unwrap() += result; /// })); /// }); @@ -156,7 +156,7 @@ use crate::sys_common::poison::{self, LockResult, TryLockError, TryLockResult}; /// // It's even more important here because we `.join` the threads after that. /// // If we had not dropped the lock, a thread could be waiting forever for /// // it, causing a deadlock. -/// std::mem::drop(data); +/// drop(data); /// // Here the lock is not assigned to a variable and so, even if the scope /// // does not end after this line, the mutex is still released: /// // there is no deadlock. From 1312d30a6a837f72c3f36f5dc1c575a29890aa2c Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Tue, 9 Jun 2020 22:40:30 +0200 Subject: [PATCH 04/21] Remove a lot of unecessary/duplicated comments --- src/libstd/sync/mutex.rs | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/libstd/sync/mutex.rs b/src/libstd/sync/mutex.rs index c2c86fae654cf..b3ef521d6ecb0 100644 --- a/src/libstd/sync/mutex.rs +++ b/src/libstd/sync/mutex.rs @@ -118,15 +118,11 @@ use crate::sys_common::poison::{self, LockResult, TryLockError, TryLockResult}; /// /// const N: usize = 3; /// -/// // Some data to work with in multiple threads. /// let data_mutex = Arc::new(Mutex::new(vec![1, 2, 3, 4])); -/// // The result of all the work across all threads. /// let res_mutex = Arc::new(Mutex::new(0)); /// -/// // Threads other than the main thread. /// let mut threads = Vec::with_capacity(N); /// (0..N).for_each(|_| { -/// // Getting clones for the mutexes. /// let data_mutex_clone = Arc::clone(&data_mutex); /// let res_mutex_clone = Arc::clone(&res_mutex); /// @@ -135,10 +131,6 @@ use crate::sys_common::poison::{self, LockResult, TryLockError, TryLockResult}; /// // This is the result of some important and long-ish work. /// let result = data.iter().fold(0, |acc, x| acc + x * 2); /// data.push(result); -/// // We drop the `data` explicitely because it's not necessary anymore -/// // and the thread still has work to do. This allow other threads to -/// // start working on the data immediately, without waiting -/// // for the rest of the unrelated work to be done here. /// drop(data); /// *res_mutex_clone.lock().unwrap() += result; /// })); @@ -153,9 +145,9 @@ use crate::sys_common::poison::{self, LockResult, TryLockError, TryLockResult}; /// // start working on the data immediately, without waiting /// // for the rest of the unrelated work to be done here. /// // -/// // It's even more important here because we `.join` the threads after that. -/// // If we had not dropped the lock, a thread could be waiting forever for -/// // it, causing a deadlock. +/// // It's even more important here than in the threads because we `.join` the +/// // threads after that. If we had not dropped the lock, a thread could be +/// // waiting forever for it, causing a deadlock. /// drop(data); /// // Here the lock is not assigned to a variable and so, even if the scope /// // does not end after this line, the mutex is still released: From c29b3fa1484c625bd34cb4d94fc76f36c6233447 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 29 May 2020 09:24:59 -0700 Subject: [PATCH 05/21] On recursive ADT, provide indirection structured suggestion --- src/librustc_errors/diagnostic.rs | 23 +++++++ src/librustc_errors/diagnostic_builder.rs | 13 ++++ src/librustc_middle/ty/util.rs | 10 ++- .../traits/error_reporting/mod.rs | 66 +++++++++++++++---- src/librustc_typeck/check/mod.rs | 6 +- .../infinite-tag-type-recursion.stderr | 9 ++- src/test/ui/issues/issue-17431-1.stderr | 11 +++- src/test/ui/issues/issue-17431-2.stderr | 22 +++++-- src/test/ui/issues/issue-17431-3.stderr | 11 +++- src/test/ui/issues/issue-17431-4.stderr | 11 +++- src/test/ui/issues/issue-17431-5.stderr | 11 +++- src/test/ui/issues/issue-17431-6.stderr | 9 ++- src/test/ui/issues/issue-17431-7.stderr | 9 ++- src/test/ui/issues/issue-2718-a.stderr | 9 ++- src/test/ui/issues/issue-3008-1.stderr | 9 ++- src/test/ui/issues/issue-3008-2.stderr | 11 +++- src/test/ui/issues/issue-3008-3.stderr | 9 ++- src/test/ui/issues/issue-32326.stderr | 5 +- src/test/ui/issues/issue-3779.stderr | 11 +++- src/test/ui/issues/issue-57271.stderr | 18 ++++- src/test/ui/recursion/recursive-enum.stderr | 9 ++- src/test/ui/sized-cycle-note.stderr | 22 +++++-- src/test/ui/span/E0072.stderr | 11 +++- src/test/ui/span/multiline-span-E0072.stderr | 11 +++- src/test/ui/span/recursive-type-field.stderr | 23 ++++--- src/test/ui/type/type-recursive.stderr | 11 +++- .../ui/union/union-nonrepresentable.stderr | 11 +++- 27 files changed, 315 insertions(+), 66 deletions(-) diff --git a/src/librustc_errors/diagnostic.rs b/src/librustc_errors/diagnostic.rs index cff83c3d5cda2..acaa26c6ad2fc 100644 --- a/src/librustc_errors/diagnostic.rs +++ b/src/librustc_errors/diagnostic.rs @@ -296,6 +296,29 @@ impl Diagnostic { self } + pub fn multipart_suggestions( + &mut self, + msg: &str, + suggestions: Vec>, + applicability: Applicability, + ) -> &mut Self { + self.suggestions.push(CodeSuggestion { + substitutions: suggestions + .into_iter() + .map(|suggestion| Substitution { + parts: suggestion + .into_iter() + .map(|(span, snippet)| SubstitutionPart { snippet, span }) + .collect(), + }) + .collect(), + msg: msg.to_owned(), + style: SuggestionStyle::ShowCode, + applicability, + }); + self + } + /// Prints out a message with for a multipart suggestion without showing the suggested code. /// /// This is intended to be used for suggestions that are obvious in what the changes need to diff --git a/src/librustc_errors/diagnostic_builder.rs b/src/librustc_errors/diagnostic_builder.rs index 2dbd9f4e52fad..22bf8fe34aa15 100644 --- a/src/librustc_errors/diagnostic_builder.rs +++ b/src/librustc_errors/diagnostic_builder.rs @@ -260,6 +260,19 @@ impl<'a> DiagnosticBuilder<'a> { self } + pub fn multipart_suggestions( + &mut self, + msg: &str, + suggestions: Vec>, + applicability: Applicability, + ) -> &mut Self { + if !self.0.allow_suggestions { + return self; + } + self.0.diagnostic.multipart_suggestions(msg, suggestions, applicability); + self + } + pub fn tool_only_multipart_suggestion( &mut self, msg: &str, diff --git a/src/librustc_middle/ty/util.rs b/src/librustc_middle/ty/util.rs index c2b794ca4bdd9..5cdfa6f90128d 100644 --- a/src/librustc_middle/ty/util.rs +++ b/src/librustc_middle/ty/util.rs @@ -827,7 +827,15 @@ impl<'tcx> ty::TyS<'tcx> { // Find non representable fields with their spans fold_repr(def.all_fields().map(|field| { let ty = field.ty(tcx, substs); - let span = tcx.hir().span_if_local(field.did).unwrap_or(sp); + let span = match field + .did + .as_local() + .map(|id| tcx.hir().as_local_hir_id(id)) + .and_then(|id| tcx.hir().find(id)) + { + Some(hir::Node::Field(field)) => field.ty.span, + _ => sp, + }; match is_type_structurally_recursive( tcx, span, diff --git a/src/librustc_trait_selection/traits/error_reporting/mod.rs b/src/librustc_trait_selection/traits/error_reporting/mod.rs index 1b72a4bf84f19..3457f7b4580c5 100644 --- a/src/librustc_trait_selection/traits/error_reporting/mod.rs +++ b/src/librustc_trait_selection/traits/error_reporting/mod.rs @@ -1747,24 +1747,62 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> { pub fn recursive_type_with_infinite_size_error( tcx: TyCtxt<'tcx>, type_def_id: DefId, -) -> DiagnosticBuilder<'tcx> { + spans: Vec, +) { assert!(type_def_id.is_local()); let span = tcx.hir().span_if_local(type_def_id).unwrap(); let span = tcx.sess.source_map().guess_head_span(span); - let mut err = struct_span_err!( - tcx.sess, - span, - E0072, - "recursive type `{}` has infinite size", - tcx.def_path_str(type_def_id) - ); + let path = tcx.def_path_str(type_def_id); + let mut err = + struct_span_err!(tcx.sess, span, E0072, "recursive type `{}` has infinite size", path); err.span_label(span, "recursive type has infinite size"); - err.help(&format!( - "insert indirection (e.g., a `Box`, `Rc`, or `&`) \ - at some point to make `{}` representable", - tcx.def_path_str(type_def_id) - )); - err + for &span in &spans { + err.span_label(span, "recursive without indirection"); + } + let short_msg = format!("insert some indirection to make `{}` representable", path); + let msg = format!( + "insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `{}` representable", + path, + ); + match &spans[..] { + [span] => { + err.multipart_suggestions( + &short_msg, + vec![ + vec![ + (span.shrink_to_lo(), "Box<".to_string()), + (span.shrink_to_hi(), ">".to_string()), + ], + vec![ + (span.shrink_to_lo(), "Rc<".to_string()), + (span.shrink_to_hi(), ">".to_string()), + ], + vec![(span.shrink_to_lo(), "&".to_string())], + ], + Applicability::HasPlaceholders, + ); + } + _ if spans.len() <= 4 => { + err.multipart_suggestion( + &msg, + spans + .iter() + .flat_map(|&span| { + vec![ + (span.shrink_to_lo(), "Box<".to_string()), + (span.shrink_to_hi(), ">".to_string()), + ] + .into_iter() + }) + .collect(), + Applicability::HasPlaceholders, + ); + } + _ => { + err.help(&msg); + } + } + err.emit(); } /// Summarizes information diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index f2aeed4f1e465..1e8a149d7d9ec 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -2390,11 +2390,7 @@ fn check_representable(tcx: TyCtxt<'_>, sp: Span, item_def_id: LocalDefId) -> bo // caught by case 1. match rty.is_representable(tcx, sp) { Representability::SelfRecursive(spans) => { - let mut err = recursive_type_with_infinite_size_error(tcx, item_def_id.to_def_id()); - for span in spans { - err.span_label(span, "recursive without indirection"); - } - err.emit(); + recursive_type_with_infinite_size_error(tcx, item_def_id.to_def_id(), spans); return false; } Representability::Representable | Representability::ContainsRecursive => (), diff --git a/src/test/ui/infinite/infinite-tag-type-recursion.stderr b/src/test/ui/infinite/infinite-tag-type-recursion.stderr index 11f82b842ba6f..b6a4d8f4cf563 100644 --- a/src/test/ui/infinite/infinite-tag-type-recursion.stderr +++ b/src/test/ui/infinite/infinite-tag-type-recursion.stderr @@ -6,7 +6,14 @@ LL | enum MList { Cons(isize, MList), Nil } | | | recursive type has infinite size | - = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `MList` representable +help: insert some indirection to make `MList` representable + | +LL | enum MList { Cons(isize, Box), Nil } + | ^^^^ ^ +LL | enum MList { Cons(isize, Rc), Nil } + | ^^^ ^ +LL | enum MList { Cons(isize, &MList), Nil } + | ^ error[E0391]: cycle detected when computing drop-check constraints for `MList` --> $DIR/infinite-tag-type-recursion.rs:1:1 diff --git a/src/test/ui/issues/issue-17431-1.stderr b/src/test/ui/issues/issue-17431-1.stderr index eb5a1366e8953..8d44154650e04 100644 --- a/src/test/ui/issues/issue-17431-1.stderr +++ b/src/test/ui/issues/issue-17431-1.stderr @@ -2,11 +2,18 @@ error[E0072]: recursive type `Foo` has infinite size --> $DIR/issue-17431-1.rs:1:1 | LL | struct Foo { foo: Option> } - | ^^^^^^^^^^ ------------------------ recursive without indirection + | ^^^^^^^^^^ ------------------- recursive without indirection | | | recursive type has infinite size | - = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `Foo` representable +help: insert some indirection to make `Foo` representable + | +LL | struct Foo { foo: Box>> } + | ^^^^ ^ +LL | struct Foo { foo: Rc>> } + | ^^^ ^ +LL | struct Foo { foo: &Option> } + | ^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-17431-2.stderr b/src/test/ui/issues/issue-17431-2.stderr index 3a7b0e9ce7997..b06184e84da28 100644 --- a/src/test/ui/issues/issue-17431-2.stderr +++ b/src/test/ui/issues/issue-17431-2.stderr @@ -2,21 +2,35 @@ error[E0072]: recursive type `Baz` has infinite size --> $DIR/issue-17431-2.rs:1:1 | LL | struct Baz { q: Option } - | ^^^^^^^^^^ -------------- recursive without indirection + | ^^^^^^^^^^ ----------- recursive without indirection | | | recursive type has infinite size | - = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `Baz` representable +help: insert some indirection to make `Baz` representable + | +LL | struct Baz { q: Box> } + | ^^^^ ^ +LL | struct Baz { q: Rc> } + | ^^^ ^ +LL | struct Baz { q: &Option } + | ^ error[E0072]: recursive type `Foo` has infinite size --> $DIR/issue-17431-2.rs:4:1 | LL | struct Foo { q: Option } - | ^^^^^^^^^^ -------------- recursive without indirection + | ^^^^^^^^^^ ----------- recursive without indirection | | | recursive type has infinite size | - = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `Foo` representable +help: insert some indirection to make `Foo` representable + | +LL | struct Foo { q: Box> } + | ^^^^ ^ +LL | struct Foo { q: Rc> } + | ^^^ ^ +LL | struct Foo { q: &Option } + | ^ error: aborting due to 2 previous errors diff --git a/src/test/ui/issues/issue-17431-3.stderr b/src/test/ui/issues/issue-17431-3.stderr index 675a2e2714209..f32bd70fd6d94 100644 --- a/src/test/ui/issues/issue-17431-3.stderr +++ b/src/test/ui/issues/issue-17431-3.stderr @@ -2,11 +2,18 @@ error[E0072]: recursive type `Foo` has infinite size --> $DIR/issue-17431-3.rs:3:1 | LL | struct Foo { foo: Mutex> } - | ^^^^^^^^^^ ----------------------- recursive without indirection + | ^^^^^^^^^^ ------------------ recursive without indirection | | | recursive type has infinite size | - = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `Foo` representable +help: insert some indirection to make `Foo` representable + | +LL | struct Foo { foo: Box>> } + | ^^^^ ^ +LL | struct Foo { foo: Rc>> } + | ^^^ ^ +LL | struct Foo { foo: &Mutex> } + | ^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-17431-4.stderr b/src/test/ui/issues/issue-17431-4.stderr index aff9071095ca0..372c5e975c844 100644 --- a/src/test/ui/issues/issue-17431-4.stderr +++ b/src/test/ui/issues/issue-17431-4.stderr @@ -2,11 +2,18 @@ error[E0072]: recursive type `Foo` has infinite size --> $DIR/issue-17431-4.rs:3:1 | LL | struct Foo { foo: Option>>, marker: marker::PhantomData } - | ^^^^^^^^^^^^^ --------------------------- recursive without indirection + | ^^^^^^^^^^^^^ ---------------------- recursive without indirection | | | recursive type has infinite size | - = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `Foo` representable +help: insert some indirection to make `Foo` representable + | +LL | struct Foo { foo: Box>>>, marker: marker::PhantomData } + | ^^^^ ^ +LL | struct Foo { foo: Rc>>>, marker: marker::PhantomData } + | ^^^ ^ +LL | struct Foo { foo: &Option>>, marker: marker::PhantomData } + | ^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-17431-5.stderr b/src/test/ui/issues/issue-17431-5.stderr index 537f9f34f55ca..01b850ac33694 100644 --- a/src/test/ui/issues/issue-17431-5.stderr +++ b/src/test/ui/issues/issue-17431-5.stderr @@ -2,11 +2,18 @@ error[E0072]: recursive type `Bar` has infinite size --> $DIR/issue-17431-5.rs:5:1 | LL | struct Bar { x: Bar , marker: marker::PhantomData } - | ^^^^^^^^^^^^^ ----------- recursive without indirection + | ^^^^^^^^^^^^^ -------- recursive without indirection | | | recursive type has infinite size | - = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `Bar` representable +help: insert some indirection to make `Bar` representable + | +LL | struct Bar { x: Box> , marker: marker::PhantomData } + | ^^^^ ^ +LL | struct Bar { x: Rc> , marker: marker::PhantomData } + | ^^^ ^ +LL | struct Bar { x: &Bar , marker: marker::PhantomData } + | ^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-17431-6.stderr b/src/test/ui/issues/issue-17431-6.stderr index cb2dab9501488..ce6c2db07fb6d 100644 --- a/src/test/ui/issues/issue-17431-6.stderr +++ b/src/test/ui/issues/issue-17431-6.stderr @@ -6,7 +6,14 @@ LL | enum Foo { X(Mutex>) } | | | recursive type has infinite size | - = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `Foo` representable +help: insert some indirection to make `Foo` representable + | +LL | enum Foo { X(Box>>) } + | ^^^^ ^ +LL | enum Foo { X(Rc>>) } + | ^^^ ^ +LL | enum Foo { X(&Mutex>) } + | ^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-17431-7.stderr b/src/test/ui/issues/issue-17431-7.stderr index de70851da4b5f..4fb563ba502f2 100644 --- a/src/test/ui/issues/issue-17431-7.stderr +++ b/src/test/ui/issues/issue-17431-7.stderr @@ -6,7 +6,14 @@ LL | enum Foo { Voo(Option>) } | | | recursive type has infinite size | - = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `Foo` representable +help: insert some indirection to make `Foo` representable + | +LL | enum Foo { Voo(Box>>) } + | ^^^^ ^ +LL | enum Foo { Voo(Rc>>) } + | ^^^ ^ +LL | enum Foo { Voo(&Option>) } + | ^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-2718-a.stderr b/src/test/ui/issues/issue-2718-a.stderr index 0f52c79192843..48b7a61e059ea 100644 --- a/src/test/ui/issues/issue-2718-a.stderr +++ b/src/test/ui/issues/issue-2718-a.stderr @@ -7,7 +7,14 @@ LL | pub struct Pong(SendPacket); | | recursive without indirection | recursive type has infinite size | - = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `pingpong::Pong` representable +help: insert some indirection to make `pingpong::Pong` representable + | +LL | pub struct Pong(Box>); + | ^^^^ ^ +LL | pub struct Pong(Rc>); + | ^^^ ^ +LL | pub struct Pong(&SendPacket); + | ^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-3008-1.stderr b/src/test/ui/issues/issue-3008-1.stderr index f12274134ee05..d1173a4b3334c 100644 --- a/src/test/ui/issues/issue-3008-1.stderr +++ b/src/test/ui/issues/issue-3008-1.stderr @@ -7,7 +7,14 @@ LL | enum Bar { LL | BarSome(Bar) | --- recursive without indirection | - = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `Bar` representable +help: insert some indirection to make `Bar` representable + | +LL | BarSome(Box) + | ^^^^ ^ +LL | BarSome(Rc) + | ^^^ ^ +LL | BarSome(&Bar) + | ^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-3008-2.stderr b/src/test/ui/issues/issue-3008-2.stderr index acc15f4b57c73..4fd60639b21a8 100644 --- a/src/test/ui/issues/issue-3008-2.stderr +++ b/src/test/ui/issues/issue-3008-2.stderr @@ -2,11 +2,18 @@ error[E0072]: recursive type `Bar` has infinite size --> $DIR/issue-3008-2.rs:2:1 | LL | struct Bar { x: Bar } - | ^^^^^^^^^^ ------ recursive without indirection + | ^^^^^^^^^^ --- recursive without indirection | | | recursive type has infinite size | - = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `Bar` representable +help: insert some indirection to make `Bar` representable + | +LL | struct Bar { x: Box } + | ^^^^ ^ +LL | struct Bar { x: Rc } + | ^^^ ^ +LL | struct Bar { x: &Bar } + | ^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-3008-3.stderr b/src/test/ui/issues/issue-3008-3.stderr index d08a3d9708db3..e6efad9188300 100644 --- a/src/test/ui/issues/issue-3008-3.stderr +++ b/src/test/ui/issues/issue-3008-3.stderr @@ -6,7 +6,14 @@ LL | enum E2 { V2(E2, marker::PhantomData), } | | | recursive type has infinite size | - = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `E2` representable +help: insert some indirection to make `E2` representable + | +LL | enum E2 { V2(Box>, marker::PhantomData), } + | ^^^^ ^ +LL | enum E2 { V2(Rc>, marker::PhantomData), } + | ^^^ ^ +LL | enum E2 { V2(&E2, marker::PhantomData), } + | ^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-32326.stderr b/src/test/ui/issues/issue-32326.stderr index 5967627e51a4b..0f3d3690b732e 100644 --- a/src/test/ui/issues/issue-32326.stderr +++ b/src/test/ui/issues/issue-32326.stderr @@ -8,7 +8,10 @@ LL | Plus(Expr, Expr), | | | recursive without indirection | - = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `Expr` representable +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `Expr` representable + | +LL | Plus(Box, Box), + | ^^^^ ^ ^^^^ ^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-3779.stderr b/src/test/ui/issues/issue-3779.stderr index ba1e842c610ba..9b50ddec12a44 100644 --- a/src/test/ui/issues/issue-3779.stderr +++ b/src/test/ui/issues/issue-3779.stderr @@ -5,9 +5,16 @@ LL | struct S { | ^^^^^^^^ recursive type has infinite size LL | LL | element: Option - | ------------------ recursive without indirection + | --------- recursive without indirection | - = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `S` representable +help: insert some indirection to make `S` representable + | +LL | element: Box> + | ^^^^ ^ +LL | element: Rc> + | ^^^ ^ +LL | element: &Option + | ^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-57271.stderr b/src/test/ui/issues/issue-57271.stderr index 4f164624f7a53..a6fe83a9b5636 100644 --- a/src/test/ui/issues/issue-57271.stderr +++ b/src/test/ui/issues/issue-57271.stderr @@ -7,7 +7,14 @@ LL | Class(ClassTypeSignature), LL | Array(TypeSignature), | ------------- recursive without indirection | - = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `ObjectType` representable +help: insert some indirection to make `ObjectType` representable + | +LL | Array(Box), + | ^^^^ ^ +LL | Array(Rc), + | ^^^ ^ +LL | Array(&TypeSignature), + | ^ error[E0072]: recursive type `TypeSignature` has infinite size --> $DIR/issue-57271.rs:19:1 @@ -18,7 +25,14 @@ LL | Base(BaseType), LL | Object(ObjectType), | ---------- recursive without indirection | - = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `TypeSignature` representable +help: insert some indirection to make `TypeSignature` representable + | +LL | Object(Box), + | ^^^^ ^ +LL | Object(Rc), + | ^^^ ^ +LL | Object(&ObjectType), + | ^ error: aborting due to 2 previous errors diff --git a/src/test/ui/recursion/recursive-enum.stderr b/src/test/ui/recursion/recursive-enum.stderr index e4674b57a6d21..c68badd458bce 100644 --- a/src/test/ui/recursion/recursive-enum.stderr +++ b/src/test/ui/recursion/recursive-enum.stderr @@ -6,7 +6,14 @@ LL | enum List { Cons(T, List), Nil } | | | recursive type has infinite size | - = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `List` representable +help: insert some indirection to make `List` representable + | +LL | enum List { Cons(T, Box>), Nil } + | ^^^^ ^ +LL | enum List { Cons(T, Rc>), Nil } + | ^^^ ^ +LL | enum List { Cons(T, &List), Nil } + | ^ error: aborting due to previous error diff --git a/src/test/ui/sized-cycle-note.stderr b/src/test/ui/sized-cycle-note.stderr index 95bdc34942645..99d8cfd0a05c9 100644 --- a/src/test/ui/sized-cycle-note.stderr +++ b/src/test/ui/sized-cycle-note.stderr @@ -2,21 +2,35 @@ error[E0072]: recursive type `Baz` has infinite size --> $DIR/sized-cycle-note.rs:9:1 | LL | struct Baz { q: Option } - | ^^^^^^^^^^ -------------- recursive without indirection + | ^^^^^^^^^^ ----------- recursive without indirection | | | recursive type has infinite size | - = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `Baz` representable +help: insert some indirection to make `Baz` representable + | +LL | struct Baz { q: Box> } + | ^^^^ ^ +LL | struct Baz { q: Rc> } + | ^^^ ^ +LL | struct Baz { q: &Option } + | ^ error[E0072]: recursive type `Foo` has infinite size --> $DIR/sized-cycle-note.rs:11:1 | LL | struct Foo { q: Option } - | ^^^^^^^^^^ -------------- recursive without indirection + | ^^^^^^^^^^ ----------- recursive without indirection | | | recursive type has infinite size | - = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `Foo` representable +help: insert some indirection to make `Foo` representable + | +LL | struct Foo { q: Box> } + | ^^^^ ^ +LL | struct Foo { q: Rc> } + | ^^^ ^ +LL | struct Foo { q: &Option } + | ^ error: aborting due to 2 previous errors diff --git a/src/test/ui/span/E0072.stderr b/src/test/ui/span/E0072.stderr index d4a5e7400d2a4..855e4facb7b8c 100644 --- a/src/test/ui/span/E0072.stderr +++ b/src/test/ui/span/E0072.stderr @@ -5,9 +5,16 @@ LL | struct ListNode { | ^^^^^^^^^^^^^^^ recursive type has infinite size LL | head: u8, LL | tail: Option, - | ---------------------- recursive without indirection + | ---------------- recursive without indirection | - = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `ListNode` representable +help: insert some indirection to make `ListNode` representable + | +LL | tail: Box>, + | ^^^^ ^ +LL | tail: Rc>, + | ^^^ ^ +LL | tail: &Option, + | ^ error: aborting due to previous error diff --git a/src/test/ui/span/multiline-span-E0072.stderr b/src/test/ui/span/multiline-span-E0072.stderr index dd322fe833b49..260c2157e96b7 100644 --- a/src/test/ui/span/multiline-span-E0072.stderr +++ b/src/test/ui/span/multiline-span-E0072.stderr @@ -6,11 +6,18 @@ LL | | ListNode LL | | { LL | | head: u8, LL | | tail: Option, - | | ---------------------- recursive without indirection + | | ---------------- recursive without indirection LL | | } | |_^ recursive type has infinite size | - = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `ListNode` representable +help: insert some indirection to make `ListNode` representable + | +LL | tail: Box>, + | ^^^^ ^ +LL | tail: Rc>, + | ^^^ ^ +LL | tail: &Option, + | ^ error: aborting due to previous error diff --git a/src/test/ui/span/recursive-type-field.stderr b/src/test/ui/span/recursive-type-field.stderr index d240872647e50..c1a3270d0a567 100644 --- a/src/test/ui/span/recursive-type-field.stderr +++ b/src/test/ui/span/recursive-type-field.stderr @@ -4,9 +4,16 @@ error[E0072]: recursive type `Foo` has infinite size LL | struct Foo<'a> { | ^^^^^^^^^^^^^^ recursive type has infinite size LL | bar: Bar<'a>, - | ------------ recursive without indirection + | ------- recursive without indirection | - = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `Foo` representable +help: insert some indirection to make `Foo` representable + | +LL | bar: Box>, + | ^^^^ ^ +LL | bar: Rc>, + | ^^^ ^ +LL | bar: &Bar<'a>, + | ^ error[E0072]: recursive type `Bar` has infinite size --> $DIR/recursive-type-field.rs:8:1 @@ -14,18 +21,18 @@ error[E0072]: recursive type `Bar` has infinite size LL | struct Bar<'a> { | ^^^^^^^^^^^^^^ recursive type has infinite size LL | y: (Foo<'a>, Foo<'a>), - | --------------------- recursive without indirection + | ------------------ recursive without indirection LL | z: Option>, - | ------------------ recursive without indirection + | --------------- recursive without indirection ... LL | d: [Bar<'a>; 1], - | --------------- recursive without indirection + | ------------ recursive without indirection LL | e: Foo<'a>, - | ---------- recursive without indirection + | ------- recursive without indirection LL | x: Bar<'a>, - | ---------- recursive without indirection + | ------- recursive without indirection | - = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `Bar` representable + = help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `Bar` representable error: aborting due to 2 previous errors diff --git a/src/test/ui/type/type-recursive.stderr b/src/test/ui/type/type-recursive.stderr index 72bf372e561d6..b98a6eac49e04 100644 --- a/src/test/ui/type/type-recursive.stderr +++ b/src/test/ui/type/type-recursive.stderr @@ -5,9 +5,16 @@ LL | struct T1 { | ^^^^^^^^^ recursive type has infinite size LL | foo: isize, LL | foolish: T1 - | ----------- recursive without indirection + | -- recursive without indirection | - = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `T1` representable +help: insert some indirection to make `T1` representable + | +LL | foolish: Box + | ^^^^ ^ +LL | foolish: Rc + | ^^^ ^ +LL | foolish: &T1 + | ^ error: aborting due to previous error diff --git a/src/test/ui/union/union-nonrepresentable.stderr b/src/test/ui/union/union-nonrepresentable.stderr index 746c1033ea348..70863a549ad25 100644 --- a/src/test/ui/union/union-nonrepresentable.stderr +++ b/src/test/ui/union/union-nonrepresentable.stderr @@ -5,9 +5,16 @@ LL | union U { | ^^^^^^^ recursive type has infinite size LL | a: u8, LL | b: U, - | ---- recursive without indirection + | - recursive without indirection | - = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `U` representable +help: insert some indirection to make `U` representable + | +LL | b: Box, + | ^^^^ ^ +LL | b: Rc, + | ^^^ ^ +LL | b: &U, + | ^ error: aborting due to previous error From 7cde07e5cc5cfc1665dd64e4c06482c2f10b693a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 30 May 2020 19:58:49 -0700 Subject: [PATCH 06/21] review comments: only suggest one substitution --- .../traits/error_reporting/mod.rs | 49 ++++++------------- .../infinite-tag-type-recursion.stderr | 6 +-- src/test/ui/issues/issue-17431-1.stderr | 6 +-- src/test/ui/issues/issue-17431-2.stderr | 12 +---- src/test/ui/issues/issue-17431-3.stderr | 6 +-- src/test/ui/issues/issue-17431-4.stderr | 6 +-- src/test/ui/issues/issue-17431-5.stderr | 6 +-- src/test/ui/issues/issue-17431-6.stderr | 6 +-- src/test/ui/issues/issue-17431-7.stderr | 6 +-- src/test/ui/issues/issue-2718-a.stderr | 6 +-- src/test/ui/issues/issue-3008-1.stderr | 6 +-- src/test/ui/issues/issue-3008-2.stderr | 6 +-- src/test/ui/issues/issue-3008-3.stderr | 6 +-- src/test/ui/issues/issue-3779.stderr | 6 +-- src/test/ui/issues/issue-57271.stderr | 12 +---- src/test/ui/recursion/recursive-enum.stderr | 6 +-- src/test/ui/sized-cycle-note.stderr | 12 +---- src/test/ui/span/E0072.stderr | 6 +-- src/test/ui/span/multiline-span-E0072.stderr | 6 +-- src/test/ui/span/recursive-type-field.stderr | 6 +-- src/test/ui/type/type-recursive.stderr | 6 +-- .../ui/union/union-nonrepresentable.stderr | 6 +-- 22 files changed, 38 insertions(+), 155 deletions(-) diff --git a/src/librustc_trait_selection/traits/error_reporting/mod.rs b/src/librustc_trait_selection/traits/error_reporting/mod.rs index 3457f7b4580c5..d31e04cffd55f 100644 --- a/src/librustc_trait_selection/traits/error_reporting/mod.rs +++ b/src/librustc_trait_selection/traits/error_reporting/mod.rs @@ -1759,48 +1759,27 @@ pub fn recursive_type_with_infinite_size_error( for &span in &spans { err.span_label(span, "recursive without indirection"); } - let short_msg = format!("insert some indirection to make `{}` representable", path); let msg = format!( "insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `{}` representable", path, ); - match &spans[..] { - [span] => { - err.multipart_suggestions( - &short_msg, - vec![ + if spans.len() <= 4 { + err.multipart_suggestion( + &msg, + spans + .iter() + .flat_map(|&span| { vec![ (span.shrink_to_lo(), "Box<".to_string()), (span.shrink_to_hi(), ">".to_string()), - ], - vec![ - (span.shrink_to_lo(), "Rc<".to_string()), - (span.shrink_to_hi(), ">".to_string()), - ], - vec![(span.shrink_to_lo(), "&".to_string())], - ], - Applicability::HasPlaceholders, - ); - } - _ if spans.len() <= 4 => { - err.multipart_suggestion( - &msg, - spans - .iter() - .flat_map(|&span| { - vec![ - (span.shrink_to_lo(), "Box<".to_string()), - (span.shrink_to_hi(), ">".to_string()), - ] - .into_iter() - }) - .collect(), - Applicability::HasPlaceholders, - ); - } - _ => { - err.help(&msg); - } + ] + .into_iter() + }) + .collect(), + Applicability::HasPlaceholders, + ); + } else { + err.help(&msg); } err.emit(); } diff --git a/src/test/ui/infinite/infinite-tag-type-recursion.stderr b/src/test/ui/infinite/infinite-tag-type-recursion.stderr index b6a4d8f4cf563..6d1df4fda2eb0 100644 --- a/src/test/ui/infinite/infinite-tag-type-recursion.stderr +++ b/src/test/ui/infinite/infinite-tag-type-recursion.stderr @@ -6,14 +6,10 @@ LL | enum MList { Cons(isize, MList), Nil } | | | recursive type has infinite size | -help: insert some indirection to make `MList` representable +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `MList` representable | LL | enum MList { Cons(isize, Box), Nil } | ^^^^ ^ -LL | enum MList { Cons(isize, Rc), Nil } - | ^^^ ^ -LL | enum MList { Cons(isize, &MList), Nil } - | ^ error[E0391]: cycle detected when computing drop-check constraints for `MList` --> $DIR/infinite-tag-type-recursion.rs:1:1 diff --git a/src/test/ui/issues/issue-17431-1.stderr b/src/test/ui/issues/issue-17431-1.stderr index 8d44154650e04..58d087ca1998b 100644 --- a/src/test/ui/issues/issue-17431-1.stderr +++ b/src/test/ui/issues/issue-17431-1.stderr @@ -6,14 +6,10 @@ LL | struct Foo { foo: Option> } | | | recursive type has infinite size | -help: insert some indirection to make `Foo` representable +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `Foo` representable | LL | struct Foo { foo: Box>> } | ^^^^ ^ -LL | struct Foo { foo: Rc>> } - | ^^^ ^ -LL | struct Foo { foo: &Option> } - | ^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-17431-2.stderr b/src/test/ui/issues/issue-17431-2.stderr index b06184e84da28..eba4bf6d1d5ea 100644 --- a/src/test/ui/issues/issue-17431-2.stderr +++ b/src/test/ui/issues/issue-17431-2.stderr @@ -6,14 +6,10 @@ LL | struct Baz { q: Option } | | | recursive type has infinite size | -help: insert some indirection to make `Baz` representable +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `Baz` representable | LL | struct Baz { q: Box> } | ^^^^ ^ -LL | struct Baz { q: Rc> } - | ^^^ ^ -LL | struct Baz { q: &Option } - | ^ error[E0072]: recursive type `Foo` has infinite size --> $DIR/issue-17431-2.rs:4:1 @@ -23,14 +19,10 @@ LL | struct Foo { q: Option } | | | recursive type has infinite size | -help: insert some indirection to make `Foo` representable +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `Foo` representable | LL | struct Foo { q: Box> } | ^^^^ ^ -LL | struct Foo { q: Rc> } - | ^^^ ^ -LL | struct Foo { q: &Option } - | ^ error: aborting due to 2 previous errors diff --git a/src/test/ui/issues/issue-17431-3.stderr b/src/test/ui/issues/issue-17431-3.stderr index f32bd70fd6d94..f6b15d0528ae8 100644 --- a/src/test/ui/issues/issue-17431-3.stderr +++ b/src/test/ui/issues/issue-17431-3.stderr @@ -6,14 +6,10 @@ LL | struct Foo { foo: Mutex> } | | | recursive type has infinite size | -help: insert some indirection to make `Foo` representable +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `Foo` representable | LL | struct Foo { foo: Box>> } | ^^^^ ^ -LL | struct Foo { foo: Rc>> } - | ^^^ ^ -LL | struct Foo { foo: &Mutex> } - | ^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-17431-4.stderr b/src/test/ui/issues/issue-17431-4.stderr index 372c5e975c844..aa709e1ad5183 100644 --- a/src/test/ui/issues/issue-17431-4.stderr +++ b/src/test/ui/issues/issue-17431-4.stderr @@ -6,14 +6,10 @@ LL | struct Foo { foo: Option>>, marker: marker::PhantomData | | | recursive type has infinite size | -help: insert some indirection to make `Foo` representable +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `Foo` representable | LL | struct Foo { foo: Box>>>, marker: marker::PhantomData } | ^^^^ ^ -LL | struct Foo { foo: Rc>>>, marker: marker::PhantomData } - | ^^^ ^ -LL | struct Foo { foo: &Option>>, marker: marker::PhantomData } - | ^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-17431-5.stderr b/src/test/ui/issues/issue-17431-5.stderr index 01b850ac33694..1558cffb036b3 100644 --- a/src/test/ui/issues/issue-17431-5.stderr +++ b/src/test/ui/issues/issue-17431-5.stderr @@ -6,14 +6,10 @@ LL | struct Bar { x: Bar , marker: marker::PhantomData } | | | recursive type has infinite size | -help: insert some indirection to make `Bar` representable +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `Bar` representable | LL | struct Bar { x: Box> , marker: marker::PhantomData } | ^^^^ ^ -LL | struct Bar { x: Rc> , marker: marker::PhantomData } - | ^^^ ^ -LL | struct Bar { x: &Bar , marker: marker::PhantomData } - | ^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-17431-6.stderr b/src/test/ui/issues/issue-17431-6.stderr index ce6c2db07fb6d..f2aa2a79c8200 100644 --- a/src/test/ui/issues/issue-17431-6.stderr +++ b/src/test/ui/issues/issue-17431-6.stderr @@ -6,14 +6,10 @@ LL | enum Foo { X(Mutex>) } | | | recursive type has infinite size | -help: insert some indirection to make `Foo` representable +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `Foo` representable | LL | enum Foo { X(Box>>) } | ^^^^ ^ -LL | enum Foo { X(Rc>>) } - | ^^^ ^ -LL | enum Foo { X(&Mutex>) } - | ^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-17431-7.stderr b/src/test/ui/issues/issue-17431-7.stderr index 4fb563ba502f2..684c3089e85ec 100644 --- a/src/test/ui/issues/issue-17431-7.stderr +++ b/src/test/ui/issues/issue-17431-7.stderr @@ -6,14 +6,10 @@ LL | enum Foo { Voo(Option>) } | | | recursive type has infinite size | -help: insert some indirection to make `Foo` representable +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `Foo` representable | LL | enum Foo { Voo(Box>>) } | ^^^^ ^ -LL | enum Foo { Voo(Rc>>) } - | ^^^ ^ -LL | enum Foo { Voo(&Option>) } - | ^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-2718-a.stderr b/src/test/ui/issues/issue-2718-a.stderr index 48b7a61e059ea..d152ffde4e57d 100644 --- a/src/test/ui/issues/issue-2718-a.stderr +++ b/src/test/ui/issues/issue-2718-a.stderr @@ -7,14 +7,10 @@ LL | pub struct Pong(SendPacket); | | recursive without indirection | recursive type has infinite size | -help: insert some indirection to make `pingpong::Pong` representable +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `pingpong::Pong` representable | LL | pub struct Pong(Box>); | ^^^^ ^ -LL | pub struct Pong(Rc>); - | ^^^ ^ -LL | pub struct Pong(&SendPacket); - | ^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-3008-1.stderr b/src/test/ui/issues/issue-3008-1.stderr index d1173a4b3334c..87ee36df21696 100644 --- a/src/test/ui/issues/issue-3008-1.stderr +++ b/src/test/ui/issues/issue-3008-1.stderr @@ -7,14 +7,10 @@ LL | enum Bar { LL | BarSome(Bar) | --- recursive without indirection | -help: insert some indirection to make `Bar` representable +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `Bar` representable | LL | BarSome(Box) | ^^^^ ^ -LL | BarSome(Rc) - | ^^^ ^ -LL | BarSome(&Bar) - | ^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-3008-2.stderr b/src/test/ui/issues/issue-3008-2.stderr index 4fd60639b21a8..369a19d37e6f6 100644 --- a/src/test/ui/issues/issue-3008-2.stderr +++ b/src/test/ui/issues/issue-3008-2.stderr @@ -6,14 +6,10 @@ LL | struct Bar { x: Bar } | | | recursive type has infinite size | -help: insert some indirection to make `Bar` representable +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `Bar` representable | LL | struct Bar { x: Box } | ^^^^ ^ -LL | struct Bar { x: Rc } - | ^^^ ^ -LL | struct Bar { x: &Bar } - | ^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-3008-3.stderr b/src/test/ui/issues/issue-3008-3.stderr index e6efad9188300..0b162eff94a7c 100644 --- a/src/test/ui/issues/issue-3008-3.stderr +++ b/src/test/ui/issues/issue-3008-3.stderr @@ -6,14 +6,10 @@ LL | enum E2 { V2(E2, marker::PhantomData), } | | | recursive type has infinite size | -help: insert some indirection to make `E2` representable +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `E2` representable | LL | enum E2 { V2(Box>, marker::PhantomData), } | ^^^^ ^ -LL | enum E2 { V2(Rc>, marker::PhantomData), } - | ^^^ ^ -LL | enum E2 { V2(&E2, marker::PhantomData), } - | ^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-3779.stderr b/src/test/ui/issues/issue-3779.stderr index 9b50ddec12a44..7b17e91421660 100644 --- a/src/test/ui/issues/issue-3779.stderr +++ b/src/test/ui/issues/issue-3779.stderr @@ -7,14 +7,10 @@ LL | LL | element: Option | --------- recursive without indirection | -help: insert some indirection to make `S` representable +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `S` representable | LL | element: Box> | ^^^^ ^ -LL | element: Rc> - | ^^^ ^ -LL | element: &Option - | ^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-57271.stderr b/src/test/ui/issues/issue-57271.stderr index a6fe83a9b5636..b7c799e163cee 100644 --- a/src/test/ui/issues/issue-57271.stderr +++ b/src/test/ui/issues/issue-57271.stderr @@ -7,14 +7,10 @@ LL | Class(ClassTypeSignature), LL | Array(TypeSignature), | ------------- recursive without indirection | -help: insert some indirection to make `ObjectType` representable +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `ObjectType` representable | LL | Array(Box), | ^^^^ ^ -LL | Array(Rc), - | ^^^ ^ -LL | Array(&TypeSignature), - | ^ error[E0072]: recursive type `TypeSignature` has infinite size --> $DIR/issue-57271.rs:19:1 @@ -25,14 +21,10 @@ LL | Base(BaseType), LL | Object(ObjectType), | ---------- recursive without indirection | -help: insert some indirection to make `TypeSignature` representable +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `TypeSignature` representable | LL | Object(Box), | ^^^^ ^ -LL | Object(Rc), - | ^^^ ^ -LL | Object(&ObjectType), - | ^ error: aborting due to 2 previous errors diff --git a/src/test/ui/recursion/recursive-enum.stderr b/src/test/ui/recursion/recursive-enum.stderr index c68badd458bce..ab4709d8e709e 100644 --- a/src/test/ui/recursion/recursive-enum.stderr +++ b/src/test/ui/recursion/recursive-enum.stderr @@ -6,14 +6,10 @@ LL | enum List { Cons(T, List), Nil } | | | recursive type has infinite size | -help: insert some indirection to make `List` representable +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `List` representable | LL | enum List { Cons(T, Box>), Nil } | ^^^^ ^ -LL | enum List { Cons(T, Rc>), Nil } - | ^^^ ^ -LL | enum List { Cons(T, &List), Nil } - | ^ error: aborting due to previous error diff --git a/src/test/ui/sized-cycle-note.stderr b/src/test/ui/sized-cycle-note.stderr index 99d8cfd0a05c9..45062c2ea6c72 100644 --- a/src/test/ui/sized-cycle-note.stderr +++ b/src/test/ui/sized-cycle-note.stderr @@ -6,14 +6,10 @@ LL | struct Baz { q: Option } | | | recursive type has infinite size | -help: insert some indirection to make `Baz` representable +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `Baz` representable | LL | struct Baz { q: Box> } | ^^^^ ^ -LL | struct Baz { q: Rc> } - | ^^^ ^ -LL | struct Baz { q: &Option } - | ^ error[E0072]: recursive type `Foo` has infinite size --> $DIR/sized-cycle-note.rs:11:1 @@ -23,14 +19,10 @@ LL | struct Foo { q: Option } | | | recursive type has infinite size | -help: insert some indirection to make `Foo` representable +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `Foo` representable | LL | struct Foo { q: Box> } | ^^^^ ^ -LL | struct Foo { q: Rc> } - | ^^^ ^ -LL | struct Foo { q: &Option } - | ^ error: aborting due to 2 previous errors diff --git a/src/test/ui/span/E0072.stderr b/src/test/ui/span/E0072.stderr index 855e4facb7b8c..06493f05142e6 100644 --- a/src/test/ui/span/E0072.stderr +++ b/src/test/ui/span/E0072.stderr @@ -7,14 +7,10 @@ LL | head: u8, LL | tail: Option, | ---------------- recursive without indirection | -help: insert some indirection to make `ListNode` representable +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `ListNode` representable | LL | tail: Box>, | ^^^^ ^ -LL | tail: Rc>, - | ^^^ ^ -LL | tail: &Option, - | ^ error: aborting due to previous error diff --git a/src/test/ui/span/multiline-span-E0072.stderr b/src/test/ui/span/multiline-span-E0072.stderr index 260c2157e96b7..55128347f7404 100644 --- a/src/test/ui/span/multiline-span-E0072.stderr +++ b/src/test/ui/span/multiline-span-E0072.stderr @@ -10,14 +10,10 @@ LL | | tail: Option, LL | | } | |_^ recursive type has infinite size | -help: insert some indirection to make `ListNode` representable +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `ListNode` representable | LL | tail: Box>, | ^^^^ ^ -LL | tail: Rc>, - | ^^^ ^ -LL | tail: &Option, - | ^ error: aborting due to previous error diff --git a/src/test/ui/span/recursive-type-field.stderr b/src/test/ui/span/recursive-type-field.stderr index c1a3270d0a567..fb1d98b58dfbe 100644 --- a/src/test/ui/span/recursive-type-field.stderr +++ b/src/test/ui/span/recursive-type-field.stderr @@ -6,14 +6,10 @@ LL | struct Foo<'a> { LL | bar: Bar<'a>, | ------- recursive without indirection | -help: insert some indirection to make `Foo` representable +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `Foo` representable | LL | bar: Box>, | ^^^^ ^ -LL | bar: Rc>, - | ^^^ ^ -LL | bar: &Bar<'a>, - | ^ error[E0072]: recursive type `Bar` has infinite size --> $DIR/recursive-type-field.rs:8:1 diff --git a/src/test/ui/type/type-recursive.stderr b/src/test/ui/type/type-recursive.stderr index b98a6eac49e04..d6d32cc5d6f39 100644 --- a/src/test/ui/type/type-recursive.stderr +++ b/src/test/ui/type/type-recursive.stderr @@ -7,14 +7,10 @@ LL | foo: isize, LL | foolish: T1 | -- recursive without indirection | -help: insert some indirection to make `T1` representable +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `T1` representable | LL | foolish: Box | ^^^^ ^ -LL | foolish: Rc - | ^^^ ^ -LL | foolish: &T1 - | ^ error: aborting due to previous error diff --git a/src/test/ui/union/union-nonrepresentable.stderr b/src/test/ui/union/union-nonrepresentable.stderr index 70863a549ad25..c54d04de12c50 100644 --- a/src/test/ui/union/union-nonrepresentable.stderr +++ b/src/test/ui/union/union-nonrepresentable.stderr @@ -7,14 +7,10 @@ LL | a: u8, LL | b: U, | - recursive without indirection | -help: insert some indirection to make `U` representable +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `U` representable | LL | b: Box, | ^^^^ ^ -LL | b: Rc, - | ^^^ ^ -LL | b: &U, - | ^ error: aborting due to previous error From 03552ec3fa4828329874a17d1b8966a0c03809d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 10 Jun 2020 14:54:27 -0700 Subject: [PATCH 07/21] fix rebase --- src/test/ui/issues/issue-72554.stderr | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/test/ui/issues/issue-72554.stderr b/src/test/ui/issues/issue-72554.stderr index 9db65f4a2ee8f..9de94c393a711 100644 --- a/src/test/ui/issues/issue-72554.stderr +++ b/src/test/ui/issues/issue-72554.stderr @@ -6,7 +6,10 @@ LL | pub enum ElemDerived { LL | A(ElemDerived) | ----------- recursive without indirection | - = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `ElemDerived` representable +help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to make `ElemDerived` representable + | +LL | A(Box) + | ^^^^ ^ error: aborting due to previous error From c45231ca555950cea450ba65f9d2d1962e3af6cd Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Fri, 12 Jun 2020 22:12:45 -0700 Subject: [PATCH 08/21] Revert heterogeneous SocketAddr PartialEq impls These lead to inference regressions (mostly in tests) in code that looks like: let socket = std::net::SocketAddrV4::new(std::net::Ipv4Addr::new(127, 0, 0, 1), 8080); assert_eq!(socket, "127.0.0.1:8080".parse().unwrap()); That compiles as of stable 1.44.0 but fails in beta with: error[E0284]: type annotations needed --> src/main.rs:3:41 | 3 | assert_eq!(socket, "127.0.0.1:8080".parse().unwrap()); | ^^^^^ cannot infer type for type parameter `F` declared on the associated function `parse` | = note: cannot satisfy `<_ as std::str::FromStr>::Err == _` help: consider specifying the type argument in the method call | 3 | assert_eq!(socket, "127.0.0.1:8080".parse::().unwrap()); | --- src/libstd/net/addr.rs | 40 ---------------------------------------- 1 file changed, 40 deletions(-) diff --git a/src/libstd/net/addr.rs b/src/libstd/net/addr.rs index b780340884e1f..7e3c3e8f3042e 100644 --- a/src/libstd/net/addr.rs +++ b/src/libstd/net/addr.rs @@ -694,42 +694,6 @@ impl PartialEq for SocketAddrV6 { && self.inner.sin6_scope_id == other.inner.sin6_scope_id } } -#[stable(feature = "socketaddr_ordering", since = "1.45.0")] -impl PartialEq for SocketAddr { - fn eq(&self, other: &SocketAddrV4) -> bool { - match self { - SocketAddr::V4(v4) => v4 == other, - SocketAddr::V6(_) => false, - } - } -} -#[stable(feature = "socketaddr_ordering", since = "1.45.0")] -impl PartialEq for SocketAddr { - fn eq(&self, other: &SocketAddrV6) -> bool { - match self { - SocketAddr::V4(_) => false, - SocketAddr::V6(v6) => v6 == other, - } - } -} -#[stable(feature = "socketaddr_ordering", since = "1.45.0")] -impl PartialEq for SocketAddrV4 { - fn eq(&self, other: &SocketAddr) -> bool { - match other { - SocketAddr::V4(v4) => self == v4, - SocketAddr::V6(_) => false, - } - } -} -#[stable(feature = "socketaddr_ordering", since = "1.45.0")] -impl PartialEq for SocketAddrV6 { - fn eq(&self, other: &SocketAddr) -> bool { - match other { - SocketAddr::V4(_) => false, - SocketAddr::V6(v6) => self == v6, - } - } -} #[stable(feature = "rust1", since = "1.0.0")] impl Eq for SocketAddrV4 {} #[stable(feature = "rust1", since = "1.0.0")] @@ -1242,12 +1206,8 @@ mod tests { // equality assert_eq!(v4_1, v4_1); assert_eq!(v6_1, v6_1); - assert_eq!(v4_1, SocketAddr::V4(v4_1)); - assert_eq!(v6_1, SocketAddr::V6(v6_1)); assert_eq!(SocketAddr::V4(v4_1), SocketAddr::V4(v4_1)); assert_eq!(SocketAddr::V6(v6_1), SocketAddr::V6(v6_1)); - assert!(v4_1 != SocketAddr::V6(v6_1)); - assert!(v6_1 != SocketAddr::V4(v4_1)); assert!(v4_1 != v4_2); assert!(v6_1 != v6_2); From 4ff167849c6e0268110e516818e3ec227f15381a Mon Sep 17 00:00:00 2001 From: David Wood Date: Fri, 12 Jun 2020 17:42:08 +0100 Subject: [PATCH 09/21] lint: normalize projections using opaque types This commit normalizes projections which contain opaque types (opaque types are otherwise linted against, which is would have previously made the test cases added in this commit fail). Signed-off-by: David Wood --- src/librustc_lint/types.rs | 27 +++++++++++------ src/test/ui/lint/lint-ctypes-73251-1.rs | 24 ++++++++++++++++ src/test/ui/lint/lint-ctypes-73251-1.stderr | 15 ++++++++++ src/test/ui/lint/lint-ctypes-73251-2.rs | 32 +++++++++++++++++++++ src/test/ui/lint/lint-ctypes-73251-2.stderr | 15 ++++++++++ src/test/ui/lint/lint-ctypes-73251.rs | 22 ++++++++++++++ 6 files changed, 127 insertions(+), 8 deletions(-) create mode 100644 src/test/ui/lint/lint-ctypes-73251-1.rs create mode 100644 src/test/ui/lint/lint-ctypes-73251-1.stderr create mode 100644 src/test/ui/lint/lint-ctypes-73251-2.rs create mode 100644 src/test/ui/lint/lint-ctypes-73251-2.stderr create mode 100644 src/test/ui/lint/lint-ctypes-73251.rs diff --git a/src/librustc_lint/types.rs b/src/librustc_lint/types.rs index cdb0eda645a48..0280c422258a9 100644 --- a/src/librustc_lint/types.rs +++ b/src/librustc_lint/types.rs @@ -927,22 +927,33 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { fn check_for_opaque_ty(&mut self, sp: Span, ty: Ty<'tcx>) -> bool { use rustc_middle::ty::TypeFoldable; - struct ProhibitOpaqueTypes<'tcx> { + struct ProhibitOpaqueTypes<'a, 'tcx> { + cx: &'a LateContext<'a, 'tcx>, ty: Option>, }; - impl<'tcx> ty::fold::TypeVisitor<'tcx> for ProhibitOpaqueTypes<'tcx> { + impl<'a, 'tcx> ty::fold::TypeVisitor<'tcx> for ProhibitOpaqueTypes<'a, 'tcx> { fn visit_ty(&mut self, ty: Ty<'tcx>) -> bool { - if let ty::Opaque(..) = ty.kind { - self.ty = Some(ty); - true - } else { - ty.super_visit_with(self) + match ty.kind { + ty::Opaque(..) => { + self.ty = Some(ty); + true + } + // Consider opaque types within projections FFI-safe if they do not normalize + // to more opaque types. + ty::Projection(..) => { + let ty = self.cx.tcx.normalize_erasing_regions(self.cx.param_env, ty); + + // If `ty` is a opaque type directly then `super_visit_with` won't invoke + // this function again. + if ty.has_opaque_types() { self.visit_ty(ty) } else { false } + } + _ => ty.super_visit_with(self), } } } - let mut visitor = ProhibitOpaqueTypes { ty: None }; + let mut visitor = ProhibitOpaqueTypes { cx: self.cx, ty: None }; ty.visit_with(&mut visitor); if let Some(ty) = visitor.ty { self.emit_ffi_unsafe_type_lint(ty, sp, "opaque types have no C equivalent", None); diff --git a/src/test/ui/lint/lint-ctypes-73251-1.rs b/src/test/ui/lint/lint-ctypes-73251-1.rs new file mode 100644 index 0000000000000..935073a6fe4ac --- /dev/null +++ b/src/test/ui/lint/lint-ctypes-73251-1.rs @@ -0,0 +1,24 @@ +#![feature(type_alias_impl_trait)] +#![deny(improper_ctypes)] + +pub trait Baz { } + +impl Baz for u32 { } + +type Qux = impl Baz; + +pub trait Foo { + type Assoc; +} + +impl Foo for u32 { + type Assoc = Qux; +} + +fn assign() -> Qux { 1 } + +extern "C" { + pub fn lint_me() -> ::Assoc; //~ ERROR: uses type `Qux` +} + +fn main() {} diff --git a/src/test/ui/lint/lint-ctypes-73251-1.stderr b/src/test/ui/lint/lint-ctypes-73251-1.stderr new file mode 100644 index 0000000000000..bfe843de2f4d1 --- /dev/null +++ b/src/test/ui/lint/lint-ctypes-73251-1.stderr @@ -0,0 +1,15 @@ +error: `extern` block uses type `Qux`, which is not FFI-safe + --> $DIR/lint-ctypes-73251-1.rs:21:25 + | +LL | pub fn lint_me() -> ::Assoc; + | ^^^^^^^^^^^^^^^^^^^ not FFI-safe + | +note: the lint level is defined here + --> $DIR/lint-ctypes-73251-1.rs:2:9 + | +LL | #![deny(improper_ctypes)] + | ^^^^^^^^^^^^^^^ + = note: opaque types have no C equivalent + +error: aborting due to previous error + diff --git a/src/test/ui/lint/lint-ctypes-73251-2.rs b/src/test/ui/lint/lint-ctypes-73251-2.rs new file mode 100644 index 0000000000000..c8710bcc511f8 --- /dev/null +++ b/src/test/ui/lint/lint-ctypes-73251-2.rs @@ -0,0 +1,32 @@ +#![feature(type_alias_impl_trait)] +#![deny(improper_ctypes)] + +pub trait TraitA { + type Assoc; +} + +impl TraitA for u32 { + type Assoc = u32; +} + +pub trait TraitB { + type Assoc; +} + +impl TraitB for T where T: TraitA { + type Assoc = ::Assoc; +} + +type AliasA = impl TraitA; + +type AliasB = impl TraitB; + +fn use_of_a() -> AliasA { 3 } + +fn use_of_b() -> AliasB { 3 } + +extern "C" { + pub fn lint_me() -> ::Assoc; //~ ERROR: uses type `AliasA` +} + +fn main() {} diff --git a/src/test/ui/lint/lint-ctypes-73251-2.stderr b/src/test/ui/lint/lint-ctypes-73251-2.stderr new file mode 100644 index 0000000000000..a054f7a562599 --- /dev/null +++ b/src/test/ui/lint/lint-ctypes-73251-2.stderr @@ -0,0 +1,15 @@ +error: `extern` block uses type `AliasA`, which is not FFI-safe + --> $DIR/lint-ctypes-73251-2.rs:29:25 + | +LL | pub fn lint_me() -> ::Assoc; + | ^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe + | +note: the lint level is defined here + --> $DIR/lint-ctypes-73251-2.rs:2:9 + | +LL | #![deny(improper_ctypes)] + | ^^^^^^^^^^^^^^^ + = note: opaque types have no C equivalent + +error: aborting due to previous error + diff --git a/src/test/ui/lint/lint-ctypes-73251.rs b/src/test/ui/lint/lint-ctypes-73251.rs new file mode 100644 index 0000000000000..ebc2ca77b67a1 --- /dev/null +++ b/src/test/ui/lint/lint-ctypes-73251.rs @@ -0,0 +1,22 @@ +// check-pass + +#![feature(type_alias_impl_trait)] +#![deny(improper_ctypes)] + +pub trait Foo { + type Assoc; +} + +impl Foo for () { + type Assoc = u32; +} + +type Bar = impl Foo; + +fn assign() -> Bar {} + +extern "C" { + pub fn lint_me() -> ::Assoc; +} + +fn main() {} From f747073fc1751afd2cfd4395283a4822b618f2da Mon Sep 17 00:00:00 2001 From: Poliorcetics Date: Sat, 13 Jun 2020 18:41:01 +0200 Subject: [PATCH 10/21] Apply suggestions from code review Co-authored-by: David Tolnay --- src/libstd/sync/mutex.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libstd/sync/mutex.rs b/src/libstd/sync/mutex.rs index b3ef521d6ecb0..6625d4659dcac 100644 --- a/src/libstd/sync/mutex.rs +++ b/src/libstd/sync/mutex.rs @@ -108,8 +108,8 @@ use crate::sys_common::poison::{self, LockResult, TryLockError, TryLockResult}; /// *guard += 1; /// ``` /// -/// It is sometimes a good idea (or even necessary) to manually drop the mutex -/// to unlock it as soon as possible. If you need the resource until the end of +/// It is sometimes necessary to manually drop the mutex +/// guard to unlock it as soon as possible. If you need the resource until the end of /// the scope, this is not needed. /// /// ``` @@ -140,16 +140,16 @@ use crate::sys_common::poison::{self, LockResult, TryLockError, TryLockResult}; /// // This is the result of some important and long-ish work. /// let result = data.iter().fold(0, |acc, x| acc + x * 2); /// data.push(result); -/// // We drop the `data` explicitely because it's not necessary anymore +/// // We drop the `data` explicitly because it's not necessary anymore /// // and the thread still has work to do. This allow other threads to /// // start working on the data immediately, without waiting /// // for the rest of the unrelated work to be done here. /// // /// // It's even more important here than in the threads because we `.join` the -/// // threads after that. If we had not dropped the lock, a thread could be +/// // threads after that. If we had not dropped the mutex guard, a thread could be /// // waiting forever for it, causing a deadlock. /// drop(data); -/// // Here the lock is not assigned to a variable and so, even if the scope +/// // Here the mutex guard is not assigned to a variable and so, even if the scope /// // does not end after this line, the mutex is still released: /// // there is no deadlock. /// *res_mutex.lock().unwrap() += result; From 34b3ff06e101f60cb69268ee83c93c177b63c322 Mon Sep 17 00:00:00 2001 From: Poliorcetics Date: Sat, 13 Jun 2020 18:43:37 +0200 Subject: [PATCH 11/21] Clarify the scope-related explanation Based on the review made by dtolnay. --- src/libstd/sync/mutex.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libstd/sync/mutex.rs b/src/libstd/sync/mutex.rs index 6625d4659dcac..37c8125b0984a 100644 --- a/src/libstd/sync/mutex.rs +++ b/src/libstd/sync/mutex.rs @@ -108,9 +108,8 @@ use crate::sys_common::poison::{self, LockResult, TryLockError, TryLockResult}; /// *guard += 1; /// ``` /// -/// It is sometimes necessary to manually drop the mutex -/// guard to unlock it as soon as possible. If you need the resource until the end of -/// the scope, this is not needed. +/// It is sometimes necessary to manually drop the mutex guard +/// to unlock it sooner than the end of the enclosing scope. /// /// ``` /// use std::sync::{Arc, Mutex}; From c010e711ca5ec02012afb83c0d99aec9d26a9eea Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 13 Jun 2020 10:11:02 -0700 Subject: [PATCH 12/21] Rewrap comments in Mutex example --- src/libstd/sync/mutex.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/libstd/sync/mutex.rs b/src/libstd/sync/mutex.rs index 37c8125b0984a..8478457eabfc2 100644 --- a/src/libstd/sync/mutex.rs +++ b/src/libstd/sync/mutex.rs @@ -108,8 +108,8 @@ use crate::sys_common::poison::{self, LockResult, TryLockError, TryLockResult}; /// *guard += 1; /// ``` /// -/// It is sometimes necessary to manually drop the mutex guard -/// to unlock it sooner than the end of the enclosing scope. +/// It is sometimes necessary to manually drop the mutex guard to unlock it +/// sooner than the end of the enclosing scope. /// /// ``` /// use std::sync::{Arc, Mutex}; @@ -139,18 +139,18 @@ use crate::sys_common::poison::{self, LockResult, TryLockError, TryLockResult}; /// // This is the result of some important and long-ish work. /// let result = data.iter().fold(0, |acc, x| acc + x * 2); /// data.push(result); -/// // We drop the `data` explicitly because it's not necessary anymore -/// // and the thread still has work to do. This allow other threads to -/// // start working on the data immediately, without waiting -/// // for the rest of the unrelated work to be done here. +/// // We drop the `data` explicitly because it's not necessary anymore and the +/// // thread still has work to do. This allow other threads to start working on +/// // the data immediately, without waiting for the rest of the unrelated work +/// // to be done here. /// // /// // It's even more important here than in the threads because we `.join` the -/// // threads after that. If we had not dropped the mutex guard, a thread could be -/// // waiting forever for it, causing a deadlock. +/// // threads after that. If we had not dropped the mutex guard, a thread could +/// // be waiting forever for it, causing a deadlock. /// drop(data); -/// // Here the mutex guard is not assigned to a variable and so, even if the scope -/// // does not end after this line, the mutex is still released: -/// // there is no deadlock. +/// // Here the mutex guard is not assigned to a variable and so, even if the +/// // scope does not end after this line, the mutex is still released: there is +/// // no deadlock. /// *res_mutex.lock().unwrap() += result; /// /// threads.into_iter().for_each(|thread| { From 204c236ad5b632294d8794e729326be8053ab2aa Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 13 Jun 2020 10:21:11 -0700 Subject: [PATCH 13/21] Add test for comparing SocketAddr with inferred right-hand side --- src/libstd/net/addr.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/libstd/net/addr.rs b/src/libstd/net/addr.rs index 7e3c3e8f3042e..b8fa1a7f744d3 100644 --- a/src/libstd/net/addr.rs +++ b/src/libstd/net/addr.rs @@ -1228,5 +1228,10 @@ mod tests { assert!(v6_1 < v6_3); assert!(v4_3 > v4_1); assert!(v6_3 > v6_1); + + // compare with an inferred right-hand side + assert_eq!(v4_1, "224.120.45.1:23456".parse().unwrap()); + assert_eq!(v6_1, "[2001:db8:f00::1002]:23456".parse().unwrap()); + assert_eq!(SocketAddr::V4(v4_1), "224.120.45.1:23456".parse().unwrap()); } } From cecfa43fb4f12ff4fda3af372dd8502a8f275ee6 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Thu, 30 Apr 2020 21:04:46 -0700 Subject: [PATCH 14/21] Move `check_op` logic to `ops` module --- .../transform/check_consts/ops.rs | 16 ++++++++++ .../transform/check_consts/validation.rs | 29 +++++-------------- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/src/librustc_mir/transform/check_consts/ops.rs b/src/librustc_mir/transform/check_consts/ops.rs index 92bd740e27aa6..d5059c98c9511 100644 --- a/src/librustc_mir/transform/check_consts/ops.rs +++ b/src/librustc_mir/transform/check_consts/ops.rs @@ -10,6 +10,22 @@ use rustc_span::{Span, Symbol}; use super::ConstCx; +/// Emits an error if `op` is not allowed in the given const context. +pub fn non_const(ccx: &ConstCx<'_, '_>, op: O, span: Span) { + debug!("illegal_op: op={:?}", op); + + if op.is_allowed_in_item(ccx) { + return; + } + + if ccx.tcx.sess.opts.debugging_opts.unleash_the_miri_inside_of_you { + ccx.tcx.sess.miri_unleashed_feature(span, O::feature_gate()); + return; + } + + op.emit_error(ccx, span); +} + /// An operation that is not *always* allowed in a const context. pub trait NonConstOp: std::fmt::Debug { /// Returns the `Symbol` corresponding to the feature gate that would enable this operation, diff --git a/src/librustc_mir/transform/check_consts/validation.rs b/src/librustc_mir/transform/check_consts/validation.rs index ab87d70da7da3..d351b3ced0539 100644 --- a/src/librustc_mir/transform/check_consts/validation.rs +++ b/src/librustc_mir/transform/check_consts/validation.rs @@ -232,30 +232,15 @@ impl Validator<'mir, 'tcx> { self.qualifs.in_return_place(self.ccx) } - /// Emits an error at the given `span` if an expression cannot be evaluated in the current - /// context. - pub fn check_op_spanned(&mut self, op: O, span: Span) - where - O: NonConstOp, - { - debug!("check_op: op={:?}", op); - - if op.is_allowed_in_item(self) { - return; - } - - if self.tcx.sess.opts.debugging_opts.unleash_the_miri_inside_of_you { - self.tcx.sess.miri_unleashed_feature(span, O::feature_gate()); - return; - } - - op.emit_error(self, span); - } - /// Emits an error if an expression cannot be evaluated in the current context. pub fn check_op(&mut self, op: impl NonConstOp) { - let span = self.span; - self.check_op_spanned(op, span) + ops::non_const(self.ccx, op, self.span); + } + + /// Emits an error at the given `span` if an expression cannot be evaluated in the current + /// context. + pub fn check_op_spanned(&mut self, op: impl NonConstOp, span: Span) { + ops::non_const(self.ccx, op, span); } fn check_static(&mut self, def_id: DefId, span: Span) { From d73674e1e3ea255cc82ec00023a4968d43c93a46 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Thu, 30 Apr 2020 21:04:59 -0700 Subject: [PATCH 15/21] Make `Qualifs` getters public --- src/librustc_mir/transform/check_consts/validation.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/transform/check_consts/validation.rs b/src/librustc_mir/transform/check_consts/validation.rs index d351b3ced0539..cc9b593b7abc1 100644 --- a/src/librustc_mir/transform/check_consts/validation.rs +++ b/src/librustc_mir/transform/check_consts/validation.rs @@ -40,7 +40,7 @@ pub struct Qualifs<'mir, 'tcx> { } impl Qualifs<'mir, 'tcx> { - fn indirectly_mutable( + pub fn indirectly_mutable( &mut self, ccx: &'mir ConstCx<'mir, 'tcx>, local: Local, @@ -68,7 +68,7 @@ impl Qualifs<'mir, 'tcx> { /// Returns `true` if `local` is `NeedsDrop` at the given `Location`. /// /// Only updates the cursor if absolutely necessary - fn needs_drop( + pub fn needs_drop( &mut self, ccx: &'mir ConstCx<'mir, 'tcx>, local: Local, @@ -95,7 +95,7 @@ impl Qualifs<'mir, 'tcx> { /// Returns `true` if `local` is `HasMutInterior` at the given `Location`. /// /// Only updates the cursor if absolutely necessary. - fn has_mut_interior( + pub fn has_mut_interior( &mut self, ccx: &'mir ConstCx<'mir, 'tcx>, local: Local, From a77f046c699b8ecd2352246aec8f1a571187918d Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Thu, 11 Jun 2020 14:30:19 -0700 Subject: [PATCH 16/21] Add feature gate for precise live drop checking --- src/librustc_feature/active.rs | 3 +++ src/librustc_span/symbol.rs | 1 + 2 files changed, 4 insertions(+) diff --git a/src/librustc_feature/active.rs b/src/librustc_feature/active.rs index b4935236b6a97..d186f35a12b5a 100644 --- a/src/librustc_feature/active.rs +++ b/src/librustc_feature/active.rs @@ -577,6 +577,9 @@ declare_features! ( /// Allows `extern "avr-interrupt" fn()` and `extern "avr-non-blocking-interrupt" fn()`. (active, abi_avr_interrupt, "1.45.0", Some(69664), None), + /// Be more precise when looking for live drops in a const context. + (active, const_precise_live_drops, "1.46.0", Some(73255), None), + // ------------------------------------------------------------------------- // feature-group-end: actual feature gates // ------------------------------------------------------------------------- diff --git a/src/librustc_span/symbol.rs b/src/librustc_span/symbol.rs index fea5880f01ebd..fdeb58b7b7a31 100644 --- a/src/librustc_span/symbol.rs +++ b/src/librustc_span/symbol.rs @@ -227,6 +227,7 @@ symbols! { const_loop, const_mut_refs, const_panic, + const_precise_live_drops, const_raw_ptr_deref, const_raw_ptr_to_usize_cast, const_transmute, From f5370faa6c37c565eee5ae83509302f3ed16fc3d Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Sun, 3 May 2020 11:18:26 -0700 Subject: [PATCH 17/21] Add `CheckLiveDrops` pass --- .../transform/check_consts/mod.rs | 1 + .../check_consts/post_drop_elaboration.rs | 119 ++++++++++++++++++ .../transform/check_consts/validation.rs | 6 + 3 files changed, 126 insertions(+) create mode 100644 src/librustc_mir/transform/check_consts/post_drop_elaboration.rs diff --git a/src/librustc_mir/transform/check_consts/mod.rs b/src/librustc_mir/transform/check_consts/mod.rs index 7c439f80ef6ad..e4aa88e3c20a7 100644 --- a/src/librustc_mir/transform/check_consts/mod.rs +++ b/src/librustc_mir/transform/check_consts/mod.rs @@ -12,6 +12,7 @@ use rustc_middle::ty::{self, TyCtxt}; pub use self::qualifs::Qualif; mod ops; +pub mod post_drop_elaboration; pub mod qualifs; mod resolver; pub mod validation; diff --git a/src/librustc_mir/transform/check_consts/post_drop_elaboration.rs b/src/librustc_mir/transform/check_consts/post_drop_elaboration.rs new file mode 100644 index 0000000000000..226e0e2049ebd --- /dev/null +++ b/src/librustc_mir/transform/check_consts/post_drop_elaboration.rs @@ -0,0 +1,119 @@ +use rustc_hir::def_id::LocalDefId; +use rustc_middle::mir::visit::Visitor; +use rustc_middle::mir::{self, BasicBlock, Location}; +use rustc_middle::ty::TyCtxt; +use rustc_span::Span; + +use super::ops; +use super::qualifs::{NeedsDrop, Qualif}; +use super::validation::Qualifs; +use super::ConstCx; + +/// Returns `true` if we should use the more precise live drop checker that runs after drop +/// elaboration. +pub fn checking_enabled(tcx: TyCtxt<'tcx>) -> bool { + tcx.features().const_precise_live_drops +} + +/// Look for live drops in a const context. +/// +/// This is separate from the rest of the const checking logic because it must run after drop +/// elaboration. +pub fn check_live_drops(tcx: TyCtxt<'tcx>, def_id: LocalDefId, body: &mir::Body<'tcx>) { + let const_kind = tcx.hir().body_const_context(def_id); + if const_kind.is_none() { + return; + } + + if !checking_enabled(tcx) { + return; + } + + let ccx = ConstCx { + body, + tcx, + def_id: def_id.to_def_id(), + const_kind, + param_env: tcx.param_env(def_id), + }; + + let mut visitor = CheckLiveDrops { ccx: &ccx, qualifs: Qualifs::default() }; + + visitor.visit_body(body); +} + +struct CheckLiveDrops<'mir, 'tcx> { + ccx: &'mir ConstCx<'mir, 'tcx>, + qualifs: Qualifs<'mir, 'tcx>, +} + +// So we can access `body` and `tcx`. +impl std::ops::Deref for CheckLiveDrops<'mir, 'tcx> { + type Target = ConstCx<'mir, 'tcx>; + + fn deref(&self) -> &Self::Target { + &self.ccx + } +} + +impl CheckLiveDrops<'mir, 'tcx> { + fn check_live_drop(&self, span: Span) { + ops::non_const(self.ccx, ops::LiveDrop, span); + } +} + +impl Visitor<'tcx> for CheckLiveDrops<'mir, 'tcx> { + fn visit_basic_block_data(&mut self, bb: BasicBlock, block: &mir::BasicBlockData<'tcx>) { + trace!("visit_basic_block_data: bb={:?} is_cleanup={:?}", bb, block.is_cleanup); + + // Ignore drop terminators in cleanup blocks. + if block.is_cleanup { + return; + } + + self.super_basic_block_data(bb, block); + } + + fn visit_terminator(&mut self, terminator: &mir::Terminator<'tcx>, location: Location) { + trace!("visit_terminator: terminator={:?} location={:?}", terminator, location); + + match &terminator.kind { + mir::TerminatorKind::Drop { location: dropped_place, .. } => { + let dropped_ty = dropped_place.ty(self.body, self.tcx).ty; + if !NeedsDrop::in_any_value_of_ty(self.ccx, dropped_ty) { + return; + } + + if dropped_place.is_indirect() { + self.check_live_drop(terminator.source_info.span); + return; + } + + if self.qualifs.needs_drop(self.ccx, dropped_place.local, location) { + // Use the span where the dropped local was declared for the error. + let span = self.body.local_decls[dropped_place.local].source_info.span; + self.check_live_drop(span); + } + } + + mir::TerminatorKind::DropAndReplace { .. } => span_bug!( + terminator.source_info.span, + "`DropAndReplace` should be removed by drop elaboration", + ), + + mir::TerminatorKind::Abort + | mir::TerminatorKind::Call { .. } + | mir::TerminatorKind::Assert { .. } + | mir::TerminatorKind::FalseEdge { .. } + | mir::TerminatorKind::FalseUnwind { .. } + | mir::TerminatorKind::GeneratorDrop + | mir::TerminatorKind::Goto { .. } + | mir::TerminatorKind::InlineAsm { .. } + | mir::TerminatorKind::Resume + | mir::TerminatorKind::Return + | mir::TerminatorKind::SwitchInt { .. } + | mir::TerminatorKind::Unreachable + | mir::TerminatorKind::Yield { .. } => {} + } + } +} diff --git a/src/librustc_mir/transform/check_consts/validation.rs b/src/librustc_mir/transform/check_consts/validation.rs index cc9b593b7abc1..428a74bcdcbfb 100644 --- a/src/librustc_mir/transform/check_consts/validation.rs +++ b/src/librustc_mir/transform/check_consts/validation.rs @@ -562,6 +562,12 @@ impl Visitor<'tcx> for Validator<'mir, 'tcx> { // projections that cannot be `NeedsDrop`. TerminatorKind::Drop { location: dropped_place, .. } | TerminatorKind::DropAndReplace { location: dropped_place, .. } => { + // If we are checking live drops after drop-elaboration, don't emit duplicate + // errors here. + if super::post_drop_elaboration::checking_enabled(self.tcx) { + return; + } + let mut err_span = self.span; // Check to see if the type of this place can ever have a drop impl. If not, this From a43e486e8d126f8a815c4e8ebba5ce8692864b06 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Sat, 2 May 2020 21:16:17 -0700 Subject: [PATCH 18/21] Add MIR phase and query for drop elaboration --- src/librustc_middle/mir/mod.rs | 3 +- src/librustc_middle/query/mod.rs | 6 ++++ src/librustc_mir/transform/mod.rs | 49 +++++++++++++++++++++++++------ 3 files changed, 48 insertions(+), 10 deletions(-) diff --git a/src/librustc_middle/mir/mod.rs b/src/librustc_middle/mir/mod.rs index 129f9691ea521..21f5d9e7dd4c6 100644 --- a/src/librustc_middle/mir/mod.rs +++ b/src/librustc_middle/mir/mod.rs @@ -76,7 +76,8 @@ pub enum MirPhase { Build = 0, Const = 1, Validated = 2, - Optimized = 3, + DropElab = 3, + Optimized = 4, } impl MirPhase { diff --git a/src/librustc_middle/query/mod.rs b/src/librustc_middle/query/mod.rs index 4d7e7882e426c..9e3881b28b693 100644 --- a/src/librustc_middle/query/mod.rs +++ b/src/librustc_middle/query/mod.rs @@ -190,6 +190,12 @@ rustc_queries! { no_hash } + query mir_drops_elaborated_and_const_checked(key: LocalDefId) -> Steal> { + storage(ArenaCacheSelector<'tcx>) + no_hash + desc { |tcx| "elaborating drops for `{}`", tcx.def_path_str(key.to_def_id()) } + } + query mir_validated(key: LocalDefId) -> ( Steal>, diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index 26725a2ac02d5..4240b528a6124 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -49,6 +49,7 @@ pub(crate) fn provide(providers: &mut Providers<'_>) { mir_const, mir_const_qualif, mir_validated, + mir_drops_elaborated_and_const_checked, optimized_mir, is_mir_available, promoted_mir, @@ -294,12 +295,31 @@ fn mir_validated( (tcx.alloc_steal_mir(body), tcx.alloc_steal_promoted(promoted)) } -fn run_optimization_passes<'tcx>( +fn mir_drops_elaborated_and_const_checked<'tcx>( + tcx: TyCtxt<'tcx>, + def_id: LocalDefId, +) -> Steal> { + // (Mir-)Borrowck uses `mir_validated`, so we have to force it to + // execute before we can steal. + tcx.ensure().mir_borrowck(def_id); + + let (body, _) = tcx.mir_validated(def_id); + let mut body = body.steal(); + + run_post_borrowck_cleanup_passes(tcx, &mut body, def_id, None); + check_consts::post_drop_elaboration::check_live_drops(tcx, def_id, &body); + tcx.alloc_steal_mir(body) +} + +/// After this series of passes, no lifetime analysis based on borrowing can be done. +fn run_post_borrowck_cleanup_passes<'tcx>( tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>, def_id: LocalDefId, promoted: Option, ) { + debug!("post_borrowck_cleanup({:?})", def_id); + let post_borrowck_cleanup: &[&dyn MirPass<'tcx>] = &[ // Remove all things only needed by analysis &no_landing_pads::NoLandingPads::new(tcx), @@ -318,9 +338,24 @@ fn run_optimization_passes<'tcx>( // but before optimizations begin. &add_retag::AddRetag, &simplify::SimplifyCfg::new("elaborate-drops"), - // No lifetime analysis based on borrowing can be done from here on out. ]; + run_passes( + tcx, + body, + InstanceDef::Item(def_id.to_def_id()), + promoted, + MirPhase::DropElab, + &[post_borrowck_cleanup], + ); +} + +fn run_optimization_passes<'tcx>( + tcx: TyCtxt<'tcx>, + body: &mut Body<'tcx>, + def_id: LocalDefId, + promoted: Option, +) { let optimizations: &[&dyn MirPass<'tcx>] = &[ &unreachable_prop::UnreachablePropagation, &uninhabited_enum_branching::UninhabitedEnumBranching, @@ -368,6 +403,7 @@ fn run_optimization_passes<'tcx>( let mir_opt_level = tcx.sess.opts.debugging_opts.mir_opt_level; + #[rustfmt::skip] run_passes( tcx, body, @@ -375,7 +411,6 @@ fn run_optimization_passes<'tcx>( promoted, MirPhase::Optimized, &[ - post_borrowck_cleanup, if mir_opt_level > 0 { optimizations } else { no_optimizations }, pre_codegen_cleanup, ], @@ -393,12 +428,7 @@ fn optimized_mir(tcx: TyCtxt<'_>, def_id: DefId) -> Body<'_> { let def_id = def_id.expect_local(); - // (Mir-)Borrowck uses `mir_validated`, so we have to force it to - // execute before we can steal. - tcx.ensure().mir_borrowck(def_id); - - let (body, _) = tcx.mir_validated(def_id); - let mut body = body.steal(); + let mut body = tcx.mir_drops_elaborated_and_const_checked(def_id).steal(); run_optimization_passes(tcx, &mut body, def_id, None); debug_assert!(!body.has_free_regions(), "Free regions in optimized MIR"); @@ -418,6 +448,7 @@ fn promoted_mir(tcx: TyCtxt<'_>, def_id: DefId) -> IndexVec> let mut promoted = promoted.steal(); for (p, mut body) in promoted.iter_enumerated_mut() { + run_post_borrowck_cleanup_passes(tcx, &mut body, def_id, Some(p)); run_optimization_passes(tcx, &mut body, def_id, Some(p)); } From 21ddf4d903762bb0af92ee0b63140544b8f0fbd2 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Sat, 2 May 2020 21:16:55 -0700 Subject: [PATCH 19/21] Ensure that `drop_elaboration_and_check_consts` runs for all const items --- src/librustc_interface/passes.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index 1a9bf4e1e8f3d..1ed9bc3f1f509 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -847,7 +847,11 @@ fn analysis(tcx: TyCtxt<'_>, cnum: CrateNum) -> Result<()> { sess.time("MIR_effect_checking", || { for def_id in tcx.body_owners() { - mir::transform::check_unsafety::check_unsafety(tcx, def_id) + mir::transform::check_unsafety::check_unsafety(tcx, def_id); + + if tcx.hir().body_const_context(def_id).is_some() { + tcx.ensure().mir_drops_elaborated_and_const_checked(def_id); + } } }); From 9e2ee322e84eb63d3fc2c9f6d11131f74d3f4dea Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Sun, 3 May 2020 12:41:48 -0700 Subject: [PATCH 20/21] Update incorrect error code docs --- src/librustc_error_codes/error_codes/E0493.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/librustc_error_codes/error_codes/E0493.md b/src/librustc_error_codes/error_codes/E0493.md index 90a0cbce623d0..0dcc3b62b4b2f 100644 --- a/src/librustc_error_codes/error_codes/E0493.md +++ b/src/librustc_error_codes/error_codes/E0493.md @@ -1,5 +1,4 @@ -A type with a `Drop` implementation was destructured when trying to initialize -a static item. +A value with a custom `Drop` implementation may be dropped during const-eval. Erroneous code example: @@ -16,13 +15,14 @@ struct Foo { field1: DropType, } -static FOO: Foo = Foo { ..Foo { field1: DropType::A } }; // error! +static FOO: Foo = Foo { field1: (DropType::A, DropType::A).1 }; // error! ``` The problem here is that if the given type or one of its fields implements the -`Drop` trait, this `Drop` implementation cannot be called during the static -type initialization which might cause a memory leak. To prevent this issue, -you need to instantiate all the static type's fields by hand. +`Drop` trait, this `Drop` implementation cannot be called within a const +context since it may run arbitrary, non-const-checked code. To prevent this +issue, ensure all values with custom a custom `Drop` implementation escape the +initializer. ``` enum DropType { From 2dcf7dbb8635a8fc3e972eac8204a83d616e1f50 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Thu, 11 Jun 2020 15:08:53 -0700 Subject: [PATCH 21/21] Add tests for `const_precise_live_drops` --- .../control-flow/drop-fail.precise.stderr | 15 ++++++++++++++ .../{drop-failure.rs => drop-fail.rs} | 13 +++++++----- ...-failure.stderr => drop-fail.stock.stderr} | 8 ++++---- .../{drop-success.rs => drop-pass.rs} | 2 ++ .../ui/consts/control-flow/drop-precise.rs | 20 +++++++++++++++++++ 5 files changed, 49 insertions(+), 9 deletions(-) create mode 100644 src/test/ui/consts/control-flow/drop-fail.precise.stderr rename src/test/ui/consts/control-flow/{drop-failure.rs => drop-fail.rs} (68%) rename src/test/ui/consts/control-flow/{drop-failure.stderr => drop-fail.stock.stderr} (85%) rename src/test/ui/consts/control-flow/{drop-success.rs => drop-pass.rs} (89%) create mode 100644 src/test/ui/consts/control-flow/drop-precise.rs diff --git a/src/test/ui/consts/control-flow/drop-fail.precise.stderr b/src/test/ui/consts/control-flow/drop-fail.precise.stderr new file mode 100644 index 0000000000000..b4b6be8a1e5f0 --- /dev/null +++ b/src/test/ui/consts/control-flow/drop-fail.precise.stderr @@ -0,0 +1,15 @@ +error[E0493]: destructors cannot be evaluated at compile-time + --> $DIR/drop-fail.rs:10:9 + | +LL | let x = Some(Vec::new()); + | ^ constants cannot evaluate destructors + +error[E0493]: destructors cannot be evaluated at compile-time + --> $DIR/drop-fail.rs:41:9 + | +LL | let mut tmp = None; + | ^^^^^^^ constants cannot evaluate destructors + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0493`. diff --git a/src/test/ui/consts/control-flow/drop-failure.rs b/src/test/ui/consts/control-flow/drop-fail.rs similarity index 68% rename from src/test/ui/consts/control-flow/drop-failure.rs rename to src/test/ui/consts/control-flow/drop-fail.rs index 9da5546976c75..7bd36726cead5 100644 --- a/src/test/ui/consts/control-flow/drop-failure.rs +++ b/src/test/ui/consts/control-flow/drop-fail.rs @@ -1,11 +1,14 @@ +// revisions: stock precise + #![feature(const_if_match)] #![feature(const_loop)] +#![cfg_attr(precise, feature(const_precise_live_drops))] -// `x` is *not* always moved into the final value may be dropped inside the initializer. +// `x` is *not* always moved into the final value and may be dropped inside the initializer. const _: Option> = { let y: Option> = None; let x = Some(Vec::new()); - //~^ ERROR destructors cannot be evaluated at compile-time + //[stock,precise]~^ ERROR destructors cannot be evaluated at compile-time if true { x @@ -18,7 +21,7 @@ const _: Option> = { // existing analysis. const _: Vec = { let vec_tuple = (Vec::new(),); - //~^ ERROR destructors cannot be evaluated at compile-time + //[stock]~^ ERROR destructors cannot be evaluated at compile-time vec_tuple.0 }; @@ -26,7 +29,7 @@ const _: Vec = { // This applies to single-field enum variants as well. const _: Vec = { let x: Result<_, Vec> = Ok(Vec::new()); - //~^ ERROR destructors cannot be evaluated at compile-time + //[stock]~^ ERROR destructors cannot be evaluated at compile-time match x { Ok(x) | Err(x) => x, @@ -36,7 +39,7 @@ const _: Vec = { const _: Option> = { let mut some = Some(Vec::new()); let mut tmp = None; - //~^ ERROR destructors cannot be evaluated at compile-time + //[stock,precise]~^ ERROR destructors cannot be evaluated at compile-time let mut i = 0; while i < 10 { diff --git a/src/test/ui/consts/control-flow/drop-failure.stderr b/src/test/ui/consts/control-flow/drop-fail.stock.stderr similarity index 85% rename from src/test/ui/consts/control-flow/drop-failure.stderr rename to src/test/ui/consts/control-flow/drop-fail.stock.stderr index 3eec3a929a07f..77cded5c438b5 100644 --- a/src/test/ui/consts/control-flow/drop-failure.stderr +++ b/src/test/ui/consts/control-flow/drop-fail.stock.stderr @@ -1,23 +1,23 @@ error[E0493]: destructors cannot be evaluated at compile-time - --> $DIR/drop-failure.rs:7:9 + --> $DIR/drop-fail.rs:10:9 | LL | let x = Some(Vec::new()); | ^ constants cannot evaluate destructors error[E0493]: destructors cannot be evaluated at compile-time - --> $DIR/drop-failure.rs:20:9 + --> $DIR/drop-fail.rs:23:9 | LL | let vec_tuple = (Vec::new(),); | ^^^^^^^^^ constants cannot evaluate destructors error[E0493]: destructors cannot be evaluated at compile-time - --> $DIR/drop-failure.rs:28:9 + --> $DIR/drop-fail.rs:31:9 | LL | let x: Result<_, Vec> = Ok(Vec::new()); | ^ constants cannot evaluate destructors error[E0493]: destructors cannot be evaluated at compile-time - --> $DIR/drop-failure.rs:38:9 + --> $DIR/drop-fail.rs:41:9 | LL | let mut tmp = None; | ^^^^^^^ constants cannot evaluate destructors diff --git a/src/test/ui/consts/control-flow/drop-success.rs b/src/test/ui/consts/control-flow/drop-pass.rs similarity index 89% rename from src/test/ui/consts/control-flow/drop-success.rs rename to src/test/ui/consts/control-flow/drop-pass.rs index 185d6b639962b..b0afd76c4e6ef 100644 --- a/src/test/ui/consts/control-flow/drop-success.rs +++ b/src/test/ui/consts/control-flow/drop-pass.rs @@ -1,7 +1,9 @@ // run-pass +// revisions: stock precise #![feature(const_if_match)] #![feature(const_loop)] +#![cfg_attr(precise, feature(const_precise_live_drops))] // `x` is always moved into the final value and is not dropped inside the initializer. const _: Option> = { diff --git a/src/test/ui/consts/control-flow/drop-precise.rs b/src/test/ui/consts/control-flow/drop-precise.rs new file mode 100644 index 0000000000000..95df76d990554 --- /dev/null +++ b/src/test/ui/consts/control-flow/drop-precise.rs @@ -0,0 +1,20 @@ +// run-pass +// gate-test-const_precise_live_drops + +#![feature(const_if_match)] +#![feature(const_loop)] +#![feature(const_precise_live_drops)] + +const _: Vec = { + let vec_tuple = (Vec::new(),); + vec_tuple.0 +}; + +const _: Vec = { + let x: Result<_, Vec> = Ok(Vec::new()); + match x { + Ok(x) | Err(x) => x, + } +}; + +fn main() {}