From af6ace62090aa03617fee2ef185363702634cff0 Mon Sep 17 00:00:00 2001 From: Marcel Hellwig Date: Mon, 6 May 2019 14:54:27 +0200 Subject: [PATCH 01/15] convert custom try macro to `?` resolves #60580 --- src/libstd/sys/redox/process.rs | 49 +++++++++------------ src/libstd/sys/unix/process/process_unix.rs | 27 +++++------- 2 files changed, 31 insertions(+), 45 deletions(-) diff --git a/src/libstd/sys/redox/process.rs b/src/libstd/sys/redox/process.rs index 8e6f50773abfe..5039c2e3ddfb3 100644 --- a/src/libstd/sys/redox/process.rs +++ b/src/libstd/sys/redox/process.rs @@ -254,44 +254,37 @@ impl Command { // have the drop glue anyway because this code never returns (the // child will either exec() or invoke syscall::exit) unsafe fn do_exec(&mut self, stdio: ChildPipes) -> io::Error { - macro_rules! t { - ($e:expr) => (match $e { - Ok(e) => e, - Err(e) => return e, - }) - } - if let Some(fd) = stdio.stderr.fd() { - t!(cvt(syscall::dup2(fd, 2, &[]))); - let mut flags = t!(cvt(syscall::fcntl(2, syscall::F_GETFD, 0))); + cvt(syscall::dup2(fd, 2, &[]))?; + let mut flags = cvt(syscall::fcntl(2, syscall::F_GETFD, 0))?; flags &= ! syscall::O_CLOEXEC; - t!(cvt(syscall::fcntl(2, syscall::F_SETFD, flags))); + cvt(syscall::fcntl(2, syscall::F_SETFD, flags))?; } if let Some(fd) = stdio.stdout.fd() { - t!(cvt(syscall::dup2(fd, 1, &[]))); - let mut flags = t!(cvt(syscall::fcntl(1, syscall::F_GETFD, 0))); + cvt(syscall::dup2(fd, 1, &[]))?; + let mut flags = cvt(syscall::fcntl(1, syscall::F_GETFD, 0))?; flags &= ! syscall::O_CLOEXEC; - t!(cvt(syscall::fcntl(1, syscall::F_SETFD, flags))); + cvt(syscall::fcntl(1, syscall::F_SETFD, flags))?; } if let Some(fd) = stdio.stdin.fd() { - t!(cvt(syscall::dup2(fd, 0, &[]))); - let mut flags = t!(cvt(syscall::fcntl(0, syscall::F_GETFD, 0))); + cvt(syscall::dup2(fd, 0, &[]))?; + let mut flags = cvt(syscall::fcntl(0, syscall::F_GETFD, 0))?; flags &= ! syscall::O_CLOEXEC; - t!(cvt(syscall::fcntl(0, syscall::F_SETFD, flags))); + cvt(syscall::fcntl(0, syscall::F_SETFD, flags))?; } if let Some(g) = self.gid { - t!(cvt(syscall::setregid(g as usize, g as usize))); + cvt(syscall::setregid(g as usize, g as usize))?; } if let Some(u) = self.uid { - t!(cvt(syscall::setreuid(u as usize, u as usize))); + cvt(syscall::setreuid(u as usize, u as usize))?; } if let Some(ref cwd) = self.cwd { - t!(cvt(syscall::chdir(cwd))); + cvt(syscall::chdir(cwd))?; } for callback in self.closures.iter_mut() { - t!(callback()); + callback()?; } self.env.apply(); @@ -313,7 +306,7 @@ impl Command { }; let mut file = if let Some(program) = program { - t!(File::open(program.as_os_str())) + File::open(program.as_os_str())? } else { return io::Error::from_raw_os_error(syscall::ENOENT); }; @@ -327,7 +320,7 @@ impl Command { let mut shebang = [0; 2]; let mut read = 0; loop { - match t!(reader.read(&mut shebang[read..])) { + match reader.read(&mut shebang[read..])? { 0 => break, n => read += n, } @@ -338,9 +331,9 @@ impl Command { // First of all, since we'll be passing another file to // fexec(), we need to manually check that we have permission // to execute this file: - let uid = t!(cvt(syscall::getuid())); - let gid = t!(cvt(syscall::getgid())); - let meta = t!(file.metadata()); + let uid = cvt(syscall::getuid())?; + let gid = cvt(syscall::getgid())?; + let meta = file.metadata()?; let mode = if uid == meta.uid() as usize { meta.mode() >> 3*2 & 0o7 @@ -355,7 +348,7 @@ impl Command { // Second of all, we need to actually read which interpreter it wants let mut interpreter = Vec::new(); - t!(reader.read_until(b'\n', &mut interpreter)); + reader.read_until(b'\n', &mut interpreter)?; // Pop one trailing newline, if any if interpreter.ends_with(&[b'\n']) { interpreter.pop().unwrap(); @@ -373,11 +366,11 @@ impl Command { }; if let Some(ref interpreter) = interpreter { let path: &OsStr = OsStr::from_bytes(&interpreter); - file = t!(File::open(path)); + file = File::open(path)?; args.push([interpreter.as_ptr() as usize, interpreter.len()]); } else { - t!(file.seek(SeekFrom::Start(0))); + file.seek(SeekFrom::Start(0))?; } args.push([self.program.as_ptr() as usize, self.program.len()]); diff --git a/src/libstd/sys/unix/process/process_unix.rs b/src/libstd/sys/unix/process/process_unix.rs index 220b1fd453131..07f813315e8c1 100644 --- a/src/libstd/sys/unix/process/process_unix.rs +++ b/src/libstd/sys/unix/process/process_unix.rs @@ -167,26 +167,19 @@ impl Command { ) -> io::Error { use crate::sys::{self, cvt_r}; - macro_rules! t { - ($e:expr) => (match $e { - Ok(e) => e, - Err(e) => return e, - }) - } - if let Some(fd) = stdio.stdin.fd() { - t!(cvt_r(|| libc::dup2(fd, libc::STDIN_FILENO))); + cvt_r(|| libc::dup2(fd, libc::STDIN_FILENO))?; } if let Some(fd) = stdio.stdout.fd() { - t!(cvt_r(|| libc::dup2(fd, libc::STDOUT_FILENO))); + cvt_r(|| libc::dup2(fd, libc::STDOUT_FILENO))?; } if let Some(fd) = stdio.stderr.fd() { - t!(cvt_r(|| libc::dup2(fd, libc::STDERR_FILENO))); + cvt_r(|| libc::dup2(fd, libc::STDERR_FILENO))?; } if cfg!(not(any(target_os = "l4re"))) { if let Some(u) = self.get_gid() { - t!(cvt(libc::setgid(u as gid_t))); + cvt(libc::setgid(u as gid_t))?; } if let Some(u) = self.get_uid() { // When dropping privileges from root, the `setgroups` call @@ -198,11 +191,11 @@ impl Command { // privilege dropping function. let _ = libc::setgroups(0, ptr::null()); - t!(cvt(libc::setuid(u as uid_t))); + cvt(libc::setuid(u as uid_t))?; } } if let Some(ref cwd) = *self.get_cwd() { - t!(cvt(libc::chdir(cwd.as_ptr()))); + cvt(libc::chdir(cwd.as_ptr()))?; } // emscripten has no signal support. @@ -225,10 +218,10 @@ impl Command { 0, mem::size_of::()); } else { - t!(cvt(libc::sigemptyset(&mut set))); + cvt(libc::sigemptyset(&mut set))?; } - t!(cvt(libc::pthread_sigmask(libc::SIG_SETMASK, &set, - ptr::null_mut()))); + cvt(libc::pthread_sigmask(libc::SIG_SETMASK, &set, + ptr::null_mut()))?; let ret = sys::signal(libc::SIGPIPE, libc::SIG_DFL); if ret == libc::SIG_ERR { return io::Error::last_os_error() @@ -236,7 +229,7 @@ impl Command { } for callback in self.get_closures().iter_mut() { - t!(callback()); + callback()?; } // Although we're performing an exec here we may also return with an From 5458b651b1ecad5cc334b494209352ac935360ce Mon Sep 17 00:00:00 2001 From: Marcel Hellwig Date: Mon, 6 May 2019 15:40:34 +0200 Subject: [PATCH 02/15] use exhaustive_patterns to be able to use `?` --- src/libstd/sys/redox/process.rs | 16 +++++++++------- src/libstd/sys/unix/process/process_unix.rs | 11 ++++++----- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/libstd/sys/redox/process.rs b/src/libstd/sys/redox/process.rs index 5039c2e3ddfb3..2a553b2c93bd3 100644 --- a/src/libstd/sys/redox/process.rs +++ b/src/libstd/sys/redox/process.rs @@ -150,7 +150,7 @@ impl Command { match cvt(syscall::clone(0))? { 0 => { drop(input); - let err = self.do_exec(theirs); + let Err(err) = self.do_exec(theirs); let errno = err.raw_os_error().unwrap_or(syscall::EINVAL) as u32; let bytes = [ (errno >> 24) as u8, @@ -218,7 +218,10 @@ impl Command { } match self.setup_io(default, true) { - Ok((_, theirs)) => unsafe { self.do_exec(theirs) }, + Ok((_, theirs)) => unsafe { + let Err(e) = self.do_exec(theirs); + e + }, Err(e) => e, } } @@ -253,7 +256,7 @@ impl Command { // allocation). Instead we just close it manually. This will never // have the drop glue anyway because this code never returns (the // child will either exec() or invoke syscall::exit) - unsafe fn do_exec(&mut self, stdio: ChildPipes) -> io::Error { + unsafe fn do_exec(&mut self, stdio: ChildPipes) -> Result { if let Some(fd) = stdio.stderr.fd() { cvt(syscall::dup2(fd, 2, &[]))?; let mut flags = cvt(syscall::fcntl(2, syscall::F_GETFD, 0))?; @@ -308,7 +311,7 @@ impl Command { let mut file = if let Some(program) = program { File::open(program.as_os_str())? } else { - return io::Error::from_raw_os_error(syscall::ENOENT); + return Err(io::Error::from_raw_os_error(syscall::ENOENT)); }; // Push all the arguments @@ -343,7 +346,7 @@ impl Command { meta.mode() & 0o7 }; if mode & 1 == 0 { - return io::Error::from_raw_os_error(syscall::EPERM); + return Err(io::Error::from_raw_os_error(syscall::EPERM)); } // Second of all, we need to actually read which interpreter it wants @@ -389,13 +392,12 @@ impl Command { } if let Err(err) = syscall::fexec(file.as_raw_fd(), &args, &vars) { - io::Error::from_raw_os_error(err.errno as i32) + Err(io::Error::from_raw_os_error(err.errno as i32)) } else { panic!("return from exec without err"); } } - fn setup_io(&self, default: Stdio, needs_stdin: bool) -> io::Result<(StdioPipes, ChildPipes)> { let null = Stdio::Null; diff --git a/src/libstd/sys/unix/process/process_unix.rs b/src/libstd/sys/unix/process/process_unix.rs index 07f813315e8c1..80fe763aecc88 100644 --- a/src/libstd/sys/unix/process/process_unix.rs +++ b/src/libstd/sys/unix/process/process_unix.rs @@ -47,7 +47,7 @@ impl Command { match result { 0 => { drop(input); - let err = self.do_exec(theirs, envp.as_ref()); + let Err(err) = self.do_exec(theirs, envp.as_ref()); let errno = err.raw_os_error().unwrap_or(libc::EINVAL) as u32; let bytes = [ (errno >> 24) as u8, @@ -123,7 +123,8 @@ impl Command { // environment lock before we try to exec. let _lock = sys::os::env_lock(); - self.do_exec(theirs, envp.as_ref()) + let Err(e) = self.do_exec(theirs, envp.as_ref()); + e } } Err(e) => e, @@ -164,7 +165,7 @@ impl Command { &mut self, stdio: ChildPipes, maybe_envp: Option<&CStringArray> - ) -> io::Error { + ) -> Result { use crate::sys::{self, cvt_r}; if let Some(fd) = stdio.stdin.fd() { @@ -224,7 +225,7 @@ impl Command { ptr::null_mut()))?; let ret = sys::signal(libc::SIGPIPE, libc::SIG_DFL); if ret == libc::SIG_ERR { - return io::Error::last_os_error() + return Err(io::Error::last_os_error()) } } @@ -254,7 +255,7 @@ impl Command { } libc::execvp(self.get_argv()[0], self.get_argv().as_ptr()); - io::Error::last_os_error() + Err(io::Error::last_os_error()) } #[cfg(not(any(target_os = "macos", target_os = "freebsd", From c5d94017569713c12a7e3d260cb4cd0a02b26372 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 6 May 2019 11:47:03 +1000 Subject: [PATCH 03/15] Avoid symbol interning in `file_metadata`. This commit changes `created_files` so it uses strings directly as keys, rather than symbols derived from the strings. This avoids the cost of having to do the hash table lookups to produce the symbols from the strings. The commit also uses `entry` to avoid doing a repeated hash table lookup (`get` + `insert`). Note that PR #60467 improved this code somewhat; this is a further improvement. --- .../debuginfo/metadata.rs | 64 +++++++++---------- src/librustc_codegen_llvm/debuginfo/mod.rs | 4 +- 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/src/librustc_codegen_llvm/debuginfo/metadata.rs b/src/librustc_codegen_llvm/debuginfo/metadata.rs index 9bdce37b8baec..e7f54f6609d29 100644 --- a/src/librustc_codegen_llvm/debuginfo/metadata.rs +++ b/src/librustc_codegen_llvm/debuginfo/metadata.rs @@ -38,6 +38,7 @@ use rustc_data_structures::small_c_str::SmallCStr; use rustc_target::abi::HasDataLayout; use libc::{c_uint, c_longlong}; +use std::collections::hash_map::Entry; use std::ffi::CString; use std::fmt::{self, Write}; use std::hash::{Hash, Hasher}; @@ -45,7 +46,7 @@ use std::iter; use std::ptr; use std::path::{Path, PathBuf}; use syntax::ast; -use syntax::symbol::{Interner, InternedString, Symbol}; +use syntax::symbol::{Interner, InternedString}; use syntax_pos::{self, Span, FileName}; impl PartialEq for llvm::Metadata { @@ -787,49 +788,48 @@ pub fn file_metadata(cx: &CodegenCx<'ll, '_>, file_name, defining_crate); - let file_name = &file_name.to_string(); - let file_name_symbol = Symbol::intern(file_name); - if defining_crate == LOCAL_CRATE { - let directory = &cx.sess().working_dir.0.to_string_lossy(); - file_metadata_raw(cx, file_name, Some(file_name_symbol), - directory, Some(Symbol::intern(directory))) + let file_name = Some(file_name.to_string()); + let directory = if defining_crate == LOCAL_CRATE { + Some(cx.sess().working_dir.0.to_string_lossy().to_string()) } else { // If the path comes from an upstream crate we assume it has been made // independent of the compiler's working directory one way or another. - file_metadata_raw(cx, file_name, Some(file_name_symbol), "", None) - } + None + }; + file_metadata_raw(cx, file_name, directory) } pub fn unknown_file_metadata(cx: &CodegenCx<'ll, '_>) -> &'ll DIFile { - file_metadata_raw(cx, "", None, "", None) + file_metadata_raw(cx, None, None) } fn file_metadata_raw(cx: &CodegenCx<'ll, '_>, - file_name: &str, - file_name_symbol: Option, - directory: &str, - directory_symbol: Option) + file_name: Option, + directory: Option) -> &'ll DIFile { - let key = (file_name_symbol, directory_symbol); + let key = (file_name, directory); + + match debug_context(cx).created_files.borrow_mut().entry(key) { + Entry::Occupied(o) => return o.get(), + Entry::Vacant(v) => { + let (file_name, directory) = v.key(); + debug!("file_metadata: file_name: {:?}, directory: {:?}", file_name, directory); + + let file_name = SmallCStr::new( + if let Some(file_name) = file_name { &file_name } else { "" }); + let directory = SmallCStr::new( + if let Some(directory) = directory { &directory } else { "" }); + + let file_metadata = unsafe { + llvm::LLVMRustDIBuilderCreateFile(DIB(cx), + file_name.as_ptr(), + directory.as_ptr()) + }; - if let Some(file_metadata) = debug_context(cx).created_files.borrow().get(&key) { - return *file_metadata; + v.insert(file_metadata); + file_metadata + } } - - debug!("file_metadata: file_name: {}, directory: {}", file_name, directory); - - let file_name = SmallCStr::new(file_name); - let directory = SmallCStr::new(directory); - - let file_metadata = unsafe { - llvm::LLVMRustDIBuilderCreateFile(DIB(cx), - file_name.as_ptr(), - directory.as_ptr()) - }; - - let mut created_files = debug_context(cx).created_files.borrow_mut(); - created_files.insert(key, file_metadata); - file_metadata } fn basic_type_metadata(cx: &CodegenCx<'ll, 'tcx>, t: Ty<'tcx>) -> &'ll DIType { diff --git a/src/librustc_codegen_llvm/debuginfo/mod.rs b/src/librustc_codegen_llvm/debuginfo/mod.rs index f3070a03b4ed5..527290392fff4 100644 --- a/src/librustc_codegen_llvm/debuginfo/mod.rs +++ b/src/librustc_codegen_llvm/debuginfo/mod.rs @@ -37,7 +37,7 @@ use std::ffi::CString; use syntax_pos::{self, Span, Pos}; use syntax::ast; -use syntax::symbol::{Symbol, InternedString}; +use syntax::symbol::InternedString; use rustc::ty::layout::{self, LayoutOf, HasTyCtxt}; use rustc_codegen_ssa::traits::*; @@ -63,7 +63,7 @@ pub struct CrateDebugContext<'a, 'tcx> { llcontext: &'a llvm::Context, llmod: &'a llvm::Module, builder: &'a mut DIBuilder<'a>, - created_files: RefCell, Option), &'a DIFile>>, + created_files: RefCell, Option), &'a DIFile>>, created_enum_disr_types: RefCell>, type_map: RefCell>, From a7e1431941406eeb341c4e3b7e929c2e65514ac3 Mon Sep 17 00:00:00 2001 From: Brent Kerby Date: Sun, 19 May 2019 09:21:03 -0600 Subject: [PATCH 04/15] Update boxed::Box docs on memory layout --- src/liballoc/boxed.rs | 59 +++++++++++++++++++++++++++++++++---------- 1 file changed, 45 insertions(+), 14 deletions(-) diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index 207359ed6968f..90bec03beb077 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -127,24 +127,38 @@ impl Box { /// /// After calling this function, the raw pointer is owned by the /// resulting `Box`. Specifically, the `Box` destructor will call - /// the destructor of `T` and free the allocated memory. Since the - /// way `Box` allocates and releases memory is unspecified, the - /// only valid pointer to pass to this function is the one taken - /// from another `Box` via the [`Box::into_raw`] function. + /// the destructor of `T` and free the allocated memory. For this + /// to be safe, the memory must have been allocated in the precise + /// way that `Box` expects, namely, using the global allocator + /// with the correct [`Layout`] for holding a value of type `T`. In + /// particular, this will be satisfied for a pointer obtained + /// from a previously existing `Box` using [`Box::into_raw`]. + /// + /// # Safety /// /// This function is unsafe because improper use may lead to /// memory problems. For example, a double-free may occur if the /// function is called twice on the same raw pointer. /// - /// [`Box::into_raw`]: struct.Box.html#method.into_raw - /// /// # Examples - /// + /// Recreate a `Box` which was previously converted to a raw pointer using [`Box::into_raw`]: /// ``` /// let x = Box::new(5); /// let ptr = Box::into_raw(x); /// let x = unsafe { Box::from_raw(ptr) }; /// ``` + /// Manually create a `Box` from scratch by using the global allocator: + /// ``` + /// use std::alloc::{Layout, alloc}; + /// + /// let ptr = unsafe{ alloc(Layout::new::()) } as *mut i32; + /// unsafe{ *ptr = 5; } + /// let x = unsafe{ Box::from_raw(ptr) }; + /// ``` + /// + /// [`Layout`]: ../alloc/struct.Layout.html + /// [`Box::into_raw`]: struct.Box.html#method.into_raw + /// #[stable(feature = "box_raw", since = "1.4.0")] #[inline] pub unsafe fn from_raw(raw: *mut T) -> Self { @@ -158,21 +172,34 @@ impl Box { /// After calling this function, the caller is responsible for the /// memory previously managed by the `Box`. In particular, the /// caller should properly destroy `T` and release the memory. The - /// proper way to do so is to convert the raw pointer back into a - /// `Box` with the [`Box::from_raw`] function. + /// easiest way to do so is to convert the raw pointer back into a `Box` + /// with the [`Box::from_raw`] function. /// /// Note: this is an associated function, which means that you have /// to call it as `Box::into_raw(b)` instead of `b.into_raw()`. This /// is so that there is no conflict with a method on the inner type. /// - /// [`Box::from_raw`]: struct.Box.html#method.from_raw - /// /// # Examples - /// + /// Converting the raw pointer back into a `Box` with [`Box::from_raw`] + /// for automatic cleanup: /// ``` - /// let x = Box::new(5); + /// let x = Box::new(String::from("Hello")); /// let ptr = Box::into_raw(x); + /// let x = unsafe{ Box::from_raw(ptr) }; + /// ``` + /// Manual cleanup by running the destructor and deallocating the memory: /// ``` + /// use std::alloc::{Layout, dealloc}; + /// use std::ptr; + /// + /// let x = Box::new(String::from("Hello")); + /// let p = Box::into_raw(x); + /// unsafe{ ptr::drop_in_place(p); } + /// unsafe{ dealloc(p as *mut u8, Layout::new::()); } + /// ``` + /// + /// [`Box::from_raw`]: struct.Box.html#method.from_raw + /// #[stable(feature = "box_raw", since = "1.4.0")] #[inline] pub fn into_raw(b: Box) -> *mut T { @@ -184,7 +211,7 @@ impl Box { /// After calling this function, the caller is responsible for the /// memory previously managed by the `Box`. In particular, the /// caller should properly destroy `T` and release the memory. The - /// proper way to do so is to convert the `NonNull` pointer + /// easiest way to do so is to convert the `NonNull` pointer /// into a raw pointer and back into a `Box` with the [`Box::from_raw`] /// function. /// @@ -203,6 +230,10 @@ impl Box { /// fn main() { /// let x = Box::new(5); /// let ptr = Box::into_raw_non_null(x); + /// + /// // Clean up the memory by converting the NonNull pointer back + /// // into a Box and letting the Box be dropped. + /// let x = unsafe{ Box::from_raw(ptr.as_ptr()) }; /// } /// ``` #[unstable(feature = "box_into_raw_non_null", issue = "47336")] From 178b753a4a202ad96ccbd10e037194d15ca8f805 Mon Sep 17 00:00:00 2001 From: Brent Kerby Date: Sun, 19 May 2019 17:47:18 -0600 Subject: [PATCH 05/15] Remove trailing whitespaces to satisfy tidy --- src/liballoc/boxed.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index 90bec03beb077..4e712a946b85d 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -130,9 +130,9 @@ impl Box { /// the destructor of `T` and free the allocated memory. For this /// to be safe, the memory must have been allocated in the precise /// way that `Box` expects, namely, using the global allocator - /// with the correct [`Layout`] for holding a value of type `T`. In + /// with the correct [`Layout`] for holding a value of type `T`. In /// particular, this will be satisfied for a pointer obtained - /// from a previously existing `Box` using [`Box::into_raw`]. + /// from a previously existing `Box` using [`Box::into_raw`]. /// /// # Safety /// @@ -172,7 +172,7 @@ impl Box { /// After calling this function, the caller is responsible for the /// memory previously managed by the `Box`. In particular, the /// caller should properly destroy `T` and release the memory. The - /// easiest way to do so is to convert the raw pointer back into a `Box` + /// easiest way to do so is to convert the raw pointer back into a `Box` /// with the [`Box::from_raw`] function. /// /// Note: this is an associated function, which means that you have @@ -180,7 +180,7 @@ impl Box { /// is so that there is no conflict with a method on the inner type. /// /// # Examples - /// Converting the raw pointer back into a `Box` with [`Box::from_raw`] + /// Converting the raw pointer back into a `Box` with [`Box::from_raw`] /// for automatic cleanup: /// ``` /// let x = Box::new(String::from("Hello")); @@ -191,7 +191,7 @@ impl Box { /// ``` /// use std::alloc::{Layout, dealloc}; /// use std::ptr; - /// + /// /// let x = Box::new(String::from("Hello")); /// let p = Box::into_raw(x); /// unsafe{ ptr::drop_in_place(p); } From d320c7ceab4371e6fd508a1f79742be042d97d2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 20 May 2019 11:46:44 -0700 Subject: [PATCH 06/15] Do not fail on child without DefId --- src/librustc_metadata/cstore_impl.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/librustc_metadata/cstore_impl.rs b/src/librustc_metadata/cstore_impl.rs index 087256a971056..e6710d668dc3c 100644 --- a/src/librustc_metadata/cstore_impl.rs +++ b/src/librustc_metadata/cstore_impl.rs @@ -355,20 +355,20 @@ pub fn provide<'tcx>(providers: &mut Providers<'tcx>) { return; } - let child = child.res.def_id(); - - match visible_parent_map.entry(child) { - Entry::Occupied(mut entry) => { - // If `child` is defined in crate `cnum`, ensure - // that it is mapped to a parent in `cnum`. - if child.krate == cnum && entry.get().krate != cnum { + if let Some(child) = child.res.opt_def_id() { + match visible_parent_map.entry(child) { + Entry::Occupied(mut entry) => { + // If `child` is defined in crate `cnum`, ensure + // that it is mapped to a parent in `cnum`. + if child.krate == cnum && entry.get().krate != cnum { + entry.insert(parent); + } + } + Entry::Vacant(entry) => { entry.insert(parent); + bfs_queue.push_back(child); } } - Entry::Vacant(entry) => { - entry.insert(parent); - bfs_queue.push_back(child); - } } }; From 419ca9d64001da616ab3c325aca4de8bc7e3be4d Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Mon, 20 May 2019 20:13:58 -0300 Subject: [PATCH 07/15] LocalDecl push returns Local len --- src/librustc_mir/transform/generator.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index d2da1e6e3ac9d..780b3c9686b6a 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -212,8 +212,8 @@ impl<'a, 'tcx> TransformVisitor<'a, 'tcx> { // Create a statement which reads the discriminant into a temporary fn get_discr(&self, mir: &mut Mir<'tcx>) -> (Statement<'tcx>, Place<'tcx>) { let temp_decl = LocalDecl::new_internal(self.tcx.types.isize, mir.span); - let temp = Place::Base(PlaceBase::Local(Local::new(mir.local_decls.len()))); - mir.local_decls.push(temp_decl); + let local_decls_len = mir.local_decls.push(temp_decl); + let temp = Place::Base(PlaceBase::Local(local_decls_len)); let self_place = Place::Base(PlaceBase::Local(self_arg())); let assign = Statement { From 4e37785c7d6ef85d00833e93943cdee28baf97b3 Mon Sep 17 00:00:00 2001 From: Brent Kerby Date: Mon, 20 May 2019 21:03:40 -0600 Subject: [PATCH 08/15] Create and reference Memory Layout section of boxed docs --- src/liballoc/boxed.rs | 70 ++++++++++++++++++++++++------------------- 1 file changed, 39 insertions(+), 31 deletions(-) diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index 4e712a946b85d..024594517d988 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -4,16 +4,6 @@ //! heap allocation in Rust. Boxes provide ownership for this allocation, and //! drop their contents when they go out of scope. //! -//! For non-zero-sized values, a [`Box`] will use the [`Global`] allocator for -//! its allocation. It is valid to convert both ways between a [`Box`] and a -//! raw pointer allocated with the [`Global`] allocator, given that the -//! [`Layout`] used with the allocator is correct for the type. More precisely, -//! a `value: *mut T` that has been allocated with the [`Global`] allocator -//! with `Layout::for_value(&*value)` may be converted into a box using -//! `Box::::from_raw(value)`. Conversely, the memory backing a `value: *mut -//! T` obtained from `Box::::into_raw` may be deallocated using the -//! [`Global`] allocator with `Layout::for_value(&*value)`. -//! //! # Examples //! //! Move a value from the stack to the heap by creating a [`Box`]: @@ -61,6 +51,19 @@ //! for a `Cons`. By introducing a `Box`, which has a defined size, we know how //! big `Cons` needs to be. //! +//! # Memory layout +//! +//! For non-zero-sized values, a [`Box`] will use the [`Global`] allocator for +//! its allocation. It is valid to convert both ways between a [`Box`] and a +//! raw pointer allocated with the [`Global`] allocator, given that the +//! [`Layout`] used with the allocator is correct for the type. More precisely, +//! a `value: *mut T` that has been allocated with the [`Global`] allocator +//! with `Layout::for_value(&*value)` may be converted into a box using +//! `Box::::from_raw(value)`. Conversely, the memory backing a `value: *mut +//! T` obtained from `Box::::into_raw` may be deallocated using the +//! [`Global`] allocator with `Layout::for_value(&*value)`. +//! +//! //! [dereferencing]: ../../std/ops/trait.Deref.html //! [`Box`]: struct.Box.html //! [`Global`]: ../alloc/struct.Global.html @@ -128,11 +131,8 @@ impl Box { /// After calling this function, the raw pointer is owned by the /// resulting `Box`. Specifically, the `Box` destructor will call /// the destructor of `T` and free the allocated memory. For this - /// to be safe, the memory must have been allocated in the precise - /// way that `Box` expects, namely, using the global allocator - /// with the correct [`Layout`] for holding a value of type `T`. In - /// particular, this will be satisfied for a pointer obtained - /// from a previously existing `Box` using [`Box::into_raw`]. + /// to be safe, the memory must have been allocated in accordance + /// with the [memory layout] used by `Box` . /// /// # Safety /// @@ -141,7 +141,8 @@ impl Box { /// function is called twice on the same raw pointer. /// /// # Examples - /// Recreate a `Box` which was previously converted to a raw pointer using [`Box::into_raw`]: + /// Recreate a `Box` which was previously converted to a raw pointer + /// using [`Box::into_raw`]: /// ``` /// let x = Box::new(5); /// let ptr = Box::into_raw(x); @@ -149,16 +150,18 @@ impl Box { /// ``` /// Manually create a `Box` from scratch by using the global allocator: /// ``` - /// use std::alloc::{Layout, alloc}; + /// use std::alloc::{alloc, Layout}; /// - /// let ptr = unsafe{ alloc(Layout::new::()) } as *mut i32; - /// unsafe{ *ptr = 5; } - /// let x = unsafe{ Box::from_raw(ptr) }; + /// unsafe { + /// let ptr = alloc(Layout::new::()) as *mut i32; + /// *ptr = 5; + /// let x = Box::from_raw(ptr); + /// } /// ``` /// + /// [memory layout]: index.html#memory-layout /// [`Layout`]: ../alloc/struct.Layout.html /// [`Box::into_raw`]: struct.Box.html#method.into_raw - /// #[stable(feature = "box_raw", since = "1.4.0")] #[inline] pub unsafe fn from_raw(raw: *mut T) -> Self { @@ -171,9 +174,11 @@ impl Box { /// /// After calling this function, the caller is responsible for the /// memory previously managed by the `Box`. In particular, the - /// caller should properly destroy `T` and release the memory. The - /// easiest way to do so is to convert the raw pointer back into a `Box` - /// with the [`Box::from_raw`] function. + /// caller should properly destroy `T` and release the memory, taking + /// into account the [memory layout] used by `Box`. The easiest way to + /// do this is to convert the raw pointer back into a `Box` with the + /// [`Box::from_raw`] function, allowing the `Box` destructor to perform + /// the cleanup. /// /// Note: this is an associated function, which means that you have /// to call it as `Box::into_raw(b)` instead of `b.into_raw()`. This @@ -185,21 +190,24 @@ impl Box { /// ``` /// let x = Box::new(String::from("Hello")); /// let ptr = Box::into_raw(x); - /// let x = unsafe{ Box::from_raw(ptr) }; + /// let x = unsafe { Box::from_raw(ptr) }; /// ``` - /// Manual cleanup by running the destructor and deallocating the memory: + /// Manual cleanup by explicitly running the destructor and deallocating + /// the memory: /// ``` - /// use std::alloc::{Layout, dealloc}; + /// use std::alloc::{dealloc, Layout}; /// use std::ptr; /// /// let x = Box::new(String::from("Hello")); /// let p = Box::into_raw(x); - /// unsafe{ ptr::drop_in_place(p); } - /// unsafe{ dealloc(p as *mut u8, Layout::new::()); } + /// unsafe { + /// ptr::drop_in_place(p); + /// dealloc(p as *mut u8, Layout::new::()); + /// } /// ``` /// + /// [memory layout]: index.html#memory-layout /// [`Box::from_raw`]: struct.Box.html#method.from_raw - /// #[stable(feature = "box_raw", since = "1.4.0")] #[inline] pub fn into_raw(b: Box) -> *mut T { @@ -233,7 +241,7 @@ impl Box { /// /// // Clean up the memory by converting the NonNull pointer back /// // into a Box and letting the Box be dropped. - /// let x = unsafe{ Box::from_raw(ptr.as_ptr()) }; + /// let x = unsafe { Box::from_raw(ptr.as_ptr()) }; /// } /// ``` #[unstable(feature = "box_into_raw_non_null", issue = "47336")] From e186d3f3e057eef69c60c7d34e474461432ee544 Mon Sep 17 00:00:00 2001 From: topecongiro Date: Tue, 21 May 2019 13:18:20 +0900 Subject: [PATCH 09/15] Add stream_to_parser_with_base_dir --- src/libsyntax/parse/mod.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/libsyntax/parse/mod.rs b/src/libsyntax/parse/mod.rs index 0611c1d9b42a5..53d6838939d05 100644 --- a/src/libsyntax/parse/mod.rs +++ b/src/libsyntax/parse/mod.rs @@ -329,6 +329,13 @@ pub fn stream_to_parser(sess: &ParseSess, stream: TokenStream) -> Parser<'_> { Parser::new(sess, stream, None, true, false) } +/// Given stream, the `ParseSess` and the base directory, produces a parser. +pub fn stream_to_parser_with_base_dir<'a>(sess: &'a ParseSess, + stream: TokenStream, + base_dir: Directory<'a>) -> Parser<'a> { + Parser::new(sess, stream, Some(base_dir), true, false) +} + /// A sequence separator. pub struct SeqSep { /// The seperator token. From 5ea5fe30726b616a7a561d9b9737b5f4bf629118 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 21 May 2019 08:51:18 +0200 Subject: [PATCH 10/15] static_assert: make use of anonymous constants --- src/librustc_data_structures/macros.rs | 6 +++--- src/librustc_mir/transform/qualify_consts.rs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/librustc_data_structures/macros.rs b/src/librustc_data_structures/macros.rs index 7fc23999284a7..40899c414722e 100644 --- a/src/librustc_data_structures/macros.rs +++ b/src/librustc_data_structures/macros.rs @@ -1,13 +1,13 @@ /// A simple static assertion macro. The first argument should be a unique /// ALL_CAPS identifier that describes the condition. #[macro_export] -#[allow_internal_unstable(type_ascription)] +#[allow_internal_unstable(type_ascription, underscore_const_names)] macro_rules! static_assert { - ($name:ident: $test:expr) => { + ($test:expr) => { // Use the bool to access an array such that if the bool is false, the access // is out-of-bounds. #[allow(dead_code)] - static $name: () = [()][!($test: bool) as usize]; + const _: () = [()][!($test: bool) as usize]; } } diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 579f75ba51657..84e2caec68e1c 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -544,14 +544,14 @@ impl Qualif for IsNotImplicitlyPromotable { // Ensure the `IDX` values are sequential (`0..QUALIF_COUNT`). macro_rules! static_assert_seq_qualifs { ($i:expr => $first:ident $(, $rest:ident)*) => { - static_assert!(SEQ_QUALIFS: { + static_assert!({ static_assert_seq_qualifs!($i + 1 => $($rest),*); $first::IDX == $i }); }; ($i:expr =>) => { - static_assert!(SEQ_QUALIFS: QUALIF_COUNT == $i); + static_assert!(QUALIF_COUNT == $i); }; } static_assert_seq_qualifs!( From dd94dc3599dfc11fa703470525473864495c596d Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Tue, 21 May 2019 12:11:23 +0200 Subject: [PATCH 11/15] Emit error when trying to use PGO in conjunction with unwinding on Windows. --- src/librustc/session/mod.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index 4d47491661e86..71a9d17be8919 100644 --- a/src/librustc/session/mod.rs +++ b/src/librustc/session/mod.rs @@ -1272,6 +1272,18 @@ fn validate_commandline_args_with_session_available(sess: &Session) { sess.err("Linker plugin based LTO is not supported together with \ `-C prefer-dynamic` when targeting MSVC"); } + + // PGO does not work reliably with panic=unwind on Windows. Let's make it + // an error to combine the two for now. It always runs into an assertions + // if LLVM is built with assertions, but without assertions it sometimes + // does not crash and will probably generate a corrupted binary. + if sess.opts.debugging_opts.pgo_gen.enabled() && + sess.target.target.options.is_like_windows && + sess.panic_strategy() == PanicStrategy::Unwind { + sess.err("Profile-guided optimization does not yet work in conjunction \ + with `-Cpanic=unwind` on Windows. \ + See https://github.com/rust-lang/rust/issues/61002 for details."); + } } /// Hash value constructed out of all the `-C metadata` arguments passed to the From 61735abc063e4622d1c9322d70c263a6a673bf42 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 21 May 2019 13:10:28 +0200 Subject: [PATCH 12/15] adjust deprecation date of mem::uninitialized --- src/libcore/mem.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 24bee6355a7cc..56869f38a4f6b 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -466,7 +466,7 @@ pub unsafe fn zeroed() -> T { /// [`MaybeUninit`]: union.MaybeUninit.html /// [inv]: union.MaybeUninit.html#initialization-invariant #[inline] -#[rustc_deprecated(since = "1.40.0", reason = "use `mem::MaybeUninit` instead")] +#[rustc_deprecated(since = "1.38.0", reason = "use `mem::MaybeUninit` instead")] #[stable(feature = "rust1", since = "1.0.0")] pub unsafe fn uninitialized() -> T { intrinsics::panic_if_uninhabited::(); From b07dbe1d44038b84e722cbe1efe3201e8fa934d8 Mon Sep 17 00:00:00 2001 From: topecongiro Date: Tue, 21 May 2019 22:57:34 +0900 Subject: [PATCH 13/15] Add doc comment --- src/libsyntax/parse/mod.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/libsyntax/parse/mod.rs b/src/libsyntax/parse/mod.rs index 53d6838939d05..8c1810e3efa01 100644 --- a/src/libsyntax/parse/mod.rs +++ b/src/libsyntax/parse/mod.rs @@ -330,6 +330,16 @@ pub fn stream_to_parser(sess: &ParseSess, stream: TokenStream) -> Parser<'_> { } /// Given stream, the `ParseSess` and the base directory, produces a parser. +/// +/// Use this function when you are creating a parser from the token stream +/// and also care about the current working directory of the parser (e.g., +/// you are trying to resolve modules defined inside a macro invocation). +/// +/// # Note +/// +/// The main usage of this function is outside of rustc, for those who uses +/// libsyntax as a library. Please do not remove this function while refactoring +/// just because it is not used in rustc codebase! pub fn stream_to_parser_with_base_dir<'a>(sess: &'a ParseSess, stream: TokenStream, base_dir: Directory<'a>) -> Parser<'a> { From 1f1a9176e7963b43155ce56d2a4cea3bee4a4f7e Mon Sep 17 00:00:00 2001 From: topecongiro Date: Tue, 21 May 2019 23:17:59 +0900 Subject: [PATCH 14/15] Fix tidy: remove a trailing whitespace --- src/libsyntax/parse/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsyntax/parse/mod.rs b/src/libsyntax/parse/mod.rs index 8c1810e3efa01..d0af1afd8fdbd 100644 --- a/src/libsyntax/parse/mod.rs +++ b/src/libsyntax/parse/mod.rs @@ -334,7 +334,7 @@ pub fn stream_to_parser(sess: &ParseSess, stream: TokenStream) -> Parser<'_> { /// Use this function when you are creating a parser from the token stream /// and also care about the current working directory of the parser (e.g., /// you are trying to resolve modules defined inside a macro invocation). -/// +/// /// # Note /// /// The main usage of this function is outside of rustc, for those who uses From a2168b0259f67ca478255239cdde9b76603b08c8 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 21 May 2019 17:14:09 +0200 Subject: [PATCH 15/15] update doc comment --- src/librustc_data_structures/macros.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/librustc_data_structures/macros.rs b/src/librustc_data_structures/macros.rs index 40899c414722e..b851263aaf9c4 100644 --- a/src/librustc_data_structures/macros.rs +++ b/src/librustc_data_structures/macros.rs @@ -1,5 +1,4 @@ -/// A simple static assertion macro. The first argument should be a unique -/// ALL_CAPS identifier that describes the condition. +/// A simple static assertion macro. #[macro_export] #[allow_internal_unstable(type_ascription, underscore_const_names)] macro_rules! static_assert {