From 2131b153b33f7ba2dc0a7cd3ea4efd1bc4a03ba2 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sun, 3 Mar 2019 19:04:26 +0000 Subject: [PATCH 01/21] Check which blocks are cleanup in mir-opt tests --- src/librustc_mir/util/pretty.rs | 5 ++--- src/test/mir-opt/basic_assignment.rs | 2 +- src/test/mir-opt/box_expr.rs | 8 ++++---- src/test/mir-opt/issue-38669.rs | 2 +- src/test/mir-opt/issue-49232.rs | 2 +- src/test/mir-opt/loop_test.rs | 2 +- src/test/mir-opt/match_false_edges.rs | 6 +++--- src/test/mir-opt/packed-struct-drop-aligned.rs | 4 ++-- src/test/mir-opt/remove_fake_borrows.rs | 4 ++-- src/test/mir-opt/unusual-item-types.rs | 10 +++++----- 10 files changed, 22 insertions(+), 23 deletions(-) diff --git a/src/librustc_mir/util/pretty.rs b/src/librustc_mir/util/pretty.rs index c3fbee3a2a6e5..5000eb55b09f6 100644 --- a/src/librustc_mir/util/pretty.rs +++ b/src/librustc_mir/util/pretty.rs @@ -317,9 +317,8 @@ where let data = &mir[block]; // Basic block label at the top. - let cleanup_text = if data.is_cleanup { " // cleanup" } else { "" }; - let lbl = format!("{}{:?}: {{", INDENT, block); - writeln!(w, "{0:1$}{2}", lbl, ALIGN, cleanup_text)?; + let cleanup_text = if data.is_cleanup { " (cleanup)" } else { "" }; + writeln!(w, "{}{:?}{}: {{", INDENT, block, cleanup_text)?; // List of statements in the middle. let mut current_location = Location { diff --git a/src/test/mir-opt/basic_assignment.rs b/src/test/mir-opt/basic_assignment.rs index 1bbbe67a12cb8..c6ca6e41c2da4 100644 --- a/src/test/mir-opt/basic_assignment.rs +++ b/src/test/mir-opt/basic_assignment.rs @@ -48,7 +48,7 @@ fn main() { // drop(_6) -> [return: bb6, unwind: bb4]; // } // ... -// bb5: { +// bb5 (cleanup): { // drop(_6) -> bb4; // } // END rustc.main.SimplifyCfg-initial.after.mir diff --git a/src/test/mir-opt/box_expr.rs b/src/test/mir-opt/box_expr.rs index ad5cf42029e9d..14d302f0eea72 100644 --- a/src/test/mir-opt/box_expr.rs +++ b/src/test/mir-opt/box_expr.rs @@ -38,7 +38,7 @@ impl Drop for S { // (*_2) = const S::new() -> [return: bb2, unwind: bb3]; // } // -// bb1: { +// bb1 (cleanup): { // resume; // } // @@ -47,7 +47,7 @@ impl Drop for S { // drop(_2) -> bb4; // } // -// bb3: { +// bb3 (cleanup): { // drop(_2) -> bb1; // } // @@ -62,11 +62,11 @@ impl Drop for S { // drop(_4) -> [return: bb8, unwind: bb6]; // } // -// bb6: { +// bb6 (cleanup): { // drop(_1) -> bb1; // } // -// bb7: { +// bb7 (cleanup): { // drop(_4) -> bb6; // } // diff --git a/src/test/mir-opt/issue-38669.rs b/src/test/mir-opt/issue-38669.rs index 618ee2f740746..047e623941b71 100644 --- a/src/test/mir-opt/issue-38669.rs +++ b/src/test/mir-opt/issue-38669.rs @@ -18,7 +18,7 @@ fn main() { // FakeRead(ForLet, _1); // goto -> bb2; // } -// bb1: { +// bb1 (cleanup): { // resume; // } // bb2: { diff --git a/src/test/mir-opt/issue-49232.rs b/src/test/mir-opt/issue-49232.rs index 0f0401a55eaca..5f4f4ab82af24 100644 --- a/src/test/mir-opt/issue-49232.rs +++ b/src/test/mir-opt/issue-49232.rs @@ -43,7 +43,7 @@ fn main() { // FakeRead(ForMatchedPlace, _3); // switchInt(_3) -> [false: bb9, otherwise: bb8]; // } -// bb4: { +// bb4 (cleanup): { // resume; // } // bb5: { diff --git a/src/test/mir-opt/loop_test.rs b/src/test/mir-opt/loop_test.rs index e44743aa4b780..34891ee70b5c6 100644 --- a/src/test/mir-opt/loop_test.rs +++ b/src/test/mir-opt/loop_test.rs @@ -18,7 +18,7 @@ fn main() { // END RUST SOURCE // START rustc.main.SimplifyCfg-qualify-consts.after.mir // ... -// bb1: { // The cleanup block +// bb1 (cleanup): { // resume; // } // ... diff --git a/src/test/mir-opt/match_false_edges.rs b/src/test/mir-opt/match_false_edges.rs index ab6de71d2894d..7d3abdd79bdda 100644 --- a/src/test/mir-opt/match_false_edges.rs +++ b/src/test/mir-opt/match_false_edges.rs @@ -47,7 +47,7 @@ fn main() { // _3 = discriminant(_2); // switchInt(move _3) -> [0isize: bb4, 1isize: bb2, otherwise: bb7]; // } -// bb1: { +// bb1 (cleanup): { // resume; // } // bb2: { @@ -116,7 +116,7 @@ fn main() { // _3 = discriminant(_2); // switchInt(move _3) -> [0isize: bb3, 1isize: bb2, otherwise: bb7]; // } -// bb1: { +// bb1 (cleanup): { // resume; // } // bb2: { @@ -185,7 +185,7 @@ fn main() { // _3 = discriminant(_2); // switchInt(move _3) -> [1isize: bb2, otherwise: bb3]; // } -// bb1: { +// bb1 (cleanup): { // resume; // } // bb2: { diff --git a/src/test/mir-opt/packed-struct-drop-aligned.rs b/src/test/mir-opt/packed-struct-drop-aligned.rs index 01402f261563c..167a6eb349eb2 100644 --- a/src/test/mir-opt/packed-struct-drop-aligned.rs +++ b/src/test/mir-opt/packed-struct-drop-aligned.rs @@ -38,14 +38,14 @@ impl Drop for Droppy { // _6 = move (_1.0: Aligned); // drop(_6) -> [return: bb4, unwind: bb3]; // } -// bb1: { +// bb1 (cleanup): { // resume; // } // bb2: { // StorageDead(_1); // return; // } -// bb3: { +// bb3 (cleanup): { // (_1.0: Aligned) = move _4; // drop(_1) -> bb1; // } diff --git a/src/test/mir-opt/remove_fake_borrows.rs b/src/test/mir-opt/remove_fake_borrows.rs index 48d1c991b623e..144348450a91b 100644 --- a/src/test/mir-opt/remove_fake_borrows.rs +++ b/src/test/mir-opt/remove_fake_borrows.rs @@ -63,7 +63,7 @@ fn main() { // StorageDead(_8); // return; // } -// bb10: { +// bb10 (cleanup): { // resume; // } // END rustc.match_guard.CleanupNonCodegenStatements.before.mir @@ -114,7 +114,7 @@ fn main() { // StorageDead(_8); // return; // } -// bb10: { +// bb10 (cleanup): { // resume; // } // END rustc.match_guard.CleanupNonCodegenStatements.after.mir diff --git a/src/test/mir-opt/unusual-item-types.rs b/src/test/mir-opt/unusual-item-types.rs index fe85baa048e39..c56334f2edd14 100644 --- a/src/test/mir-opt/unusual-item-types.rs +++ b/src/test/mir-opt/unusual-item-types.rs @@ -22,7 +22,7 @@ fn main() { // _0 = const 2i32; // return; // } -// bb1: { +// bb1 (cleanup): { // resume; // } // END rustc.{{impl}}-ASSOCIATED_CONSTANT.mir_map.0.mir @@ -32,7 +32,7 @@ fn main() { // _0 = const 5isize; // return; // } -// bb1: { +// bb1 (cleanup): { // resume; // } // END rustc.E-V-{{constant}}.mir_map.0.mir @@ -44,16 +44,16 @@ fn main() { // bb1: { // return; // } -// bb2: { +// bb2 (cleanup): { // resume; // } // bb3: { // goto -> bb1; // } -// bb4: { +// bb4 (cleanup): { // goto -> bb2; // } -// bb5: { +// bb5 (cleanup): { // drop(((*_1).0: alloc::raw_vec::RawVec)) -> bb4; // } // bb6: { From 8a7801908c6b8f142a0b31f0582e526fb7369833 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sun, 3 Mar 2019 19:27:41 +0000 Subject: [PATCH 02/21] Don't incorrectly mark blocks in generator drop shims as cleanup This also ensure that dropping a generator won't leak upvars if dropping one of them panics --- src/librustc_mir/transform/generator.rs | 46 +++++++++++----------- src/test/mir-opt/generator-drop-cleanup.rs | 43 ++++++++++++++++++++ 2 files changed, 65 insertions(+), 24 deletions(-) create mode 100644 src/test/mir-opt/generator-drop-cleanup.rs diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index c455d38cebce8..9c212e36761c0 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -592,8 +592,15 @@ fn elaborate_generator_drops<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let param_env = tcx.param_env(def_id); let gen = self_arg(); - for block in mir.basic_blocks().indices() { - let (target, unwind, source_info) = match mir.basic_blocks()[block].terminator() { + let mut elaborator = DropShimElaborator { + mir: mir, + patch: MirPatch::new(mir), + tcx, + param_env + }; + + for (block, block_data) in mir.basic_blocks().iter_enumerated() { + let (target, unwind, source_info) = match block_data.terminator() { &Terminator { source_info, kind: TerminatorKind::Drop { @@ -604,31 +611,22 @@ fn elaborate_generator_drops<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, } if local == gen => (target, unwind, source_info), _ => continue, }; - let unwind = if let Some(unwind) = unwind { - Unwind::To(unwind) - } else { + let unwind = if block_data.is_cleanup { Unwind::InCleanup + } else { + Unwind::To(unwind.unwrap_or_else(|| elaborator.patch.resume_block())) }; - let patch = { - let mut elaborator = DropShimElaborator { - mir: &mir, - patch: MirPatch::new(mir), - tcx, - param_env - }; - elaborate_drop( - &mut elaborator, - source_info, - &Place::Base(PlaceBase::Local(gen)), - (), - target, - unwind, - block - ); - elaborator.patch - }; - patch.apply(mir); + elaborate_drop( + &mut elaborator, + source_info, + &Place::Base(PlaceBase::Local(gen)), + (), + target, + unwind, + block, + ); } + elaborator.patch.apply(mir); } fn create_generator_drop_shim<'a, 'tcx>( diff --git a/src/test/mir-opt/generator-drop-cleanup.rs b/src/test/mir-opt/generator-drop-cleanup.rs new file mode 100644 index 0000000000000..48398691271ba --- /dev/null +++ b/src/test/mir-opt/generator-drop-cleanup.rs @@ -0,0 +1,43 @@ +#![feature(generators, generator_trait)] + +// Regression test for #58892, generator drop shims should not have blocks +// spuriously marked as cleanup + +fn main() { + let gen = || { + yield; + }; +} + +// END RUST SOURCE + +// START rustc.main-{{closure}}.generator_drop.0.mir +// bb0: { +// switchInt(((*_1).0: u32)) -> [0u32: bb4, 3u32: bb7, otherwise: bb8]; +// } +// bb1: { +// goto -> bb5; +// } +// bb2: { +// return; +// } +// bb3: { +// return; +// } +// bb4: { +// goto -> bb6; +// } +// bb5: { +// goto -> bb2; +// } +// bb6: { +// goto -> bb3; +// } +// bb7: { +// StorageLive(_3); +// goto -> bb1; +// } +// bb8: { +// return; +// } +// END rustc.main-{{closure}}.generator_drop.0.mir From 5e68c5708792a945b9e1dc5b2b0299fec629a509 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sun, 3 Mar 2019 19:28:05 +0000 Subject: [PATCH 03/21] Use the correct state for poisoning a generator --- src/librustc_mir/transform/generator.rs | 33 +++++++++++-------- .../run-fail/generator-resume-after-panic.rs | 22 +++++++++++++ 2 files changed, 42 insertions(+), 13 deletions(-) create mode 100644 src/test/run-fail/generator-resume-after-panic.rs diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index 9c212e36761c0..38fc1243f24af 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -26,7 +26,7 @@ //! } //! //! This pass computes the meaning of the state field and the MIR locals which are live -//! across a suspension point. There are however two hardcoded generator states: +//! across a suspension point. There are however three hardcoded generator states: //! 0 - Generator have not been resumed yet //! 1 - Generator has returned / is completed //! 2 - Generator has been poisoned @@ -144,6 +144,13 @@ fn self_arg() -> Local { Local::new(1) } +/// Generator have not been resumed yet +const UNRESUMED: u32 = 0; +/// Generator has returned / is completed +const RETURNED: u32 = 1; +/// Generator has been poisoned +const POISONED: u32 = 2; + struct SuspensionPoint { state: u32, resume: BasicBlock, @@ -278,7 +285,7 @@ impl<'a, 'tcx> MutVisitor<'tcx> for TransformVisitor<'a, 'tcx> { state } else { // Return - 1 // state for returned + RETURNED // state for returned }; data.statements.push(self.set_state(state, source_info)); data.terminator.as_mut().unwrap().kind = TerminatorKind::Return; @@ -643,10 +650,10 @@ fn create_generator_drop_shim<'a, 'tcx>( let mut cases = create_cases(&mut mir, transform, |point| point.drop); - cases.insert(0, (0, drop_clean)); + cases.insert(0, (UNRESUMED, drop_clean)); - // The returned state (1) and the poisoned state (2) falls through to - // the default case which is just to return + // The returned state and the poisoned state fall through to the default + // case which is just to return insert_switch(tcx, &mut mir, cases, &transform, TerminatorKind::Return); @@ -762,7 +769,7 @@ fn create_generator_resume_function<'a, 'tcx>( for block in mir.basic_blocks_mut() { let source_info = block.terminator().source_info; if let &TerminatorKind::Resume = &block.terminator().kind { - block.statements.push(transform.set_state(1, source_info)); + block.statements.push(transform.set_state(POISONED, source_info)); } } @@ -773,12 +780,12 @@ fn create_generator_resume_function<'a, 'tcx>( GeneratorResumedAfterReturn, }; - // Jump to the entry point on the 0 state - cases.insert(0, (0, BasicBlock::new(0))); - // Panic when resumed on the returned (1) state - cases.insert(1, (1, insert_panic_block(tcx, mir, GeneratorResumedAfterReturn))); - // Panic when resumed on the poisoned (2) state - cases.insert(2, (2, insert_panic_block(tcx, mir, GeneratorResumedAfterPanic))); + // Jump to the entry point on the unresumed + cases.insert(0, (UNRESUMED, BasicBlock::new(0))); + // Panic when resumed on the returned state + cases.insert(1, (RETURNED, insert_panic_block(tcx, mir, GeneratorResumedAfterReturn))); + // Panic when resumed on the poisoned state + cases.insert(2, (POISONED, insert_panic_block(tcx, mir, GeneratorResumedAfterPanic))); insert_switch(tcx, mir, cases, &transform, TerminatorKind::Unreachable); @@ -942,7 +949,7 @@ impl MirPass for StateTransform { mir.generator_layout = Some(layout); // Insert `drop(generator_struct)` which is used to drop upvars for generators in - // the unresumed (0) state. + // the unresumed state. // This is expanded to a drop ladder in `elaborate_generator_drops`. let drop_clean = insert_clean_drop(mir); diff --git a/src/test/run-fail/generator-resume-after-panic.rs b/src/test/run-fail/generator-resume-after-panic.rs new file mode 100644 index 0000000000000..910b4903bf6a3 --- /dev/null +++ b/src/test/run-fail/generator-resume-after-panic.rs @@ -0,0 +1,22 @@ +// error-pattern:generator resumed after panicking + +// Test that we get the correct message for resuming a panicked generator. + +#![feature(generators, generator_trait)] + +use std::{ + ops::Generator, + pin::Pin, + panic, +}; + +fn main() { + let mut g = || { + panic!(); + yield; + }; + panic::catch_unwind(panic::AssertUnwindSafe(|| { + let x = Pin::new(&mut g).resume(); + })); + Pin::new(&mut g).resume(); +} From 96e361f6f4319782159164ceac6e1d660e383fcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Vicente=20Milack?= Date: Mon, 4 Mar 2019 10:37:15 -0300 Subject: [PATCH 04/21] Fix buffer invalidation for BufRead There are two moments when a BufRead object needs to empty it's internal buffer: - In a seek call; - In a read call when all data in the internal buffer had been already consumed and the output buffer has a greater or equal size than the internal buffer. In both cases, the buffer was not being properly emptied, but only marked as consumed (self.pos = self.cap). That should be no problem if the inner reader is only Read, but if it is Seek as well, then it's possible to access the data in the buffer by using the seek_relative method. In order to prevent this from happening, both self.pos and self.cap should be set to 0. Two test cases were added to detect that failure: - test_buffered_reader_invalidated_after_read - test_buffered_reader_invalidated_after_seek Both tests are very similar to each other. The inner reader contains the following data: [5, 6, 7, 0, 1, 2, 3, 4]. The buffer capacity is 3 bytes. - First, we call fill_buffer, which loads [5, 6, 7] into the internal buffer, and then consume those 3 bytes. - Then we either read the 5 remaining bytes in a single read call or we move to the end of the stream by calling seek. In both cases the buffer should be emptied to prevent the previous data [5, 6, 7] from being read. - We now call seek_relative(-2) and read two bytes, which should give us the last 2 bytes of the stream: [3, 4]. Before this commit, the the seek_relative method would consider that we're still in the range of the internal buffer, so instead of fetching data from the inner reader, it would return the two last bytes that were incorrectly still in the buffer: [6, 7]. Therefore, the test would fail. Now, when seek_relative is called the buffer is empty. So the expected data [3, 4] is fetched from the inner reader and the test passes. --- src/libstd/io/buffered.rs | 45 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/src/libstd/io/buffered.rs b/src/libstd/io/buffered.rs index 559a54d3c8aca..365b1e5974737 100644 --- a/src/libstd/io/buffered.rs +++ b/src/libstd/io/buffered.rs @@ -225,6 +225,9 @@ impl Read for BufReader { // (larger than our internal buffer), bypass our internal buffer // entirely. if self.pos == self.cap && buf.len() >= self.buf.len() { + // Empty the buffer + self.cap = 0; + self.pos = 0; return self.inner.read(buf); } let nread = { @@ -323,14 +326,18 @@ impl Seek for BufReader { } else { // seek backwards by our remainder, and then by the offset self.inner.seek(SeekFrom::Current(-remainder))?; - self.pos = self.cap; // empty the buffer + // Empty the buffer + self.cap = 0; + self.pos = 0; result = self.inner.seek(SeekFrom::Current(n))?; } } else { // Seeking with Start/End doesn't care about our buffer length. result = self.inner.seek(pos)?; } - self.pos = self.cap; // empty the buffer + // Empty the buffer + self.cap = 0; + self.pos = 0; Ok(result) } } @@ -1066,6 +1073,40 @@ mod tests { assert_eq!(reader.fill_buf().ok(), Some(&[2, 3][..])); } + #[test] + fn test_buffered_reader_invalidated_after_read() { + let inner: &[u8] = &[5, 6, 7, 0, 1, 2, 3, 4]; + let mut reader = BufReader::with_capacity(3, io::Cursor::new(inner)); + + assert_eq!(reader.fill_buf().ok(), Some(&[5, 6, 7][..])); + reader.consume(3); + + let mut buffer = [0, 0, 0, 0, 0]; + assert_eq!(reader.read(&mut buffer).ok(), Some(5)); + assert_eq!(buffer, [0, 1, 2, 3, 4]); + + assert!(reader.seek_relative(-2).is_ok()); + let mut buffer = [0, 0]; + assert_eq!(reader.read(&mut buffer).ok(), Some(2)); + assert_eq!(buffer, [3, 4]); + } + + #[test] + fn test_buffered_reader_invalidated_after_seek() { + let inner: &[u8] = &[5, 6, 7, 0, 1, 2, 3, 4]; + let mut reader = BufReader::with_capacity(3, io::Cursor::new(inner)); + + assert_eq!(reader.fill_buf().ok(), Some(&[5, 6, 7][..])); + reader.consume(3); + + assert!(reader.seek(SeekFrom::Current(5)).is_ok()); + + assert!(reader.seek_relative(-2).is_ok()); + let mut buffer = [0, 0]; + assert_eq!(reader.read(&mut buffer).ok(), Some(2)); + assert_eq!(buffer, [3, 4]); + } + #[test] fn test_buffered_reader_seek_underflow() { // gimmick reader that yields its position modulo 256 for each byte From c36d91c5cc446a4688186be1071e0e139ba9d38f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Vicente=20Milack?= Date: Wed, 6 Mar 2019 16:22:28 -0300 Subject: [PATCH 05/21] Fix buffer invalidation at BufReader.read_vectored --- src/libstd/io/buffered.rs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/libstd/io/buffered.rs b/src/libstd/io/buffered.rs index 365b1e5974737..fd4b582d7f2da 100644 --- a/src/libstd/io/buffered.rs +++ b/src/libstd/io/buffered.rs @@ -191,6 +191,13 @@ impl BufReader { /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub fn into_inner(self) -> R { self.inner } + + /// Invalidates all data in the internal buffer. + #[inline] + fn discard_buffer(&mut self) { + self.pos = 0; + self.cap = 0; + } } impl BufReader { @@ -225,9 +232,7 @@ impl Read for BufReader { // (larger than our internal buffer), bypass our internal buffer // entirely. if self.pos == self.cap && buf.len() >= self.buf.len() { - // Empty the buffer - self.cap = 0; - self.pos = 0; + self.discard_buffer(); return self.inner.read(buf); } let nread = { @@ -241,6 +246,7 @@ impl Read for BufReader { fn read_vectored(&mut self, bufs: &mut [IoVecMut<'_>]) -> io::Result { let total_len = bufs.iter().map(|b| b.len()).sum::(); if self.pos == self.cap && total_len >= self.buf.len() { + self.discard_buffer(); return self.inner.read_vectored(bufs); } let nread = { @@ -326,18 +332,14 @@ impl Seek for BufReader { } else { // seek backwards by our remainder, and then by the offset self.inner.seek(SeekFrom::Current(-remainder))?; - // Empty the buffer - self.cap = 0; - self.pos = 0; + self.discard_buffer(); result = self.inner.seek(SeekFrom::Current(n))?; } } else { // Seeking with Start/End doesn't care about our buffer length. result = self.inner.seek(pos)?; } - // Empty the buffer - self.cap = 0; - self.pos = 0; + self.discard_buffer(); Ok(result) } } From 83534874444f8981c540685eba1ddb8fb42999bc Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 7 Mar 2019 17:57:18 +0100 Subject: [PATCH 06/21] refactor build-mainfest. --- src/tools/build-manifest/src/main.rs | 401 +++++++++++++-------------- 1 file changed, 197 insertions(+), 204 deletions(-) diff --git a/src/tools/build-manifest/src/main.rs b/src/tools/build-manifest/src/main.rs index d44a51a9635e9..ae297e23061f1 100644 --- a/src/tools/build-manifest/src/main.rs +++ b/src/tools/build-manifest/src/main.rs @@ -10,7 +10,7 @@ use std::io::{self, Read, Write}; use std::path::{PathBuf, Path}; use std::process::{Command, Stdio}; -static HOSTS: &'static [&'static str] = &[ +static HOSTS: &[&str] = &[ "aarch64-unknown-linux-gnu", "arm-unknown-linux-gnueabi", "arm-unknown-linux-gnueabihf", @@ -35,7 +35,7 @@ static HOSTS: &'static [&'static str] = &[ "x86_64-unknown-netbsd", ]; -static TARGETS: &'static [&'static str] = &[ +static TARGETS: &[&str] = &[ "aarch64-apple-ios", "aarch64-fuchsia", "aarch64-linux-android", @@ -115,7 +115,7 @@ static TARGETS: &'static [&'static str] = &[ "x86_64-unknown-redox", ]; -static DOCS_TARGETS: &'static [&'static str] = &[ +static DOCS_TARGETS: &[&str] = &[ "i686-apple-darwin", "i686-pc-windows-gnu", "i686-pc-windows-msvc", @@ -126,7 +126,7 @@ static DOCS_TARGETS: &'static [&'static str] = &[ "x86_64-unknown-linux-gnu", ]; -static MINGW: &'static [&'static str] = &[ +static MINGW: &[&str] = &[ "i686-pc-windows-gnu", "x86_64-pc-windows-gnu", ]; @@ -153,7 +153,7 @@ struct Rename { to: String, } -#[derive(Serialize)] +#[derive(Serialize, Default)] struct Target { available: bool, url: Option, @@ -165,17 +165,7 @@ struct Target { } impl Target { - fn unavailable() -> Target { - Target { - available: false, - url: None, - hash: None, - xz_url: None, - xz_hash: None, - components: None, - extensions: None, - } - } + fn unavailable() -> Self { Self::default() } } #[derive(Serialize)] @@ -184,6 +174,12 @@ struct Component { target: String, } +impl Component { + fn from_str(pkg: &str, target: &str) -> Self { + Self { pkg: pkg.to_string(), target: target.to_string() } + } +} + macro_rules! t { ($e:expr) => (match $e { Ok(e) => e, @@ -301,6 +297,25 @@ fn main() { }.build(); } +enum PkgType { RustSrc, Cargo, Rls, Clippy, Rustfmt, LlvmTools, Lldb, Miri, Other } + +impl PkgType { + fn from_component(component: &str) -> Self { + use PkgType::*; + match component { + "rust-src" => RustSrc, + "cargo" => Cargo, + "rls" | "rls-preview" => Rls, + "clippy" | "clippy-preview" => Clippy, + "rustfmt" | "rustfmt-preview" => Rustfmt, + "llvm-tools" | "llvm-tools-preview" => LlvmTools, + "lldb" | "lldb-preview" => Lldb, + "miri" | "miri-preview" => Miri, + _ => Other, + } + } +} + impl Builder { fn build(&mut self) { self.rust_version = self.version("rust", "x86_64-unknown-linux-gnu"); @@ -349,39 +364,56 @@ impl Builder { renames: BTreeMap::new(), profiles: BTreeMap::new(), }; + self.add_packages_to(&mut manifest); + self.add_profiles_to(&mut manifest); + self.add_renames_to(&mut manifest); + manifest.pkg.insert("rust".to_string(), self.rust_package(&manifest)); + manifest + } - self.package("rustc", &mut manifest.pkg, HOSTS); - self.package("cargo", &mut manifest.pkg, HOSTS); - self.package("rust-mingw", &mut manifest.pkg, MINGW); - self.package("rust-std", &mut manifest.pkg, TARGETS); - self.package("rust-docs", &mut manifest.pkg, DOCS_TARGETS); - self.package("rust-src", &mut manifest.pkg, &["*"]); - self.package("rls-preview", &mut manifest.pkg, HOSTS); - self.package("clippy-preview", &mut manifest.pkg, HOSTS); - self.package("miri", &mut manifest.pkg, HOSTS); - self.package("rustfmt-preview", &mut manifest.pkg, HOSTS); - self.package("rust-analysis", &mut manifest.pkg, TARGETS); - self.package("llvm-tools-preview", &mut manifest.pkg, TARGETS); - self.package("lldb-preview", &mut manifest.pkg, TARGETS); - - self.profile("minimal", - &mut manifest.profiles, - &["rustc", "cargo", "rust-std", "rust-mingw"]); - self.profile("default", - &mut manifest.profiles, - &["rustc", "cargo", "rust-std", "rust-mingw", - "rust-docs", "rustfmt-preview", "clippy-preview"]); - self.profile("complete", - &mut manifest.profiles, - &["rustc", "cargo", "rust-std", "rust-mingw", - "rust-docs", "rustfmt-preview", "clippy-preview", - "rls-preview", "rust-src", "llvm-tools-preview", - "lldb-preview", "rust-analysis", "miri"]); - - manifest.renames.insert("rls".to_owned(), Rename { to: "rls-preview".to_owned() }); - manifest.renames.insert("rustfmt".to_owned(), Rename { to: "rustfmt-preview".to_owned() }); - manifest.renames.insert("clippy".to_owned(), Rename { to: "clippy-preview".to_owned() }); + fn add_packages_to(&mut self, manifest: &mut Manifest) { + let mut package = |name, targets| self.package(name, &mut manifest.pkg, targets); + package("rustc", HOSTS); + package("cargo", HOSTS); + package("rust-mingw", MINGW); + package("rust-std", TARGETS); + package("rust-docs", DOCS_TARGETS); + package("rust-src", &["*"]); + package("rls-preview", HOSTS); + package("clippy-preview", HOSTS); + package("miri", HOSTS); + package("rustfmt-preview", HOSTS); + package("rust-analysis", TARGETS); + package("llvm-tools-preview", TARGETS); + package("lldb-preview", TARGETS); + } + fn add_profiles_to(&mut self, manifest: &mut Manifest) { + let mut profile = |name, pkgs| self.profile(name, &mut manifest.profiles, pkgs); + profile("minimal", &["rustc", "cargo", "rust-std", "rust-mingw"]); + profile("default", &[ + "rustc", "cargo", "rust-std", "rust-mingw", + "rust-docs", "rustfmt-preview", "clippy-preview" + ]); + profile("complete", &[ + "rustc", "cargo", "rust-std", "rust-mingw", + "rust-docs", "rustfmt-preview", "clippy-preview", + "rls-preview", "rust-src", "llvm-tools-preview", + "lldb-preview", "rust-analysis", "miri" + ]); + } + + fn add_renames_to(&self, manifest: &mut Manifest) { + let mut rename = |from: &str, to: &str| manifest.renames.insert( + from.to_owned(), + Rename { to: to.to_owned() } + ); + rename("rls", "rls-preview"); + rename("rustfmt", "rustfmt-preview"); + rename("clippy", "clippy-preview"); + } + + fn rust_package(&mut self, manifest: &Manifest) -> Package { let mut pkg = Package { version: self.cached_version("rust") .as_ref() @@ -391,90 +423,82 @@ impl Builder { target: BTreeMap::new(), }; for host in HOSTS { - let filename = self.filename("rust", host); - let digest = match self.digests.remove(&filename) { - Some(digest) => digest, - None => { - pkg.target.insert(host.to_string(), Target::unavailable()); - continue - } - }; - let xz_filename = filename.replace(".tar.gz", ".tar.xz"); - let xz_digest = self.digests.remove(&xz_filename); - let mut components = Vec::new(); - let mut extensions = Vec::new(); - - // rustc/rust-std/cargo/docs are all required, and so is rust-mingw - // if it's available for the target. - components.extend(vec![ - Component { pkg: "rustc".to_string(), target: host.to_string() }, - Component { pkg: "rust-std".to_string(), target: host.to_string() }, - Component { pkg: "cargo".to_string(), target: host.to_string() }, - Component { pkg: "rust-docs".to_string(), target: host.to_string() }, - ]); - if host.contains("pc-windows-gnu") { - components.push(Component { - pkg: "rust-mingw".to_string(), - target: host.to_string(), - }); - } - - // Tools are always present in the manifest, but might be marked as unavailable if they - // weren't built - extensions.extend(vec![ - Component { pkg: "clippy-preview".to_string(), target: host.to_string() }, - Component { pkg: "miri".to_string(), target: host.to_string() }, - Component { pkg: "rls-preview".to_string(), target: host.to_string() }, - Component { pkg: "rustfmt-preview".to_string(), target: host.to_string() }, - Component { pkg: "llvm-tools-preview".to_string(), target: host.to_string() }, - Component { pkg: "lldb-preview".to_string(), target: host.to_string() }, - Component { pkg: "rust-analysis".to_string(), target: host.to_string() }, - ]); - - for target in TARGETS { - if target != host { - extensions.push(Component { - pkg: "rust-std".to_string(), - target: target.to_string(), - }); - } - } - extensions.push(Component { - pkg: "rust-src".to_string(), - target: "*".to_string(), - }); - - // If the components/extensions don't actually exist for this - // particular host/target combination then nix it entirely from our - // lists. - { - let has_component = |c: &Component| { - if c.target == "*" { - return true - } - let pkg = match manifest.pkg.get(&c.pkg) { - Some(p) => p, - None => return false, - }; - pkg.target.get(&c.target).is_some() - }; - extensions.retain(&has_component); - components.retain(&has_component); + if let Some(target) = self.target_host_combination(host, &manifest) { + pkg.target.insert(host.to_string(), target); + } else { + pkg.target.insert(host.to_string(), Target::unavailable()); + continue } + } + pkg + } - pkg.target.insert(host.to_string(), Target { - available: true, - url: Some(self.url(&filename)), - hash: Some(digest), - xz_url: xz_digest.as_ref().map(|_| self.url(&xz_filename)), - xz_hash: xz_digest, - components: Some(components), - extensions: Some(extensions), - }); + fn target_host_combination(&mut self, host: &str, manifest: &Manifest) -> Option { + let filename = self.filename("rust", host); + let digest = self.digests.remove(&filename)?; + let xz_filename = filename.replace(".tar.gz", ".tar.xz"); + let xz_digest = self.digests.remove(&xz_filename); + let mut components = Vec::new(); + let mut extensions = Vec::new(); + + let host_component = |pkg| Component::from_str(pkg, host); + + // rustc/rust-std/cargo/docs are all required, + // and so is rust-mingw if it's available for the target. + components.extend(vec![ + host_component("rustc"), + host_component("rust-std"), + host_component("cargo"), + host_component("rust-docs"), + ]); + if host.contains("pc-windows-gnu") { + components.push(host_component("rust-mingw")); } - manifest.pkg.insert("rust".to_string(), pkg); - manifest + // Tools are always present in the manifest, + // but might be marked as unavailable if they weren't built. + extensions.extend(vec![ + host_component("clippy-preview"), + host_component("miri"), + host_component("rls-preview"), + host_component("rustfmt-preview"), + host_component("llvm-tools-preview"), + host_component("lldb-preview"), + host_component("rust-analysis"), + ]); + + extensions.extend( + TARGETS.iter() + .filter(|&&target| target != host) + .map(|target| Component::from_str("rust-std", target)) + ); + extensions.push(Component::from_str("rust-src", "*")); + + // If the components/extensions don't actually exist for this + // particular host/target combination then nix it entirely from our + // lists. + let has_component = |c: &Component| { + if c.target == "*" { + return true + } + let pkg = match manifest.pkg.get(&c.pkg) { + Some(p) => p, + None => return false, + }; + pkg.target.get(&c.target).is_some() + }; + extensions.retain(&has_component); + components.retain(&has_component); + + Some(Target { + available: true, + url: Some(self.url(&filename)), + hash: Some(digest), + xz_url: xz_digest.as_ref().map(|_| self.url(&xz_filename)), + xz_hash: xz_digest, + components: Some(components), + extensions: Some(extensions), + }) } fn profile(&mut self, @@ -488,10 +512,11 @@ impl Builder { pkgname: &str, dst: &mut BTreeMap, targets: &[&str]) { - let (version, is_present) = match *self.cached_version(pkgname) { - Some(ref version) => (version.clone(), true), - None => (String::new(), false), - }; + let (version, is_present) = self.cached_version(pkgname) + .as_ref() + .cloned() + .map(|version| (version, true)) + .unwrap_or_default(); let targets = targets.iter().map(|name| { if is_present { @@ -515,15 +540,7 @@ impl Builder { } else { // If the component is not present for this build add it anyway but mark it as // unavailable -- this way rustup won't allow upgrades without --force - (name.to_string(), Target { - available: false, - url: None, - hash: None, - xz_url: None, - xz_hash: None, - components: None, - extensions: None, - }) + (name.to_string(), Target::unavailable()) } }).collect(); @@ -542,89 +559,65 @@ impl Builder { } fn filename(&self, component: &str, target: &str) -> String { - if component == "rust-src" { - format!("rust-src-{}.tar.gz", self.rust_release) - } else if component == "cargo" { - format!("cargo-{}-{}.tar.gz", self.cargo_release, target) - } else if component == "rls" || component == "rls-preview" { - format!("rls-{}-{}.tar.gz", self.rls_release, target) - } else if component == "clippy" || component == "clippy-preview" { - format!("clippy-{}-{}.tar.gz", self.clippy_release, target) - } else if component == "rustfmt" || component == "rustfmt-preview" { - format!("rustfmt-{}-{}.tar.gz", self.rustfmt_release, target) - } else if component == "llvm-tools" || component == "llvm-tools-preview" { - format!("llvm-tools-{}-{}.tar.gz", self.llvm_tools_release, target) - } else if component == "lldb" || component == "lldb-preview" { - format!("lldb-{}-{}.tar.gz", self.lldb_release, target) - } else if component == "miri" || component == "miri-preview" { - format!("miri-{}-{}.tar.gz", self.miri_release, target) - } else { - format!("{}-{}-{}.tar.gz", component, self.rust_release, target) + use PkgType::*; + match PkgType::from_component(component) { + RustSrc => format!("rust-src-{}.tar.gz", self.rust_release), + Cargo => format!("cargo-{}-{}.tar.gz", self.cargo_release, target), + Rls => format!("rls-{}-{}.tar.gz", self.rls_release, target), + Clippy => format!("clippy-{}-{}.tar.gz", self.clippy_release, target), + Rustfmt => format!("rustfmt-{}-{}.tar.gz", self.rustfmt_release, target), + LlvmTools => format!("llvm-tools-{}-{}.tar.gz", self.llvm_tools_release, target), + Lldb => format!("lldb-{}-{}.tar.gz", self.lldb_release, target), + Miri => format!("miri-{}-{}.tar.gz", self.miri_release, target), + Other => format!("{}-{}-{}.tar.gz", component, self.rust_release, target), } } fn cached_version(&self, component: &str) -> &Option { - if component == "cargo" { - &self.cargo_version - } else if component == "rls" || component == "rls-preview" { - &self.rls_version - } else if component == "clippy" || component == "clippy-preview" { - &self.clippy_version - } else if component == "rustfmt" || component == "rustfmt-preview" { - &self.rustfmt_version - } else if component == "llvm-tools" || component == "llvm-tools-preview" { - &self.llvm_tools_version - } else if component == "lldb" || component == "lldb-preview" { - &self.lldb_version - } else if component == "miri" || component == "miri-preview" { - &self.miri_version - } else { - &self.rust_version + use PkgType::*; + match PkgType::from_component(component) { + Cargo => &self.cargo_version, + Rls => &self.rls_version, + Clippy => &self.clippy_version, + Rustfmt => &self.rustfmt_version, + LlvmTools => &self.llvm_tools_version, + Lldb => &self.lldb_version, + Miri => &self.miri_version, + _ => &self.rust_version, } } fn cached_git_commit_hash(&self, component: &str) -> &Option { - if component == "cargo" { - &self.cargo_git_commit_hash - } else if component == "rls" || component == "rls-preview" { - &self.rls_git_commit_hash - } else if component == "clippy" || component == "clippy-preview" { - &self.clippy_git_commit_hash - } else if component == "rustfmt" || component == "rustfmt-preview" { - &self.rustfmt_git_commit_hash - } else if component == "llvm-tools" || component == "llvm-tools-preview" { - &self.llvm_tools_git_commit_hash - } else if component == "lldb" || component == "lldb-preview" { - &self.lldb_git_commit_hash - } else if component == "miri" || component == "miri-preview" { - &self.miri_git_commit_hash - } else { - &self.rust_git_commit_hash + use PkgType::*; + match PkgType::from_component(component) { + Cargo => &self.cargo_git_commit_hash, + Rls => &self.rls_git_commit_hash, + Clippy => &self.clippy_git_commit_hash, + Rustfmt => &self.rustfmt_git_commit_hash, + LlvmTools => &self.llvm_tools_git_commit_hash, + Lldb => &self.lldb_git_commit_hash, + Miri => &self.miri_git_commit_hash, + _ => &self.rust_git_commit_hash, } } fn version(&self, component: &str, target: &str) -> Option { - let mut cmd = Command::new("tar"); - let filename = self.filename(component, target); - cmd.arg("xf") - .arg(self.input.join(&filename)) - .arg(format!("{}/version", filename.replace(".tar.gz", ""))) - .arg("-O"); - let output = t!(cmd.output()); - if output.status.success() { - Some(String::from_utf8_lossy(&output.stdout).trim().to_string()) - } else { - // Perhaps we didn't build this package. - None - } + self.untar(component, target, |filename| format!("{}/version", filename)) } fn git_commit_hash(&self, component: &str, target: &str) -> Option { + self.untar(component, target, |filename| format!("{}/git-commit-hash", filename)) + } + + fn untar(&self, component: &str, target: &str, dir: F) -> Option + where + F: FnOnce(String) -> String + { let mut cmd = Command::new("tar"); let filename = self.filename(component, target); cmd.arg("xf") .arg(self.input.join(&filename)) - .arg(format!("{}/git-commit-hash", filename.replace(".tar.gz", ""))) + .arg(dir(filename.replace(".tar.gz", ""))) .arg("-O"); let output = t!(cmd.output()); if output.status.success() { From ff5e31ffb5224f458711a35392e0acddb1d4080a Mon Sep 17 00:00:00 2001 From: varkor Date: Thu, 14 Mar 2019 01:02:56 +0000 Subject: [PATCH 07/21] Fix a broken link to the rustc-guide --- CONTRIBUTING.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e785f03d7de2b..1fc942c7cd98a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -28,6 +28,7 @@ can give you a good example of how a typical contribution would go. [pound-rust-internals]: https://chat.mibbit.com/?server=irc.mozilla.org&channel=%23rust-internals [internals]: https://internals.rust-lang.org [coc]: https://www.rust-lang.org/conduct.html +[rustc-guide]: https://rust-lang.github.io/rustc-guide/ [walkthrough]: https://rust-lang.github.io/rustc-guide/walkthrough.html ## Feature Requests From 6e449daceacd5959b830d24a8c9fc9971a2fc8ea Mon Sep 17 00:00:00 2001 From: varkor Date: Thu, 14 Mar 2019 01:03:11 +0000 Subject: [PATCH 08/21] Remove trailing newlines --- CONTRIBUTING.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1fc942c7cd98a..02777621d6571 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -35,7 +35,7 @@ can give you a good example of how a typical contribution would go. [feature-requests]: #feature-requests To request a change to the way the Rust language works, please head over -to the [RFCs repository](https://github.com/rust-lang/rfcs) and view the +to the [RFCs repository](https://github.com/rust-lang/rfcs) and view the [README](https://github.com/rust-lang/rfcs/blob/master/README.md) for instructions. @@ -191,7 +191,7 @@ before the PR is merged. [breaking-tools-built-with-the-compiler]: #breaking-tools-built-with-the-compiler Rust's build system builds a number of tools that make use of the -internals of the compiler. This includes +internals of the compiler. This includes [Clippy](https://github.com/rust-lang/rust-clippy), [RLS](https://github.com/rust-lang/rls) and [rustfmt](https://github.com/rust-lang/rustfmt). If these tools From 205ab0c260e983615de3a2ed71c8e82e5c48dd8d Mon Sep 17 00:00:00 2001 From: varkor Date: Thu, 14 Mar 2019 01:04:19 +0000 Subject: [PATCH 09/21] Add a link to the Discord and Zulip servers --- CONTRIBUTING.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 02777621d6571..35a741babd23f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -15,7 +15,7 @@ links to the major sections: * [Helpful Links and Information](#helpful-links-and-information) If you have questions, please make a post on [internals.rust-lang.org][internals] or -hop on [#rust-internals][pound-rust-internals]. +hop on the [Rust Discord server][rust-discord], [Rust Zulip server][rust-zulip] or [#rust-internals][pound-rust-internals]. As a reminder, all contributors are expected to follow our [Code of Conduct][coc]. @@ -27,6 +27,8 @@ can give you a good example of how a typical contribution would go. [pound-rust-internals]: https://chat.mibbit.com/?server=irc.mozilla.org&channel=%23rust-internals [internals]: https://internals.rust-lang.org +[rust-discord]: http://discord.gg/rust-lang +[rust-zulip]: https://rust-lang.zulipchat.com [coc]: https://www.rust-lang.org/conduct.html [rustc-guide]: https://rust-lang.github.io/rustc-guide/ [walkthrough]: https://rust-lang.github.io/rustc-guide/walkthrough.html From 6f3fda9d1d603084c8e6a323bdbea7b780ba87ca Mon Sep 17 00:00:00 2001 From: varkor Date: Thu, 14 Mar 2019 01:07:00 +0000 Subject: [PATCH 10/21] Add links to @rust-highfive and @bors --- CONTRIBUTING.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 35a741babd23f..dde4ac3bbeb77 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -132,7 +132,7 @@ request); you can add [git hooks](https://git-scm.com/book/en/v2/Customizing-Git before every push to make sure you never forget to make this check. All pull requests are reviewed by another person. We have a bot, -@rust-highfive, that will automatically assign a random person to review your +[@rust-highfive][rust-highfive], that will automatically assign a random person to review your request. If you want to request that a specific person reviews your pull request, @@ -149,11 +149,13 @@ on the pull request with an `r+`. It will look something like this: @bors: r+ 38fe8d2 -This tells @bors, our lovable integration bot, that your pull request has +This tells [@bors][bors], our lovable integration bot, that your pull request has been approved. The PR then enters the [merge queue][merge-queue], where @bors will run all the tests on every platform we support. If it all works out, @bors will merge your code into `master` and close the pull request. +[rust-highfive]: https://github.com/rust-highfive +[bors]: https://github.com/bors [merge-queue]: https://buildbot2.rust-lang.org/homu/queue/rust Speaking of tests, Rust has a comprehensive test suite. More information about From eadb8443f3ea4269b7010afa3bd5de21515c23b1 Mon Sep 17 00:00:00 2001 From: varkor Date: Thu, 14 Mar 2019 01:09:32 +0000 Subject: [PATCH 11/21] Update r+ syntax --- CONTRIBUTING.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index dde4ac3bbeb77..5276171728f0e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -147,7 +147,7 @@ of a random person. This is entirely optional. After someone has reviewed your pull request, they will leave an annotation on the pull request with an `r+`. It will look something like this: - @bors: r+ 38fe8d2 + @bors r+ This tells [@bors][bors], our lovable integration bot, that your pull request has been approved. The PR then enters the [merge queue][merge-queue], where @bors @@ -302,7 +302,7 @@ from the source code itself. Documentation pull requests function in the same way as other pull requests, though you may see a slightly different form of `r+`: - @bors: r+ 38fe8d2 rollup + @bors r+ rollup That additional `rollup` tells @bors that this change is eligible for a 'rollup'. To save @bors some work, and to get small changes through more quickly, when From 308a002eb16425b5164c1d2931d26d802ae89f6f Mon Sep 17 00:00:00 2001 From: varkor Date: Thu, 14 Mar 2019 01:13:26 +0000 Subject: [PATCH 12/21] Make all references to @bors or users links --- CONTRIBUTING.md | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5276171728f0e..d6253dcb2338a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -136,12 +136,12 @@ All pull requests are reviewed by another person. We have a bot, request. If you want to request that a specific person reviews your pull request, -you can add an `r?` to the message. For example, Steve usually reviews +you can add an `r?` to the message. For example, [Steve][steveklabnik] usually reviews documentation changes. So if you were to make a documentation change, add r? @steveklabnik -to the end of the message, and @rust-highfive will assign @steveklabnik instead +to the end of the message, and @rust-highfive will assign [@steveklabnik][steveklabnik] instead of a random person. This is entirely optional. After someone has reviewed your pull request, they will leave an annotation @@ -150,11 +150,12 @@ on the pull request with an `r+`. It will look something like this: @bors r+ This tells [@bors][bors], our lovable integration bot, that your pull request has -been approved. The PR then enters the [merge queue][merge-queue], where @bors +been approved. The PR then enters the [merge queue][merge-queue], where [@bors][bors] will run all the tests on every platform we support. If it all works out, -@bors will merge your code into `master` and close the pull request. +[@bors][bors] will merge your code into `master` and close the pull request. [rust-highfive]: https://github.com/rust-highfive +[steveklabnik]: https://github.com/steveklabnik [bors]: https://github.com/bors [merge-queue]: https://buildbot2.rust-lang.org/homu/queue/rust @@ -304,9 +305,9 @@ though you may see a slightly different form of `r+`: @bors r+ rollup -That additional `rollup` tells @bors that this change is eligible for a 'rollup'. -To save @bors some work, and to get small changes through more quickly, when -@bors attempts to merge a commit that's rollup-eligible, it will also merge +That additional `rollup` tells [@bors][bors] that this change is eligible for a 'rollup'. +To save [@bors][bors] some work, and to get small changes through more quickly, when +[@bors][bors] attempts to merge a commit that's rollup-eligible, it will also merge the other rollup-eligible patches too, and they'll get tested and merged at the same time. @@ -433,7 +434,7 @@ are: * Although out of date, [Tom Lee's great blog article][tlgba] is very helpful * [rustaceans.org][ro] is helpful, but mostly dedicated to IRC * The [Rust Compiler Testing Docs][rctd] -* For @bors, [this cheat sheet][cheatsheet] is helpful (Remember to replace `@homu` with `@bors` in the commands that you use.) +* For [@bors][bors], [this cheat sheet][cheatsheet] is helpful (Remember to replace `@homu` with `@bors` in the commands that you use.) * **Google!** ([search only in Rust Documentation][gsearchdocs] to find types, traits, etc. quickly) * Don't be afraid to ask! The Rust community is friendly and helpful. From 3a00649256c64cacfdbf5f0e7b370d660291d06e Mon Sep 17 00:00:00 2001 From: varkor Date: Thu, 14 Mar 2019 01:17:49 +0000 Subject: [PATCH 13/21] Move rollup description earlier --- CONTRIBUTING.md | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d6253dcb2338a..ee56f5bf9954e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -154,6 +154,15 @@ been approved. The PR then enters the [merge queue][merge-queue], where [@bors][ will run all the tests on every platform we support. If it all works out, [@bors][bors] will merge your code into `master` and close the pull request. +Depending on the scale of the change, you may see a slightly different form of `r+`: + + @bors r+ rollup + +The additional `rollup` tells [@bors][bors] that this change is eligible for to be +"rolled up". Changes that are rolled up are tested and merged at the same time, to +speed the process up. Typically only small changes that are expected not to conflict +with one another are rolled up. + [rust-highfive]: https://github.com/rust-highfive [steveklabnik]: https://github.com/steveklabnik [bors]: https://github.com/bors @@ -298,18 +307,8 @@ the submodule to. Running `./x.py build` should work now. Documentation improvements are very welcome. The source of `doc.rust-lang.org` is located in `src/doc` in the tree, and standard API documentation is generated -from the source code itself. - -Documentation pull requests function in the same way as other pull requests, -though you may see a slightly different form of `r+`: - - @bors r+ rollup - -That additional `rollup` tells [@bors][bors] that this change is eligible for a 'rollup'. -To save [@bors][bors] some work, and to get small changes through more quickly, when -[@bors][bors] attempts to merge a commit that's rollup-eligible, it will also merge -the other rollup-eligible patches too, and they'll get tested and merged at -the same time. +from the source code itself. Documentation pull requests function in the same way +as other pull requests. To find documentation-related issues, sort by the [T-doc label][tdoc]. From 037596c4ce1548543479ace50500ee3dc2f6720e Mon Sep 17 00:00:00 2001 From: varkor Date: Thu, 14 Mar 2019 01:20:41 +0000 Subject: [PATCH 14/21] Fix capitalisation problem --- CONTRIBUTING.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ee56f5bf9954e..fa408935cc8cc 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -433,7 +433,8 @@ are: * Although out of date, [Tom Lee's great blog article][tlgba] is very helpful * [rustaceans.org][ro] is helpful, but mostly dedicated to IRC * The [Rust Compiler Testing Docs][rctd] -* For [@bors][bors], [this cheat sheet][cheatsheet] is helpful (Remember to replace `@homu` with `@bors` in the commands that you use.) +* For [@bors][bors], [this cheat sheet][cheatsheet] is helpful +(though you'll need to replace `@homu` with `@bors` in any commands) * **Google!** ([search only in Rust Documentation][gsearchdocs] to find types, traits, etc. quickly) * Don't be afraid to ask! The Rust community is friendly and helpful. From 541ad45a83482e3132c75fbbc55fb2afc03a6031 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 5 Mar 2019 02:29:21 +0100 Subject: [PATCH 15/21] Add default keyword handling in rustdoc --- src/librustdoc/clean/mod.rs | 24 ++++++++++++++++++++---- src/librustdoc/html/format.rs | 11 +++++++++++ src/librustdoc/html/render.rs | 8 +++++--- src/test/rustdoc/default_trait_method.rs | 15 +++++++++++++++ 4 files changed, 51 insertions(+), 7 deletions(-) create mode 100644 src/test/rustdoc/default_trait_method.rs diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index f2bf7ead5619b..86958e06d5c5b 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -507,6 +507,18 @@ impl Item { .as_ref() .or_else(|| self.stability.as_ref().and_then(|s| s.deprecation.as_ref())) } + pub fn is_default(&self) -> bool { + match self.inner { + ItemEnum::MethodItem(ref meth) => { + if let Some(defaultness) = meth.defaultness { + defaultness.has_value() && !defaultness.is_final() + } else { + false + } + } + _ => false, + } + } } #[derive(Clone, RustcEncodable, RustcDecodable, Debug)] @@ -1699,9 +1711,11 @@ pub struct Method { pub generics: Generics, pub decl: FnDecl, pub header: hir::FnHeader, + pub defaultness: Option, } -impl<'a> Clean for (&'a hir::MethodSig, &'a hir::Generics, hir::BodyId) { +impl<'a> Clean for (&'a hir::MethodSig, &'a hir::Generics, hir::BodyId, + Option) { fn clean(&self, cx: &DocContext<'_>) -> Method { let (generics, decl) = enter_impl_trait(cx, || { (self.1.clean(cx), (&*self.0.decl, self.2).clean(cx)) @@ -1710,6 +1724,7 @@ impl<'a> Clean for (&'a hir::MethodSig, &'a hir::Generics, hir::BodyId) decl, generics, header: self.0.header, + defaultness: self.3, } } } @@ -2015,7 +2030,7 @@ impl Clean for hir::TraitItem { default.map(|e| print_const_expr(cx, e))) } hir::TraitItemKind::Method(ref sig, hir::TraitMethod::Provided(body)) => { - MethodItem((sig, &self.generics, body).clean(cx)) + MethodItem((sig, &self.generics, body, None).clean(cx)) } hir::TraitItemKind::Method(ref sig, hir::TraitMethod::Required(ref names)) => { let (generics, decl) = enter_impl_trait(cx, || { @@ -2053,7 +2068,7 @@ impl Clean for hir::ImplItem { Some(print_const_expr(cx, expr))) } hir::ImplItemKind::Method(ref sig, body) => { - MethodItem((sig, &self.generics, body).clean(cx)) + MethodItem((sig, &self.generics, body, Some(self.defaultness)).clean(cx)) } hir::ImplItemKind::Type(ref ty) => TypedefItem(Typedef { type_: ty.clean(cx), @@ -2136,7 +2151,8 @@ impl<'tcx> Clean for ty::AssociatedItem { abi: sig.abi(), constness, asyncness: hir::IsAsync::NotAsync, - } + }, + defaultness: Some(self.defaultness), }) } else { TyMethodItem(TyMethod { diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index d204a179ca62c..48baf27bae20a 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -45,6 +45,7 @@ pub struct GenericBounds<'a>(pub &'a [clean::GenericBound]); /// Wrapper struct for emitting a comma-separated list of items pub struct CommaSep<'a, T>(pub &'a [T]); pub struct AbiSpace(pub Abi); +pub struct DefaultSpace(pub bool); /// Wrapper struct for properly emitting a function or method declaration. pub struct Function<'a> { @@ -1057,3 +1058,13 @@ impl fmt::Display for AbiSpace { } } } + +impl fmt::Display for DefaultSpace { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if self.0 { + write!(f, "default ") + } else { + Ok(()) + } + } +} diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index d711e4514a049..f02e4a01ec56c 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -61,7 +61,7 @@ use crate::doctree; use crate::fold::DocFolder; use crate::html::escape::Escape; use crate::html::format::{AsyncSpace, ConstnessSpace}; -use crate::html::format::{GenericBounds, WhereClause, href, AbiSpace}; +use crate::html::format::{GenericBounds, WhereClause, href, AbiSpace, DefaultSpace}; use crate::html::format::{VisSpace, Function, UnsafetySpace, MutableSpace}; use crate::html::format::fmt_impl_for_trait_page; use crate::html::item_type::ItemType; @@ -3434,11 +3434,12 @@ fn render_assoc_item(w: &mut fmt::Formatter<'_>, } }; let mut header_len = format!( - "{}{}{}{}{:#}fn {}{:#}", + "{}{}{}{}{}{:#}fn {}{:#}", VisSpace(&meth.visibility), ConstnessSpace(header.constness), UnsafetySpace(header.unsafety), AsyncSpace(header.asyncness), + DefaultSpace(meth.is_default()), AbiSpace(header.abi), name, *g @@ -3450,12 +3451,13 @@ fn render_assoc_item(w: &mut fmt::Formatter<'_>, (0, true) }; render_attributes(w, meth)?; - write!(w, "{}{}{}{}{}fn {name}\ + write!(w, "{}{}{}{}{}{}fn {name}\ {generics}{decl}{where_clause}", VisSpace(&meth.visibility), ConstnessSpace(header.constness), UnsafetySpace(header.unsafety), AsyncSpace(header.asyncness), + DefaultSpace(meth.is_default()), AbiSpace(header.abi), href = href, name = name, diff --git a/src/test/rustdoc/default_trait_method.rs b/src/test/rustdoc/default_trait_method.rs new file mode 100644 index 0000000000000..dfbd8f2210fa4 --- /dev/null +++ b/src/test/rustdoc/default_trait_method.rs @@ -0,0 +1,15 @@ +#![feature(specialization)] + +pub trait Item { + fn foo(); + fn bar(); +} + +// @has default_trait_method/trait.Item.html +// @has - '//*[@id="method.foo"]' 'default fn foo()' +// @has - '//*[@id="method.bar"]' 'fn bar()' +// @!has - '//*[@id="method.bar"]' 'default fn bar()' +impl Item for T { + default fn foo() {} + fn bar() {} +} From e07d1635af6e5ffc3cc7dc43010b72620ff240c6 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Sat, 16 Mar 2019 18:07:58 +0100 Subject: [PATCH 16/21] Fix undefined behavior in hint::spin_loop for x86 targets without SSE2 The pause instruction requires SSE2 but was being unconditionally used on targets without it, resulting in undefined behavior. This PR fixes that by only using the pause intrinsic if SSE2 is available. It also removes the inline assembly which was not required since these instructions are available in core::arch, and extends support of the spin_loop hint to arm targets with the v6 feature which also support the yield instruction. Closes #59237 . --- src/libcore/hint.rs | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/src/libcore/hint.rs b/src/libcore/hint.rs index 89de5c1bc8af8..c6d1412f6f93b 100644 --- a/src/libcore/hint.rs +++ b/src/libcore/hint.rs @@ -2,7 +2,7 @@ //! Hints to compiler that affects how code should be emitted or optimized. -use intrinsics; +use {arch, intrinsics}; /// Informs the compiler that this point in the code is not reachable, enabling /// further optimizations. @@ -62,13 +62,32 @@ pub unsafe fn unreachable_unchecked() -> ! { #[inline] #[unstable(feature = "renamed_spin_loop", issue = "55002")] pub fn spin_loop() { - #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] - unsafe { - asm!("pause" ::: "memory" : "volatile"); + #[cfg( + all( + any(target_arch = "x86", target_arch = "x86_64"), + target_feature = "sse2" + ) + )] { + #[cfg(target_arch = "x86")] { + unsafe { arch::x86::_mm_pause() }; + } + + #[cfg(target_arch = "x86_64")] { + unsafe { arch::x86_64::_mm_pause() }; + } } - #[cfg(target_arch = "aarch64")] - unsafe { - asm!("yield" ::: "memory" : "volatile"); + #[cfg( + any( + target_arch = "aarch64", + all(target_arch = "arm", target_feature = "v6") + ) + )] { + #[cfg(target_arch = "aarch64")] { + unsafe { arch::aarch64::__yield() }; + } + #[cfg(target_arch = "arm")] { + unsafe { arch::arm::__yield() }; + } } } From 64303189f0b3a111c3b1e59d77cdaad91ca90efe Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sun, 17 Mar 2019 10:36:10 +0000 Subject: [PATCH 17/21] Use a valid name for graphviz graphs --- src/librustc_driver/pretty.rs | 12 +++++++++- src/librustc_mir/borrow_check/mod.rs | 8 +++---- src/librustc_mir/dataflow/graphviz.rs | 17 +++++++------- src/librustc_mir/dataflow/mod.rs | 12 +++++----- src/librustc_mir/transform/elaborate_drops.rs | 12 +++++----- src/librustc_mir/transform/generator.rs | 6 ++--- src/librustc_mir/transform/rustc_peek.rs | 19 ++++++++------- src/librustc_mir/util/graphviz.rs | 14 ++++++++++- src/librustc_mir/util/mod.rs | 2 +- src/test/mir-opt/graphviz.rs | 23 +++++++++++++++++++ 10 files changed, 85 insertions(+), 40 deletions(-) create mode 100644 src/test/mir-opt/graphviz.rs diff --git a/src/librustc_driver/pretty.rs b/src/librustc_driver/pretty.rs index dde88a212408d..ace5198deaf2e 100644 --- a/src/librustc_driver/pretty.rs +++ b/src/librustc_driver/pretty.rs @@ -633,10 +633,20 @@ fn print_flowgraph<'a, 'tcx, W: Write>(variants: Vec, let body = tcx.hir().body(body_id); let cfg = cfg::CFG::new(tcx, &body); let labelled_edges = mode != PpFlowGraphMode::UnlabelledEdges; + let hir_id = code.id(); + // We have to disassemble the hir_id because name must be ASCII + // alphanumeric. This does not appear in the rendered graph, so it does not + // have to be user friendly. + let name = format!( + "hir_id_{}_{}_{}", + hir_id.owner.address_space().index(), + hir_id.owner.as_array_index(), + hir_id.local_id.index(), + ); let lcfg = LabelledCFG { tcx, cfg: &cfg, - name: format!("node_{}", code.id()), + name, labelled_edges, }; diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index c4e371d5afedb..551f18b95fe52 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -156,7 +156,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( let mut flow_inits = FlowAtLocation::new(do_dataflow( tcx, mir, - id, + def_id, &attributes, &dead_unwinds, MaybeInitializedPlaces::new(tcx, mir, &mdpe), @@ -191,7 +191,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( let flow_borrows = FlowAtLocation::new(do_dataflow( tcx, mir, - id, + def_id, &attributes, &dead_unwinds, Borrows::new(tcx, mir, regioncx.clone(), &borrow_set), @@ -200,7 +200,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( let flow_uninits = FlowAtLocation::new(do_dataflow( tcx, mir, - id, + def_id, &attributes, &dead_unwinds, MaybeUninitializedPlaces::new(tcx, mir, &mdpe), @@ -209,7 +209,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( let flow_ever_inits = FlowAtLocation::new(do_dataflow( tcx, mir, - id, + def_id, &attributes, &dead_unwinds, EverInitializedPlaces::new(tcx, mir, &mdpe), diff --git a/src/librustc_mir/dataflow/graphviz.rs b/src/librustc_mir/dataflow/graphviz.rs index da9cc118f5521..d68377681f1ca 100644 --- a/src/librustc_mir/dataflow/graphviz.rs +++ b/src/librustc_mir/dataflow/graphviz.rs @@ -1,6 +1,6 @@ //! Hook into libgraphviz for rendering dataflow graphs for MIR. -use rustc::hir::HirId; +use rustc::hir::def_id::DefId; use rustc::mir::{BasicBlock, Mir}; use std::fs; @@ -8,13 +8,15 @@ use std::io; use std::marker::PhantomData; use std::path::Path; +use crate::util::graphviz_safe_def_name; + use super::{BitDenotation, DataflowState}; use super::DataflowBuilder; use super::DebugFormatted; pub trait MirWithFlowState<'tcx> { type BD: BitDenotation<'tcx>; - fn hir_id(&self) -> HirId; + fn def_id(&self) -> DefId; fn mir(&self) -> &Mir<'tcx>; fn flow_state(&self) -> &DataflowState<'tcx, Self::BD>; } @@ -23,7 +25,7 @@ impl<'a, 'tcx, BD> MirWithFlowState<'tcx> for DataflowBuilder<'a, 'tcx, BD> where BD: BitDenotation<'tcx> { type BD = BD; - fn hir_id(&self) -> HirId { self.hir_id } + fn def_id(&self) -> DefId { self.def_id } fn mir(&self) -> &Mir<'tcx> { self.flow_state.mir() } fn flow_state(&self) -> &DataflowState<'tcx, Self::BD> { &self.flow_state.flow_state } } @@ -47,8 +49,8 @@ pub(crate) fn print_borrowck_graph_to<'a, 'tcx, BD, P>( let g = Graph { mbcx, phantom: PhantomData, render_idx }; let mut v = Vec::new(); dot::render(&g, &mut v)?; - debug!("print_borrowck_graph_to path: {} hir_id: {}", - path.display(), mbcx.hir_id); + debug!("print_borrowck_graph_to path: {} def_id: {:?}", + path.display(), mbcx.def_id); fs::write(path, v) } @@ -69,9 +71,8 @@ impl<'a, 'tcx, MWF, P> dot::Labeller<'a> for Graph<'a, 'tcx, MWF, P> type Node = Node; type Edge = Edge; fn graph_id(&self) -> dot::Id<'_> { - dot::Id::new(format!("graph_for_node_{}", - self.mbcx.hir_id())) - .unwrap() + let name = graphviz_safe_def_name(self.mbcx.def_id()); + dot::Id::new(format!("graph_for_def_id_{}", name)).unwrap() } fn node_id(&self, n: &Node) -> dot::Id<'_> { diff --git a/src/librustc_mir/dataflow/mod.rs b/src/librustc_mir/dataflow/mod.rs index 03f8ac6743617..d0035bf7d3bc2 100644 --- a/src/librustc_mir/dataflow/mod.rs +++ b/src/librustc_mir/dataflow/mod.rs @@ -4,7 +4,7 @@ use rustc_data_structures::bit_set::{BitSet, BitSetOperator, HybridBitSet}; use rustc_data_structures::indexed_vec::Idx; use rustc_data_structures::work_queue::WorkQueue; -use rustc::hir::HirId; +use rustc::hir::def_id::DefId; use rustc::ty::{self, TyCtxt}; use rustc::mir::{self, Mir, BasicBlock, BasicBlockData, Location, Statement, Terminator}; use rustc::mir::traversal; @@ -39,7 +39,7 @@ pub(crate) struct DataflowBuilder<'a, 'tcx: 'a, BD> where BD: BitDenotation<'tcx> { - hir_id: HirId, + def_id: DefId, flow_state: DataflowAnalysis<'a, 'tcx, BD>, print_preflow_to: Option, print_postflow_to: Option, @@ -117,7 +117,7 @@ pub struct MoveDataParamEnv<'gcx, 'tcx> { pub(crate) fn do_dataflow<'a, 'gcx, 'tcx, BD, P>(tcx: TyCtxt<'a, 'gcx, 'tcx>, mir: &'a Mir<'tcx>, - hir_id: HirId, + def_id: DefId, attributes: &[ast::Attribute], dead_unwinds: &BitSet, bd: BD, @@ -127,14 +127,14 @@ pub(crate) fn do_dataflow<'a, 'gcx, 'tcx, BD, P>(tcx: TyCtxt<'a, 'gcx, 'tcx>, P: Fn(&BD, BD::Idx) -> DebugFormatted { let flow_state = DataflowAnalysis::new(mir, dead_unwinds, bd); - flow_state.run(tcx, hir_id, attributes, p) + flow_state.run(tcx, def_id, attributes, p) } impl<'a, 'gcx: 'tcx, 'tcx: 'a, BD> DataflowAnalysis<'a, 'tcx, BD> where BD: BitDenotation<'tcx> { pub(crate) fn run

