From fa9bfebfc9256369c03cbe8bba2e737de3cb38fc Mon Sep 17 00:00:00 2001 From: Stein Somers Date: Tue, 4 Feb 2020 22:19:05 +0100 Subject: [PATCH 01/11] Fix and test implementation of BTreeMap's first_entry, last_entry, pop_first, pop_last --- src/liballoc/collections/btree/map.rs | 24 ++++++++++++++---------- src/liballoc/tests/btree/map.rs | 5 +++++ src/liballoc/tests/btree/set.rs | 27 ++++++++++++++++----------- 3 files changed, 35 insertions(+), 21 deletions(-) diff --git a/src/liballoc/collections/btree/map.rs b/src/liballoc/collections/btree/map.rs index 8eabc1773042f..c1778f2065d40 100644 --- a/src/liballoc/collections/btree/map.rs +++ b/src/liballoc/collections/btree/map.rs @@ -675,13 +675,15 @@ impl BTreeMap { T: Ord, K: Borrow, { - match self.length { - 0 => None, - _ => Some(OccupiedEntry { - handle: self.root.as_mut().first_kv(), + let front = self.root.as_mut().first_leaf_edge(); + if let Ok(kv) = front.right_kv() { + Some(OccupiedEntry { + handle: kv.forget_node_type(), length: &mut self.length, _marker: PhantomData, - }), + }) + } else { + None } } @@ -736,13 +738,15 @@ impl BTreeMap { T: Ord, K: Borrow, { - match self.length { - 0 => None, - _ => Some(OccupiedEntry { - handle: self.root.as_mut().last_kv(), + let back = self.root.as_mut().last_leaf_edge(); + if let Ok(kv) = back.left_kv() { + Some(OccupiedEntry { + handle: kv.forget_node_type(), length: &mut self.length, _marker: PhantomData, - }), + }) + } else { + None } } diff --git a/src/liballoc/tests/btree/map.rs b/src/liballoc/tests/btree/map.rs index 0d009507fc7aa..0a26d7bf427ab 100644 --- a/src/liballoc/tests/btree/map.rs +++ b/src/liballoc/tests/btree/map.rs @@ -23,6 +23,11 @@ fn test_basic_large() { assert_eq!(map.len(), i + 1); } + assert_eq!(map.first_key_value(), Some((&0, &0))); + assert_eq!(map.last_key_value(), Some((&(size - 1), &(10 * (size - 1))))); + assert_eq!(map.first_entry().unwrap().key(), &0); + assert_eq!(map.last_entry().unwrap().key(), &(size - 1)); + for i in 0..size { assert_eq!(map.get(&i).unwrap(), &(i * 10)); } diff --git a/src/liballoc/tests/btree/set.rs b/src/liballoc/tests/btree/set.rs index 265ef758cc5bc..1a2b62d026b2e 100644 --- a/src/liballoc/tests/btree/set.rs +++ b/src/liballoc/tests/btree/set.rs @@ -487,21 +487,26 @@ fn test_first_last() { a.insert(2); assert_eq!(a.first(), Some(&1)); assert_eq!(a.last(), Some(&2)); - a.insert(3); + for i in 3..=12 { + a.insert(i); + } assert_eq!(a.first(), Some(&1)); - assert_eq!(a.last(), Some(&3)); - - assert_eq!(a.len(), 3); + assert_eq!(a.last(), Some(&12)); assert_eq!(a.pop_first(), Some(1)); - assert_eq!(a.len(), 2); - assert_eq!(a.pop_last(), Some(3)); - assert_eq!(a.len(), 1); + assert_eq!(a.pop_last(), Some(12)); assert_eq!(a.pop_first(), Some(2)); - assert_eq!(a.len(), 0); - assert_eq!(a.pop_last(), None); - assert_eq!(a.len(), 0); + assert_eq!(a.pop_last(), Some(11)); + assert_eq!(a.pop_first(), Some(3)); + assert_eq!(a.pop_last(), Some(10)); + assert_eq!(a.pop_first(), Some(4)); + assert_eq!(a.pop_first(), Some(5)); + assert_eq!(a.pop_first(), Some(6)); + assert_eq!(a.pop_first(), Some(7)); + assert_eq!(a.pop_first(), Some(8)); + assert_eq!(a.clone().pop_last(), Some(9)); + assert_eq!(a.pop_first(), Some(9)); assert_eq!(a.pop_first(), None); - assert_eq!(a.len(), 0); + assert_eq!(a.pop_last(), None); } fn rand_data(len: usize) -> Vec { From c00d8aa5179ef38f7d83067a8d010bcc26fa0cc3 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Wed, 5 Feb 2020 16:28:13 +0800 Subject: [PATCH 02/11] Reorder declarations of Result::export/unwrap to match Option --- src/libcore/result.rs | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/libcore/result.rs b/src/libcore/result.rs index bc70dbd62eb52..3361ab6c97d80 100644 --- a/src/libcore/result.rs +++ b/src/libcore/result.rs @@ -935,8 +935,8 @@ impl Result { /// /// # Panics /// - /// Panics if the value is an [`Err`], with a panic message provided by the - /// [`Err`]'s value. + /// Panics if the value is an [`Err`], with a panic message including the + /// passed message, and the content of the [`Err`]. /// /// [`Ok`]: enum.Result.html#variant.Ok /// [`Err`]: enum.Result.html#variant.Err @@ -945,22 +945,17 @@ impl Result { /// /// Basic usage: /// - /// ``` - /// let x: Result = Ok(2); - /// assert_eq!(x.unwrap(), 2); - /// ``` - /// /// ```{.should_panic} /// let x: Result = Err("emergency failure"); - /// x.unwrap(); // panics with `emergency failure` + /// x.expect("Testing expect"); // panics with `Testing expect: emergency failure` /// ``` #[inline] #[track_caller] - #[stable(feature = "rust1", since = "1.0.0")] - pub fn unwrap(self) -> T { + #[stable(feature = "result_expect", since = "1.4.0")] + pub fn expect(self, msg: &str) -> T { match self { Ok(t) => t, - Err(e) => unwrap_failed("called `Result::unwrap()` on an `Err` value", &e), + Err(e) => unwrap_failed(msg, &e), } } @@ -968,8 +963,8 @@ impl Result { /// /// # Panics /// - /// Panics if the value is an [`Err`], with a panic message including the - /// passed message, and the content of the [`Err`]. + /// Panics if the value is an [`Err`], with a panic message provided by the + /// [`Err`]'s value. /// /// [`Ok`]: enum.Result.html#variant.Ok /// [`Err`]: enum.Result.html#variant.Err @@ -978,17 +973,22 @@ impl Result { /// /// Basic usage: /// + /// ``` + /// let x: Result = Ok(2); + /// assert_eq!(x.unwrap(), 2); + /// ``` + /// /// ```{.should_panic} /// let x: Result = Err("emergency failure"); - /// x.expect("Testing expect"); // panics with `Testing expect: emergency failure` + /// x.unwrap(); // panics with `emergency failure` /// ``` #[inline] #[track_caller] - #[stable(feature = "result_expect", since = "1.4.0")] - pub fn expect(self, msg: &str) -> T { + #[stable(feature = "rust1", since = "1.0.0")] + pub fn unwrap(self) -> T { match self { Ok(t) => t, - Err(e) => unwrap_failed(msg, &e), + Err(e) => unwrap_failed("called `Result::unwrap()` on an `Err` value", &e), } } } From db9b578b71190540cfd84f16f5d310d6ce4cb659 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Wed, 5 Feb 2020 16:31:12 +0800 Subject: [PATCH 03/11] Reorder declarations of Result::expect_err/unwrap_err to match Option --- src/libcore/result.rs | 44 +++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/libcore/result.rs b/src/libcore/result.rs index 3361ab6c97d80..ee28946f0da7a 100644 --- a/src/libcore/result.rs +++ b/src/libcore/result.rs @@ -998,30 +998,26 @@ impl Result { /// /// # Panics /// - /// Panics if the value is an [`Ok`], with a custom panic message provided - /// by the [`Ok`]'s value. + /// Panics if the value is an [`Ok`], with a panic message including the + /// passed message, and the content of the [`Ok`]. /// /// [`Ok`]: enum.Result.html#variant.Ok /// [`Err`]: enum.Result.html#variant.Err /// - /// /// # Examples /// - /// ```{.should_panic} - /// let x: Result = Ok(2); - /// x.unwrap_err(); // panics with `2` - /// ``` + /// Basic usage: /// - /// ``` - /// let x: Result = Err("emergency failure"); - /// assert_eq!(x.unwrap_err(), "emergency failure"); + /// ```{.should_panic} + /// let x: Result = Ok(10); + /// x.expect_err("Testing expect_err"); // panics with `Testing expect_err: 10` /// ``` #[inline] #[track_caller] - #[stable(feature = "rust1", since = "1.0.0")] - pub fn unwrap_err(self) -> E { + #[stable(feature = "result_expect_err", since = "1.17.0")] + pub fn expect_err(self, msg: &str) -> E { match self { - Ok(t) => unwrap_failed("called `Result::unwrap_err()` on an `Ok` value", &t), + Ok(t) => unwrap_failed(msg, &t), Err(e) => e, } } @@ -1030,26 +1026,30 @@ impl Result { /// /// # Panics /// - /// Panics if the value is an [`Ok`], with a panic message including the - /// passed message, and the content of the [`Ok`]. + /// Panics if the value is an [`Ok`], with a custom panic message provided + /// by the [`Ok`]'s value. /// /// [`Ok`]: enum.Result.html#variant.Ok /// [`Err`]: enum.Result.html#variant.Err /// - /// # Examples /// - /// Basic usage: + /// # Examples /// /// ```{.should_panic} - /// let x: Result = Ok(10); - /// x.expect_err("Testing expect_err"); // panics with `Testing expect_err: 10` + /// let x: Result = Ok(2); + /// x.unwrap_err(); // panics with `2` + /// ``` + /// + /// ``` + /// let x: Result = Err("emergency failure"); + /// assert_eq!(x.unwrap_err(), "emergency failure"); /// ``` #[inline] #[track_caller] - #[stable(feature = "result_expect_err", since = "1.17.0")] - pub fn expect_err(self, msg: &str) -> E { + #[stable(feature = "rust1", since = "1.0.0")] + pub fn unwrap_err(self) -> E { match self { - Ok(t) => unwrap_failed(msg, &t), + Ok(t) => unwrap_failed("called `Result::unwrap_err()` on an `Ok` value", &t), Err(e) => e, } } From 8251e12950159c5802dd3995b14be7cf4fa99acd Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Fri, 7 Feb 2020 13:59:43 +0800 Subject: [PATCH 04/11] Don't use the word 'unwrap' to describe core unwrapping functions It's tautological, and Rust-specific Jargon. This changes various Option/Result methods to consistently describe unwrapping behavior using the words "return", "contain", "consume". It also renames the closure argument of `Return::unwrap_or_else` to `default` to be consistent with `Option`. --- src/libcore/option.rs | 26 ++++++++++++++++---------- src/libcore/result.rs | 38 ++++++++++++++++++++++---------------- 2 files changed, 38 insertions(+), 26 deletions(-) diff --git a/src/libcore/option.rs b/src/libcore/option.rs index ad0491f888cc7..c7b36d8ad6362 100644 --- a/src/libcore/option.rs +++ b/src/libcore/option.rs @@ -317,7 +317,7 @@ impl Option { // Getting to contained values ///////////////////////////////////////////////////////////////////////// - /// Unwraps an option, yielding the content of a [`Some`]. + /// Returns the contained [`Some`] value, consuming the `self` value. /// /// # Panics /// @@ -348,17 +348,22 @@ impl Option { } } - /// Moves the value `v` out of the `Option` if it is [`Some(v)`]. + /// Returns the contained [`Some`] value, consuming the `self` value. /// - /// In general, because this function may panic, its use is discouraged. + /// Because this function may panic, its use is generally discouraged. /// Instead, prefer to use pattern matching and handle the [`None`] - /// case explicitly. + /// case explicitly, or call [`unwrap_or`], [`unwrap_or_else`], or + /// [`unwrap_or_default`]. + /// + /// [`unwrap_or`]: #method.unwrap_or + /// [`unwrap_or_else`]: #method.unwrap_or_else + /// [`unwrap_or_default`]: #method.unwrap_or_default /// /// # Panics /// /// Panics if the self value equals [`None`]. /// - /// [`Some(v)`]: #variant.Some + /// [`Some`]: #variant.Some /// [`None`]: #variant.None /// /// # Examples @@ -382,12 +387,13 @@ impl Option { } } - /// Returns the contained value or a default. + /// Returns the contained [`Some`] value or a provided default. /// /// Arguments passed to `unwrap_or` are eagerly evaluated; if you are passing /// the result of a function call, it is recommended to use [`unwrap_or_else`], /// which is lazily evaluated. /// + /// [`Some`]: #variant.Some /// [`unwrap_or_else`]: #method.unwrap_or_else /// /// # Examples @@ -405,7 +411,7 @@ impl Option { } } - /// Returns the contained value or computes it from a closure. + /// Returns the contained [`Some`] value or computes it from a closure. /// /// # Examples /// @@ -980,7 +986,7 @@ impl Option<&mut T> { } impl Option { - /// Unwraps an option, expecting [`None`] and returning nothing. + /// Consumes `self` while expecting [`None`] and returning nothing. /// /// # Panics /// @@ -1023,7 +1029,7 @@ impl Option { } } - /// Unwraps an option, expecting [`None`] and returning nothing. + /// Consumes `self` while expecting [`None`] and returning nothing. /// /// # Panics /// @@ -1068,7 +1074,7 @@ impl Option { } impl Option { - /// Returns the contained value or a default + /// Returns the contained [`Some`] value or a default /// /// Consumes the `self` argument then, if [`Some`], returns the contained /// value, otherwise if [`None`], returns the [default value] for that diff --git a/src/libcore/result.rs b/src/libcore/result.rs index ee28946f0da7a..022701c949815 100644 --- a/src/libcore/result.rs +++ b/src/libcore/result.rs @@ -792,8 +792,7 @@ impl Result { } } - /// Unwraps a result, yielding the content of an [`Ok`]. - /// Else, it returns `optb`. + /// Returns the contained [`Ok`] value or a provided default. /// /// Arguments passed to `unwrap_or` are eagerly evaluated; if you are passing /// the result of a function call, it is recommended to use [`unwrap_or_else`], @@ -808,27 +807,25 @@ impl Result { /// Basic usage: /// /// ``` - /// let optb = 2; + /// let default = 2; /// let x: Result = Ok(9); - /// assert_eq!(x.unwrap_or(optb), 9); + /// assert_eq!(x.unwrap_or(default), 9); /// /// let x: Result = Err("error"); - /// assert_eq!(x.unwrap_or(optb), optb); + /// assert_eq!(x.unwrap_or(default), default); /// ``` #[inline] #[stable(feature = "rust1", since = "1.0.0")] - pub fn unwrap_or(self, optb: T) -> T { + pub fn unwrap_or(self, default: T) -> T { match self { Ok(t) => t, - Err(_) => optb, + Err(_) => default, } } - /// Unwraps a result, yielding the content of an [`Ok`]. - /// If the value is an [`Err`] then it calls `op` with its value. + /// Returns the contained [`Ok`] value or computes it from a closure. /// /// [`Ok`]: enum.Result.html#variant.Ok - /// [`Err`]: enum.Result.html#variant.Err /// /// # Examples /// @@ -931,7 +928,7 @@ impl Result<&mut T, E> { } impl Result { - /// Unwraps a result, yielding the content of an [`Ok`]. + /// Returns the contained [`Ok`] value, consuming the `self` value. /// /// # Panics /// @@ -959,7 +956,16 @@ impl Result { } } - /// Unwraps a result, yielding the content of an [`Ok`]. + /// Returns the contained [`Ok`] value, consuming the `self` value. + /// + /// Because this function may panic, its use is generally discouraged. + /// Instead, prefer to use pattern matching and handle the [`Err`] + /// case explicitly, or call [`unwrap_or`], [`unwrap_or_else`], or + /// [`unwrap_or_default`]. + /// + /// [`unwrap_or`]: #method.unwrap_or + /// [`unwrap_or_else`]: #method.unwrap_or_else + /// [`unwrap_or_default`]: #method.unwrap_or_default /// /// # Panics /// @@ -994,7 +1000,7 @@ impl Result { } impl Result { - /// Unwraps a result, yielding the content of an [`Err`]. + /// Returns the contained [`Err`] value, consuming the `self` value. /// /// # Panics /// @@ -1022,7 +1028,7 @@ impl Result { } } - /// Unwraps a result, yielding the content of an [`Err`]. + /// Returns the contained [`Err`] value, consuming the `self` value. /// /// # Panics /// @@ -1056,7 +1062,7 @@ impl Result { } impl Result { - /// Returns the contained value or a default + /// Returns the contained [`Ok`] value or a default /// /// Consumes the `self` argument then, if [`Ok`], returns the contained /// value, otherwise if [`Err`], returns the default value for that @@ -1095,7 +1101,7 @@ impl Result { #[unstable(feature = "unwrap_infallible", reason = "newly added", issue = "61695")] impl> Result { - /// Unwraps a result that can never be an [`Err`], yielding the content of the [`Ok`]. + /// Returns the contained [`Ok`] value, but never panics. /// /// Unlike [`unwrap`], this method is known to never panic on the /// result types it is implemented for. Therefore, it can be used From 1177d0669ac213f12aee6c998b47ca1e8904ccf0 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 8 Feb 2020 15:47:29 +0100 Subject: [PATCH 05/11] Clean up E0277 and E0282 explanations --- src/librustc_error_codes/error_codes/E0277.md | 4 +++- src/librustc_error_codes/error_codes/E0282.md | 18 +++++++++++------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/librustc_error_codes/error_codes/E0277.md b/src/librustc_error_codes/error_codes/E0277.md index 2034a5b988cbf..2e2cd5e01fb6a 100644 --- a/src/librustc_error_codes/error_codes/E0277.md +++ b/src/librustc_error_codes/error_codes/E0277.md @@ -1,5 +1,7 @@ You tried to use a type which doesn't implement some trait in a place which -expected that trait. Erroneous code example: +expected that trait. + +Erroneous code example: ```compile_fail,E0277 // here we declare the Foo trait with a bar method diff --git a/src/librustc_error_codes/error_codes/E0282.md b/src/librustc_error_codes/error_codes/E0282.md index 54a9de0025a3d..49d2205f92c2a 100644 --- a/src/librustc_error_codes/error_codes/E0282.md +++ b/src/librustc_error_codes/error_codes/E0282.md @@ -1,3 +1,11 @@ +The compiler could not infer a type and asked for a type annotation. + +Erroneous code example: + +```compile_fail,E0282 +let x = "hello".chars().rev().collect(); +``` + This error indicates that type inference did not result in one unique possible type, and extra information is required. In most cases this can be provided by adding a type annotation. Sometimes you need to specify a generic type @@ -8,13 +16,9 @@ parameter with a `FromIterator` bound, which for a `char` iterator is implemented by `Vec` and `String` among others. Consider the following snippet that reverses the characters of a string: -```compile_fail,E0282 -let x = "hello".chars().rev().collect(); -``` - -In this case, the compiler cannot infer what the type of `x` should be: -`Vec` and `String` are both suitable candidates. To specify which type to -use, you can use a type annotation on `x`: +In the first code example, the compiler cannot infer what the type of `x` should +be: `Vec` and `String` are both suitable candidates. To specify which type +to use, you can use a type annotation on `x`: ``` let x: Vec = "hello".chars().rev().collect(); From d6ccbf6ff80ce66884ab70fdb679353245f05e5b Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Sat, 8 Feb 2020 17:58:00 +0200 Subject: [PATCH 06/11] rustc_codegen_llvm: remove unnecessary special-casing of root scopes' children. --- .../debuginfo/create_scope_map.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/librustc_codegen_llvm/debuginfo/create_scope_map.rs b/src/librustc_codegen_llvm/debuginfo/create_scope_map.rs index eba05ed5d7787..cdb9657e1ff3c 100644 --- a/src/librustc_codegen_llvm/debuginfo/create_scope_map.rs +++ b/src/librustc_codegen_llvm/debuginfo/create_scope_map.rs @@ -66,14 +66,8 @@ fn make_mir_scope( if !has_variables.contains(scope) { // Do not create a DIScope if there are no variables // defined in this MIR Scope, to avoid debuginfo bloat. - - // However, we don't skip creating a nested scope if - // our parent is the root, because we might want to - // put arguments in the root and not have shadowing. - if parent_scope.scope_metadata.unwrap() != fn_metadata { - debug_context.scopes[scope] = parent_scope; - return; - } + debug_context.scopes[scope] = parent_scope; + return; } let loc = span_start(cx, scope_data.span); From 1385fc4c40cb96eca5f3df2c3043425889073646 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Sat, 8 Feb 2020 18:07:44 +0200 Subject: [PATCH 07/11] rustc_codegen_llvm: remove InternalDebugLocation and simplify dbg_var_addr. --- src/librustc_codegen_llvm/debuginfo/mod.rs | 25 ++----- .../debuginfo/source_loc.rs | 74 +++++++------------ src/librustc_codegen_llvm/llvm/ffi.rs | 2 - src/librustc_codegen_ssa/mir/debuginfo.rs | 6 -- src/librustc_codegen_ssa/traits/debuginfo.rs | 1 - 5 files changed, 35 insertions(+), 73 deletions(-) diff --git a/src/librustc_codegen_llvm/debuginfo/mod.rs b/src/librustc_codegen_llvm/debuginfo/mod.rs index c4a52a73e25d9..46f186191e3ab 100644 --- a/src/librustc_codegen_llvm/debuginfo/mod.rs +++ b/src/librustc_codegen_llvm/debuginfo/mod.rs @@ -5,7 +5,6 @@ use rustc_codegen_ssa::mir::debuginfo::VariableKind::*; use self::metadata::{file_metadata, type_metadata, TypeMap}; use self::namespace::mangled_name_of_instance; -use self::source_loc::InternalDebugLocation::{self, UnknownLocation}; use self::type_names::compute_debuginfo_type_name; use self::utils::{create_DIArray, is_node_local_to_unit, span_start, DIB}; @@ -38,7 +37,7 @@ use std::ffi::CString; use rustc::ty::layout::{self, HasTyCtxt, LayoutOf, Size}; use rustc_codegen_ssa::traits::*; use rustc_span::symbol::Symbol; -use rustc_span::{self, BytePos, Pos, Span}; +use rustc_span::{self, BytePos, Span}; use smallvec::SmallVec; use syntax::ast; @@ -148,7 +147,6 @@ impl DebugInfoBuilderMethods for Builder<'a, 'll, 'tcx> { // names (choose between `dbg`, `debug`, `debuginfo`, `debug_info` etc.). fn dbg_var_addr( &mut self, - dbg_context: &FunctionDebugContext<&'ll DIScope>, dbg_var: &'ll DIVariable, scope_metadata: &'ll DIScope, variable_alloca: Self::Value, @@ -156,12 +154,11 @@ impl DebugInfoBuilderMethods for Builder<'a, 'll, 'tcx> { indirect_offsets: &[Size], span: Span, ) { - assert!(!dbg_context.source_locations_enabled); let cx = self.cx(); - let loc = span_start(cx, span); - // Convert the direct and indirect offsets to address ops. + // FIXME(eddyb) use `const`s instead of getting the values via FFI, + // the values should match the ones in the DWARF standard anyway. let op_deref = || unsafe { llvm::LLVMRustDIBuilderCreateOpDeref() }; let op_plus_uconst = || unsafe { llvm::LLVMRustDIBuilderCreateOpPlusUconst() }; let mut addr_ops = SmallVec::<[_; 8]>::new(); @@ -178,28 +175,22 @@ impl DebugInfoBuilderMethods for Builder<'a, 'll, 'tcx> { } } - // FIXME(eddyb) maybe this information could be extracted from `var`, + // FIXME(eddyb) maybe this information could be extracted from `dbg_var`, // to avoid having to pass it down in both places? - source_loc::set_debug_location( - self, - InternalDebugLocation::new(scope_metadata, loc.line, loc.col.to_usize()), - ); + // NB: `var` doesn't seem to know about the column, so that's a limitation. + let dbg_loc = cx.create_debug_loc(scope_metadata, span); unsafe { - let debug_loc = llvm::LLVMGetCurrentDebugLocation(self.llbuilder); // FIXME(eddyb) replace `llvm.dbg.declare` with `llvm.dbg.addr`. - let instr = llvm::LLVMRustDIBuilderInsertDeclareAtEnd( + llvm::LLVMRustDIBuilderInsertDeclareAtEnd( DIB(cx), variable_alloca, dbg_var, addr_ops.as_ptr(), addr_ops.len() as c_uint, - debug_loc, + dbg_loc, self.llbb(), ); - - llvm::LLVMSetInstDebugLocation(self.llbuilder, instr); } - source_loc::set_debug_location(self, UnknownLocation); } fn set_source_location( diff --git a/src/librustc_codegen_llvm/debuginfo/source_loc.rs b/src/librustc_codegen_llvm/debuginfo/source_loc.rs index 6afaca44e0e96..78dc1ac250e6e 100644 --- a/src/librustc_codegen_llvm/debuginfo/source_loc.rs +++ b/src/librustc_codegen_llvm/debuginfo/source_loc.rs @@ -1,12 +1,11 @@ -use self::InternalDebugLocation::*; - use super::metadata::UNKNOWN_COLUMN_NUMBER; use super::utils::{debug_context, span_start}; use rustc_codegen_ssa::mir::debuginfo::FunctionDebugContext; use crate::builder::Builder; -use crate::llvm; +use crate::common::CodegenCx; use crate::llvm::debuginfo::DIScope; +use crate::llvm::{self, Value}; use log::debug; use rustc_codegen_ssa::traits::*; @@ -24,56 +23,37 @@ pub fn set_source_location( ) { let dbg_loc = if debug_context.source_locations_enabled { debug!("set_source_location: {}", bx.sess().source_map().span_to_string(span)); - let loc = span_start(bx.cx(), span); - InternalDebugLocation::new(scope, loc.line, loc.col.to_usize()) + Some(bx.cx().create_debug_loc(scope, span)) } else { - UnknownLocation + None }; - set_debug_location(bx, dbg_loc); -} -#[derive(Copy, Clone, PartialEq)] -pub enum InternalDebugLocation<'ll> { - KnownLocation { scope: &'ll DIScope, line: usize, col: usize }, - UnknownLocation, -} - -impl InternalDebugLocation<'ll> { - pub fn new(scope: &'ll DIScope, line: usize, col: usize) -> Self { - KnownLocation { scope, line, col } + unsafe { + llvm::LLVMSetCurrentDebugLocation(bx.llbuilder, dbg_loc); } } -pub fn set_debug_location(bx: &Builder<'_, 'll, '_>, debug_location: InternalDebugLocation<'ll>) { - let metadata_node = match debug_location { - KnownLocation { scope, line, col } => { - // For MSVC, set the column number to zero. - // Otherwise, emit it. This mimics clang behaviour. - // See discussion in https://github.com/rust-lang/rust/issues/42921 - let col_used = if bx.sess().target.target.options.is_like_msvc { - UNKNOWN_COLUMN_NUMBER - } else { - col as c_uint - }; - debug!("setting debug location to {} {}", line, col); - - unsafe { - Some(llvm::LLVMRustDIBuilderCreateDebugLocation( - debug_context(bx.cx()).llcontext, - line as c_uint, - col_used, - scope, - None, - )) - } - } - UnknownLocation => { - debug!("clearing debug location "); - None +impl CodegenCx<'ll, '_> { + pub fn create_debug_loc(&self, scope: &'ll DIScope, span: Span) -> &'ll Value { + let loc = span_start(self, span); + + // For MSVC, set the column number to zero. + // Otherwise, emit it. This mimics clang behaviour. + // See discussion in https://github.com/rust-lang/rust/issues/42921 + let col_used = if self.sess().target.target.options.is_like_msvc { + UNKNOWN_COLUMN_NUMBER + } else { + loc.col.to_usize() as c_uint + }; + + unsafe { + llvm::LLVMRustDIBuilderCreateDebugLocation( + debug_context(self).llcontext, + loc.line as c_uint, + col_used, + scope, + None, + ) } - }; - - unsafe { - llvm::LLVMSetCurrentDebugLocation(bx.llbuilder, metadata_node); } } diff --git a/src/librustc_codegen_llvm/llvm/ffi.rs b/src/librustc_codegen_llvm/llvm/ffi.rs index 3f37f86676c98..87a6449a8ea7a 100644 --- a/src/librustc_codegen_llvm/llvm/ffi.rs +++ b/src/librustc_codegen_llvm/llvm/ffi.rs @@ -910,8 +910,6 @@ extern "C" { // Metadata pub fn LLVMSetCurrentDebugLocation(Builder: &Builder<'a>, L: Option<&'a Value>); - pub fn LLVMGetCurrentDebugLocation(Builder: &Builder<'a>) -> &'a Value; - pub fn LLVMSetInstDebugLocation(Builder: &Builder<'a>, Inst: &'a Value); // Terminators pub fn LLVMBuildRetVoid(B: &Builder<'a>) -> &'a Value; diff --git a/src/librustc_codegen_ssa/mir/debuginfo.rs b/src/librustc_codegen_ssa/mir/debuginfo.rs index f66496d10fb16..362bb18abb829 100644 --- a/src/librustc_codegen_ssa/mir/debuginfo.rs +++ b/src/librustc_codegen_ssa/mir/debuginfo.rs @@ -210,11 +210,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { return; } - let debug_context = match &self.debug_context { - Some(debug_context) => debug_context, - None => return, - }; - // FIXME(eddyb) add debuginfo for unsized places too. let base = match local_ref { LocalRef::Place(place) => place, @@ -264,7 +259,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { if let Some(scope) = scope { if let Some(dbg_var) = var.dbg_var { bx.dbg_var_addr( - debug_context, dbg_var, scope, base.llval, diff --git a/src/librustc_codegen_ssa/traits/debuginfo.rs b/src/librustc_codegen_ssa/traits/debuginfo.rs index 22a4e96b9e4bf..f1ba6d4daa809 100644 --- a/src/librustc_codegen_ssa/traits/debuginfo.rs +++ b/src/librustc_codegen_ssa/traits/debuginfo.rs @@ -49,7 +49,6 @@ pub trait DebugInfoBuilderMethods: BackendTypes { // names (choose between `dbg`, `debug`, `debuginfo`, `debug_info` etc.). fn dbg_var_addr( &mut self, - dbg_context: &FunctionDebugContext, dbg_var: Self::DIVariable, scope_metadata: Self::DIScope, variable_alloca: Self::Value, From bdb72e7b5ac304c918710ec5c968eaece7e6b57c Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Sat, 8 Feb 2020 18:27:49 +0200 Subject: [PATCH 08/11] rustc_codegen_ssa: remove unnecessary source_locations_enabled. --- src/librustc_codegen_llvm/debuginfo/mod.rs | 17 +++++++------ .../debuginfo/source_loc.rs | 24 ------------------- src/librustc_codegen_llvm/llvm/ffi.rs | 2 +- src/librustc_codegen_ssa/mir/debuginfo.rs | 8 +++---- src/librustc_codegen_ssa/mir/mod.rs | 7 ------ src/librustc_codegen_ssa/traits/debuginfo.rs | 7 +----- 6 files changed, 13 insertions(+), 52 deletions(-) diff --git a/src/librustc_codegen_llvm/debuginfo/mod.rs b/src/librustc_codegen_llvm/debuginfo/mod.rs index 46f186191e3ab..c1a7bb5087824 100644 --- a/src/librustc_codegen_llvm/debuginfo/mod.rs +++ b/src/librustc_codegen_llvm/debuginfo/mod.rs @@ -51,7 +51,6 @@ mod utils; pub use self::create_scope_map::compute_mir_scopes; pub use self::metadata::create_global_var_metadata; pub use self::metadata::extend_scope_to_file; -pub use self::source_loc::set_source_location; #[allow(non_upper_case_globals)] const DW_TAG_auto_variable: c_uint = 0x100; @@ -193,13 +192,14 @@ impl DebugInfoBuilderMethods for Builder<'a, 'll, 'tcx> { } } - fn set_source_location( - &mut self, - debug_context: &mut FunctionDebugContext<&'ll DIScope>, - scope: &'ll DIScope, - span: Span, - ) { - set_source_location(debug_context, &self, scope, span) + fn set_source_location(&mut self, scope: &'ll DIScope, span: Span) { + debug!("set_source_location: {}", self.sess().source_map().span_to_string(span)); + + let dbg_loc = self.cx().create_debug_loc(scope, span); + + unsafe { + llvm::LLVMSetCurrentDebugLocation(self.llbuilder, dbg_loc); + } } fn insert_reference_to_gdb_debug_scripts_section_global(&mut self) { gdb::insert_reference_to_gdb_debug_scripts_section_global(self) @@ -333,7 +333,6 @@ impl DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> { }; let mut fn_debug_context = FunctionDebugContext { scopes: IndexVec::from_elem(null_scope, &mir.source_scopes), - source_locations_enabled: false, defining_crate: def_id.krate, }; diff --git a/src/librustc_codegen_llvm/debuginfo/source_loc.rs b/src/librustc_codegen_llvm/debuginfo/source_loc.rs index 78dc1ac250e6e..1f871c7d207a4 100644 --- a/src/librustc_codegen_llvm/debuginfo/source_loc.rs +++ b/src/librustc_codegen_llvm/debuginfo/source_loc.rs @@ -1,38 +1,14 @@ use super::metadata::UNKNOWN_COLUMN_NUMBER; use super::utils::{debug_context, span_start}; -use rustc_codegen_ssa::mir::debuginfo::FunctionDebugContext; -use crate::builder::Builder; use crate::common::CodegenCx; use crate::llvm::debuginfo::DIScope; use crate::llvm::{self, Value}; -use log::debug; use rustc_codegen_ssa::traits::*; use libc::c_uint; use rustc_span::{Pos, Span}; -/// Sets the current debug location at the beginning of the span. -/// -/// Maps to a call to llvm::LLVMSetCurrentDebugLocation(...). -pub fn set_source_location( - debug_context: &FunctionDebugContext, - bx: &Builder<'_, 'll, '_>, - scope: &'ll DIScope, - span: Span, -) { - let dbg_loc = if debug_context.source_locations_enabled { - debug!("set_source_location: {}", bx.sess().source_map().span_to_string(span)); - Some(bx.cx().create_debug_loc(scope, span)) - } else { - None - }; - - unsafe { - llvm::LLVMSetCurrentDebugLocation(bx.llbuilder, dbg_loc); - } -} - impl CodegenCx<'ll, '_> { pub fn create_debug_loc(&self, scope: &'ll DIScope, span: Span) -> &'ll Value { let loc = span_start(self, span); diff --git a/src/librustc_codegen_llvm/llvm/ffi.rs b/src/librustc_codegen_llvm/llvm/ffi.rs index 87a6449a8ea7a..146b7d3d76c5e 100644 --- a/src/librustc_codegen_llvm/llvm/ffi.rs +++ b/src/librustc_codegen_llvm/llvm/ffi.rs @@ -909,7 +909,7 @@ extern "C" { pub fn LLVMDisposeBuilder(Builder: &'a mut Builder<'a>); // Metadata - pub fn LLVMSetCurrentDebugLocation(Builder: &Builder<'a>, L: Option<&'a Value>); + pub fn LLVMSetCurrentDebugLocation(Builder: &Builder<'a>, L: &'a Value); // Terminators pub fn LLVMBuildRetVoid(B: &Builder<'a>) -> &'a Value; diff --git a/src/librustc_codegen_ssa/mir/debuginfo.rs b/src/librustc_codegen_ssa/mir/debuginfo.rs index 362bb18abb829..976a656a29b19 100644 --- a/src/librustc_codegen_ssa/mir/debuginfo.rs +++ b/src/librustc_codegen_ssa/mir/debuginfo.rs @@ -14,7 +14,6 @@ use super::{FunctionCx, LocalRef}; pub struct FunctionDebugContext { pub scopes: IndexVec>, - pub source_locations_enabled: bool, pub defining_crate: CrateNum, } @@ -53,11 +52,10 @@ impl DebugScope { } impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { - pub fn set_debug_loc(&mut self, bx: &mut Bx, source_info: mir::SourceInfo) { + pub fn set_debug_loc(&self, bx: &mut Bx, source_info: mir::SourceInfo) { let (scope, span) = self.debug_loc(source_info); - if let Some(debug_context) = &mut self.debug_context { - // FIXME(eddyb) get rid of this unwrap somehow. - bx.set_source_location(debug_context, scope.unwrap(), span); + if let Some(scope) = scope { + bx.set_source_location(scope, span); } } diff --git a/src/librustc_codegen_ssa/mir/mod.rs b/src/librustc_codegen_ssa/mir/mod.rs index 8a6284479c722..64ead19b35869 100644 --- a/src/librustc_codegen_ssa/mir/mod.rs +++ b/src/librustc_codegen_ssa/mir/mod.rs @@ -230,13 +230,6 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( bx.br(fx.blocks[mir::START_BLOCK]); } - // Up until here, IR instructions for this function have explicitly not been annotated with - // source code location, so we don't step into call setup code. From here on, source location - // emitting should be enabled. - if let Some(debug_context) = &mut fx.debug_context { - debug_context.source_locations_enabled = true; - } - let rpo = traversal::reverse_postorder(&mir_body); let mut visited = BitSet::new_empty(mir_body.basic_blocks().len()); diff --git a/src/librustc_codegen_ssa/traits/debuginfo.rs b/src/librustc_codegen_ssa/traits/debuginfo.rs index f1ba6d4daa809..3688ae51b3918 100644 --- a/src/librustc_codegen_ssa/traits/debuginfo.rs +++ b/src/librustc_codegen_ssa/traits/debuginfo.rs @@ -57,12 +57,7 @@ pub trait DebugInfoBuilderMethods: BackendTypes { indirect_offsets: &[Size], span: Span, ); - fn set_source_location( - &mut self, - debug_context: &mut FunctionDebugContext, - scope: Self::DIScope, - span: Span, - ); + fn set_source_location(&mut self, scope: Self::DIScope, span: Span); fn insert_reference_to_gdb_debug_scripts_section_global(&mut self); fn set_var_name(&mut self, value: Self::Value, name: &str); } From 4ac468c038e2d24efd2228c201fd0b3211ca8ba5 Mon Sep 17 00:00:00 2001 From: Michael Bradshaw Date: Fri, 7 Feb 2020 22:13:36 -0800 Subject: [PATCH 09/11] Mark several functions and methods in core::cmp as #[must_use] --- src/libcore/cmp.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/libcore/cmp.rs b/src/libcore/cmp.rs index e41a7afd3e223..604be7d5f68d0 100644 --- a/src/libcore/cmp.rs +++ b/src/libcore/cmp.rs @@ -361,6 +361,7 @@ impl Ordering { /// assert!(data == b); /// ``` #[inline] + #[must_use] #[stable(feature = "rust1", since = "1.0.0")] pub fn reverse(self) -> Ordering { match self { @@ -398,6 +399,7 @@ impl Ordering { /// assert_eq!(result, Ordering::Less); /// ``` #[inline] + #[must_use] #[stable(feature = "ordering_chaining", since = "1.17.0")] pub fn then(self, other: Ordering) -> Ordering { match self { @@ -435,6 +437,7 @@ impl Ordering { /// assert_eq!(result, Ordering::Less); /// ``` #[inline] + #[must_use] #[stable(feature = "ordering_chaining", since = "1.17.0")] pub fn then_with Ordering>(self, f: F) -> Ordering { match self { @@ -576,6 +579,7 @@ pub trait Ord: Eq + PartialOrd { /// assert_eq!(10.cmp(&5), Ordering::Greater); /// assert_eq!(5.cmp(&5), Ordering::Equal); /// ``` + #[must_use] #[stable(feature = "rust1", since = "1.0.0")] fn cmp(&self, other: &Self) -> Ordering; @@ -591,6 +595,7 @@ pub trait Ord: Eq + PartialOrd { /// ``` #[stable(feature = "ord_max_min", since = "1.21.0")] #[inline] + #[must_use] fn max(self, other: Self) -> Self where Self: Sized, @@ -610,6 +615,7 @@ pub trait Ord: Eq + PartialOrd { /// ``` #[stable(feature = "ord_max_min", since = "1.21.0")] #[inline] + #[must_use] fn min(self, other: Self) -> Self where Self: Sized, @@ -635,6 +641,7 @@ pub trait Ord: Eq + PartialOrd { /// assert!(0.clamp(-2, 1) == 0); /// assert!(2.clamp(-2, 1) == 1); /// ``` + #[must_use] #[unstable(feature = "clamp", issue = "44095")] fn clamp(self, min: Self, max: Self) -> Self where @@ -915,6 +922,7 @@ pub macro PartialOrd($item:item) { /// assert_eq!(2, cmp::min(2, 2)); /// ``` #[inline] +#[must_use] #[stable(feature = "rust1", since = "1.0.0")] pub fn min(v1: T, v2: T) -> T { v1.min(v2) @@ -935,6 +943,7 @@ pub fn min(v1: T, v2: T) -> T { /// assert_eq!(cmp::min_by(-2, 2, |x: &i32, y: &i32| x.abs().cmp(&y.abs())), -2); /// ``` #[inline] +#[must_use] #[unstable(feature = "cmp_min_max_by", issue = "64460")] pub fn min_by Ordering>(v1: T, v2: T, compare: F) -> T { match compare(&v1, &v2) { @@ -958,6 +967,7 @@ pub fn min_by Ordering>(v1: T, v2: T, compare: F) -> T { /// assert_eq!(cmp::min_by_key(-2, 2, |x: &i32| x.abs()), -2); /// ``` #[inline] +#[must_use] #[unstable(feature = "cmp_min_max_by", issue = "64460")] pub fn min_by_key K, K: Ord>(v1: T, v2: T, mut f: F) -> T { min_by(v1, v2, |v1, v2| f(v1).cmp(&f(v2))) @@ -978,6 +988,7 @@ pub fn min_by_key K, K: Ord>(v1: T, v2: T, mut f: F) -> T { /// assert_eq!(2, cmp::max(2, 2)); /// ``` #[inline] +#[must_use] #[stable(feature = "rust1", since = "1.0.0")] pub fn max(v1: T, v2: T) -> T { v1.max(v2) @@ -998,6 +1009,7 @@ pub fn max(v1: T, v2: T) -> T { /// assert_eq!(cmp::max_by(-2, 2, |x: &i32, y: &i32| x.abs().cmp(&y.abs())), 2); /// ``` #[inline] +#[must_use] #[unstable(feature = "cmp_min_max_by", issue = "64460")] pub fn max_by Ordering>(v1: T, v2: T, compare: F) -> T { match compare(&v1, &v2) { @@ -1021,6 +1033,7 @@ pub fn max_by Ordering>(v1: T, v2: T, compare: F) -> T { /// assert_eq!(cmp::max_by_key(-2, 2, |x: &i32| x.abs()), 2); /// ``` #[inline] +#[must_use] #[unstable(feature = "cmp_min_max_by", issue = "64460")] pub fn max_by_key K, K: Ord>(v1: T, v2: T, mut f: F) -> T { max_by(v1, v2, |v1, v2| f(v1).cmp(&f(v2))) From 51b891ae2cf55ce0a265fa99e92bdb7b19244112 Mon Sep 17 00:00:00 2001 From: Markus Westerlind Date: Wed, 8 Jan 2020 22:06:25 +0100 Subject: [PATCH 10/11] Reduce Vec allocations in normalization by passing &mut Vec --- src/librustc/traits/mod.rs | 4 +- src/librustc/traits/project.rs | 81 +++++++++++++++++++++++++--------- src/librustc/traits/select.rs | 56 +++++++++++------------ src/librustc/traits/wf.rs | 20 ++++----- 4 files changed, 100 insertions(+), 61 deletions(-) diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index e88f4e65c7eb1..cec9eac6850d2 100644 --- a/src/librustc/traits/mod.rs +++ b/src/librustc/traits/mod.rs @@ -50,7 +50,9 @@ pub use self::object_safety::MethodViolationCode; pub use self::object_safety::ObjectSafetyViolation; pub use self::on_unimplemented::{OnUnimplementedDirective, OnUnimplementedNote}; pub use self::project::MismatchedProjectionTypes; -pub use self::project::{normalize, normalize_projection_type, poly_project_and_unify_type}; +pub use self::project::{ + normalize, normalize_projection_type, normalize_to, poly_project_and_unify_type, +}; pub use self::project::{Normalized, ProjectionCache, ProjectionCacheSnapshot}; pub use self::select::{IntercrateAmbiguityCause, SelectionContext}; pub use self::specialize::find_associated_item; diff --git a/src/librustc/traits/project.rs b/src/librustc/traits/project.rs index fffcf66075f93..a1d785cf4447a 100644 --- a/src/librustc/traits/project.rs +++ b/src/librustc/traits/project.rs @@ -216,7 +216,22 @@ pub fn normalize<'a, 'b, 'tcx, T>( where T: TypeFoldable<'tcx>, { - normalize_with_depth(selcx, param_env, cause, 0, value) + let mut obligations = Vec::new(); + let value = normalize_to(selcx, param_env, cause, value, &mut obligations); + Normalized { value, obligations } +} + +pub fn normalize_to<'a, 'b, 'tcx, T>( + selcx: &'a mut SelectionContext<'b, 'tcx>, + param_env: ty::ParamEnv<'tcx>, + cause: ObligationCause<'tcx>, + value: &T, + obligations: &mut Vec>, +) -> T +where + T: TypeFoldable<'tcx>, +{ + normalize_with_depth_to(selcx, param_env, cause, 0, value, obligations) } /// As `normalize`, but with a custom depth. @@ -227,11 +242,27 @@ pub fn normalize_with_depth<'a, 'b, 'tcx, T>( depth: usize, value: &T, ) -> Normalized<'tcx, T> +where + T: TypeFoldable<'tcx>, +{ + let mut obligations = Vec::new(); + let value = normalize_with_depth_to(selcx, param_env, cause, depth, value, &mut obligations); + Normalized { value, obligations } +} + +pub fn normalize_with_depth_to<'a, 'b, 'tcx, T>( + selcx: &'a mut SelectionContext<'b, 'tcx>, + param_env: ty::ParamEnv<'tcx>, + cause: ObligationCause<'tcx>, + depth: usize, + value: &T, + obligations: &mut Vec>, +) -> T where T: TypeFoldable<'tcx>, { debug!("normalize_with_depth(depth={}, value={:?})", depth, value); - let mut normalizer = AssocTypeNormalizer::new(selcx, param_env, cause, depth); + let mut normalizer = AssocTypeNormalizer::new(selcx, param_env, cause, depth, obligations); let result = normalizer.fold(value); debug!( "normalize_with_depth: depth={} result={:?} with {} obligations", @@ -240,14 +271,14 @@ where normalizer.obligations.len() ); debug!("normalize_with_depth: depth={} obligations={:?}", depth, normalizer.obligations); - Normalized { value: result, obligations: normalizer.obligations } + result } struct AssocTypeNormalizer<'a, 'b, 'tcx> { selcx: &'a mut SelectionContext<'b, 'tcx>, param_env: ty::ParamEnv<'tcx>, cause: ObligationCause<'tcx>, - obligations: Vec>, + obligations: &'a mut Vec>, depth: usize, } @@ -257,8 +288,9 @@ impl<'a, 'b, 'tcx> AssocTypeNormalizer<'a, 'b, 'tcx> { param_env: ty::ParamEnv<'tcx>, cause: ObligationCause<'tcx>, depth: usize, + obligations: &'a mut Vec>, ) -> AssocTypeNormalizer<'a, 'b, 'tcx> { - AssocTypeNormalizer { selcx, param_env, cause, obligations: vec![], depth } + AssocTypeNormalizer { selcx, param_env, cause, obligations, depth } } fn fold>(&mut self, value: &T) -> T { @@ -343,7 +375,7 @@ impl<'a, 'b, 'tcx> TypeFolder<'tcx> for AssocTypeNormalizer<'a, 'b, 'tcx> { ); debug!( "AssocTypeNormalizer: depth={} normalized {:?} to {:?}, \ - now with {} obligations", + now with {} obligations", self.depth, ty, normalized_ty, @@ -441,8 +473,8 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>( debug!( "opt_normalize_projection_type(\ - projection_ty={:?}, \ - depth={})", + projection_ty={:?}, \ + depth={})", projection_ty, depth ); @@ -469,7 +501,7 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>( // changes debug!( "opt_normalize_projection_type: \ - found cache entry: ambiguous" + found cache entry: ambiguous" ); if !projection_ty.has_closure_types() { return None; @@ -498,7 +530,7 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>( debug!( "opt_normalize_projection_type: \ - found cache entry: in-progress" + found cache entry: in-progress" ); // But for now, let's classify this as an overflow: @@ -521,7 +553,7 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>( // evaluations can causes ICEs (e.g., #43132). debug!( "opt_normalize_projection_type: \ - found normalized ty `{:?}`", + found normalized ty `{:?}`", ty ); @@ -546,7 +578,7 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>( Err(ProjectionCacheEntry::Error) => { debug!( "opt_normalize_projection_type: \ - found error" + found error" ); let result = normalize_to_error(selcx, param_env, projection_ty, cause, depth); obligations.extend(result.obligations); @@ -567,23 +599,28 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>( debug!( "opt_normalize_projection_type: \ - projected_ty={:?} \ - depth={} \ - projected_obligations={:?}", + projected_ty={:?} \ + depth={} \ + projected_obligations={:?}", projected_ty, depth, projected_obligations ); let result = if projected_ty.has_projections() { - let mut normalizer = AssocTypeNormalizer::new(selcx, param_env, cause, depth + 1); + let mut normalizer = AssocTypeNormalizer::new( + selcx, + param_env, + cause, + depth + 1, + &mut projected_obligations, + ); let normalized_ty = normalizer.fold(&projected_ty); debug!( "opt_normalize_projection_type: \ - normalized_ty={:?} depth={}", + normalized_ty={:?} depth={}", normalized_ty, depth ); - projected_obligations.extend(normalizer.obligations); Normalized { value: normalized_ty, obligations: projected_obligations } } else { Normalized { value: projected_ty, obligations: projected_obligations } @@ -597,7 +634,7 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>( Ok(ProjectedTy::NoProgress(projected_ty)) => { debug!( "opt_normalize_projection_type: \ - projected_ty={:?} no progress", + projected_ty={:?} no progress", projected_ty ); let result = Normalized { value: projected_ty, obligations: vec![] }; @@ -608,7 +645,7 @@ fn opt_normalize_projection_type<'a, 'b, 'tcx>( Err(ProjectionTyError::TooManyCandidates) => { debug!( "opt_normalize_projection_type: \ - too many candidates" + too many candidates" ); infcx.projection_cache.borrow_mut().ambiguous(cache_key); None @@ -930,7 +967,7 @@ fn assemble_candidates_from_predicates<'cx, 'tcx, I>( debug!( "assemble_candidates_from_predicates: candidate={:?} \ - is_match={} same_def_id={}", + is_match={} same_def_id={}", data, is_match, same_def_id ); @@ -1192,7 +1229,7 @@ fn confirm_object_candidate<'cx, 'tcx>( None => { debug!( "confirm_object_candidate: no env-predicate \ - found in object type `{:?}`; ill-formed", + found in object type `{:?}`; ill-formed", object_ty ); return Progress::error(selcx.tcx()); diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index e4ef68c167f94..26f2a4ddb385a 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -9,7 +9,9 @@ use self::SelectionCandidate::*; use super::coherence::{self, Conflict}; use super::project; -use super::project::{normalize_with_depth, Normalized, ProjectionCacheKey}; +use super::project::{ + normalize_with_depth, normalize_with_depth_to, Normalized, ProjectionCacheKey, +}; use super::util; use super::util::{closure_trait_ref_and_return_type, predicate_for_trait_def}; use super::wf; @@ -1019,7 +1021,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { if let Some(value) = value { debug!( "filter_negative_and_reservation_impls: \ - reservation impl ambiguity on {:?}", + reservation impl ambiguity on {:?}", def_id ); intercrate_ambiguity_clauses.push( @@ -1317,7 +1319,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { if !self.can_cache_candidate(&candidate) { debug!( "insert_candidate_cache(trait_ref={:?}, candidate={:?} -\ - candidate is not cacheable", + candidate is not cacheable", trait_ref, candidate ); return; @@ -3484,25 +3486,23 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { // that order. let predicates = tcx.predicates_of(def_id); assert_eq!(predicates.parent, None); - let mut predicates: Vec<_> = predicates - .predicates - .iter() - .flat_map(|(predicate, _)| { - let predicate = normalize_with_depth( - self, - param_env, - cause.clone(), - recursion_depth, - &predicate.subst(tcx, substs), - ); - predicate.obligations.into_iter().chain(Some(Obligation { - cause: cause.clone(), - recursion_depth, - param_env, - predicate: predicate.value, - })) - }) - .collect(); + let mut obligations = Vec::new(); + for (predicate, _) in predicates.predicates { + let predicate = normalize_with_depth_to( + self, + param_env, + cause.clone(), + recursion_depth, + &predicate.subst(tcx, substs), + &mut obligations, + ); + obligations.push(Obligation { + cause: cause.clone(), + recursion_depth, + param_env, + predicate, + }); + } // We are performing deduplication here to avoid exponential blowups // (#38528) from happening, but the real cause of the duplication is @@ -3513,20 +3513,20 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { // This code is hot enough that it's worth avoiding the allocation // required for the FxHashSet when possible. Special-casing lengths 0, // 1 and 2 covers roughly 75-80% of the cases. - if predicates.len() <= 1 { + if obligations.len() <= 1 { // No possibility of duplicates. - } else if predicates.len() == 2 { + } else if obligations.len() == 2 { // Only two elements. Drop the second if they are equal. - if predicates[0] == predicates[1] { - predicates.truncate(1); + if obligations[0] == obligations[1] { + obligations.truncate(1); } } else { // Three or more elements. Use a general deduplication process. let mut seen = FxHashSet::default(); - predicates.retain(|i| seen.insert(i.clone())); + obligations.retain(|i| seen.insert(i.clone())); } - predicates + obligations } } diff --git a/src/librustc/traits/wf.rs b/src/librustc/traits/wf.rs index 9fa3c87477951..fbcb77a403183 100644 --- a/src/librustc/traits/wf.rs +++ b/src/librustc/traits/wf.rs @@ -8,7 +8,6 @@ use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_span::symbol::{kw, Ident}; use rustc_span::Span; -use std::iter::once; /// Returns the set of obligations needed to make `ty` well-formed. /// If `ty` contains unresolved inference variables, this may include @@ -26,6 +25,7 @@ pub fn obligations<'a, 'tcx>( let mut wf = WfPredicates { infcx, param_env, body_id, span, out: vec![], item: None }; if wf.compute(ty) { debug!("wf::obligations({:?}, body_id={:?}) = {:?}", ty, body_id, wf.out); + let result = wf.normalize(); debug!("wf::obligations({:?}, body_id={:?}) ~~> {:?}", ty, body_id, result); Some(result) @@ -143,15 +143,15 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> { let cause = self.cause(traits::MiscObligation); let infcx = &mut self.infcx; let param_env = self.param_env; - self.out - .iter() - .inspect(|pred| assert!(!pred.has_escaping_bound_vars())) - .flat_map(|pred| { - let mut selcx = traits::SelectionContext::new(infcx); - let pred = traits::normalize(&mut selcx, param_env, cause.clone(), pred); - once(pred.value).chain(pred.obligations) - }) - .collect() + let mut obligations = Vec::new(); + self.out.iter().inspect(|pred| assert!(!pred.has_escaping_bound_vars())).for_each(|pred| { + let mut selcx = traits::SelectionContext::new(infcx); + let i = obligations.len(); + let value = + traits::normalize_to(&mut selcx, param_env, cause.clone(), pred, &mut obligations); + obligations.insert(i, value); + }); + obligations } /// Pushes the obligations required for `trait_ref` to be WF into `self.out`. From 619051e4f080e3f3ccb94d7789bbef58282e64a7 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sat, 8 Feb 2020 15:06:31 -0500 Subject: [PATCH 11/11] Move librustc_hir/def_id.rs to librustc_span/def_id.rs For noww, librustc_hir re-exports the `def_id` module from librustc_span, so the rest of rustc can continue to reference rustc_hir::def_id --- src/librustc/ich/hcx.rs | 6 +++++ src/librustc/ich/impls_hir.rs | 6 ----- src/librustc_hir/lib.rs | 2 +- src/librustc_hir/stable_hash_impls.rs | 8 ------ src/{librustc_hir => librustc_span}/def_id.rs | 26 ++++++++++++------- src/librustc_span/lib.rs | 4 ++- 6 files changed, 27 insertions(+), 25 deletions(-) rename src/{librustc_hir => librustc_span}/def_id.rs (95%) diff --git a/src/librustc/ich/hcx.rs b/src/librustc/ich/hcx.rs index aade4c3f74c54..76e4b5f01b775 100644 --- a/src/librustc/ich/hcx.rs +++ b/src/librustc/ich/hcx.rs @@ -249,6 +249,12 @@ impl<'a> rustc_span::HashStableContext for StableHashingContext<'a> { self.hash_spans } + #[inline] + fn hash_def_id(&mut self, def_id: DefId, hasher: &mut StableHasher) { + let hcx = self; + hcx.def_path_hash(def_id).hash_stable(hcx, hasher); + } + fn byte_pos_to_line_and_col( &mut self, byte: BytePos, diff --git a/src/librustc/ich/impls_hir.rs b/src/librustc/ich/impls_hir.rs index 0155861549773..625d8a4670f22 100644 --- a/src/librustc/ich/impls_hir.rs +++ b/src/librustc/ich/impls_hir.rs @@ -11,12 +11,6 @@ use smallvec::SmallVec; use std::mem; impl<'ctx> rustc_hir::HashStableContext for StableHashingContext<'ctx> { - #[inline] - fn hash_def_id(&mut self, def_id: DefId, hasher: &mut StableHasher) { - let hcx = self; - hcx.def_path_hash(def_id).hash_stable(hcx, hasher); - } - #[inline] fn hash_hir_id(&mut self, hir_id: hir::HirId, hasher: &mut StableHasher) { let hcx = self; diff --git a/src/librustc_hir/lib.rs b/src/librustc_hir/lib.rs index f54fa291bd6e8..e4edd34bd6e23 100644 --- a/src/librustc_hir/lib.rs +++ b/src/librustc_hir/lib.rs @@ -12,7 +12,7 @@ extern crate rustc_data_structures; pub mod def; -pub mod def_id; +pub use rustc_span::def_id; mod hir; pub mod hir_id; pub mod intravisit; diff --git a/src/librustc_hir/stable_hash_impls.rs b/src/librustc_hir/stable_hash_impls.rs index 294074cd3e5a4..e8407b537011b 100644 --- a/src/librustc_hir/stable_hash_impls.rs +++ b/src/librustc_hir/stable_hash_impls.rs @@ -1,6 +1,5 @@ use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; -use crate::def_id::DefId; use crate::hir::{BodyId, Expr, ImplItemId, ItemId, Mod, TraitItemId, Ty, VisibilityKind}; use crate::hir_id::HirId; @@ -8,7 +7,6 @@ use crate::hir_id::HirId; /// This is a hack to allow using the `HashStable_Generic` derive macro /// instead of implementing everything in librustc. pub trait HashStableContext: syntax::HashStableContext + rustc_target::HashStableContext { - fn hash_def_id(&mut self, _: DefId, hasher: &mut StableHasher); fn hash_hir_id(&mut self, _: HirId, hasher: &mut StableHasher); fn hash_body_id(&mut self, _: BodyId, hasher: &mut StableHasher); fn hash_reference_to_item(&mut self, _: HirId, hasher: &mut StableHasher); @@ -24,12 +22,6 @@ impl HashStable for HirId { } } -impl HashStable for DefId { - fn hash_stable(&self, hcx: &mut HirCtx, hasher: &mut StableHasher) { - hcx.hash_def_id(*self, hasher) - } -} - impl HashStable for BodyId { fn hash_stable(&self, hcx: &mut HirCtx, hasher: &mut StableHasher) { hcx.hash_body_id(*self, hasher) diff --git a/src/librustc_hir/def_id.rs b/src/librustc_span/def_id.rs similarity index 95% rename from src/librustc_hir/def_id.rs rename to src/librustc_span/def_id.rs index 7ee778ddd8ec7..6cdfd0500ca84 100644 --- a/src/librustc_hir/def_id.rs +++ b/src/librustc_span/def_id.rs @@ -1,3 +1,5 @@ +use crate::HashStableContext; +use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; use rustc_data_structures::AtomicRef; use rustc_index::vec::Idx; use rustc_serialize::{Decoder, Encoder}; @@ -18,15 +20,6 @@ pub enum CrateNum { Index(CrateId), } -impl ::std::fmt::Debug for CrateNum { - fn fmt(&self, fmt: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result { - match self { - CrateNum::Index(id) => write!(fmt, "crate{}", id.private), - CrateNum::ReservedForIncrCompCache => write!(fmt, "crate for decoding incr comp cache"), - } - } -} - /// Item definitions in the currently-compiled crate would have the `CrateNum` /// `LOCAL_CRATE` in their `DefId`. pub const LOCAL_CRATE: CrateNum = CrateNum::Index(CrateId::from_u32_const(0)); @@ -100,6 +93,15 @@ impl rustc_serialize::UseSpecializedDecodable for CrateNum { } } +impl ::std::fmt::Debug for CrateNum { + fn fmt(&self, fmt: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result { + match self { + CrateNum::Index(id) => write!(fmt, "crate{}", id.private), + CrateNum::ReservedForIncrCompCache => write!(fmt, "crate for decoding incr comp cache"), + } + } +} + rustc_index::newtype_index! { /// A DefIndex is an index into the hir-map for a crate, identifying a /// particular definition. It should really be considered an interned @@ -207,3 +209,9 @@ impl fmt::Debug for LocalDefId { impl rustc_serialize::UseSpecializedEncodable for LocalDefId {} impl rustc_serialize::UseSpecializedDecodable for LocalDefId {} + +impl HashStable for DefId { + fn hash_stable(&self, hcx: &mut CTX, hasher: &mut StableHasher) { + hcx.hash_def_id(*self, hasher) + } +} diff --git a/src/librustc_span/lib.rs b/src/librustc_span/lib.rs index 413bd77daae24..87342d6a30120 100644 --- a/src/librustc_span/lib.rs +++ b/src/librustc_span/lib.rs @@ -25,7 +25,8 @@ use edition::Edition; pub mod hygiene; use hygiene::Transparency; pub use hygiene::{DesugaringKind, ExpnData, ExpnId, ExpnKind, MacroKind, SyntaxContext}; - +pub mod def_id; +use def_id::DefId; mod span_encoding; pub use span_encoding::{Span, DUMMY_SP}; @@ -1561,6 +1562,7 @@ fn lookup_line(lines: &[BytePos], pos: BytePos) -> isize { /// instead of implementing everything in librustc. pub trait HashStableContext { fn hash_spans(&self) -> bool; + fn hash_def_id(&mut self, _: DefId, hasher: &mut StableHasher); fn byte_pos_to_line_and_col( &mut self, byte: BytePos,