From 318c5d6c963d0e5d8ece89e804b4e696c6011955 Mon Sep 17 00:00:00 2001 From: Stephane Raux Date: Mon, 8 Jul 2019 21:34:36 -0700 Subject: Clarify `Box` representation and its use in FFI This officializes what was only shown as a code example in [the unsafe code guidelines](https://rust-lang.github.io/unsafe-code-guidelines/layout/function-pointers.html?highlight=box#use) and follows [the discussion](https://github.com/rust-lang/unsafe-code-guidelines/issues/157) in the corresponding repository. It is also related to [the issue](https://github.com/rust-lang/rust/issues/52976) regarding marking `Box` `#[repr(transparent)]`. --- src/liballoc/boxed.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'src') diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index 01dee0a3943..50174c3f279 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -63,6 +63,28 @@ //! T` obtained from `Box::::into_raw` may be deallocated using the //! [`Global`] allocator with `Layout::for_value(&*value)`. //! +//! `Box` has the same representation as `*mut T`. In particular, when +//! `T: Sized`, this means that `Box` has the same representation as +//! a C pointer, making the following code valid in FFI: +//! +//! ```c +//! /* C header */ +//! struct Foo* foo(); /* Returns ownership */ +//! void bar(struct Foo*); /* `bar` takes ownership */ +//! ``` +//! +//! ``` +//! #[repr(C)] +//! pub struct Foo; +//! +//! #[no_mangle] +//! pub extern "C" fn foo() -> Box { +//! Box::new(Foo) +//! } +//! +//! #[no_mangle] +//! pub extern "C" fn bar(_: Option>) {} +//! ``` //! //! [dereferencing]: ../../std/ops/trait.Deref.html //! [`Box`]: struct.Box.html -- cgit 1.4.1-3-g733a5 From aea94230c476fea2ee073a1bc672125e4586d4b5 Mon Sep 17 00:00:00 2001 From: Stephane Raux Date: Sun, 25 Aug 2019 23:25:56 -0700 Subject: Update Box representation comment based on reviews --- src/liballoc/boxed.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index 50174c3f279..b2b23cc9671 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -63,9 +63,8 @@ //! T` obtained from `Box::::into_raw` may be deallocated using the //! [`Global`] allocator with `Layout::for_value(&*value)`. //! -//! `Box` has the same representation as `*mut T`. In particular, when -//! `T: Sized`, this means that `Box` has the same representation as -//! a C pointer, making the following code valid in FFI: +//! `Box` has the same ABI as `&mut T`. In particular, when `T: Sized`, +//! this allows using `Box` in FFI: //! //! ```c //! /* C header */ -- cgit 1.4.1-3-g733a5 From 812ec6a3bf775c1564ed3b12374c4ee81bfa94b8 Mon Sep 17 00:00:00 2001 From: Stephane Raux Date: Wed, 30 Oct 2019 09:43:04 -0700 Subject: Update FFI example - Use meaningful names - Clarify comments - Fix C function declaration --- src/liballoc/boxed.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index b2b23cc9671..a502a5b0a0b 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -68,8 +68,8 @@ //! //! ```c //! /* C header */ -//! struct Foo* foo(); /* Returns ownership */ -//! void bar(struct Foo*); /* `bar` takes ownership */ +//! struct Foo* foo_new(void); /* Returns ownership to the caller */ +//! void foo_delete(struct Foo*); /* Takes ownership from the caller */ //! ``` //! //! ``` @@ -77,12 +77,12 @@ //! pub struct Foo; //! //! #[no_mangle] -//! pub extern "C" fn foo() -> Box { +//! pub extern "C" fn foo_new() -> Box { //! Box::new(Foo) //! } //! //! #[no_mangle] -//! pub extern "C" fn bar(_: Option>) {} +//! pub extern "C" fn foo_delete(_: Option>) {} //! ``` //! //! [dereferencing]: ../../std/ops/trait.Deref.html -- cgit 1.4.1-3-g733a5 From 3aa99020250e1369dfac10952625d4ad6d9d8190 Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Tue, 3 Dec 2019 12:35:09 +0100 Subject: Fix #66295 --- src/librustc_lint/unused.rs | 8 ++++++-- src/test/ui/lint/lint-unnecessary-parens.rs | 9 +++++++++ src/test/ui/lint/lint-unnecessary-parens.stderr | 22 +++++++++++----------- 3 files changed, 26 insertions(+), 13 deletions(-) (limited to 'src') diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index f7de7ec7e18..25c351f6234 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -17,7 +17,7 @@ use syntax::print::pprust; use syntax::symbol::{kw, sym}; use syntax::symbol::Symbol; use syntax::util::parser; -use syntax_pos::{Span, BytePos}; +use syntax_pos::{MultiSpan, Span, BytePos}; use log::debug; @@ -355,7 +355,11 @@ impl UnusedParens { match value.kind { ast::ExprKind::Paren(ref inner) => { if !Self::is_expr_parens_necessary(inner, followed_by_block) && - value.attrs.is_empty() { + value.attrs.is_empty() && + !MultiSpan::from(value.span).primary_span() + .map(|span| span.from_expansion()) + .unwrap_or(false) + { let expr_text = if let Ok(snippet) = cx.sess().source_map() .span_to_snippet(value.span) { snippet diff --git a/src/test/ui/lint/lint-unnecessary-parens.rs b/src/test/ui/lint/lint-unnecessary-parens.rs index 9f42b855a87..12ffb6d3c66 100644 --- a/src/test/ui/lint/lint-unnecessary-parens.rs +++ b/src/test/ui/lint/lint-unnecessary-parens.rs @@ -25,6 +25,12 @@ fn passes_unused_parens_lint() -> &'static (dyn Trait) { panic!() } +macro_rules! baz { + ($($foo:expr),+) => { + ($($foo),*) + } +} + fn main() { foo(); bar((true)); //~ ERROR unnecessary parentheses around function argument @@ -55,4 +61,7 @@ fn main() { let mut _a = (0); //~ ERROR unnecessary parentheses around assigned value _a = (0); //~ ERROR unnecessary parentheses around assigned value _a += (1); //~ ERROR unnecessary parentheses around assigned value + + let _a = baz!(3, 4); + let _b = baz!(3); } diff --git a/src/test/ui/lint/lint-unnecessary-parens.stderr b/src/test/ui/lint/lint-unnecessary-parens.stderr index adc1069b64d..541ae7aa4b5 100644 --- a/src/test/ui/lint/lint-unnecessary-parens.stderr +++ b/src/test/ui/lint/lint-unnecessary-parens.stderr @@ -23,25 +23,25 @@ LL | fn unused_parens_around_return_type() -> (u32) { | ^^^^^ help: remove these parentheses error: unnecessary parentheses around function argument - --> $DIR/lint-unnecessary-parens.rs:30:9 + --> $DIR/lint-unnecessary-parens.rs:36:9 | LL | bar((true)); | ^^^^^^ help: remove these parentheses error: unnecessary parentheses around `if` condition - --> $DIR/lint-unnecessary-parens.rs:32:8 + --> $DIR/lint-unnecessary-parens.rs:38:8 | LL | if (true) {} | ^^^^^^ help: remove these parentheses error: unnecessary parentheses around `while` condition - --> $DIR/lint-unnecessary-parens.rs:33:11 + --> $DIR/lint-unnecessary-parens.rs:39:11 | LL | while (true) {} | ^^^^^^ help: remove these parentheses warning: denote infinite loops with `loop { ... }` - --> $DIR/lint-unnecessary-parens.rs:33:5 + --> $DIR/lint-unnecessary-parens.rs:39:5 | LL | while (true) {} | ^^^^^^^^^^^^ help: use `loop` @@ -49,43 +49,43 @@ LL | while (true) {} = note: `#[warn(while_true)]` on by default error: unnecessary parentheses around `match` head expression - --> $DIR/lint-unnecessary-parens.rs:35:11 + --> $DIR/lint-unnecessary-parens.rs:41:11 | LL | match (true) { | ^^^^^^ help: remove these parentheses error: unnecessary parentheses around `let` head expression - --> $DIR/lint-unnecessary-parens.rs:38:16 + --> $DIR/lint-unnecessary-parens.rs:44:16 | LL | if let 1 = (1) {} | ^^^ help: remove these parentheses error: unnecessary parentheses around `let` head expression - --> $DIR/lint-unnecessary-parens.rs:39:19 + --> $DIR/lint-unnecessary-parens.rs:45:19 | LL | while let 1 = (2) {} | ^^^ help: remove these parentheses error: unnecessary parentheses around method argument - --> $DIR/lint-unnecessary-parens.rs:53:24 + --> $DIR/lint-unnecessary-parens.rs:59:24 | LL | X { y: false }.foo((true)); | ^^^^^^ help: remove these parentheses error: unnecessary parentheses around assigned value - --> $DIR/lint-unnecessary-parens.rs:55:18 + --> $DIR/lint-unnecessary-parens.rs:61:18 | LL | let mut _a = (0); | ^^^ help: remove these parentheses error: unnecessary parentheses around assigned value - --> $DIR/lint-unnecessary-parens.rs:56:10 + --> $DIR/lint-unnecessary-parens.rs:62:10 | LL | _a = (0); | ^^^ help: remove these parentheses error: unnecessary parentheses around assigned value - --> $DIR/lint-unnecessary-parens.rs:57:11 + --> $DIR/lint-unnecessary-parens.rs:63:11 | LL | _a += (1); | ^^^ help: remove these parentheses -- cgit 1.4.1-3-g733a5 From cc4ced88404d417fadb94d706b184f01d6064fbe Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Tue, 3 Dec 2019 14:07:45 +0000 Subject: Apply suggestions from code review Co-Authored-By: lzutao --- src/librustc_lint/unused.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'src') diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index 25c351f6234..ee644acd099 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -357,8 +357,7 @@ impl UnusedParens { if !Self::is_expr_parens_necessary(inner, followed_by_block) && value.attrs.is_empty() && !MultiSpan::from(value.span).primary_span() - .map(|span| span.from_expansion()) - .unwrap_or(false) + .map_or(false, |span| span.from_expansion()) { let expr_text = if let Ok(snippet) = cx.sess().source_map() .span_to_snippet(value.span) { -- cgit 1.4.1-3-g733a5 From 9598cf416dcfba4e15770f57447b3996d1853fc1 Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Fri, 6 Dec 2019 15:09:01 +0100 Subject: Remove failing test case --- .../issue-47775-nested-macro-unnecessary-parens-arg.rs | 3 --- ...issue-47775-nested-macro-unnecessary-parens-arg.stderr | 15 --------------- 2 files changed, 18 deletions(-) delete mode 100644 src/test/ui/lint/issue-47775-nested-macro-unnecessary-parens-arg.stderr (limited to 'src') diff --git a/src/test/ui/lint/issue-47775-nested-macro-unnecessary-parens-arg.rs b/src/test/ui/lint/issue-47775-nested-macro-unnecessary-parens-arg.rs index ab9baa79b8b..0a951cfa91c 100644 --- a/src/test/ui/lint/issue-47775-nested-macro-unnecessary-parens-arg.rs +++ b/src/test/ui/lint/issue-47775-nested-macro-unnecessary-parens-arg.rs @@ -17,10 +17,7 @@ macro_rules! the_worship_the_heart_lifts_above { macro_rules! and_the_heavens_reject_not { () => { - // ↓ But let's test that we still lint for unused parens around - // function args inside of simple, one-deep macros. #[allow(dead_code)] fn the_night_for_the_morrow() -> Option { Some((2)) } - //~^ WARN unnecessary parentheses around function argument } } diff --git a/src/test/ui/lint/issue-47775-nested-macro-unnecessary-parens-arg.stderr b/src/test/ui/lint/issue-47775-nested-macro-unnecessary-parens-arg.stderr deleted file mode 100644 index 57cdcd70e9d..00000000000 --- a/src/test/ui/lint/issue-47775-nested-macro-unnecessary-parens-arg.stderr +++ /dev/null @@ -1,15 +0,0 @@ -warning: unnecessary parentheses around function argument - --> $DIR/issue-47775-nested-macro-unnecessary-parens-arg.rs:22:83 - | -LL | #[allow(dead_code)] fn the_night_for_the_morrow() -> Option { Some((2)) } - | ^^^ help: remove these parentheses -... -LL | and_the_heavens_reject_not!(); - | ------------------------------ in this macro invocation - | -note: lint level defined here - --> $DIR/issue-47775-nested-macro-unnecessary-parens-arg.rs:3:9 - | -LL | #![warn(unused_parens)] - | ^^^^^^^^^^^^^ - -- cgit 1.4.1-3-g733a5 From ead115949017533de244049c58f4b6886243eda7 Mon Sep 17 00:00:00 2001 From: Stephane Raux Date: Mon, 9 Dec 2019 22:49:59 -0800 Subject: Use Niko's wording --- src/liballoc/boxed.rs | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index a502a5b0a0b..a7e09d72b4a 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -63,14 +63,25 @@ //! T` obtained from `Box::::into_raw` may be deallocated using the //! [`Global`] allocator with `Layout::for_value(&*value)`. //! -//! `Box` has the same ABI as `&mut T`. In particular, when `T: Sized`, -//! this allows using `Box` in FFI: +//! So long as `T: Sized`, a `Box` is guaranteed to be represented as a +//! single pointer and is also ABI-compatible with C pointers (i.e. the C type +//! `T*`). This means that you have Rust code which passes ownership of a +//! `Box` to C code by using `Box` as the type on the Rust side, and +//! `T*` as the corresponding type on the C side. As an example, consider this +//! C header which declares functions that create and destroy some kind of +//! `Foo` value: //! //! ```c //! /* C header */ //! struct Foo* foo_new(void); /* Returns ownership to the caller */ //! void foo_delete(struct Foo*); /* Takes ownership from the caller */ //! ``` +//! +//! These two functions might be implemented in Rust as follows. Here, the +//! `struct Foo*` type from C is translated to `Box`, which captures +//! the ownership constraints. Note also that the nullable argument to +//! `foo_delete` is represented in Rust as `Option>`, since `Box` +//! cannot be null. //! //! ``` //! #[repr(C)] @@ -84,6 +95,14 @@ //! #[no_mangle] //! pub extern "C" fn foo_delete(_: Option>) {} //! ``` +//! +//! Even though `Box` has the same representation and C ABI as a C pointer, +//! this does not mean that you can convert an arbitrary `T*` into a `Box` +//! and expect things to work. `Box` values will always be fully aligned, +//! non-null pointers. Moreover, the destructor for `Box` will attempt to +//! free the value with the global allocator. In general, the best practice +//! is to only use `Box` for pointers that originated from the global +//! allocator. //! //! [dereferencing]: ../../std/ops/trait.Deref.html //! [`Box`]: struct.Box.html -- cgit 1.4.1-3-g733a5 From fe6ddd5d15c4f0108c32fb731b688f34fbeba788 Mon Sep 17 00:00:00 2001 From: Stephane Raux Date: Mon, 9 Dec 2019 22:56:37 -0800 Subject: Specify behavior when passed a null pointer --- src/liballoc/boxed.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index a7e09d72b4a..2f24086bf2c 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -73,8 +73,12 @@ //! //! ```c //! /* C header */ -//! struct Foo* foo_new(void); /* Returns ownership to the caller */ -//! void foo_delete(struct Foo*); /* Takes ownership from the caller */ +//! +//! /* Returns ownership to the caller */ +//! struct Foo* foo_new(void); +//! +//! /* Takes ownership from the caller; no-op when invoked with NULL */ +//! void foo_delete(struct Foo*); //! ``` //! //! These two functions might be implemented in Rust as follows. Here, the -- cgit 1.4.1-3-g733a5 From 1a26df77272d3bafc9e9e689a5a4a314896050f5 Mon Sep 17 00:00:00 2001 From: Stephane Raux Date: Tue, 10 Dec 2019 00:00:25 -0800 Subject: Remove trailing whitespace --- src/liballoc/boxed.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index 2f24086bf2c..c3732b245f4 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -73,14 +73,14 @@ //! //! ```c //! /* C header */ -//! +//! //! /* Returns ownership to the caller */ //! struct Foo* foo_new(void); -//! +//! //! /* Takes ownership from the caller; no-op when invoked with NULL */ //! void foo_delete(struct Foo*); //! ``` -//! +//! //! These two functions might be implemented in Rust as follows. Here, the //! `struct Foo*` type from C is translated to `Box`, which captures //! the ownership constraints. Note also that the nullable argument to @@ -99,7 +99,7 @@ //! #[no_mangle] //! pub extern "C" fn foo_delete(_: Option>) {} //! ``` -//! +//! //! Even though `Box` has the same representation and C ABI as a C pointer, //! this does not mean that you can convert an arbitrary `T*` into a `Box` //! and expect things to work. `Box` values will always be fully aligned, -- cgit 1.4.1-3-g733a5 From c681841ca078d6f29776d961ba8e44196662413e Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 11 Dec 2019 14:17:41 +1100 Subject: Fix `-Z print-type-sizes`'s handling of zero-sized fields. Currently, the type `struct S { x: u32, y: u32, tag: () }` is incorrectly described like this: ``` print-type-size type: `S`: 8 bytes, alignment: 4 bytes print-type-size field `.x`: 4 bytes print-type-size field `.tag`: 0 bytes, offset: 0 bytes, alignment: 1 bytes print-type-size padding: 4 bytes print-type-size field `.y`: 4 bytes, alignment: 4 bytes ``` Specifically: - The `padding` line is wrong. (There is no padding.) - The `offset` and `alignment` on the `.tag` line shouldn't be printed. The problem is that multiple fields can end up with the same offset, and the printing code doesn't handle this correctly. This commit fixes it by adjusting the field sorting so that zero-sized fields are dealt with before non-zero-sized fields. With that in place, the printing code works correctly. The commit also corrects the "something is very wrong" comment. The new output looks like this: ``` print-type-size type: `S`: 8 bytes, alignment: 4 bytes print-type-size field `.tag`: 0 bytes print-type-size field `.x`: 4 bytes print-type-size field `.y`: 4 bytes ``` --- src/librustc_session/code_stats.rs | 9 +++-- src/test/ui/print_type_sizes/zero-sized-fields.rs | 46 ++++++++++++++++++++++ .../ui/print_type_sizes/zero-sized-fields.stdout | 16 ++++++++ 3 files changed, 68 insertions(+), 3 deletions(-) create mode 100644 src/test/ui/print_type_sizes/zero-sized-fields.rs create mode 100644 src/test/ui/print_type_sizes/zero-sized-fields.stdout (limited to 'src') diff --git a/src/librustc_session/code_stats.rs b/src/librustc_session/code_stats.rs index 5baf0c5948f..764d6d8eaee 100644 --- a/src/librustc_session/code_stats.rs +++ b/src/librustc_session/code_stats.rs @@ -132,9 +132,12 @@ impl CodeStats { let mut min_offset = discr_size; - // We want to print fields by increasing offset. + // We want to print fields by increasing offset. We also want + // zero-sized fields before non-zero-sized fields, otherwise + // the loop below goes wrong; hence the `f.size` in the sort + // key. let mut fields = fields.clone(); - fields.sort_by_key(|f| f.offset); + fields.sort_by_key(|f| (f.offset, f.size)); for field in fields.iter() { let FieldInfo { ref name, offset, size, align } = *field; @@ -146,7 +149,7 @@ impl CodeStats { } if offset < min_offset { - // if this happens something is very wrong + // If this happens it's probably a union. println!("print-type-size {}field `.{}`: {} bytes, \ offset: {} bytes, \ alignment: {} bytes", diff --git a/src/test/ui/print_type_sizes/zero-sized-fields.rs b/src/test/ui/print_type_sizes/zero-sized-fields.rs new file mode 100644 index 00000000000..2ad488e8d8f --- /dev/null +++ b/src/test/ui/print_type_sizes/zero-sized-fields.rs @@ -0,0 +1,46 @@ +// compile-flags: -Z print-type-sizes +// build-pass (FIXME(62277): could be check-pass?) + +// At one point, zero-sized fields such as those in this file were causing +// incorrect output from `-Z print-type-sizes`. + +#![feature(start)] + +struct S1 { + x: u32, + y: u32, + tag: (), +} + +struct Void(); +struct Empty {} + +struct S5 { + tagw: TagW, + w: u32, + unit: (), + x: u32, + void: Void, + y: u32, + empty: Empty, + z: u32, + tagz: TagZ, +} + +#[start] +fn start(_: isize, _: *const *const u8) -> isize { + let _s1: S1 = S1 { x: 0, y: 0, tag: () }; + + let _s5: S5<(), Empty> = S5 { + tagw: (), + w: 1, + unit: (), + x: 2, + void: Void(), + y: 3, + empty: Empty {}, + z: 4, + tagz: Empty {}, + }; + 0 +} diff --git a/src/test/ui/print_type_sizes/zero-sized-fields.stdout b/src/test/ui/print_type_sizes/zero-sized-fields.stdout new file mode 100644 index 00000000000..72f59c4bb57 --- /dev/null +++ b/src/test/ui/print_type_sizes/zero-sized-fields.stdout @@ -0,0 +1,16 @@ +print-type-size type: `S5<(), Empty>`: 16 bytes, alignment: 4 bytes +print-type-size field `.tagw`: 0 bytes +print-type-size field `.unit`: 0 bytes +print-type-size field `.void`: 0 bytes +print-type-size field `.empty`: 0 bytes +print-type-size field `.tagz`: 0 bytes +print-type-size field `.w`: 4 bytes +print-type-size field `.x`: 4 bytes +print-type-size field `.y`: 4 bytes +print-type-size field `.z`: 4 bytes +print-type-size type: `S1`: 8 bytes, alignment: 4 bytes +print-type-size field `.tag`: 0 bytes +print-type-size field `.x`: 4 bytes +print-type-size field `.y`: 4 bytes +print-type-size type: `Empty`: 0 bytes, alignment: 1 bytes +print-type-size type: `Void`: 0 bytes, alignment: 1 bytes -- cgit 1.4.1-3-g733a5 From cb1cc1181e884d0f53c444af0b6a21189af83bea Mon Sep 17 00:00:00 2001 From: Stephane Raux Date: Tue, 10 Dec 2019 22:18:17 -0800 Subject: Fix description based on review --- src/liballoc/boxed.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index c3732b245f4..d50b4b8dc4a 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -65,7 +65,7 @@ //! //! So long as `T: Sized`, a `Box` is guaranteed to be represented as a //! single pointer and is also ABI-compatible with C pointers (i.e. the C type -//! `T*`). This means that you have Rust code which passes ownership of a +//! `T*`). This means that you can have Rust code which passes ownership of a //! `Box` to C code by using `Box` as the type on the Rust side, and //! `T*` as the corresponding type on the C side. As an example, consider this //! C header which declares functions that create and destroy some kind of -- cgit 1.4.1-3-g733a5 From ab3f4fd709fc2fcb19cda89305da08d9ae989c47 Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Wed, 11 Dec 2019 10:10:41 +0100 Subject: Apply review suggestions --- src/librustc_lint/unused.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index ee644acd099..e6f39cca6dc 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -17,7 +17,7 @@ use syntax::print::pprust; use syntax::symbol::{kw, sym}; use syntax::symbol::Symbol; use syntax::util::parser; -use syntax_pos::{MultiSpan, Span, BytePos}; +use syntax_pos::{Span, BytePos}; use log::debug; @@ -356,8 +356,7 @@ impl UnusedParens { ast::ExprKind::Paren(ref inner) => { if !Self::is_expr_parens_necessary(inner, followed_by_block) && value.attrs.is_empty() && - !MultiSpan::from(value.span).primary_span() - .map_or(false, |span| span.from_expansion()) + !value.span.from_expansion() { let expr_text = if let Ok(snippet) = cx.sess().source_map() .span_to_snippet(value.span) { -- cgit 1.4.1-3-g733a5 From fafa4897985f932490960e90ddd2ff39134e967e Mon Sep 17 00:00:00 2001 From: Nicholas Matsakis Date: Wed, 11 Dec 2019 10:32:11 -0500 Subject: clarify that `Box` should only be used when defined *in Rust* --- src/liballoc/boxed.rs | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index d50b4b8dc4a..c25495bec41 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -63,13 +63,14 @@ //! T` obtained from `Box::::into_raw` may be deallocated using the //! [`Global`] allocator with `Layout::for_value(&*value)`. //! -//! So long as `T: Sized`, a `Box` is guaranteed to be represented as a -//! single pointer and is also ABI-compatible with C pointers (i.e. the C type -//! `T*`). This means that you can have Rust code which passes ownership of a -//! `Box` to C code by using `Box` as the type on the Rust side, and -//! `T*` as the corresponding type on the C side. As an example, consider this -//! C header which declares functions that create and destroy some kind of -//! `Foo` value: +//! So long as `T: Sized`, a `Box` is guaranteed to be represented +//! as a single pointer and is also ABI-compatible with C pointers +//! (i.e. the C type `T*`). This means that if you have extern "C" +//! Rust functions that will be called from C, you can define those +//! Rust functions using `Box` types, and use `T*` as corresponding +//! type on the C side. As an example, consider this C header which +//! declares functions that create and destroy some kind of `Foo` +//! value: //! //! ```c //! /* C header */ @@ -108,6 +109,14 @@ //! is to only use `Box` for pointers that originated from the global //! allocator. //! +//! **Important.** At least at present, you should avoid using +//! `Box` types for functions that are defined in C but invoked +//! from Rust. In those cases, you should directly mirror the C types +//! as closely as possible. Using types like `Box` where the C +//! definition is just using `T*` can lead to undefined behavior, as +//! described in [rust-lang/unsafe-code-guidelines#198][ucg#198]. +//! +//! [ucg#198]: https://github.com/rust-lang/unsafe-code-guidelines/issues/198 //! [dereferencing]: ../../std/ops/trait.Deref.html //! [`Box`]: struct.Box.html //! [`Global`]: ../alloc/struct.Global.html -- cgit 1.4.1-3-g733a5 From e4852da576bda4a5d26341961e0be7d6626ff5c0 Mon Sep 17 00:00:00 2001 From: chansuke Date: Thu, 12 Dec 2019 01:00:37 +0900 Subject: Remove irelevant comment on `register_dtor` --- src/libstd/sys/unix/fast_thread_local.rs | 2 -- 1 file changed, 2 deletions(-) (limited to 'src') diff --git a/src/libstd/sys/unix/fast_thread_local.rs b/src/libstd/sys/unix/fast_thread_local.rs index dfb9307daea..0861432f8a9 100644 --- a/src/libstd/sys/unix/fast_thread_local.rs +++ b/src/libstd/sys/unix/fast_thread_local.rs @@ -8,8 +8,6 @@ // Note, however, that we run on lots older linuxes, as well as cross // compiling from a newer linux to an older linux, so we also have a // fallback implementation to use as well. -// -// Due to rust-lang/rust#18804, make sure this is not generic! #[cfg(any( target_os = "linux", target_os = "fuchsia", -- cgit 1.4.1-3-g733a5 From 914c9aa78d7d43d268690e2cf1ae634795cd29aa Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Wed, 11 Dec 2019 21:58:00 +0300 Subject: resolve: Always resolve visibilities on impl items --- src/librustc_resolve/build_reduced_graph.rs | 24 ++++++++++----------- src/test/ui/resolve/impl-items-vis-unresolved.rs | 25 ++++++++++++++++++++++ .../ui/resolve/impl-items-vis-unresolved.stderr | 9 ++++++++ 3 files changed, 46 insertions(+), 12 deletions(-) create mode 100644 src/test/ui/resolve/impl-items-vis-unresolved.rs create mode 100644 src/test/ui/resolve/impl-items-vis-unresolved.stderr (limited to 'src') diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 566ba129074..e2578d67e73 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -647,8 +647,6 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { self.r.define(parent, ident, TypeNS, imported_binding); } - ItemKind::GlobalAsm(..) => {} - ItemKind::Mod(..) if ident.name == kw::Invalid => {} // Crate root ItemKind::Mod(..) => { @@ -667,9 +665,6 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { self.parent_scope.module = module; } - // Handled in `rustc_metadata::{native_libs,link_args}` - ItemKind::ForeignMod(..) => {} - // These items live in the value namespace. ItemKind::Static(..) => { let res = Res::Def(DefKind::Static, self.r.definitions.local_def_id(item.id)); @@ -765,12 +760,6 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { self.insert_field_names_local(def_id, vdata); } - ItemKind::Impl(.., ref impl_items) => { - for impl_item in impl_items { - self.resolve_visibility(&impl_item.vis); - } - } - ItemKind::Trait(..) => { let def_id = self.r.definitions.local_def_id(item.id); @@ -785,6 +774,9 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { self.parent_scope.module = module; } + // These items do not add names to modules. + ItemKind::Impl(..) | ItemKind::ForeignMod(..) | ItemKind::GlobalAsm(..) => {} + ItemKind::MacroDef(..) | ItemKind::Mac(_) => unreachable!(), } } @@ -1118,7 +1110,6 @@ macro_rules! method { } impl<'a, 'b> Visitor<'b> for BuildReducedGraphVisitor<'a, 'b> { - method!(visit_impl_item: ast::ImplItem, ast::ImplItemKind::Macro, walk_impl_item); method!(visit_expr: ast::Expr, ast::ExprKind::Mac, walk_expr); method!(visit_pat: ast::Pat, ast::PatKind::Mac, walk_pat); method!(visit_ty: ast::Ty, ast::TyKind::Mac, walk_ty); @@ -1202,6 +1193,15 @@ impl<'a, 'b> Visitor<'b> for BuildReducedGraphVisitor<'a, 'b> { visit::walk_trait_item(self, item); } + fn visit_impl_item(&mut self, item: &'b ast::ImplItem) { + if let ast::ImplItemKind::Macro(..) = item.kind { + self.visit_invoc(item.id); + } else { + self.resolve_visibility(&item.vis); + visit::walk_impl_item(self, item); + } + } + fn visit_token(&mut self, t: Token) { if let token::Interpolated(nt) = t.kind { if let token::NtExpr(ref expr) = *nt { diff --git a/src/test/ui/resolve/impl-items-vis-unresolved.rs b/src/test/ui/resolve/impl-items-vis-unresolved.rs new file mode 100644 index 00000000000..9b4fe498239 --- /dev/null +++ b/src/test/ui/resolve/impl-items-vis-unresolved.rs @@ -0,0 +1,25 @@ +// Visibilities on impl items expanded from macros are resolved (issue #64705). + +macro_rules! perftools_inline { + ($($item:tt)*) => ( + $($item)* + ); +} + +mod state { + pub struct RawFloatState; + impl RawFloatState { + perftools_inline! { + pub(super) fn new() {} // OK + } + } +} + +pub struct RawFloatState; +impl RawFloatState { + perftools_inline! { + pub(super) fn new() {} //~ ERROR failed to resolve: there are too many initial `super`s + } +} + +fn main() {} diff --git a/src/test/ui/resolve/impl-items-vis-unresolved.stderr b/src/test/ui/resolve/impl-items-vis-unresolved.stderr new file mode 100644 index 00000000000..8e285e53124 --- /dev/null +++ b/src/test/ui/resolve/impl-items-vis-unresolved.stderr @@ -0,0 +1,9 @@ +error[E0433]: failed to resolve: there are too many initial `super`s. + --> $DIR/impl-items-vis-unresolved.rs:21:13 + | +LL | pub(super) fn new() {} + | ^^^^^ there are too many initial `super`s. + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0433`. -- cgit 1.4.1-3-g733a5 From 7f87dd1bc008d49cc27bee8d5a866e3c2de463b3 Mon Sep 17 00:00:00 2001 From: Andre Bogus Date: Wed, 11 Dec 2019 20:49:46 +0100 Subject: Some small readability improvements --- src/libcore/char/methods.rs | 15 +++++---------- src/libcore/str/pattern.rs | 7 +------ 2 files changed, 6 insertions(+), 16 deletions(-) (limited to 'src') diff --git a/src/libcore/char/methods.rs b/src/libcore/char/methods.rs index 1ec614edbe2..5c63eebf595 100644 --- a/src/libcore/char/methods.rs +++ b/src/libcore/char/methods.rs @@ -553,8 +553,7 @@ impl char { pub fn is_alphabetic(self) -> bool { match self { 'a'..='z' | 'A'..='Z' => true, - c if c > '\x7f' => derived_property::Alphabetic(c), - _ => false, + c => c > '\x7f' && derived_property::Alphabetic(c), } } @@ -585,8 +584,7 @@ impl char { pub fn is_lowercase(self) -> bool { match self { 'a'..='z' => true, - c if c > '\x7f' => derived_property::Lowercase(c), - _ => false, + c => c > '\x7f' && derived_property::Lowercase(c), } } @@ -617,8 +615,7 @@ impl char { pub fn is_uppercase(self) -> bool { match self { 'A'..='Z' => true, - c if c > '\x7f' => derived_property::Uppercase(c), - _ => false, + c => c > '\x7f' && derived_property::Uppercase(c), } } @@ -646,8 +643,7 @@ impl char { pub fn is_whitespace(self) -> bool { match self { ' ' | '\x09'..='\x0d' => true, - c if c > '\x7f' => property::White_Space(c), - _ => false, + c => c > '\x7f' && property::White_Space(c), } } @@ -744,8 +740,7 @@ impl char { pub fn is_numeric(self) -> bool { match self { '0'..='9' => true, - c if c > '\x7f' => general_category::N(c), - _ => false, + c => c > '\x7f' && general_category::N(c), } } diff --git a/src/libcore/str/pattern.rs b/src/libcore/str/pattern.rs index a494274118a..1037da14b5f 100644 --- a/src/libcore/str/pattern.rs +++ b/src/libcore/str/pattern.rs @@ -296,12 +296,7 @@ unsafe impl<'a> Searcher<'a> for CharSearcher<'a> { fn next_match(&mut self) -> Option<(usize, usize)> { loop { // get the haystack after the last character found - let bytes = if let Some(slice) = self.haystack.as_bytes() - .get(self.finger..self.finger_back) { - slice - } else { - return None; - }; + let bytes = self.haystack.as_bytes().get(self.finger..self.finger_back)?; // the last byte of the utf8 encoded needle let last_byte = unsafe { *self.utf8_encoded.get_unchecked(self.utf8_size - 1) }; if let Some(index) = memchr::memchr(last_byte, bytes) { -- cgit 1.4.1-3-g733a5 From 24227977852b22eb0ab3a228b7135611ae49bb8d Mon Sep 17 00:00:00 2001 From: Andre Bogus Date: Wed, 11 Dec 2019 21:01:33 +0100 Subject: Small Cow improvements --- src/liballoc/borrow.rs | 18 +++++------------- src/liballoc/tests/cow_str.rs | 3 +++ 2 files changed, 8 insertions(+), 13 deletions(-) (limited to 'src') diff --git a/src/liballoc/borrow.rs b/src/liballoc/borrow.rs index d2bdda83fa9..fc960451968 100644 --- a/src/liballoc/borrow.rs +++ b/src/liballoc/borrow.rs @@ -195,14 +195,10 @@ impl Clone for Cow<'_, B> { } fn clone_from(&mut self, source: &Self) { - if let Owned(ref mut dest) = *self { - if let Owned(ref o) = *source { - o.borrow().clone_into(dest); - return; - } + match (self, source) { + (&mut Owned(ref mut dest), &Owned(ref o)) => o.borrow().clone_into(dest), + (t, s) => *t = s.clone(), } - - *self = source.clone(); } } @@ -449,9 +445,7 @@ impl<'a> AddAssign<&'a str> for Cow<'a, str> { fn add_assign(&mut self, rhs: &'a str) { if self.is_empty() { *self = Cow::Borrowed(rhs) - } else if rhs.is_empty() { - return; - } else { + } else if !rhs.is_empty() { if let Cow::Borrowed(lhs) = *self { let mut s = String::with_capacity(lhs.len() + rhs.len()); s.push_str(lhs); @@ -467,9 +461,7 @@ impl<'a> AddAssign> for Cow<'a, str> { fn add_assign(&mut self, rhs: Cow<'a, str>) { if self.is_empty() { *self = rhs - } else if rhs.is_empty() { - return; - } else { + } else if !rhs.is_empty() { if let Cow::Borrowed(lhs) = *self { let mut s = String::with_capacity(lhs.len() + rhs.len()); s.push_str(lhs); diff --git a/src/liballoc/tests/cow_str.rs b/src/liballoc/tests/cow_str.rs index 6f357eda9b8..62a5c245a54 100644 --- a/src/liballoc/tests/cow_str.rs +++ b/src/liballoc/tests/cow_str.rs @@ -138,4 +138,7 @@ fn check_cow_clone_from() { let c2: Cow<'_, str> = Cow::Owned(s); c1.clone_from(&c2); assert!(c1.into_owned().capacity() >= 25); + let mut c3: Cow<'_, str> = Cow::Borrowed("bye"); + c3.clone_from(&c2); + assert_eq!(c2, c3); } -- cgit 1.4.1-3-g733a5 From ab3afc0f0426ea7e2801043a874dd96b2831a1eb Mon Sep 17 00:00:00 2001 From: Andre Bogus Date: Wed, 11 Dec 2019 21:05:28 +0100 Subject: Make TinyList::remove iterate instead of recurse --- src/librustc_data_structures/tiny_list.rs | 40 +++++++++++++------------------ 1 file changed, 16 insertions(+), 24 deletions(-) (limited to 'src') diff --git a/src/librustc_data_structures/tiny_list.rs b/src/librustc_data_structures/tiny_list.rs index 371f0f6fa0b..78cbc1240b1 100644 --- a/src/librustc_data_structures/tiny_list.rs +++ b/src/librustc_data_structures/tiny_list.rs @@ -16,41 +16,29 @@ mod tests; #[derive(Clone)] pub struct TinyList { - head: Option> + head: Option>, } impl TinyList { #[inline] pub fn new() -> TinyList { - TinyList { - head: None - } + TinyList { head: None } } #[inline] pub fn new_single(data: T) -> TinyList { - TinyList { - head: Some(Element { - data, - next: None, - }) - } + TinyList { head: Some(Element { data, next: None }) } } #[inline] pub fn insert(&mut self, data: T) { - self.head = Some(Element { - data, - next: self.head.take().map(Box::new) - }); + self.head = Some(Element { data, next: self.head.take().map(Box::new) }); } #[inline] pub fn remove(&mut self, data: &T) -> bool { self.head = match self.head { - Some(ref mut head) if head.data == *data => { - head.next.take().map(|x| *x) - } + Some(ref mut head) if head.data == *data => head.next.take().map(|x| *x), Some(ref mut head) => return head.remove_next(data), None => return false, }; @@ -88,12 +76,16 @@ struct Element { impl Element { fn remove_next(&mut self, data: &T) -> bool { - let new_next = match self.next { - Some(ref mut next) if next.data == *data => next.next.take(), - Some(ref mut next) => return next.remove_next(data), - None => return false, - }; - self.next = new_next; - true + let mut n = self; + loop { + match n.next { + Some(ref mut next) if next.data == *data => { + n.next = next.next.take(); + return true; + } + Some(ref mut next) => n = next, + None => return false, + } + } } } -- cgit 1.4.1-3-g733a5