(self, tcx: TyCtxt<'a, 'gcx, 'tcx>, - hir_id: HirId, + def_id: DefId, attributes: &[ast::Attribute], p: P) -> DataflowResults<'tcx, BD> where P: Fn(&BD, BD::Idx) -> DebugFormatted @@ -159,7 +159,7 @@ impl<'a, 'gcx: 'tcx, 'tcx: 'a, BD> DataflowAnalysis<'a, 'tcx, BD> where BD: BitD name_found(tcx.sess, attributes, "borrowck_graphviz_postflow"); let mut mbcx = DataflowBuilder { - hir_id, + def_id, print_preflow_to, print_postflow_to, flow_state: self, }; diff --git a/src/librustc_mir/transform/elaborate_drops.rs b/src/librustc_mir/transform/elaborate_drops.rs index 32c027d90a0ce..3b0db48a94f18 100644 --- a/src/librustc_mir/transform/elaborate_drops.rs +++ b/src/librustc_mir/transform/elaborate_drops.rs @@ -28,7 +28,7 @@ impl MirPass for ElaborateDrops { { debug!("elaborate_drops({:?} @ {:?})", src, mir.span); - let id = tcx.hir().as_local_hir_id(src.def_id()).unwrap(); + let def_id = src.def_id(); let param_env = tcx.param_env(src.def_id()).with_reveal_all(); let move_data = match MoveData::gather_moves(mir, tcx) { Ok(move_data) => move_data, @@ -50,13 +50,13 @@ impl MirPass for ElaborateDrops { move_data, param_env, }; - let dead_unwinds = find_dead_unwinds(tcx, mir, id, &env); + let dead_unwinds = find_dead_unwinds(tcx, mir, def_id, &env); let flow_inits = - do_dataflow(tcx, mir, id, &[], &dead_unwinds, + do_dataflow(tcx, mir, def_id, &[], &dead_unwinds, MaybeInitializedPlaces::new(tcx, mir, &env), |bd, p| DebugFormatted::new(&bd.move_data().move_paths[p])); let flow_uninits = - do_dataflow(tcx, mir, id, &[], &dead_unwinds, + do_dataflow(tcx, mir, def_id, &[], &dead_unwinds, MaybeUninitializedPlaces::new(tcx, mir, &env), |bd, p| DebugFormatted::new(&bd.move_data().move_paths[p])); @@ -80,7 +80,7 @@ impl MirPass for ElaborateDrops { fn find_dead_unwinds<'a, 'tcx>( tcx: TyCtxt<'a, 'tcx, 'tcx>, mir: &Mir<'tcx>, - id: hir::HirId, + def_id: hir::def_id::DefId, env: &MoveDataParamEnv<'tcx, 'tcx>) -> BitSet { @@ -89,7 +89,7 @@ fn find_dead_unwinds<'a, 'tcx>( // reach cleanup blocks, which can't have unwind edges themselves. let mut dead_unwinds = BitSet::new_empty(mir.basic_blocks().len()); let flow_inits = - do_dataflow(tcx, mir, id, &[], &dead_unwinds, + do_dataflow(tcx, mir, def_id, &[], &dead_unwinds, MaybeInitializedPlaces::new(tcx, mir, &env), |bd, p| DebugFormatted::new(&bd.move_data().move_paths[p])); for (bb, bb_data) in mir.basic_blocks().iter_enumerated() { diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index 1f59802f8c6c0..98dcc3a16f209 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -383,13 +383,13 @@ fn locals_live_across_suspend_points( FxHashMap, ) { let dead_unwinds = BitSet::new_empty(mir.basic_blocks().len()); - let hir_id = tcx.hir().as_local_hir_id(source.def_id()).unwrap(); + let def_id = source.def_id(); // Calculate when MIR locals have live storage. This gives us an upper bound of their // lifetimes. let storage_live_analysis = MaybeStorageLive::new(mir); let storage_live = - do_dataflow(tcx, mir, hir_id, &[], &dead_unwinds, storage_live_analysis, + do_dataflow(tcx, mir, def_id, &[], &dead_unwinds, storage_live_analysis, |bd, p| DebugFormatted::new(&bd.mir().local_decls[p])); // Find the MIR locals which do not use StorageLive/StorageDead statements. @@ -403,7 +403,7 @@ fn locals_live_across_suspend_points( let borrowed_locals = if !movable { let analysis = HaveBeenBorrowedLocals::new(mir); let result = - do_dataflow(tcx, mir, hir_id, &[], &dead_unwinds, analysis, + do_dataflow(tcx, mir, def_id, &[], &dead_unwinds, analysis, |bd, p| DebugFormatted::new(&bd.mir().local_decls[p])); Some((analysis, result)) } else { diff --git a/src/librustc_mir/transform/rustc_peek.rs b/src/librustc_mir/transform/rustc_peek.rs index f9f8abbe6c065..246f876235d71 100644 --- a/src/librustc_mir/transform/rustc_peek.rs +++ b/src/librustc_mir/transform/rustc_peek.rs @@ -3,7 +3,7 @@ use syntax::ast; use syntax_pos::Span; use rustc::ty::{self, TyCtxt}; -use rustc::hir; +use rustc::hir::def_id::DefId; use rustc::mir::{self, Mir, Location}; use rustc_data_structures::bit_set::BitSet; use crate::transform::{MirPass, MirSource}; @@ -27,7 +27,6 @@ impl MirPass for SanityCheck { fn run_pass<'a, 'tcx>(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>, src: MirSource<'tcx>, mir: &mut Mir<'tcx>) { let def_id = src.def_id(); - let id = tcx.hir().as_local_hir_id(def_id).unwrap(); if !tcx.has_attr(def_id, "rustc_mir") { debug!("skipping rustc_peek::SanityCheck on {}", tcx.def_path_str(def_id)); return; @@ -41,26 +40,26 @@ impl MirPass for SanityCheck { let mdpe = MoveDataParamEnv { move_data: move_data, param_env: param_env }; let dead_unwinds = BitSet::new_empty(mir.basic_blocks().len()); let flow_inits = - do_dataflow(tcx, mir, id, &attributes, &dead_unwinds, + do_dataflow(tcx, mir, def_id, &attributes, &dead_unwinds, MaybeInitializedPlaces::new(tcx, mir, &mdpe), |bd, i| DebugFormatted::new(&bd.move_data().move_paths[i])); let flow_uninits = - do_dataflow(tcx, mir, id, &attributes, &dead_unwinds, + do_dataflow(tcx, mir, def_id, &attributes, &dead_unwinds, MaybeUninitializedPlaces::new(tcx, mir, &mdpe), |bd, i| DebugFormatted::new(&bd.move_data().move_paths[i])); let flow_def_inits = - do_dataflow(tcx, mir, id, &attributes, &dead_unwinds, + do_dataflow(tcx, mir, def_id, &attributes, &dead_unwinds, DefinitelyInitializedPlaces::new(tcx, mir, &mdpe), |bd, i| DebugFormatted::new(&bd.move_data().move_paths[i])); if has_rustc_mir_with(&attributes, "rustc_peek_maybe_init").is_some() { - sanity_check_via_rustc_peek(tcx, mir, id, &attributes, &flow_inits); + sanity_check_via_rustc_peek(tcx, mir, def_id, &attributes, &flow_inits); } if has_rustc_mir_with(&attributes, "rustc_peek_maybe_uninit").is_some() { - sanity_check_via_rustc_peek(tcx, mir, id, &attributes, &flow_uninits); + sanity_check_via_rustc_peek(tcx, mir, def_id, &attributes, &flow_uninits); } if has_rustc_mir_with(&attributes, "rustc_peek_definite_init").is_some() { - sanity_check_via_rustc_peek(tcx, mir, id, &attributes, &flow_def_inits); + sanity_check_via_rustc_peek(tcx, mir, def_id, &attributes, &flow_def_inits); } if has_rustc_mir_with(&attributes, "stop_after_dataflow").is_some() { tcx.sess.fatal("stop_after_dataflow ended compilation"); @@ -86,12 +85,12 @@ impl MirPass for SanityCheck { /// errors are not intended to be used for unit tests.) pub fn sanity_check_via_rustc_peek<'a, 'tcx, O>(tcx: TyCtxt<'a, 'tcx, 'tcx>, mir: &Mir<'tcx>, - id: hir::HirId, + def_id: DefId, _attributes: &[ast::Attribute], results: &DataflowResults<'tcx, O>) where O: BitDenotation<'tcx, Idx=MovePathIndex> + HasMoveData<'tcx> { - debug!("sanity_check_via_rustc_peek id: {:?}", id); + debug!("sanity_check_via_rustc_peek def_id: {:?}", def_id); // FIXME: this is not DRY. Figure out way to abstract this and // `dataflow::build_sets`. (But note it is doing non-standard // stuff, so such generalization may not be realistic.) diff --git a/src/librustc_mir/util/graphviz.rs b/src/librustc_mir/util/graphviz.rs index 69a2adcfce026..f87714b58c442 100644 --- a/src/librustc_mir/util/graphviz.rs +++ b/src/librustc_mir/util/graphviz.rs @@ -1,6 +1,7 @@ use rustc::hir::def_id::DefId; use rustc::mir::*; use rustc::ty::TyCtxt; +use rustc_data_structures::indexed_vec::Idx; use std::fmt::Debug; use std::io::{self, Write}; @@ -20,6 +21,17 @@ pub fn write_mir_graphviz<'tcx, W>(tcx: TyCtxt<'_, '_, 'tcx>, Ok(()) } +// Must match `[0-9A-Za-z_]*`. This does not appear in the rendered graph, so +// it does not have to be user friendly. +pub fn graphviz_safe_def_name(def_id: DefId) -> String { + format!( + "{}_{}_{}", + def_id.krate.index(), + def_id.index.address_space().index(), + def_id.index.as_array_index(), + ) +} + /// Write a graphviz DOT graph of the MIR. pub fn write_mir_fn_graphviz<'tcx, W>(tcx: TyCtxt<'_, '_, 'tcx>, def_id: DefId, @@ -27,7 +39,7 @@ pub fn write_mir_fn_graphviz<'tcx, W>(tcx: TyCtxt<'_, '_, 'tcx>, w: &mut W) -> io::Result<()> where W: Write { - writeln!(w, "digraph Mir_{} {{", tcx.hir().as_local_hir_id(def_id).unwrap())?; + writeln!(w, "digraph Mir_{} {{", graphviz_safe_def_name(def_id))?; // Global graph properties writeln!(w, r#" graph [fontname="monospace"];"#)?; diff --git a/src/librustc_mir/util/mod.rs b/src/librustc_mir/util/mod.rs index 29614a33f8e27..1a5a2a92247dd 100644 --- a/src/librustc_mir/util/mod.rs +++ b/src/librustc_mir/util/mod.rs @@ -15,7 +15,7 @@ pub mod collect_writes; pub use self::alignment::is_disaligned; pub use self::pretty::{dump_enabled, dump_mir, write_mir_pretty, PassWhere}; -pub use self::graphviz::{write_mir_graphviz}; +pub use self::graphviz::{graphviz_safe_def_name, write_mir_graphviz}; pub use self::graphviz::write_node_label as write_graphviz_node_label; /// If possible, suggest replacing `ref` with `ref mut`. diff --git a/src/test/mir-opt/graphviz.rs b/src/test/mir-opt/graphviz.rs new file mode 100644 index 0000000000000..660576996e5d4 --- /dev/null +++ b/src/test/mir-opt/graphviz.rs @@ -0,0 +1,23 @@ +// Test graphviz output +// compile-flags: -Z dump-mir-graphviz + +// ignore-tidy-linelength + +fn main() {} + +// END RUST SOURCE +// START rustc.main.mir_map.0.dot +// digraph Mir_0_0_3 { // The name here MUST be an ASCII identifier. +// graph [fontname="monospace"]; +// node [fontname="monospace"]; +// edge [fontname="monospace"]; +// label=>; +// bb0 [shape="none", label=<
0
_0 = ()
goto
+// >]; +// bb1 [shape="none", label=<
1
resume
+// >]; +// bb2 [shape="none", label=<
2
return
+// >]; +// bb0 -> bb2 [label=""]; +// } +// END rustc.main.mir_map.0.dot From db74efce69711fcee03d3338afcbca67c27ceee8 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sun, 17 Mar 2019 14:17:47 +0300 Subject: [PATCH 18/21] Make meta-item API compatible with `LocalInternedString::get` soundness fix --- src/librustc/hir/check_attr.rs | 12 +---- src/librustc/lint/levels.rs | 2 +- src/librustc/lint/mod.rs | 7 +-- src/librustc/middle/lib_features.rs | 6 +-- src/librustc/middle/stability.rs | 11 ++--- src/librustc/traits/on_unimplemented.rs | 4 +- .../persist/dirty_clean.rs | 4 +- src/librustc_lint/builtin.rs | 3 +- src/librustc_lint/unused.rs | 6 +-- src/librustc_passes/layout_test.rs | 5 +- src/librustc_plugin/load.rs | 12 ++--- src/librustdoc/core.rs | 12 ++--- src/librustdoc/html/render.rs | 14 +++--- src/libsyntax/attr/builtin.rs | 46 +++++++++---------- src/libsyntax/attr/mod.rs | 14 +++--- src/libsyntax/feature_gate.rs | 16 +++---- src/libsyntax_ext/deriving/generic/mod.rs | 7 +-- 17 files changed, 84 insertions(+), 97 deletions(-) diff --git a/src/librustc/hir/check_attr.rs b/src/librustc/hir/check_attr.rs index 8602d159ba960..4c527f80d0f5d 100644 --- a/src/librustc/hir/check_attr.rs +++ b/src/librustc/hir/check_attr.rs @@ -177,16 +177,8 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> { let mut is_transparent = false; for hint in &hints { - let name = if let Some(name) = hint.ident_str() { - name - } else { - // Invalid repr hint like repr(42). We don't check for unrecognized hints here - // (libsyntax does that), so just ignore it. - continue; - }; - - let (article, allowed_targets) = match name { - "C" | "align" => { + let (article, allowed_targets) = match hint.name_or_empty().get() { + name @ "C" | name @ "align" => { is_c |= name == "C"; if target != Target::Struct && target != Target::Union && diff --git a/src/librustc/lint/levels.rs b/src/librustc/lint/levels.rs index 19e899ceb421f..2e5bd8add0cdc 100644 --- a/src/librustc/lint/levels.rs +++ b/src/librustc/lint/levels.rs @@ -194,7 +194,7 @@ impl<'a> LintLevelsBuilder<'a> { struct_span_err!(sess, span, E0452, "malformed lint attribute") }; for attr in attrs { - let level = match attr.ident_str().and_then(|name| Level::from_str(name)) { + let level = match Level::from_str(&attr.name_or_empty()) { None => continue, Some(lvl) => lvl, }; diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs index b1ff66eb64fa6..f92e33582c4c3 100644 --- a/src/librustc/lint/mod.rs +++ b/src/librustc/lint/mod.rs @@ -723,12 +723,7 @@ pub fn struct_lint_level<'a>(sess: &'a Session, pub fn maybe_lint_level_root(tcx: TyCtxt<'_, '_, '_>, id: hir::HirId) -> bool { let attrs = tcx.hir().attrs_by_hir_id(id); - for attr in attrs { - if attr.ident_str().and_then(Level::from_str).is_some() { - return true; - } - } - false + attrs.iter().any(|attr| Level::from_str(&attr.name_or_empty()).is_some()) } fn lint_levels<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, cnum: CrateNum) diff --git a/src/librustc/middle/lib_features.rs b/src/librustc/middle/lib_features.rs index 237b00db575a9..ee8dd9e58b5ff 100644 --- a/src/librustc/middle/lib_features.rs +++ b/src/librustc/middle/lib_features.rs @@ -65,9 +65,9 @@ impl<'a, 'tcx> LibFeatureCollector<'a, 'tcx> { for meta in metas { if let Some(mi) = meta.meta_item() { // Find the `feature = ".."` meta-item. - match (mi.ident_str(), mi.value_str()) { - (Some("feature"), val) => feature = val, - (Some("since"), val) => since = val, + match (mi.name_or_empty().get(), mi.value_str()) { + ("feature", val) => feature = val, + ("since", val) => since = val, _ => {} } } diff --git a/src/librustc/middle/stability.rs b/src/librustc/middle/stability.rs index 2664d6eaa2857..e63665574b940 100644 --- a/src/librustc/middle/stability.rs +++ b/src/librustc/middle/stability.rs @@ -194,12 +194,11 @@ impl<'a, 'tcx: 'a> Annotator<'a, 'tcx> { } else { // Emit errors for non-staged-api crates. for attr in attrs { - if let Some(tag) = attr.ident_str() { - if tag == "unstable" || tag == "stable" || tag == "rustc_deprecated" { - attr::mark_used(attr); - self.tcx.sess.span_err(attr.span, "stability attributes may not be used \ - outside of the standard library"); - } + let name = attr.name_or_empty(); + if ["unstable", "stable", "rustc_deprecated"].contains(&name.get()) { + attr::mark_used(attr); + self.tcx.sess.span_err(attr.span, "stability attributes may not be used \ + outside of the standard library"); } } diff --git a/src/librustc/traits/on_unimplemented.rs b/src/librustc/traits/on_unimplemented.rs index d0acaf674ae74..2b286ee1b97fb 100644 --- a/src/librustc/traits/on_unimplemented.rs +++ b/src/librustc/traits/on_unimplemented.rs @@ -177,9 +177,9 @@ impl<'a, 'gcx, 'tcx> OnUnimplementedDirective { for command in self.subcommands.iter().chain(Some(self)).rev() { if let Some(ref condition) = command.condition { if !attr::eval_condition(condition, &tcx.sess.parse_sess, &mut |c| { - c.ident_str().map_or(false, |name| { + c.ident().map_or(false, |ident| { options.contains(&( - name.to_string(), + ident.to_string(), c.value_str().map(|s| s.as_str().to_string()) )) }) diff --git a/src/librustc_incremental/persist/dirty_clean.rs b/src/librustc_incremental/persist/dirty_clean.rs index 5551cf6b3b604..0a43624c687bb 100644 --- a/src/librustc_incremental/persist/dirty_clean.rs +++ b/src/librustc_incremental/persist/dirty_clean.rs @@ -576,8 +576,8 @@ fn expect_associated_value(tcx: TyCtxt<'_, '_, '_>, item: &NestedMetaItem) -> as if let Some(value) = item.value_str() { value } else { - let msg = if let Some(name) = item.ident_str() { - format!("associated value expected for `{}`", name) + let msg = if let Some(ident) = item.ident() { + format!("associated value expected for `{}`", ident) } else { "expected an associated value".to_string() }; diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 80c5eeeeac345..37aa8b0db8be5 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -759,8 +759,9 @@ impl LintPass for DeprecatedAttr { impl EarlyLintPass for DeprecatedAttr { fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &ast::Attribute) { + let name = attr.name_or_empty(); for &&(n, _, _, ref g) in &self.depr_attrs { - if attr.ident_str() == Some(n) { + if name == n { if let &AttributeGate::Gated(Stability::Deprecated(link, suggestion), ref name, ref reason, diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index 648cae30da6c8..ebbd27798419b 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -267,21 +267,21 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedAttributes { } } - let name = attr.ident_str(); + let name = attr.name_or_empty(); if !attr::is_used(attr) { debug!("Emitting warning for: {:?}", attr); cx.span_lint(UNUSED_ATTRIBUTES, attr.span, "unused attribute"); // Is it a builtin attribute that must be used at the crate level? let known_crate = BUILTIN_ATTRIBUTES.iter() .find(|&&(builtin, ty, ..)| { - name == Some(builtin) && ty == AttributeType::CrateLevel + name == builtin && ty == AttributeType::CrateLevel }) .is_some(); // Has a plugin registered this attribute as one that must be used at // the crate level? let plugin_crate = plugin_attributes.iter() - .find(|&&(ref x, t)| name == Some(x) && AttributeType::CrateLevel == t) + .find(|&&(ref x, t)| name == x.as_str() && AttributeType::CrateLevel == t) .is_some(); if known_crate || plugin_crate { let msg = match attr.style { diff --git a/src/librustc_passes/layout_test.rs b/src/librustc_passes/layout_test.rs index 373bcf7f0e2f3..6940f8f442ee9 100644 --- a/src/librustc_passes/layout_test.rs +++ b/src/librustc_passes/layout_test.rs @@ -53,8 +53,7 @@ impl<'a, 'tcx> VarianceTest<'a, 'tcx> { // The `..` are the names of fields to dump. let meta_items = attr.meta_item_list().unwrap_or_default(); for meta_item in meta_items { - let name = meta_item.ident_str().unwrap_or(""); - match name { + match meta_item.name_or_empty().get() { "abi" => { self.tcx .sess @@ -84,7 +83,7 @@ impl<'a, 'tcx> VarianceTest<'a, 'tcx> { ); } - _ => { + name => { self.tcx.sess.span_err( meta_item.span(), &format!("unrecognized field name `{}`", name), diff --git a/src/librustc_plugin/load.rs b/src/librustc_plugin/load.rs index bd11e0ce8023a..8b86bddb29f4e 100644 --- a/src/librustc_plugin/load.rs +++ b/src/librustc_plugin/load.rs @@ -56,12 +56,12 @@ pub fn load_plugins(sess: &Session, for plugin in plugins { // plugins must have a name and can't be key = value - match plugin.ident_str() { - Some(name) if !plugin.is_value_str() => { - let args = plugin.meta_item_list().map(ToOwned::to_owned); - loader.load_plugin(plugin.span(), name, args.unwrap_or_default()); - }, - _ => call_malformed_plugin_attribute(sess, attr.span), + let name = plugin.name_or_empty(); + if !name.is_empty() && !plugin.is_value_str() { + let args = plugin.meta_item_list().map(ToOwned::to_owned); + loader.load_plugin(plugin.span(), &name, args.unwrap_or_default()); + } else { + call_malformed_plugin_attribute(sess, attr.span); } } } diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index dca6458c701c5..7a64773054234 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -521,21 +521,21 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt for attr in krate.module.as_ref().unwrap().attrs.lists("doc") { let diag = ctxt.sess().diagnostic(); - let name = attr.ident_str(); + let name = attr.name_or_empty(); if attr.is_word() { - if name == Some("no_default_passes") { + if name == "no_default_passes" { report_deprecated_attr("no_default_passes", diag); if default_passes == passes::DefaultPassOption::Default { default_passes = passes::DefaultPassOption::None; } } } else if let Some(value) = attr.value_str() { - let sink = match name { - Some("passes") => { + let sink = match name.get() { + "passes" => { report_deprecated_attr("passes = \"...\"", diag); &mut manual_passes }, - Some("plugins") => { + "plugins" => { report_deprecated_attr("plugins = \"...\"", diag); eprintln!("WARNING: #![doc(plugins = \"...\")] no longer functions; \ see CVE-2018-1000622"); @@ -548,7 +548,7 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt } } - if attr.is_word() && name == Some("document_private_items") { + if attr.is_word() && name == "document_private_items" { if default_passes == passes::DefaultPassOption::Default { default_passes = passes::DefaultPassOption::Private; } diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index b3a816d17f56c..d6850ed3bb972 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -562,23 +562,23 @@ pub fn run(mut krate: clean::Crate, // going to emit HTML if let Some(attrs) = krate.module.as_ref().map(|m| &m.attrs) { for attr in attrs.lists("doc") { - match (attr.ident_str(), attr.value_str()) { - (Some("html_favicon_url"), Some(s)) => { + match (attr.name_or_empty().get(), attr.value_str()) { + ("html_favicon_url", Some(s)) => { scx.layout.favicon = s.to_string(); } - (Some("html_logo_url"), Some(s)) => { + ("html_logo_url", Some(s)) => { scx.layout.logo = s.to_string(); } - (Some("html_playground_url"), Some(s)) => { + ("html_playground_url", Some(s)) => { markdown::PLAYGROUND.with(|slot| { let name = krate.name.clone(); *slot.borrow_mut() = Some((Some(name), s.to_string())); }); } - (Some("issue_tracker_base_url"), Some(s)) => { + ("issue_tracker_base_url", Some(s)) => { scx.issue_tracker_base_url = Some(s.to_string()); } - (Some("html_no_source"), None) if attr.is_word() => { + ("html_no_source", None) if attr.is_word() => { scx.include_sources = false; } _ => {} @@ -3749,7 +3749,7 @@ fn render_attributes(w: &mut fmt::Formatter<'_>, it: &clean::Item) -> fmt::Resul let mut attrs = String::new(); for attr in &it.attrs.other_attrs { - if !attr.ident_str().map_or(false, |name| ATTRIBUTE_WHITELIST.contains(&name)) { + if !ATTRIBUTE_WHITELIST.contains(&attr.name_or_empty().get()) { continue; } if let Some(s) = render_attribute(&attr.meta().unwrap()) { diff --git a/src/libsyntax/attr/builtin.rs b/src/libsyntax/attr/builtin.rs index cdfb83c6e56c8..74c952b076cd7 100644 --- a/src/libsyntax/attr/builtin.rs +++ b/src/libsyntax/attr/builtin.rs @@ -222,9 +222,9 @@ fn find_stability_generic<'a, I>(sess: &ParseSess, )+ for meta in metas { if let Some(mi) = meta.meta_item() { - match mi.ident_str() { + match mi.name_or_empty().get() { $( - Some(stringify!($name)) + stringify!($name) => if !get(mi, &mut $name) { continue 'outer }, )+ _ => { @@ -252,7 +252,7 @@ fn find_stability_generic<'a, I>(sess: &ParseSess, } } - match meta.ident_str().expect("not a stability level") { + match meta.name_or_empty().get() { "rustc_deprecated" => { if rustc_depr.is_some() { span_err!(diagnostic, item_sp, E0540, @@ -306,10 +306,10 @@ fn find_stability_generic<'a, I>(sess: &ParseSess, let mut issue = None; for meta in metas { if let Some(mi) = meta.meta_item() { - match mi.ident_str() { - Some("feature") => if !get(mi, &mut feature) { continue 'outer }, - Some("reason") => if !get(mi, &mut reason) { continue 'outer }, - Some("issue") => if !get(mi, &mut issue) { continue 'outer }, + match mi.name_or_empty().get() { + "feature" => if !get(mi, &mut feature) { continue 'outer }, + "reason" => if !get(mi, &mut reason) { continue 'outer }, + "issue" => if !get(mi, &mut issue) { continue 'outer }, _ => { handle_errors( sess, @@ -377,10 +377,10 @@ fn find_stability_generic<'a, I>(sess: &ParseSess, for meta in metas { match meta { NestedMetaItem::MetaItem(mi) => { - match mi.ident_str() { - Some("feature") => + match mi.name_or_empty().get() { + "feature" => if !get(mi, &mut feature) { continue 'outer }, - Some("since") => + "since" => if !get(mi, &mut since) { continue 'outer }, _ => { handle_errors( @@ -532,14 +532,14 @@ pub fn eval_condition(cfg: &ast::MetaItem, sess: &ParseSess, eval: &mut F) // The unwraps below may look dangerous, but we've already asserted // that they won't fail with the loop above. - match cfg.ident_str() { - Some("any") => mis.iter().any(|mi| { + match cfg.name_or_empty().get() { + "any" => mis.iter().any(|mi| { eval_condition(mi.meta_item().unwrap(), sess, eval) }), - Some("all") => mis.iter().all(|mi| { + "all" => mis.iter().all(|mi| { eval_condition(mi.meta_item().unwrap(), sess, eval) }), - Some("not") => { + "not" => { if mis.len() != 1 { span_err!(sess.span_diagnostic, cfg.span, E0536, "expected 1 cfg-pattern"); return false; @@ -635,9 +635,9 @@ fn find_deprecation_generic<'a, I>(sess: &ParseSess, for meta in list { match meta { NestedMetaItem::MetaItem(mi) => { - match mi.ident_str() { - Some("since") => if !get(mi, &mut since) { continue 'outer }, - Some("note") => if !get(mi, &mut note) { continue 'outer }, + match mi.name_or_empty().get() { + "since" => if !get(mi, &mut since) { continue 'outer }, + "note" => if !get(mi, &mut note) { continue 'outer }, _ => { handle_errors( sess, @@ -729,12 +729,12 @@ pub fn find_repr_attrs(sess: &ParseSess, attr: &Attribute) -> Vec { let mut recognised = false; if item.is_word() { - let hint = match item.ident_str() { - Some("C") => Some(ReprC), - Some("packed") => Some(ReprPacked(1)), - Some("simd") => Some(ReprSimd), - Some("transparent") => Some(ReprTransparent), - name => name.and_then(|name| int_type_of_word(name)).map(ReprInt), + let hint = match item.name_or_empty().get() { + "C" => Some(ReprC), + "packed" => Some(ReprPacked(1)), + "simd" => Some(ReprSimd), + "transparent" => Some(ReprTransparent), + name => int_type_of_word(name).map(ReprInt), }; if let Some(h) = hint { diff --git a/src/libsyntax/attr/mod.rs b/src/libsyntax/attr/mod.rs index 1a8faa43fff7c..c0bd5c79b1dd1 100644 --- a/src/libsyntax/attr/mod.rs +++ b/src/libsyntax/attr/mod.rs @@ -22,7 +22,7 @@ use crate::parse::parser::Parser; use crate::parse::{self, ParseSess, PResult}; use crate::parse::token::{self, Token}; use crate::ptr::P; -use crate::symbol::Symbol; +use crate::symbol::{keywords, LocalInternedString, Symbol}; use crate::ThinVec; use crate::tokenstream::{TokenStream, TokenTree, DelimSpan}; use crate::GLOBALS; @@ -89,8 +89,8 @@ impl NestedMetaItem { pub fn ident(&self) -> Option { self.meta_item().and_then(|meta_item| meta_item.ident()) } - pub fn ident_str(&self) -> Option<&str> { - self.ident().map(|name| name.as_str().get()) + pub fn name_or_empty(&self) -> LocalInternedString { + self.ident().unwrap_or(keywords::Invalid.ident()).name.as_str() } /// Gets the string value if self is a MetaItem and the MetaItem is a @@ -167,8 +167,8 @@ impl Attribute { None } } - pub fn ident_str(&self) -> Option<&str> { - self.ident().map(|name| name.as_str().get()) + pub fn name_or_empty(&self) -> LocalInternedString { + self.ident().unwrap_or(keywords::Invalid.ident()).name.as_str() } pub fn value_str(&self) -> Option { @@ -205,8 +205,8 @@ impl MetaItem { None } } - pub fn ident_str(&self) -> Option<&str> { - self.ident().map(|name| name.as_str().get()) + pub fn name_or_empty(&self) -> LocalInternedString { + self.ident().unwrap_or(keywords::Invalid.ident()).name.as_str() } // #[attribute(name = "value")] diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index 9beaabb0cd58b..d1d0d79a705da 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -1341,9 +1341,9 @@ macro_rules! gate_feature { impl<'a> Context<'a> { fn check_attribute(&self, attr: &ast::Attribute, is_macro: bool) { debug!("check_attribute(attr = {:?})", attr); - let name = attr.ident_str(); + let name = attr.name_or_empty(); for &(n, ty, _template, ref gateage) in BUILTIN_ATTRIBUTES { - if name == Some(n) { + if name == n { if let Gated(_, name, desc, ref has_feature) = *gateage { if !attr.span.allows_unstable(name) { gate_feature_fn!( @@ -1373,7 +1373,7 @@ impl<'a> Context<'a> { } } if !attr::is_known(attr) { - if name.map_or(false, |name| name.starts_with("rustc_")) { + if name.starts_with("rustc_") { let msg = "unless otherwise specified, attributes with the prefix `rustc_` \ are reserved for internal compiler diagnostics"; gate_feature!(self, rustc_attrs, attr.span, msg); @@ -2054,12 +2054,12 @@ pub fn get_features(span_handler: &Handler, krate_attrs: &[ast::Attribute], }; for mi in list { - let name = match mi.ident_str() { - Some(name) if mi.is_word() => name, - _ => continue, - }; + if !mi.is_word() { + continue; + } - if incomplete_features.iter().any(|f| *f == name) { + let name = mi.name_or_empty(); + if incomplete_features.iter().any(|f| name == *f) { span_handler.struct_span_warn( mi.span(), &format!( diff --git a/src/libsyntax_ext/deriving/generic/mod.rs b/src/libsyntax_ext/deriving/generic/mod.rs index 2bb98c1bf625c..c790e6ba7f17c 100644 --- a/src/libsyntax_ext/deriving/generic/mod.rs +++ b/src/libsyntax_ext/deriving/generic/mod.rs @@ -463,9 +463,10 @@ impl<'a> TraitDef<'a> { let mut attrs = newitem.attrs.clone(); attrs.extend(item.attrs .iter() - .filter(|a| a.ident_str().map_or(false, |name| { - ["allow", "warn", "deny", "forbid", "stable", "unstable"].contains(&name) - })) + .filter(|a| { + ["allow", "warn", "deny", "forbid", "stable", "unstable"] + .contains(&a.name_or_empty().get()) + }) .cloned()); push(Annotatable::Item(P(ast::Item { attrs: attrs, ..(*newitem).clone() }))) } From 6007e6f649feac9a0cb39270444033f8dbfa9259 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 17 Mar 2019 20:09:53 -0700 Subject: [PATCH 19/21] Do not complain about non-existing fields after parse recovery When failing to parse struct-like enum variants, the ADT gets recorded as having no fields. Record that we have actually recovered during parsing of this variant to avoid complaing about non-existing fields when actually using it. --- src/librustc/hir/lowering.rs | 3 +- src/librustc/hir/mod.rs | 4 +-- src/librustc/ty/mod.rs | 28 +++++++++++-------- src/librustc_metadata/decoder.rs | 3 +- src/librustc_save_analysis/dump_visitor.rs | 6 ++-- src/librustc_save_analysis/sig.rs | 18 +++++++----- src/librustc_typeck/check/_match.rs | 20 +++++++------ src/librustc_typeck/check/mod.rs | 17 +++++++---- src/librustc_typeck/collect.rs | 7 ++++- src/libsyntax/ast.rs | 6 ++-- src/libsyntax/config.rs | 2 +- src/libsyntax/mut_visit.rs | 2 +- src/libsyntax/parse/parser.rs | 28 ++++++++++++------- .../ui/parser/recovered-struct-variant.rs | 13 +++++++++ .../ui/parser/recovered-struct-variant.stderr | 8 ++++++ 15 files changed, 108 insertions(+), 57 deletions(-) create mode 100644 src/test/ui/parser/recovered-struct-variant.rs create mode 100644 src/test/ui/parser/recovered-struct-variant.stderr diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 949fdd2682b96..73c3b3026d98b 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -2672,7 +2672,7 @@ impl<'a> LoweringContext<'a> { fn lower_variant_data(&mut self, vdata: &VariantData) -> hir::VariantData { match *vdata { - VariantData::Struct(ref fields, id) => { + VariantData::Struct(ref fields, id, recovered) => { let LoweredNodeId { node_id: _, hir_id } = self.lower_node_id(id); hir::VariantData::Struct( @@ -2682,6 +2682,7 @@ impl<'a> LoweringContext<'a> { .map(|f| self.lower_struct_field(f)) .collect(), hir_id, + recovered, ) }, VariantData::Tuple(ref fields, id) => { diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 88ab58d10fc34..785d7745b297b 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -2173,7 +2173,7 @@ impl StructField { /// Id of the whole struct lives in `Item`. #[derive(Clone, RustcEncodable, RustcDecodable, Debug, HashStable)] pub enum VariantData { - Struct(HirVec, HirId), + Struct(HirVec, HirId, bool), Tuple(HirVec, HirId), Unit(HirId), } @@ -2187,7 +2187,7 @@ impl VariantData { } pub fn hir_id(&self) -> HirId { match *self { - VariantData::Struct(_, hir_id) + VariantData::Struct(_, hir_id, _) | VariantData::Tuple(_, hir_id) | VariantData::Unit(hir_id) => hir_id, } diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 882e2dc62b1c3..47dbed914ddc3 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -1811,6 +1811,7 @@ pub struct VariantDef { pub fields: Vec, pub ctor_kind: CtorKind, flags: VariantFlags, + pub recovered: bool, } impl<'a, 'gcx, 'tcx> VariantDef { @@ -1829,16 +1830,17 @@ impl<'a, 'gcx, 'tcx> VariantDef { /// /// If someone speeds up attribute loading to not be a performance concern, they can /// remove this hack and use the constructor `DefId` everywhere. - pub fn new(tcx: TyCtxt<'a, 'gcx, 'tcx>, - did: DefId, - ident: Ident, - discr: VariantDiscr, - fields: Vec, - adt_kind: AdtKind, - ctor_kind: CtorKind, - attribute_def_id: DefId) - -> Self - { + pub fn new( + tcx: TyCtxt<'a, 'gcx, 'tcx>, + did: DefId, + ident: Ident, + discr: VariantDiscr, + fields: Vec, + adt_kind: AdtKind, + ctor_kind: CtorKind, + attribute_def_id: DefId, + recovered: bool, + ) -> Self { debug!("VariantDef::new({:?}, {:?}, {:?}, {:?}, {:?}, {:?}, {:?})", did, ident, discr, fields, adt_kind, ctor_kind, attribute_def_id); let mut flags = VariantFlags::NO_VARIANT_FLAGS; @@ -1852,7 +1854,8 @@ impl<'a, 'gcx, 'tcx> VariantDef { discr, fields, ctor_kind, - flags + flags, + recovered, } } @@ -1868,7 +1871,8 @@ impl_stable_hash_for!(struct VariantDef { discr, fields, ctor_kind, - flags + flags, + recovered }); #[derive(Copy, Clone, Debug, PartialEq, Eq, RustcEncodable, RustcDecodable, HashStable)] diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs index 6fe00a4ad2ff2..c608c03095aa3 100644 --- a/src/librustc_metadata/decoder.rs +++ b/src/librustc_metadata/decoder.rs @@ -576,7 +576,8 @@ impl<'a, 'tcx> CrateMetadata { }).collect(), adt_kind, data.ctor_kind, - attribute_def_id + attribute_def_id, + false, ) } diff --git a/src/librustc_save_analysis/dump_visitor.rs b/src/librustc_save_analysis/dump_visitor.rs index 3fea515ae401e..01bb643c1d587 100644 --- a/src/librustc_save_analysis/dump_visitor.rs +++ b/src/librustc_save_analysis/dump_visitor.rs @@ -481,8 +481,8 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> { }; let (value, fields) = match item.node { - ast::ItemKind::Struct(ast::VariantData::Struct(ref fields, _), _) | - ast::ItemKind::Union(ast::VariantData::Struct(ref fields, _), _) => { + ast::ItemKind::Struct(ast::VariantData::Struct(ref fields, ..), _) | + ast::ItemKind::Union(ast::VariantData::Struct(ref fields, ..), _) => { let include_priv_fields = !self.save_ctxt.config.pub_only; let fields_str = fields .iter() @@ -560,7 +560,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> { let name_span = variant.node.ident.span; match variant.node.data { - ast::VariantData::Struct(ref fields, _) => { + ast::VariantData::Struct(ref fields, ..) => { let fields_str = fields .iter() .enumerate() diff --git a/src/librustc_save_analysis/sig.rs b/src/librustc_save_analysis/sig.rs index 64a2c92d04dbd..6e47ae6b15984 100644 --- a/src/librustc_save_analysis/sig.rs +++ b/src/librustc_save_analysis/sig.rs @@ -703,7 +703,7 @@ impl Sig for ast::Variant_ { fn make(&self, offset: usize, _parent_id: Option, scx: &SaveContext<'_, '_>) -> Result { let mut text = self.ident.to_string(); match self.data { - ast::VariantData::Struct(ref fields, id) => { + ast::VariantData::Struct(ref fields, id, r) => { let name_def = SigElement { id: id_from_node_id(id, scx), start: offset, @@ -712,12 +712,16 @@ impl Sig for ast::Variant_ { text.push_str(" { "); let mut defs = vec![name_def]; let mut refs = vec![]; - for f in fields { - let field_sig = f.make(offset + text.len(), Some(id), scx)?; - text.push_str(&field_sig.text); - text.push_str(", "); - defs.extend(field_sig.defs.into_iter()); - refs.extend(field_sig.refs.into_iter()); + if r { + text.push_str("/* parse error */ "); + } else { + for f in fields { + let field_sig = f.make(offset + text.len(), Some(id), scx)?; + text.push_str(&field_sig.text); + text.push_str(", "); + defs.extend(field_sig.defs.into_iter()); + refs.extend(field_sig.refs.into_iter()); + } } text.push('}'); Ok(Signature { text, defs, refs }) diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index c6b66393dd2f1..c30b9d65fec83 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -918,14 +918,16 @@ https://doc.rust-lang.org/reference/types.html#trait-objects"); pat_ty } - fn check_struct_pat_fields(&self, - adt_ty: Ty<'tcx>, - pat_id: hir::HirId, - span: Span, - variant: &'tcx ty::VariantDef, - fields: &'gcx [Spanned], - etc: bool, - def_bm: ty::BindingMode) -> bool { + fn check_struct_pat_fields( + &self, + adt_ty: Ty<'tcx>, + pat_id: hir::HirId, + span: Span, + variant: &'tcx ty::VariantDef, + fields: &'gcx [Spanned], + etc: bool, + def_bm: ty::BindingMode, + ) -> bool { let tcx = self.tcx; let (substs, adt) = match adt_ty.sty { @@ -985,7 +987,7 @@ https://doc.rust-lang.org/reference/types.html#trait-objects"); .map(|field| field.ident.modern()) .filter(|ident| !used_fields.contains_key(&ident)) .collect::>(); - if inexistent_fields.len() > 0 { + if inexistent_fields.len() > 0 && !variant.recovered { let (field_names, t, plural) = if inexistent_fields.len() == 1 { (format!("a field named `{}`", inexistent_fields[0].1), "this", "") } else { diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index fa4bb02189f20..e38a575c560e3 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -3689,12 +3689,17 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { field, expr_t) } - fn report_unknown_field(&self, - ty: Ty<'tcx>, - variant: &'tcx ty::VariantDef, - field: &hir::Field, - skip_fields: &[hir::Field], - kind_name: &str) { + fn report_unknown_field( + &self, + ty: Ty<'tcx>, + variant: &'tcx ty::VariantDef, + field: &hir::Field, + skip_fields: &[hir::Field], + kind_name: &str, + ) { + if variant.recovered { + return; + } let mut err = self.type_error_struct_with_diag( field.ident.span, |actual| match ty.sty { diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 10e9613bf21a2..c0739db3df6a2 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -598,6 +598,10 @@ fn convert_variant<'a, 'tcx>( } }) .collect(); + let recovered = match def { + hir::VariantData::Struct(_, _, r) => *r, + _ => false, + }; ty::VariantDef::new(tcx, did, ident, @@ -605,7 +609,8 @@ fn convert_variant<'a, 'tcx>( fields, adt_kind, CtorKind::from_hir(def), - attribute_def_id + attribute_def_id, + recovered, ) } diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index 1a0da73880cfc..5da00ef8ebee2 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -2133,7 +2133,7 @@ pub enum VariantData { /// Struct variant. /// /// E.g., `Bar { .. }` as in `enum Foo { Bar { .. } }`. - Struct(Vec, NodeId), + Struct(Vec, NodeId, bool), /// Tuple variant. /// /// E.g., `Bar(..)` as in `enum Foo { Bar(..) }`. @@ -2147,13 +2147,13 @@ pub enum VariantData { impl VariantData { pub fn fields(&self) -> &[StructField] { match *self { - VariantData::Struct(ref fields, _) | VariantData::Tuple(ref fields, _) => fields, + VariantData::Struct(ref fields, ..) | VariantData::Tuple(ref fields, _) => fields, _ => &[], } } pub fn id(&self) -> NodeId { match *self { - VariantData::Struct(_, id) | VariantData::Tuple(_, id) | VariantData::Unit(id) => id, + VariantData::Struct(_, id, _) | VariantData::Tuple(_, id) | VariantData::Unit(id) => id, } } pub fn is_struct(&self) -> bool { diff --git a/src/libsyntax/config.rs b/src/libsyntax/config.rs index c300ffc2d61b9..7159c949513ac 100644 --- a/src/libsyntax/config.rs +++ b/src/libsyntax/config.rs @@ -225,7 +225,7 @@ impl<'a> StripUnconfigured<'a> { fn configure_variant_data(&mut self, vdata: &mut ast::VariantData) { match vdata { - ast::VariantData::Struct(fields, _id) | + ast::VariantData::Struct(fields, _id, _) | ast::VariantData::Tuple(fields, _id) => fields.flat_map_in_place(|field| self.configure(field)), ast::VariantData::Unit(_id) => {} diff --git a/src/libsyntax/mut_visit.rs b/src/libsyntax/mut_visit.rs index 462346df7d76d..5bb1d8a4b9476 100644 --- a/src/libsyntax/mut_visit.rs +++ b/src/libsyntax/mut_visit.rs @@ -765,7 +765,7 @@ pub fn noop_visit_where_predicate(pred: &mut WherePredicate, vis: pub fn noop_visit_variant_data(vdata: &mut VariantData, vis: &mut T) { match vdata { - VariantData::Struct(fields, id) | + VariantData::Struct(fields, id, _) | VariantData::Tuple(fields, id) => { visit_vec(fields, |field| vis.visit_struct_field(field)); vis.visit_id(id); diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index aa70c54a1ef8a..360a101a64faf 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -6829,14 +6829,16 @@ impl<'a> Parser<'a> { VariantData::Unit(ast::DUMMY_NODE_ID) } else { // If we see: `struct Foo where T: Copy { ... }` - VariantData::Struct(self.parse_record_struct_body()?, ast::DUMMY_NODE_ID) + let (fields, recovered) = self.parse_record_struct_body()?; + VariantData::Struct(fields, ast::DUMMY_NODE_ID, recovered) } // No `where` so: `struct Foo;` } else if self.eat(&token::Semi) { VariantData::Unit(ast::DUMMY_NODE_ID) // Record-style struct definition } else if self.token == token::OpenDelim(token::Brace) { - VariantData::Struct(self.parse_record_struct_body()?, ast::DUMMY_NODE_ID) + let (fields, recovered) = self.parse_record_struct_body()?; + VariantData::Struct(fields, ast::DUMMY_NODE_ID, recovered) // Tuple-style struct definition with optional where-clause. } else if self.token == token::OpenDelim(token::Paren) { let body = VariantData::Tuple(self.parse_tuple_struct_body()?, ast::DUMMY_NODE_ID); @@ -6864,9 +6866,11 @@ impl<'a> Parser<'a> { let vdata = if self.token.is_keyword(keywords::Where) { generics.where_clause = self.parse_where_clause()?; - VariantData::Struct(self.parse_record_struct_body()?, ast::DUMMY_NODE_ID) + let (fields, recovered) = self.parse_record_struct_body()?; + VariantData::Struct(fields, ast::DUMMY_NODE_ID, recovered) } else if self.token == token::OpenDelim(token::Brace) { - VariantData::Struct(self.parse_record_struct_body()?, ast::DUMMY_NODE_ID) + let (fields, recovered) = self.parse_record_struct_body()?; + VariantData::Struct(fields, ast::DUMMY_NODE_ID, recovered) } else { let token_str = self.this_token_descr(); let mut err = self.fatal(&format!( @@ -6898,12 +6902,14 @@ impl<'a> Parser<'a> { } } - fn parse_record_struct_body(&mut self) -> PResult<'a, Vec> { + fn parse_record_struct_body(&mut self) -> PResult<'a, (Vec, bool)> { let mut fields = Vec::new(); + let mut recovered = false; if self.eat(&token::OpenDelim(token::Brace)) { while self.token != token::CloseDelim(token::Brace) { let field = self.parse_struct_decl_field().map_err(|e| { self.recover_stmt(); + recovered = true; e }); match field { @@ -6922,7 +6928,7 @@ impl<'a> Parser<'a> { return Err(err); } - Ok(fields) + Ok((fields, recovered)) } fn parse_tuple_struct_body(&mut self) -> PResult<'a, Vec> { @@ -7684,12 +7690,14 @@ impl<'a> Parser<'a> { if self.check(&token::OpenDelim(token::Brace)) { // Parse a struct variant. all_nullary = false; - struct_def = VariantData::Struct(self.parse_record_struct_body()?, - ast::DUMMY_NODE_ID); + let (fields, recovered) = self.parse_record_struct_body()?; + struct_def = VariantData::Struct(fields, ast::DUMMY_NODE_ID, recovered); } else if self.check(&token::OpenDelim(token::Paren)) { all_nullary = false; - struct_def = VariantData::Tuple(self.parse_tuple_struct_body()?, - ast::DUMMY_NODE_ID); + struct_def = VariantData::Tuple( + self.parse_tuple_struct_body()?, + ast::DUMMY_NODE_ID, + ); } else if self.eat(&token::Eq) { disr_expr = Some(AnonConst { id: ast::DUMMY_NODE_ID, diff --git a/src/test/ui/parser/recovered-struct-variant.rs b/src/test/ui/parser/recovered-struct-variant.rs new file mode 100644 index 0000000000000..5b195dcc37878 --- /dev/null +++ b/src/test/ui/parser/recovered-struct-variant.rs @@ -0,0 +1,13 @@ +enum Foo { + A { a, b: usize } + //~^ ERROR expected `:`, found `,` +} + +fn main() { + // no complaints about non-existing fields + let f = Foo::A { a:3, b: 4}; + match f { + // no complaints about non-existing fields + Foo::A {a, b} => {} + } +} diff --git a/src/test/ui/parser/recovered-struct-variant.stderr b/src/test/ui/parser/recovered-struct-variant.stderr new file mode 100644 index 0000000000000..51aaf8bb3cfbe --- /dev/null +++ b/src/test/ui/parser/recovered-struct-variant.stderr @@ -0,0 +1,8 @@ +error: expected `:`, found `,` + --> $DIR/recovered-struct-variant.rs:2:10 + | +LL | A { a, b: usize } + | ^ expected `:` + +error: aborting due to previous error + From 30d5dc9a0ac48b7fae1e5ca79987fa004b00eec1 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 19 Mar 2019 21:32:17 +0300 Subject: [PATCH 20/21] Do not encode gensymed imports in metadata --- src/librustc_resolve/build_reduced_graph.rs | 4 ++-- src/librustc_resolve/resolve_imports.rs | 6 ++++-- src/libsyntax_pos/symbol.rs | 4 ++++ src/test/ui/imports/auxiliary/gensymed.rs | 3 +++ src/test/ui/imports/gensymed.rs | 7 +++++++ 5 files changed, 20 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/imports/auxiliary/gensymed.rs create mode 100644 src/test/ui/imports/gensymed.rs diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 6fad4b2db9713..f312ef216822d 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -303,7 +303,7 @@ impl<'a> Resolver<'a> { } // Empty groups `a::b::{}` are turned into synthetic `self` imports - // `a::b::c::{self as _}`, so that their prefixes are correctly + // `a::b::c::{self as __dummy}`, so that their prefixes are correctly // resolved and checked for privacy/stability/etc. if items.is_empty() && !empty_for_self(&prefix) { let new_span = prefix[prefix.len() - 1].ident.span; @@ -312,7 +312,7 @@ impl<'a> Resolver<'a> { Ident::new(keywords::SelfLower.name(), new_span) ), kind: ast::UseTreeKind::Simple( - Some(Ident::new(keywords::Underscore.name().gensymed(), new_span)), + Some(Ident::new(Name::gensym("__dummy"), new_span)), ast::DUMMY_NODE_ID, ast::DUMMY_NODE_ID, ), diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 9daffd522bf2c..bda59c6c46c8b 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -1295,9 +1295,11 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { None => continue, }; - // Filter away "empty import canaries" and ambiguous imports. + // Filter away ambiguous and gensymed imports. Gensymed imports + // (e.g. implicitly injected `std`) cannot be properly encoded in metadata, + // so they can cause name conflict errors downstream. let is_good_import = binding.is_import() && !binding.is_ambiguity() && - binding.vis != ty::Visibility::Invisible; + !(ident.name.is_gensymed() && ident.name != "_"); if is_good_import || binding.is_macro_def() { let def = binding.def(); if def != Def::Err { diff --git a/src/libsyntax_pos/symbol.rs b/src/libsyntax_pos/symbol.rs index e8d215a562e2c..f61aa4284d29b 100644 --- a/src/libsyntax_pos/symbol.rs +++ b/src/libsyntax_pos/symbol.rs @@ -179,6 +179,10 @@ impl Symbol { with_interner(|interner| interner.gensymed(self)) } + pub fn is_gensymed(self) -> bool { + with_interner(|interner| interner.is_gensymed(self)) + } + pub fn as_str(self) -> LocalInternedString { with_interner(|interner| unsafe { LocalInternedString { diff --git a/src/test/ui/imports/auxiliary/gensymed.rs b/src/test/ui/imports/auxiliary/gensymed.rs new file mode 100644 index 0000000000000..bbb19f5ec6519 --- /dev/null +++ b/src/test/ui/imports/auxiliary/gensymed.rs @@ -0,0 +1,3 @@ +// edition:2018 + +mod std {} diff --git a/src/test/ui/imports/gensymed.rs b/src/test/ui/imports/gensymed.rs new file mode 100644 index 0000000000000..317441079ff87 --- /dev/null +++ b/src/test/ui/imports/gensymed.rs @@ -0,0 +1,7 @@ +// compile-pass +// edition:2018 +// aux-build:gensymed.rs + +extern crate gensymed; + +fn main() {} From 757eb679927e2c85457f567487e887eb080eb7cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 19 Mar 2019 13:17:25 -0700 Subject: [PATCH 21/21] review comments --- src/librustc/hir/mod.rs | 2 +- src/libsyntax/ast.rs | 2 +- src/libsyntax/parse/parser.rs | 4 +++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 785d7745b297b..51d91cc562f68 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -2173,7 +2173,7 @@ impl StructField { /// Id of the whole struct lives in `Item`. #[derive(Clone, RustcEncodable, RustcDecodable, Debug, HashStable)] pub enum VariantData { - Struct(HirVec, HirId, bool), + Struct(HirVec, HirId, /* recovered */ bool), Tuple(HirVec, HirId), Unit(HirId), } diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index 5da00ef8ebee2..2cbd2dfeb25d6 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -620,7 +620,7 @@ pub enum PatKind { /// A struct or struct variant pattern (e.g., `Variant {x, y, ..}`). /// The `bool` is `true` in the presence of a `..`. - Struct(Path, Vec>, bool), + Struct(Path, Vec>, /* recovered */ bool), /// A tuple struct/variant pattern (`Variant(x, y, .., z)`). /// If the `..` pattern fragment is present, then `Option` denotes its position. diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 360a101a64faf..361ecce5bc438 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -6902,7 +6902,9 @@ impl<'a> Parser<'a> { } } - fn parse_record_struct_body(&mut self) -> PResult<'a, (Vec, bool)> { + fn parse_record_struct_body( + &mut self, + ) -> PResult<'a, (Vec, /* recovered */ bool)> { let mut fields = Vec::new(); let mut recovered = false; if self.eat(&token::OpenDelim(token::Brace)) {