From f45bdcce69c754a3e5308b71c810e233a100f65c Mon Sep 17 00:00:00 2001 From: Xavientois Date: Fri, 15 Jan 2021 14:20:51 -0500 Subject: [PATCH 01/63] Implement size_hint for BufReader --- library/std/src/io/mod.rs | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index db3b0e2628f2a..0fc99baa66639 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -2216,7 +2216,11 @@ impl Read for Chain { unsafe fn initializer(&self) -> Initializer { let initializer = self.first.initializer(); - if initializer.should_initialize() { initializer } else { self.second.initializer() } + if initializer.should_initialize() { + initializer + } else { + self.second.initializer() + } } } @@ -2235,7 +2239,11 @@ impl BufRead for Chain { } fn consume(&mut self, amt: usize) { - if !self.done_first { self.first.consume(amt) } else { self.second.consume(amt) } + if !self.done_first { + self.first.consume(amt) + } else { + self.second.consume(amt) + } } } @@ -2465,6 +2473,17 @@ impl Iterator for Bytes { }; } } + + default fn size_hint(&self) -> (usize, Option) { + (0, None) + } +} + +#[stable(feature = "bufreader_size_hint", since = "1.51.0")] +impl Iterator for Bytes> { + fn size_hint(&self) -> (usize, Option) { + (self.inner.buffer().len(), None) + } } /// An iterator over the contents of an instance of `BufRead` split on a From c3e47d974ab1c1c4d4b00989631586e60d78b554 Mon Sep 17 00:00:00 2001 From: Xavientois Date: Fri, 15 Jan 2021 14:43:20 -0500 Subject: [PATCH 02/63] Fix implementation to specialize --- library/std/src/io/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index 0fc99baa66639..e02ee58cdbcb2 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -2480,7 +2480,7 @@ impl Iterator for Bytes { } #[stable(feature = "bufreader_size_hint", since = "1.51.0")] -impl Iterator for Bytes> { +impl Iterator for Bytes> { fn size_hint(&self) -> (usize, Option) { (self.inner.buffer().len(), None) } From fa76db3104c11416f125be7fcc27d2435dcb84c5 Mon Sep 17 00:00:00 2001 From: Xavientois Date: Fri, 15 Jan 2021 17:37:29 -0500 Subject: [PATCH 03/63] Use helper trait to follow min_specialization rules --- library/std/src/io/mod.rs | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index e02ee58cdbcb2..c98f8e383eda0 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -2459,7 +2459,7 @@ pub struct Bytes { } #[stable(feature = "rust1", since = "1.0.0")] -impl Iterator for Bytes { +impl Iterator for Bytes { type Item = Result; fn next(&mut self) -> Option> { @@ -2475,14 +2475,34 @@ impl Iterator for Bytes { } default fn size_hint(&self) -> (usize, Option) { - (0, None) + self.inner.size_hint() } } #[stable(feature = "bufreader_size_hint", since = "1.51.0")] -impl Iterator for Bytes> { +trait SizeHint { + fn lower_bound(&self) -> usize; + + fn upper_bound(&self) -> Option { + None + } + fn size_hint(&self) -> (usize, Option) { - (self.inner.buffer().len(), None) + (self.lower_bound(), self.upper_bound()) + } +} + +#[stable(feature = "bufreader_size_hint", since = "1.51.0")] +impl SizeHint for T { + fn lower_bound(&self) -> usize { + 0 + } +} + +#[stable(feature = "bufreader_size_hint", since = "1.51.0")] +impl SizeHint for BufReader { + fn lower_bound(&self) -> usize { + self.buffer().len() } } From 11c49f6a2a6cc7aa5b0223c4d2ec59fb4fc9b739 Mon Sep 17 00:00:00 2001 From: Xavientois Date: Fri, 15 Jan 2021 17:41:42 -0500 Subject: [PATCH 04/63] Add missing generic --- library/std/src/io/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index c98f8e383eda0..ae01972339498 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -2493,7 +2493,7 @@ trait SizeHint { } #[stable(feature = "bufreader_size_hint", since = "1.51.0")] -impl SizeHint for T { +impl SizeHint for T { fn lower_bound(&self) -> usize { 0 } From 260a270f7c531a3b8f951b69c9e793e3afd67c89 Mon Sep 17 00:00:00 2001 From: Xavientois Date: Fri, 15 Jan 2021 17:47:25 -0500 Subject: [PATCH 05/63] Move default to trait definition --- library/std/src/io/mod.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index ae01972339498..cd54b8cc5699e 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -2481,7 +2481,9 @@ impl Iterator for Bytes { #[stable(feature = "bufreader_size_hint", since = "1.51.0")] trait SizeHint { - fn lower_bound(&self) -> usize; + fn lower_bound(&self) -> usize { + 0 + } fn upper_bound(&self) -> Option { None @@ -2493,11 +2495,7 @@ trait SizeHint { } #[stable(feature = "bufreader_size_hint", since = "1.51.0")] -impl SizeHint for T { - fn lower_bound(&self) -> usize { - 0 - } -} +impl SizeHint for T; #[stable(feature = "bufreader_size_hint", since = "1.51.0")] impl SizeHint for BufReader { From 5f60a3048ed0d6b2b56e253769fbd6593ccb1d71 Mon Sep 17 00:00:00 2001 From: Xavientois Date: Fri, 15 Jan 2021 17:52:55 -0500 Subject: [PATCH 06/63] Fix incorrect token --- library/std/src/io/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index cd54b8cc5699e..53da24409459d 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -2495,7 +2495,7 @@ trait SizeHint { } #[stable(feature = "bufreader_size_hint", since = "1.51.0")] -impl SizeHint for T; +impl SizeHint for T {} #[stable(feature = "bufreader_size_hint", since = "1.51.0")] impl SizeHint for BufReader { From eea99f491b6000a836a8887a29d1ce85c666fe47 Mon Sep 17 00:00:00 2001 From: Xavientois Date: Fri, 15 Jan 2021 18:45:00 -0500 Subject: [PATCH 07/63] Add default keyword for specialization --- library/std/src/io/mod.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index 53da24409459d..712e808a02474 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -2481,10 +2481,6 @@ impl Iterator for Bytes { #[stable(feature = "bufreader_size_hint", since = "1.51.0")] trait SizeHint { - fn lower_bound(&self) -> usize { - 0 - } - fn upper_bound(&self) -> Option { None } @@ -2495,7 +2491,11 @@ trait SizeHint { } #[stable(feature = "bufreader_size_hint", since = "1.51.0")] -impl SizeHint for T {} +impl SizeHint for T { + default fn lower_bound(&self) -> usize { + 0 + } +} #[stable(feature = "bufreader_size_hint", since = "1.51.0")] impl SizeHint for BufReader { From 7e56637c749c4427ce0456022ea84e8393e5b0f7 Mon Sep 17 00:00:00 2001 From: Xavientois Date: Fri, 15 Jan 2021 19:29:49 -0500 Subject: [PATCH 08/63] Add back lower_bound as memeber --- library/std/src/io/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index 712e808a02474..018fe1c52bfb9 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -2481,6 +2481,8 @@ impl Iterator for Bytes { #[stable(feature = "bufreader_size_hint", since = "1.51.0")] trait SizeHint { + fn lower_bound(&self) -> usize { + fn upper_bound(&self) -> Option { None } From 442de9ac4531cc31955b62fbbebf80d2b99b4bb0 Mon Sep 17 00:00:00 2001 From: Xavientois Date: Fri, 15 Jan 2021 19:43:52 -0500 Subject: [PATCH 09/63] Fix semicolon --- library/std/src/io/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index 018fe1c52bfb9..e9c99e5a62738 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -2481,7 +2481,7 @@ impl Iterator for Bytes { #[stable(feature = "bufreader_size_hint", since = "1.51.0")] trait SizeHint { - fn lower_bound(&self) -> usize { + fn lower_bound(&self) -> usize; fn upper_bound(&self) -> Option { None From 1190321b7633369ec9e678d2fe4ac986b8157cc1 Mon Sep 17 00:00:00 2001 From: Xavientois Date: Fri, 15 Jan 2021 20:02:55 -0500 Subject: [PATCH 10/63] Remove exposing private trait --- library/std/src/io/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index e9c99e5a62738..a05adedaed2d6 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -2459,7 +2459,7 @@ pub struct Bytes { } #[stable(feature = "rust1", since = "1.0.0")] -impl Iterator for Bytes { +impl Iterator for Bytes { type Item = Result; fn next(&mut self) -> Option> { @@ -2475,7 +2475,7 @@ impl Iterator for Bytes { } default fn size_hint(&self) -> (usize, Option) { - self.inner.size_hint() + (&self.inner as &SizeHint).size_hint() } } From 421b40cd6a72f039b319f8da6b80c67c385e9965 Mon Sep 17 00:00:00 2001 From: Xavientois Date: Fri, 15 Jan 2021 20:09:43 -0500 Subject: [PATCH 11/63] Add dyn for SizeHint cast --- library/std/src/io/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index a05adedaed2d6..e81bfd27a9ad1 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -2475,7 +2475,7 @@ impl Iterator for Bytes { } default fn size_hint(&self) -> (usize, Option) { - (&self.inner as &SizeHint).size_hint() + (&self.inner as &dyn SizeHint).size_hint() } } From 265db94dc29b4e089e134795eac24b1b0a2faab1 Mon Sep 17 00:00:00 2001 From: Xavientois Date: Fri, 15 Jan 2021 20:25:32 -0500 Subject: [PATCH 12/63] Fix formatting --- library/std/src/io/mod.rs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index e81bfd27a9ad1..766da2ed6264f 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -2216,11 +2216,7 @@ impl Read for Chain { unsafe fn initializer(&self) -> Initializer { let initializer = self.first.initializer(); - if initializer.should_initialize() { - initializer - } else { - self.second.initializer() - } + if initializer.should_initialize() { initializer } else { self.second.initializer() } } } @@ -2239,11 +2235,7 @@ impl BufRead for Chain { } fn consume(&mut self, amt: usize) { - if !self.done_first { - self.first.consume(amt) - } else { - self.second.consume(amt) - } + if !self.done_first { self.first.consume(amt) } else { self.second.consume(amt) } } } From 93870c8d5ff5750db2e138f35f783078e8ad8dd0 Mon Sep 17 00:00:00 2001 From: Xavientois Date: Fri, 15 Jan 2021 20:40:10 -0500 Subject: [PATCH 13/63] Remove stable annotation --- library/std/src/io/mod.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index 766da2ed6264f..754e22f228821 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -2471,7 +2471,6 @@ impl Iterator for Bytes { } } -#[stable(feature = "bufreader_size_hint", since = "1.51.0")] trait SizeHint { fn lower_bound(&self) -> usize; @@ -2484,14 +2483,12 @@ trait SizeHint { } } -#[stable(feature = "bufreader_size_hint", since = "1.51.0")] impl SizeHint for T { default fn lower_bound(&self) -> usize { 0 } } -#[stable(feature = "bufreader_size_hint", since = "1.51.0")] impl SizeHint for BufReader { fn lower_bound(&self) -> usize { self.buffer().len() From 7869371bf1e6ab88bb61771c5dca217cc92ba0c9 Mon Sep 17 00:00:00 2001 From: Xavientois Date: Fri, 15 Jan 2021 20:45:43 -0500 Subject: [PATCH 14/63] Remove unnecessary default keyword --- library/std/src/io/mod.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index 754e22f228821..d7fd811cdb64b 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -2466,7 +2466,7 @@ impl Iterator for Bytes { } } - default fn size_hint(&self) -> (usize, Option) { + fn size_hint(&self) -> (usize, Option) { (&self.inner as &dyn SizeHint).size_hint() } } @@ -2474,9 +2474,7 @@ impl Iterator for Bytes { trait SizeHint { fn lower_bound(&self) -> usize; - fn upper_bound(&self) -> Option { - None - } + fn upper_bound(&self) -> Option; fn size_hint(&self) -> (usize, Option) { (self.lower_bound(), self.upper_bound()) @@ -2487,6 +2485,10 @@ impl SizeHint for T { default fn lower_bound(&self) -> usize { 0 } + + default fn upper_bound(&self) -> Option { + None + } } impl SizeHint for BufReader { From c8e0f8aaa3e8a21158b596814d8a53cfe604a294 Mon Sep 17 00:00:00 2001 From: Xavientois Date: Sat, 16 Jan 2021 10:42:52 -0500 Subject: [PATCH 15/63] Use fully qualified syntax to avoid dyn --- library/std/src/io/mod.rs | 2 +- library/std/src/io/tests.rs | 22 +++++++++++++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index d7fd811cdb64b..3839f7cd28d17 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -2467,7 +2467,7 @@ impl Iterator for Bytes { } fn size_hint(&self) -> (usize, Option) { - (&self.inner as &dyn SizeHint).size_hint() + SizeHint::size_hint(&self.inner) } } diff --git a/library/std/src/io/tests.rs b/library/std/src/io/tests.rs index f176c2f088cb3..03ed8ba74c520 100644 --- a/library/std/src/io/tests.rs +++ b/library/std/src/io/tests.rs @@ -1,7 +1,7 @@ use super::{repeat, Cursor, SeekFrom}; use crate::cmp::{self, min}; use crate::io::{self, IoSlice, IoSliceMut}; -use crate::io::{BufRead, Read, Seek, Write}; +use crate::io::{BufRead, BufReader, Read, Seek, Write}; use crate::ops::Deref; #[test] @@ -198,6 +198,26 @@ fn chain_bufread() { cmp_bufread(chain1, chain2, &testdata[..]); } +#[test] +fn bufreader_size_hint() { + let testdata = b"ABCDEFGHIJKL"; + let mut buf_reader = BufReader::new(&testdata[..]); + assert_eq!(buf_reader.buffer().len(), 0); + + let buffer = buf_reader.fill_buf().unwrap(); + let buffer_length = buffer.len(); + + // Check that size hint matches buffer contents + let mut buffered_bytes = buf_reader.bytes(); + let (lower_bound, _upper_bound) = buffered_bytes.size_hint(); + assert_eq!(lower_bound, buffer_length); + + // Check that size hint matches buffer contents after advancing + buffered_bytes.next().unwrap().unwrap(); + let (lower_bound, _upper_bound) = buffered_bytes.size_hint(); + assert_eq!(lower_bound, buffer_length - 1); +} + #[test] fn chain_zero_length_read_is_not_eof() { let a = b"A"; From 96255f82c9209fc2366ef32937c2bb53817687c1 Mon Sep 17 00:00:00 2001 From: Xavientois Date: Sat, 16 Jan 2021 15:03:58 -0500 Subject: [PATCH 16/63] Implement SizeHint trait for BufReader, Emtpy, and Chain --- library/std/src/io/buffered/bufreader.rs | 9 ++++++++- library/std/src/io/mod.rs | 20 ++++++++++++++------ library/std/src/io/util.rs | 8 +++++++- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/library/std/src/io/buffered/bufreader.rs b/library/std/src/io/buffered/bufreader.rs index 987371f50ec22..e737a104014f4 100644 --- a/library/std/src/io/buffered/bufreader.rs +++ b/library/std/src/io/buffered/bufreader.rs @@ -1,6 +1,6 @@ use crate::cmp; use crate::fmt; -use crate::io::{self, BufRead, Initializer, IoSliceMut, Read, Seek, SeekFrom, DEFAULT_BUF_SIZE}; +use crate::io::{self, BufRead, Initializer, IoSliceMut, Read, Seek, SeekFrom, SizeHint, DEFAULT_BUF_SIZE}; /// The `BufReader` struct adds buffering to any reader. /// @@ -435,3 +435,10 @@ impl Seek for BufReader { }) } } + +impl SizeHint for BufReader { + fn lower_bound(&self) -> usize { + self.buffer().len() + } +} + diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index 3839f7cd28d17..08b258bb816ec 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -2239,6 +2239,20 @@ impl BufRead for Chain { } } +impl SizeHint for Chain { + fn lower_bound(&self) -> usize { + SizeHint::lower_bound(&self.first) + SizeHint::lower_bound(&self.second) + } + + fn upper_bound(&self) -> Option { + match (SizeHint::upper_bound(&self.first), SizeHint::upper_bound(&self.second)) { + (Some(first), Some(second)) => Some(first + second), + _ => None, + } + } +} + + /// Reader adaptor which limits the bytes read from an underlying reader. /// /// This struct is generally created by calling [`take`] on a reader. @@ -2491,12 +2505,6 @@ impl SizeHint for T { } } -impl SizeHint for BufReader { - fn lower_bound(&self) -> usize { - self.buffer().len() - } -} - /// An iterator over the contents of an instance of `BufRead` split on a /// particular byte. /// diff --git a/library/std/src/io/util.rs b/library/std/src/io/util.rs index e43ce4cdb4b8e..7de37c67abebc 100644 --- a/library/std/src/io/util.rs +++ b/library/std/src/io/util.rs @@ -4,7 +4,7 @@ mod tests; use crate::fmt; -use crate::io::{self, BufRead, Initializer, IoSlice, IoSliceMut, Read, Seek, SeekFrom, Write}; +use crate::io::{self, BufRead, Initializer, IoSlice, IoSliceMut, Read, Seek, SeekFrom, SizeHint, Write}; /// A reader which is always at EOF. /// @@ -80,6 +80,12 @@ impl fmt::Debug for Empty { } } +impl SizeHint for Empty { + fn upper_bound(&self) -> Option { + Some(0) + } +} + /// A reader which yields one byte over and over and over and over and over and... /// /// This struct is generally created by calling [`repeat()`]. Please From 389e638c054434c4b9df9427dab29b0f325923ac Mon Sep 17 00:00:00 2001 From: Xavientois Date: Sat, 16 Jan 2021 15:04:26 -0500 Subject: [PATCH 17/63] Add tests for SizeHint implementations --- library/std/src/io/tests.rs | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/library/std/src/io/tests.rs b/library/std/src/io/tests.rs index 03ed8ba74c520..a85dd0d982715 100644 --- a/library/std/src/io/tests.rs +++ b/library/std/src/io/tests.rs @@ -204,8 +204,8 @@ fn bufreader_size_hint() { let mut buf_reader = BufReader::new(&testdata[..]); assert_eq!(buf_reader.buffer().len(), 0); - let buffer = buf_reader.fill_buf().unwrap(); - let buffer_length = buffer.len(); + let buffer_length = testdata.len(); + buf_reader.fill_buf().unwrap(); // Check that size hint matches buffer contents let mut buffered_bytes = buf_reader.bytes(); @@ -218,6 +218,33 @@ fn bufreader_size_hint() { assert_eq!(lower_bound, buffer_length - 1); } +#[test] +fn empty_size_hint() { + let size_hint = io::empty().bytes().size_hint(); + assert_eq!(size_hint, (0, Some(0))); +} + +#[test] +fn chain_empty_size_hint() { + let chain = io::empty().chain(io::empty()); + let size_hint = chain.bytes().size_hint(); + assert_eq!(size_hint, (0, Some(0))); +} + +#[test] +fn chain_size_hint() { + let testdata = b"ABCDEFGHIJKL"; + let mut buf_reader_1 = BufReader::new(&testdata[..6]); + let mut buf_reader_2 = BufReader::new(&testdata[6..]); + + buf_reader_1.fill_buf().unwrap(); + buf_reader_2.fill_buf().unwrap(); + + let chain = buf_reader_1.chain(buf_reader_2); + let size_hint = chain.bytes().size_hint(); + assert_eq!(size_hint, (testdata.len(), None)); +} + #[test] fn chain_zero_length_read_is_not_eof() { let a = b"A"; From b837f3a99b8dc767c4a6a3216ad3940f23896932 Mon Sep 17 00:00:00 2001 From: Xavientois Date: Sat, 16 Jan 2021 15:20:40 -0500 Subject: [PATCH 18/63] Remove trailing newline --- library/std/src/io/buffered/bufreader.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/library/std/src/io/buffered/bufreader.rs b/library/std/src/io/buffered/bufreader.rs index e737a104014f4..839c64ee5c443 100644 --- a/library/std/src/io/buffered/bufreader.rs +++ b/library/std/src/io/buffered/bufreader.rs @@ -1,6 +1,8 @@ use crate::cmp; use crate::fmt; -use crate::io::{self, BufRead, Initializer, IoSliceMut, Read, Seek, SeekFrom, SizeHint, DEFAULT_BUF_SIZE}; +use crate::io::{ + self, BufRead, Initializer, IoSliceMut, Read, Seek, SeekFrom, SizeHint, DEFAULT_BUF_SIZE, +}; /// The `BufReader` struct adds buffering to any reader. /// @@ -441,4 +443,3 @@ impl SizeHint for BufReader { self.buffer().len() } } - From 81aba388f16502cbb7b305d64de9ccc95057b339 Mon Sep 17 00:00:00 2001 From: Xavientois Date: Sat, 16 Jan 2021 15:36:59 -0500 Subject: [PATCH 19/63] Add space for proper indentation --- library/std/src/io/util.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/io/util.rs b/library/std/src/io/util.rs index 7de37c67abebc..e5e5f29b22d09 100644 --- a/library/std/src/io/util.rs +++ b/library/std/src/io/util.rs @@ -82,7 +82,7 @@ impl fmt::Debug for Empty { impl SizeHint for Empty { fn upper_bound(&self) -> Option { - Some(0) + Some(0) } } From fc9cd4a14be3ea1d5ee29b495045fa51edfc0810 Mon Sep 17 00:00:00 2001 From: Xavientois Date: Sat, 16 Jan 2021 17:48:22 -0500 Subject: [PATCH 20/63] Fix formatting on mod --- library/std/src/io/mod.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index 08b258bb816ec..b7ccf9f21efb0 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -2244,7 +2244,7 @@ impl SizeHint for Chain { SizeHint::lower_bound(&self.first) + SizeHint::lower_bound(&self.second) } - fn upper_bound(&self) -> Option { + fn upper_bound(&self) -> Option { match (SizeHint::upper_bound(&self.first), SizeHint::upper_bound(&self.second)) { (Some(first), Some(second)) => Some(first + second), _ => None, @@ -2252,7 +2252,6 @@ impl SizeHint for Chain { } } - /// Reader adaptor which limits the bytes read from an underlying reader. /// /// This struct is generally created by calling [`take`] on a reader. From 7674ae1a4e10ab6bafbda57f9fc2175fdf012f5e Mon Sep 17 00:00:00 2001 From: Xavientois Date: Sun, 31 Jan 2021 08:52:57 -0500 Subject: [PATCH 21/63] Fix line length format --- library/std/src/io/util.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library/std/src/io/util.rs b/library/std/src/io/util.rs index e5e5f29b22d09..f472361f916db 100644 --- a/library/std/src/io/util.rs +++ b/library/std/src/io/util.rs @@ -4,7 +4,9 @@ mod tests; use crate::fmt; -use crate::io::{self, BufRead, Initializer, IoSlice, IoSliceMut, Read, Seek, SeekFrom, SizeHint, Write}; +use crate::io::{ + self, BufRead, Initializer, IoSlice, IoSliceMut, Read, Seek, SeekFrom, SizeHint, Write, +}; /// A reader which is always at EOF. /// From 7b021aacb57d7a120f280302358d7bdd04a67bbc Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Fri, 29 Jan 2021 00:59:20 +0300 Subject: [PATCH 22/63] resolve: Reduce scope of `pub_use_of_private_extern_crate` deprecation lint --- compiler/rustc_resolve/src/imports.rs | 32 +++++++++------ src/test/rustdoc/extern-links.rs | 2 +- src/test/rustdoc/issue-28927.rs | 2 +- .../ui/pub/pub-reexport-priv-extern-crate.rs | 8 +--- .../pub/pub-reexport-priv-extern-crate.stderr | 39 +++++++++++-------- 5 files changed, 47 insertions(+), 36 deletions(-) diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index bd0296751a535..61f4c00a4ca42 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -156,6 +156,21 @@ impl<'a> NameResolution<'a> { } } +// Reexports of the form `pub use foo as bar;` where `foo` is `extern crate foo;` +// are permitted for backward-compatibility under a deprecation lint. +fn pub_use_of_private_extern_crate_hack(import: &Import<'_>, binding: &NameBinding<'_>) -> bool { + match (&import.kind, &binding.kind) { + ( + ImportKind::Single { .. }, + NameBindingKind::Import { + import: Import { kind: ImportKind::ExternCrate { .. }, .. }, + .. + }, + ) => import.vis.get() == ty::Visibility::Public, + _ => false, + } +} + impl<'a> Resolver<'a> { crate fn resolve_ident_in_module_unadjusted( &mut self, @@ -263,10 +278,7 @@ impl<'a> Resolver<'a> { return Err((Determined, Weak::No)); } } - // `extern crate` are always usable for backwards compatibility, see issue #37020, - // remove this together with `PUB_USE_OF_PRIVATE_EXTERN_CRATE`. - let usable = this.is_accessible_from(binding.vis, parent_scope.module) - || binding.is_extern_crate(); + let usable = this.is_accessible_from(binding.vis, parent_scope.module); if usable { Ok(binding) } else { Err((Determined, Weak::No)) } }; @@ -309,10 +321,7 @@ impl<'a> Resolver<'a> { } } - if !(self.is_accessible_from(binding.vis, parent_scope.module) || - // Remove this together with `PUB_USE_OF_PRIVATE_EXTERN_CRATE` - (self.last_import_segment && binding.is_extern_crate())) - { + if !self.is_accessible_from(binding.vis, parent_scope.module) { self.privacy_errors.push(PrivacyError { ident, binding, @@ -455,9 +464,8 @@ impl<'a> Resolver<'a> { binding: &'a NameBinding<'a>, import: &'a Import<'a>, ) -> &'a NameBinding<'a> { - let vis = if binding.vis.is_at_least(import.vis.get(), self) || - // cf. `PUB_USE_OF_PRIVATE_EXTERN_CRATE` - !import.is_glob() && binding.is_extern_crate() + let vis = if binding.vis.is_at_least(import.vis.get(), self) + || pub_use_of_private_extern_crate_hack(import, binding) { import.vis.get() } else { @@ -1188,7 +1196,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> { // All namespaces must be re-exported with extra visibility for an error to occur. if !any_successful_reexport { let (ns, binding) = reexport_error.unwrap(); - if ns == TypeNS && binding.is_extern_crate() { + if pub_use_of_private_extern_crate_hack(import, binding) { let msg = format!( "extern crate `{}` is private, and cannot be \ re-exported (error E0365), consider declaring with \ diff --git a/src/test/rustdoc/extern-links.rs b/src/test/rustdoc/extern-links.rs index 991f869138d93..0383ccf7db666 100644 --- a/src/test/rustdoc/extern-links.rs +++ b/src/test/rustdoc/extern-links.rs @@ -3,7 +3,7 @@ #![crate_name = "foo"] -extern crate extern_links; +pub extern crate extern_links; // @!has foo/index.html '//a' 'extern_links' #[doc(no_inline)] diff --git a/src/test/rustdoc/issue-28927.rs b/src/test/rustdoc/issue-28927.rs index 7b535f33bf7e3..38a520850b6dd 100644 --- a/src/test/rustdoc/issue-28927.rs +++ b/src/test/rustdoc/issue-28927.rs @@ -2,5 +2,5 @@ // aux-build:issue-28927-1.rs // ignore-cross-compile -extern crate issue_28927_1 as inner1; +pub extern crate issue_28927_1 as inner1; pub use inner1 as foo; diff --git a/src/test/ui/pub/pub-reexport-priv-extern-crate.rs b/src/test/ui/pub/pub-reexport-priv-extern-crate.rs index e95d6924026ca..dd5cd420fa546 100644 --- a/src/test/ui/pub/pub-reexport-priv-extern-crate.rs +++ b/src/test/ui/pub/pub-reexport-priv-extern-crate.rs @@ -1,5 +1,3 @@ -#![allow(unused)] - extern crate core; pub use core as reexported_core; //~ ERROR `core` is private, and cannot be re-exported //~^ WARN this was previously accepted @@ -9,16 +7,14 @@ mod foo1 { } mod foo2 { - use foo1::core; //~ ERROR `core` is private, and cannot be re-exported - //~^ WARN this was previously accepted + use foo1::core; //~ ERROR crate import `core` is private pub mod bar { extern crate core; } } mod baz { - pub use foo2::bar::core; //~ ERROR `core` is private, and cannot be re-exported - //~^ WARN this was previously accepted + pub use foo2::bar::core; //~ ERROR crate import `core` is private } fn main() {} diff --git a/src/test/ui/pub/pub-reexport-priv-extern-crate.stderr b/src/test/ui/pub/pub-reexport-priv-extern-crate.stderr index 0b44c5a6525f6..e4d73c6475dc4 100644 --- a/src/test/ui/pub/pub-reexport-priv-extern-crate.stderr +++ b/src/test/ui/pub/pub-reexport-priv-extern-crate.stderr @@ -1,30 +1,37 @@ -error: extern crate `core` is private, and cannot be re-exported (error E0365), consider declaring with `pub` - --> $DIR/pub-reexport-priv-extern-crate.rs:4:9 +error[E0603]: crate import `core` is private + --> $DIR/pub-reexport-priv-extern-crate.rs:10:15 | -LL | pub use core as reexported_core; - | ^^^^^^^^^^^^^^^^^^^^^^^ +LL | use foo1::core; + | ^^^^ private crate import | - = note: `#[deny(pub_use_of_private_extern_crate)]` on by default - = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! - = note: for more information, see issue #34537 +note: the crate import `core` is defined here + --> $DIR/pub-reexport-priv-extern-crate.rs:6:5 + | +LL | extern crate core; + | ^^^^^^^^^^^^^^^^^^ -error: extern crate `core` is private, and cannot be re-exported (error E0365), consider declaring with `pub` - --> $DIR/pub-reexport-priv-extern-crate.rs:12:9 +error[E0603]: crate import `core` is private + --> $DIR/pub-reexport-priv-extern-crate.rs:17:24 | -LL | use foo1::core; - | ^^^^^^^^^^ +LL | pub use foo2::bar::core; + | ^^^^ private crate import | - = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! - = note: for more information, see issue #34537 +note: the crate import `core` is defined here + --> $DIR/pub-reexport-priv-extern-crate.rs:12:9 + | +LL | extern crate core; + | ^^^^^^^^^^^^^^^^^^ error: extern crate `core` is private, and cannot be re-exported (error E0365), consider declaring with `pub` - --> $DIR/pub-reexport-priv-extern-crate.rs:20:13 + --> $DIR/pub-reexport-priv-extern-crate.rs:2:9 | -LL | pub use foo2::bar::core; - | ^^^^^^^^^^^^^^^ +LL | pub use core as reexported_core; + | ^^^^^^^^^^^^^^^^^^^^^^^ | + = note: `#[deny(pub_use_of_private_extern_crate)]` on by default = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #34537 error: aborting due to 3 previous errors +For more information about this error, try `rustc --explain E0603`. From a3db47ab6c976137a26976d5c8060e7eadc05232 Mon Sep 17 00:00:00 2001 From: Kevin Per Date: Tue, 9 Feb 2021 17:18:28 +0000 Subject: [PATCH 23/63] Add suggestion for iterators in iterators --- .../diagnostics/conflict_errors.rs | 33 ++++++++++++++++--- library/core/src/iter/traits/iterator.rs | 1 + src/test/ui/issues/issue-81584.fixed | 8 +++++ src/test/ui/issues/issue-81584.rs | 8 +++++ src/test/ui/issues/issue-81584.stderr | 14 ++++++++ .../ui/static/static-reference-to-fn-2.stderr | 2 ++ 6 files changed, 62 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/issues/issue-81584.fixed create mode 100644 src/test/ui/issues/issue-81584.rs create mode 100644 src/test/ui/issues/issue-81584.stderr diff --git a/compiler/rustc_mir/src/borrow_check/diagnostics/conflict_errors.rs b/compiler/rustc_mir/src/borrow_check/diagnostics/conflict_errors.rs index b0b58a8d00367..24b9408ffb657 100644 --- a/compiler/rustc_mir/src/borrow_check/diagnostics/conflict_errors.rs +++ b/compiler/rustc_mir/src/borrow_check/diagnostics/conflict_errors.rs @@ -10,16 +10,18 @@ use rustc_middle::mir::{ FakeReadCause, Local, LocalDecl, LocalInfo, LocalKind, Location, Operand, Place, PlaceRef, ProjectionElem, Rvalue, Statement, StatementKind, Terminator, TerminatorKind, VarBindingForm, }; -use rustc_middle::ty::{self, suggest_constraining_type_param, Instance, Ty}; -use rustc_span::{source_map::DesugaringKind, symbol::sym, Span}; +use rustc_middle::ty::{self, suggest_constraining_type_param, Ty, TypeFoldable}; +use rustc_span::source_map::DesugaringKind; +use rustc_span::symbol::sym; +use rustc_span::Span; use crate::dataflow::drop_flag_effects; use crate::dataflow::indexes::{MoveOutIndex, MovePathIndex}; use crate::util::borrowck_errors; use crate::borrow_check::{ - borrow_set::BorrowData, prefixes::IsPrefixOf, InitializationRequiringAction, MirBorrowckCtxt, - PrefixSet, WriteKind, + borrow_set::BorrowData, diagnostics::Instance, prefixes::IsPrefixOf, + InitializationRequiringAction, MirBorrowckCtxt, PrefixSet, WriteKind, }; use super::{ @@ -1267,6 +1269,29 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { if return_span != borrow_span { err.span_label(borrow_span, note); + + let tcx = self.infcx.tcx; + let ty_params = ty::List::empty(); + + let return_ty = self.regioncx.universal_regions().unnormalized_output_ty; + let return_ty = tcx.erase_regions(return_ty); + + // to avoid panics + if !return_ty.has_infer_types() { + if let Some(iter_trait) = tcx.get_diagnostic_item(sym::Iterator) { + if tcx.type_implements_trait((iter_trait, return_ty, ty_params, self.param_env)) + { + if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(return_span) { + err.span_suggestion_hidden( + return_span, + "use `.collect()` to allocate the iterator", + format!("{}{}", snippet, ".collect::>()"), + Applicability::MaybeIncorrect, + ); + } + } + } + } } Some(err) diff --git a/library/core/src/iter/traits/iterator.rs b/library/core/src/iter/traits/iterator.rs index f28c4673cc033..e179ce01c417a 100644 --- a/library/core/src/iter/traits/iterator.rs +++ b/library/core/src/iter/traits/iterator.rs @@ -93,6 +93,7 @@ fn _assert_is_object_safe(_: &dyn Iterator) {} message = "`{Self}` is not an iterator" )] #[doc(spotlight)] +#[rustc_diagnostic_item = "Iterator"] #[must_use = "iterators are lazy and do nothing unless consumed"] pub trait Iterator { /// The type of the elements being iterated over. diff --git a/src/test/ui/issues/issue-81584.fixed b/src/test/ui/issues/issue-81584.fixed new file mode 100644 index 0000000000000..1cad59f1062c6 --- /dev/null +++ b/src/test/ui/issues/issue-81584.fixed @@ -0,0 +1,8 @@ +// run-rustfix +fn main() { + let _ = vec![vec![0, 1], vec![2]] + .into_iter() + .map(|y| y.iter().map(|x| x + 1).collect::>()) + //~^ ERROR cannot return value referencing function parameter `y` + .collect::>(); +} diff --git a/src/test/ui/issues/issue-81584.rs b/src/test/ui/issues/issue-81584.rs new file mode 100644 index 0000000000000..452288db08bd8 --- /dev/null +++ b/src/test/ui/issues/issue-81584.rs @@ -0,0 +1,8 @@ +// run-rustfix +fn main() { + let _ = vec![vec![0, 1], vec![2]] + .into_iter() + .map(|y| y.iter().map(|x| x + 1)) + //~^ ERROR cannot return value referencing function parameter `y` + .collect::>(); +} diff --git a/src/test/ui/issues/issue-81584.stderr b/src/test/ui/issues/issue-81584.stderr new file mode 100644 index 0000000000000..d57f1b778df17 --- /dev/null +++ b/src/test/ui/issues/issue-81584.stderr @@ -0,0 +1,14 @@ +error[E0515]: cannot return value referencing function parameter `y` + --> $DIR/issue-81584.rs:5:22 + | +LL | .map(|y| y.iter().map(|x| x + 1)) + | -^^^^^^^^^^^^^^^^^^^^^^ + | | + | returns a value referencing data owned by the current function + | `y` is borrowed here + | + = help: use `.collect()` to allocate the iterator + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0515`. diff --git a/src/test/ui/static/static-reference-to-fn-2.stderr b/src/test/ui/static/static-reference-to-fn-2.stderr index 028e11a60cef4..ff15884bd445d 100644 --- a/src/test/ui/static/static-reference-to-fn-2.stderr +++ b/src/test/ui/static/static-reference-to-fn-2.stderr @@ -40,6 +40,8 @@ LL | | statefn: &id(state1 as StateMachineFunc) | | ------------------------------ temporary value created here LL | | } | |_____^ returns a value referencing data owned by the current function + | + = help: use `.collect()` to allocate the iterator error: aborting due to 4 previous errors From 040735c110026bbd494a23c86182ebda201d720b Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Tue, 5 Jan 2021 10:07:50 +0100 Subject: [PATCH 24/63] First version of noop-lint --- compiler/rustc_lint/src/lib.rs | 3 + compiler/rustc_lint/src/noop_method_call.rs | 71 +++++++++++++++++++++ compiler/rustc_span/src/symbol.rs | 1 + library/core/src/clone.rs | 2 + 4 files changed, 77 insertions(+) create mode 100644 compiler/rustc_lint/src/noop_method_call.rs diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 638b73c27a8d7..b0c22154c0b5b 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -57,6 +57,7 @@ mod methods; mod non_ascii_idents; mod non_fmt_panic; mod nonstandard_style; +mod noop_method_call; mod passes; mod redundant_semicolon; mod traits; @@ -83,6 +84,7 @@ use methods::*; use non_ascii_idents::*; use non_fmt_panic::NonPanicFmt; use nonstandard_style::*; +use noop_method_call::*; use redundant_semicolon::*; use traits::*; use types::*; @@ -170,6 +172,7 @@ macro_rules! late_lint_passes { DropTraitConstraints: DropTraitConstraints, TemporaryCStringAsPtr: TemporaryCStringAsPtr, NonPanicFmt: NonPanicFmt, + NoopMethodCall: NoopMethodCall, ] ); }; diff --git a/compiler/rustc_lint/src/noop_method_call.rs b/compiler/rustc_lint/src/noop_method_call.rs new file mode 100644 index 0000000000000..098c50d5abe19 --- /dev/null +++ b/compiler/rustc_lint/src/noop_method_call.rs @@ -0,0 +1,71 @@ +use crate::context::LintContext; +use crate::rustc_middle::ty::TypeFoldable; +use crate::LateContext; +use crate::LateLintPass; +use rustc_hir::def::DefKind; +use rustc_hir::{Expr, ExprKind}; +use rustc_middle::ty; +use rustc_span::symbol::sym; + +declare_lint! { + /// The `noop_method_call` lint detects specific calls to noop methods + /// such as a calling `<&T as Clone>::clone` where `T: !Clone`. + /// + /// ### Example + /// + /// ```rust + /// # #![allow(unused)] + /// struct Foo; + /// let foo = &Foo; + /// let clone: &Foo = foo.clone(); + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Some method calls are noops meaning that they do nothing. Usually such methods + /// are the result of blanket implementations that happen to create some method invocations + /// that end up not doing anything. For instance, `Clone` is implemented on all `&T`, but + /// calling `clone` on a `&T` where `T` does not implement clone, actually doesn't do anything + /// as references are copy. This lint detects these calls and warns the user about them. + pub NOOP_METHOD_CALL, + Warn, + "detects the use of well-known noop methods" +} + +declare_lint_pass!(NoopMethodCall => [NOOP_METHOD_CALL]); + +impl<'tcx> LateLintPass<'tcx> for NoopMethodCall { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + // We only care about method calls + if let ExprKind::MethodCall(..) = expr.kind { + // Get the `DefId` only when dealing with an `AssocFn` + if let Some((DefKind::AssocFn, did)) = + cx.typeck_results().type_dependent_def(expr.hir_id) + { + // Check that we're dealing with a trait method + if let Some(trait_id) = cx.tcx.trait_of_item(did) { + let substs = cx.typeck_results().node_substs(expr.hir_id); + // We can't resolve on types that recursively require monomorphization, + // so check that we don't need to perfom substitution + if !substs.needs_subst() { + let param_env = cx.tcx.param_env(trait_id); + // Resolve the trait method instance + if let Ok(Some(i)) = ty::Instance::resolve(cx.tcx, param_env, did, substs) { + // Check that it implements the noop diagnostic + if cx.tcx.is_diagnostic_item(sym::ref_clone_method, i.def_id()) { + let span = expr.span; + + cx.struct_span_lint(NOOP_METHOD_CALL, span, |lint| { + let message = "Call to noop method"; + lint.build(&message).emit() + }); + } + } + } + } + } + } + } +} diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 27bb45bcc8512..2207deb8fb5a0 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -915,6 +915,7 @@ symbols! { receiver, recursion_limit, reexport_test_harness_main, + ref_clone_method, reference, reflect, reg, diff --git a/library/core/src/clone.rs b/library/core/src/clone.rs index a953a3a4182bc..12f0f9629a31f 100644 --- a/library/core/src/clone.rs +++ b/library/core/src/clone.rs @@ -104,6 +104,7 @@ /// [impls]: #implementors #[stable(feature = "rust1", since = "1.0.0")] #[lang = "clone"] +#[rustc_diagnostic_item = "Clone"] pub trait Clone: Sized { /// Returns a copy of the value. /// @@ -221,6 +222,7 @@ mod impls { #[stable(feature = "rust1", since = "1.0.0")] impl Clone for &T { #[inline] + #[rustc_diagnostic_item = "ref_clone_method"] fn clone(&self) -> Self { *self } From f49ed7a6b7aa3a44dd0444b508a1d0ddc09b0f15 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Tue, 5 Jan 2021 16:14:39 +0100 Subject: [PATCH 25/63] Add tests and support two more noop methods --- compiler/rustc_lint/src/noop_method_call.rs | 12 +++++- compiler/rustc_span/src/symbol.rs | 4 +- library/core/src/borrow.rs | 1 + library/core/src/clone.rs | 3 +- library/core/src/ops/deref.rs | 1 + src/test/ui/lint/noop-method-call.rs | 45 +++++++++++++++++++++ src/test/ui/lint/noop-method-call.stderr | 28 +++++++++++++ 7 files changed, 89 insertions(+), 5 deletions(-) create mode 100644 src/test/ui/lint/noop-method-call.rs create mode 100644 src/test/ui/lint/noop-method-call.stderr diff --git a/compiler/rustc_lint/src/noop_method_call.rs b/compiler/rustc_lint/src/noop_method_call.rs index 098c50d5abe19..3ab3fab4272c7 100644 --- a/compiler/rustc_lint/src/noop_method_call.rs +++ b/compiler/rustc_lint/src/noop_method_call.rs @@ -54,11 +54,19 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall { // Resolve the trait method instance if let Ok(Some(i)) = ty::Instance::resolve(cx.tcx, param_env, did, substs) { // Check that it implements the noop diagnostic - if cx.tcx.is_diagnostic_item(sym::ref_clone_method, i.def_id()) { + tracing::debug!("Resolves to: {:?}", i.def_id()); + if [ + sym::noop_method_borrow, + sym::noop_method_clone, + sym::noop_method_deref, + ] + .iter() + .any(|s| cx.tcx.is_diagnostic_item(*s, i.def_id())) + { let span = expr.span; cx.struct_span_lint(NOOP_METHOD_CALL, span, |lint| { - let message = "Call to noop method"; + let message = "call to noop method"; lint.build(&message).emit() }); } diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 2207deb8fb5a0..d39fbd61962e7 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -789,6 +789,9 @@ symbols! { none_error, nontemporal_store, nontrapping_dash_fptoint: "nontrapping-fptoint", + noop_method_borrow, + noop_method_clone, + noop_method_deref, noreturn, nostack, not, @@ -915,7 +918,6 @@ symbols! { receiver, recursion_limit, reexport_test_harness_main, - ref_clone_method, reference, reflect, reg, diff --git a/library/core/src/borrow.rs b/library/core/src/borrow.rs index c9040cd0a1670..af2ad12dddf0a 100644 --- a/library/core/src/borrow.rs +++ b/library/core/src/borrow.rs @@ -219,6 +219,7 @@ impl BorrowMut for T { #[stable(feature = "rust1", since = "1.0.0")] impl Borrow for &T { + #[rustc_diagnostic_item = "noop_method_borrow"] fn borrow(&self) -> &T { &**self } diff --git a/library/core/src/clone.rs b/library/core/src/clone.rs index 12f0f9629a31f..12b4feeb14ae3 100644 --- a/library/core/src/clone.rs +++ b/library/core/src/clone.rs @@ -104,7 +104,6 @@ /// [impls]: #implementors #[stable(feature = "rust1", since = "1.0.0")] #[lang = "clone"] -#[rustc_diagnostic_item = "Clone"] pub trait Clone: Sized { /// Returns a copy of the value. /// @@ -222,7 +221,7 @@ mod impls { #[stable(feature = "rust1", since = "1.0.0")] impl Clone for &T { #[inline] - #[rustc_diagnostic_item = "ref_clone_method"] + #[rustc_diagnostic_item = "noop_method_clone"] fn clone(&self) -> Self { *self } diff --git a/library/core/src/ops/deref.rs b/library/core/src/ops/deref.rs index 2419771eae212..d503a30174191 100644 --- a/library/core/src/ops/deref.rs +++ b/library/core/src/ops/deref.rs @@ -78,6 +78,7 @@ pub trait Deref { impl Deref for &T { type Target = T; + #[rustc_diagnostic_item = "noop_method_deref"] fn deref(&self) -> &T { *self } diff --git a/src/test/ui/lint/noop-method-call.rs b/src/test/ui/lint/noop-method-call.rs new file mode 100644 index 0000000000000..4b81e04d3f7f9 --- /dev/null +++ b/src/test/ui/lint/noop-method-call.rs @@ -0,0 +1,45 @@ +// check-pass + +#![allow(unused)] + +use std::borrow::Borrow; +use std::ops::Deref; + +struct Foo(T); + +#[derive(Clone)] +struct Bar(T); + +struct DerefExample(T); + +impl Deref for DerefExample { + type Target = T; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +fn main() { + let foo = &Foo(1u32); + let foo_clone: &Foo = foo.clone(); //~ WARNING call to noop method + + let bar = &Bar(1u32); + let bar_clone: Bar = bar.clone(); + + let deref = &&DerefExample(12u32); + let derefed: &DerefExample = deref.deref(); //~ WARNING call to noop method + + let deref = &DerefExample(12u32); + let derefed: &u32 = deref.deref(); + + let a = &&Foo(1u32); + let borrowed: &Foo = a.borrow(); //~ WARNING call to noop method +} + +fn generic(foo: &Foo) { + foo.clone(); +} + +fn non_generic(foo: &Foo) { + foo.clone(); //~ WARNING call to noop method +} diff --git a/src/test/ui/lint/noop-method-call.stderr b/src/test/ui/lint/noop-method-call.stderr new file mode 100644 index 0000000000000..1120adee121b1 --- /dev/null +++ b/src/test/ui/lint/noop-method-call.stderr @@ -0,0 +1,28 @@ +warning: call to noop method + --> $DIR/noop-method-call.rs:24:32 + | +LL | let foo_clone: &Foo = foo.clone(); + | ^^^^^^^^^^^ + | + = note: `#[warn(noop_method_call)]` on by default + +warning: call to noop method + --> $DIR/noop-method-call.rs:30:39 + | +LL | let derefed: &DerefExample = deref.deref(); + | ^^^^^^^^^^^^^ + +warning: call to noop method + --> $DIR/noop-method-call.rs:36:31 + | +LL | let borrowed: &Foo = a.borrow(); + | ^^^^^^^^^^ + +warning: call to noop method + --> $DIR/noop-method-call.rs:44:5 + | +LL | foo.clone(); + | ^^^^^^^^^^^ + +warning: 4 warnings emitted + From a6d926d80db4a52398dea0cf29e6c501eab50170 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Tue, 5 Jan 2021 16:46:50 +0100 Subject: [PATCH 26/63] Fix tests --- compiler/rustc_codegen_ssa/src/back/rpath.rs | 2 +- compiler/rustc_lint/src/noop_method_call.rs | 9 ++++++++- compiler/rustc_middle/src/ty/error.rs | 2 -- compiler/rustc_mir/src/borrow_check/invalidation.rs | 8 ++++---- compiler/rustc_mir_build/src/build/matches/test.rs | 2 +- compiler/rustc_span/src/symbol.rs | 2 ++ .../src/traits/error_reporting/mod.rs | 2 +- compiler/rustc_traits/src/chalk/mod.rs | 2 +- compiler/rustc_typeck/src/check/callee.rs | 2 +- compiler/rustc_typeck/src/check/expr.rs | 2 +- library/core/src/borrow.rs | 1 + library/core/src/clone.rs | 1 + library/core/src/ops/deref.rs | 1 + src/test/ui/issues/issue-11820.rs | 10 ++++++---- src/test/ui/underscore-imports/cycle.rs | 1 + src/test/ui/underscore-imports/hygiene-2.rs | 1 + src/test/ui/underscore-imports/macro-expanded.rs | 1 + 17 files changed, 32 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/rpath.rs b/compiler/rustc_codegen_ssa/src/back/rpath.rs index 005d2efdd3b26..5f21046b05e47 100644 --- a/compiler/rustc_codegen_ssa/src/back/rpath.rs +++ b/compiler/rustc_codegen_ssa/src/back/rpath.rs @@ -24,7 +24,7 @@ pub fn get_rpath_flags(config: &mut RPathConfig<'_>) -> Vec { debug!("preparing the RPATH!"); - let libs = config.used_crates.clone(); + let libs = config.used_crates; let libs = libs.iter().filter_map(|&(_, ref l)| l.option()).collect::>(); let rpaths = get_rpaths(config, &libs); let mut flags = rpaths_to_flags(&rpaths); diff --git a/compiler/rustc_lint/src/noop_method_call.rs b/compiler/rustc_lint/src/noop_method_call.rs index 3ab3fab4272c7..dad557128f8b7 100644 --- a/compiler/rustc_lint/src/noop_method_call.rs +++ b/compiler/rustc_lint/src/noop_method_call.rs @@ -46,6 +46,14 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall { { // Check that we're dealing with a trait method if let Some(trait_id) = cx.tcx.trait_of_item(did) { + // Check we're dealing with one of the traits we care about + if ![sym::Clone, sym::Deref, sym::Borrow] + .iter() + .any(|s| cx.tcx.is_diagnostic_item(*s, trait_id)) + { + return; + } + let substs = cx.typeck_results().node_substs(expr.hir_id); // We can't resolve on types that recursively require monomorphization, // so check that we don't need to perfom substitution @@ -54,7 +62,6 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall { // Resolve the trait method instance if let Ok(Some(i)) = ty::Instance::resolve(cx.tcx, param_env, did, substs) { // Check that it implements the noop diagnostic - tracing::debug!("Resolves to: {:?}", i.def_id()); if [ sym::noop_method_borrow, sym::noop_method_clone, diff --git a/compiler/rustc_middle/src/ty/error.rs b/compiler/rustc_middle/src/ty/error.rs index bf315c81588a9..f19cc99844926 100644 --- a/compiler/rustc_middle/src/ty/error.rs +++ b/compiler/rustc_middle/src/ty/error.rs @@ -12,7 +12,6 @@ use rustc_target::spec::abi; use std::borrow::Cow; use std::fmt; -use std::ops::Deref; #[derive(Clone, Copy, Debug, PartialEq, Eq, TypeFoldable)] pub struct ExpectedFound { @@ -548,7 +547,6 @@ impl Trait for X { TargetFeatureCast(def_id) => { let attrs = self.get_attrs(*def_id); let target_spans = attrs - .deref() .iter() .filter(|attr| attr.has_name(sym::target_feature)) .map(|attr| attr.span); diff --git a/compiler/rustc_mir/src/borrow_check/invalidation.rs b/compiler/rustc_mir/src/borrow_check/invalidation.rs index 8c05e6fd5d0e4..e423e449746fc 100644 --- a/compiler/rustc_mir/src/borrow_check/invalidation.rs +++ b/compiler/rustc_mir/src/borrow_check/invalidation.rs @@ -165,7 +165,7 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> { self.consume_operand(location, value); // Invalidate all borrows of local places - let borrow_set = self.borrow_set.clone(); + let borrow_set = self.borrow_set; let resume = self.location_table.start_index(resume.start_location()); for (i, data) in borrow_set.iter_enumerated() { if borrow_of_local_data(data.borrowed_place) { @@ -177,7 +177,7 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> { } TerminatorKind::Resume | TerminatorKind::Return | TerminatorKind::GeneratorDrop => { // Invalidate all borrows of local places - let borrow_set = self.borrow_set.clone(); + let borrow_set = self.borrow_set; let start = self.location_table.start_index(location); for (i, data) in borrow_set.iter_enumerated() { if borrow_of_local_data(data.borrowed_place) { @@ -369,7 +369,7 @@ impl<'cx, 'tcx> InvalidationGenerator<'cx, 'tcx> { ); let tcx = self.tcx; let body = self.body; - let borrow_set = self.borrow_set.clone(); + let borrow_set = self.borrow_set; let indices = self.borrow_set.indices(); each_borrow_involving_path( self, @@ -377,7 +377,7 @@ impl<'cx, 'tcx> InvalidationGenerator<'cx, 'tcx> { body, location, (sd, place), - &borrow_set.clone(), + borrow_set, indices, |this, borrow_index, borrow| { match (rw, borrow.kind) { diff --git a/compiler/rustc_mir_build/src/build/matches/test.rs b/compiler/rustc_mir_build/src/build/matches/test.rs index 126fb957a6a99..4db7debee7e8f 100644 --- a/compiler/rustc_mir_build/src/build/matches/test.rs +++ b/compiler/rustc_mir_build/src/build/matches/test.rs @@ -51,7 +51,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { PatKind::Constant { value } => Test { span: match_pair.pattern.span, - kind: TestKind::Eq { value, ty: match_pair.pattern.ty.clone() }, + kind: TestKind::Eq { value, ty: match_pair.pattern.ty }, }, PatKind::Range(range) => { diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index d39fbd61962e7..61f9a080a5217 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -129,6 +129,7 @@ symbols! { BTreeMap, BTreeSet, BinaryHeap, + Borrow, C, CString, Center, @@ -141,6 +142,7 @@ symbols! { Decodable, Decoder, Default, + Deref, Encodable, Encoder, Eq, diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs index bfb5ebcea58b1..a3faf4cb7d4c1 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -819,7 +819,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { sig.decl .inputs .iter() - .map(|arg| match arg.clone().kind { + .map(|arg| match arg.kind { hir::TyKind::Tup(ref tys) => ArgKind::Tuple( Some(arg.span), vec![("_".to_owned(), "_".to_owned()); tys.len()], diff --git a/compiler/rustc_traits/src/chalk/mod.rs b/compiler/rustc_traits/src/chalk/mod.rs index d98f18182c843..b7275bac19048 100644 --- a/compiler/rustc_traits/src/chalk/mod.rs +++ b/compiler/rustc_traits/src/chalk/mod.rs @@ -165,7 +165,7 @@ crate fn evaluate_goal<'tcx>( // let's just ignore that let sol = Canonical { max_universe: ty::UniverseIndex::from_usize(0), - variables: obligation.variables.clone(), + variables: obligation.variables, value: QueryResponse { var_values: CanonicalVarValues { var_values: IndexVec::new() } .make_identity(tcx), diff --git a/compiler/rustc_typeck/src/check/callee.rs b/compiler/rustc_typeck/src/check/callee.rs index ca1e79fac73e9..6ef63bcbbbf21 100644 --- a/compiler/rustc_typeck/src/check/callee.rs +++ b/compiler/rustc_typeck/src/check/callee.rs @@ -465,7 +465,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let expected_arg_tys = self.expected_inputs_for_expected_output( call_expr.span, expected, - fn_sig.output().clone(), + fn_sig.output(), fn_sig.inputs(), ); diff --git a/compiler/rustc_typeck/src/check/expr.rs b/compiler/rustc_typeck/src/check/expr.rs index 2faf128c491fd..48740e533da8e 100644 --- a/compiler/rustc_typeck/src/check/expr.rs +++ b/compiler/rustc_typeck/src/check/expr.rs @@ -711,7 +711,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { }); let ret_ty = ret_coercion.borrow().expected_ty(); - let return_expr_ty = self.check_expr_with_hint(return_expr, ret_ty.clone()); + let return_expr_ty = self.check_expr_with_hint(return_expr, ret_ty); ret_coercion.borrow_mut().coerce( self, &self.cause(return_expr.span, ObligationCauseCode::ReturnValue(return_expr.hir_id)), diff --git a/library/core/src/borrow.rs b/library/core/src/borrow.rs index af2ad12dddf0a..a0cdf681f67e1 100644 --- a/library/core/src/borrow.rs +++ b/library/core/src/borrow.rs @@ -153,6 +153,7 @@ /// [`HashMap`]: ../../std/collections/struct.HashMap.html /// [`String`]: ../../std/string/struct.String.html #[stable(feature = "rust1", since = "1.0.0")] +#[rustc_diagnostic_item = "Borrow"] pub trait Borrow { /// Immutably borrows from an owned value. /// diff --git a/library/core/src/clone.rs b/library/core/src/clone.rs index 12b4feeb14ae3..957769cdc5a62 100644 --- a/library/core/src/clone.rs +++ b/library/core/src/clone.rs @@ -104,6 +104,7 @@ /// [impls]: #implementors #[stable(feature = "rust1", since = "1.0.0")] #[lang = "clone"] +#[rustc_diagnostic_item = "Clone"] pub trait Clone: Sized { /// Returns a copy of the value. /// diff --git a/library/core/src/ops/deref.rs b/library/core/src/ops/deref.rs index d503a30174191..10e3ce67448c8 100644 --- a/library/core/src/ops/deref.rs +++ b/library/core/src/ops/deref.rs @@ -60,6 +60,7 @@ #[doc(alias = "*")] #[doc(alias = "&*")] #[stable(feature = "rust1", since = "1.0.0")] +#[rustc_diagnostic_item = "Deref"] pub trait Deref { /// The resulting type after dereferencing. #[stable(feature = "rust1", since = "1.0.0")] diff --git a/src/test/ui/issues/issue-11820.rs b/src/test/ui/issues/issue-11820.rs index 7ffe9652797cf..8a26624a05dc9 100644 --- a/src/test/ui/issues/issue-11820.rs +++ b/src/test/ui/issues/issue-11820.rs @@ -1,12 +1,14 @@ // run-pass // pretty-expanded FIXME #23616 +#![allow(noop_method_call)] + struct NoClone; fn main() { - let rnc = &NoClone; - let rsnc = &Some(NoClone); + let rnc = &NoClone; + let rsnc = &Some(NoClone); - let _: &NoClone = rnc.clone(); - let _: &Option = rsnc.clone(); + let _: &NoClone = rnc.clone(); + let _: &Option = rsnc.clone(); } diff --git a/src/test/ui/underscore-imports/cycle.rs b/src/test/ui/underscore-imports/cycle.rs index bacf9b2d5a96a..987410fa84b20 100644 --- a/src/test/ui/underscore-imports/cycle.rs +++ b/src/test/ui/underscore-imports/cycle.rs @@ -14,5 +14,6 @@ mod y { pub fn main() { use x::*; + #[allow(noop_method_call)] (&0).deref(); } diff --git a/src/test/ui/underscore-imports/hygiene-2.rs b/src/test/ui/underscore-imports/hygiene-2.rs index bea61eae6b51a..510d91d0d4624 100644 --- a/src/test/ui/underscore-imports/hygiene-2.rs +++ b/src/test/ui/underscore-imports/hygiene-2.rs @@ -29,5 +29,6 @@ m!(y); fn main() { use crate::y::*; + #[allow(noop_method_call)] (&()).deref(); } diff --git a/src/test/ui/underscore-imports/macro-expanded.rs b/src/test/ui/underscore-imports/macro-expanded.rs index 43f527bc9a408..55e86e848558d 100644 --- a/src/test/ui/underscore-imports/macro-expanded.rs +++ b/src/test/ui/underscore-imports/macro-expanded.rs @@ -3,6 +3,7 @@ // check-pass #![feature(decl_macro, rustc_attrs)] +#![allow(noop_method_call)] mod x { pub use std::ops::Not as _; From 618c395a8da1e284a2963b8da02450eb162e0478 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Wed, 6 Jan 2021 14:48:06 +0100 Subject: [PATCH 27/63] Bless test where order of error message changed --- src/test/ui/panic-handler/weak-lang-item.stderr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/ui/panic-handler/weak-lang-item.stderr b/src/test/ui/panic-handler/weak-lang-item.stderr index 68e3e21df3e08..b7c040c7a850b 100644 --- a/src/test/ui/panic-handler/weak-lang-item.stderr +++ b/src/test/ui/panic-handler/weak-lang-item.stderr @@ -10,10 +10,10 @@ help: you can use `as` to change the binding name of the import LL | extern crate core as other_core; | -error: language item required, but not found: `eh_personality` - error: `#[panic_handler]` function required, but not found +error: language item required, but not found: `eh_personality` + error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0259`. From 3a86184777e98935bef9b9662d090d6886c3c041 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Wed, 6 Jan 2021 17:56:34 +0100 Subject: [PATCH 28/63] Fix ui-full-deps suite --- library/alloc/src/collections/btree/map/tests.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/alloc/src/collections/btree/map/tests.rs b/library/alloc/src/collections/btree/map/tests.rs index 4e48db7f49305..e636e490e1bf4 100644 --- a/library/alloc/src/collections/btree/map/tests.rs +++ b/library/alloc/src/collections/btree/map/tests.rs @@ -1801,11 +1801,11 @@ fn test_occupied_entry_key() { let key = "hello there"; let value = "value goes here"; assert!(a.is_empty()); - a.insert(key.clone(), value.clone()); + a.insert(key, value); assert_eq!(a.len(), 1); assert_eq!(a[key], value); - match a.entry(key.clone()) { + match a.entry(key) { Vacant(_) => panic!(), Occupied(e) => assert_eq!(key, *e.key()), } @@ -1821,11 +1821,11 @@ fn test_vacant_entry_key() { let value = "value goes here"; assert!(a.is_empty()); - match a.entry(key.clone()) { + match a.entry(key) { Occupied(_) => panic!(), Vacant(e) => { assert_eq!(key, *e.key()); - e.insert(value.clone()); + e.insert(value); } } assert_eq!(a.len(), 1); From ee65416f0d50331b5fef4360ce36538028edebf9 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Wed, 6 Jan 2021 21:11:08 +0100 Subject: [PATCH 29/63] Fix core tests --- library/core/src/clone.rs | 1 + library/core/tests/clone.rs | 2 ++ library/core/tests/iter/adapters/intersperse.rs | 4 ++-- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/library/core/src/clone.rs b/library/core/src/clone.rs index 957769cdc5a62..51a2dc03de318 100644 --- a/library/core/src/clone.rs +++ b/library/core/src/clone.rs @@ -111,6 +111,7 @@ pub trait Clone: Sized { /// # Examples /// /// ``` + /// # #![allow(noop_method_call)] /// let hello = "Hello"; // &str implements Clone /// /// assert_eq!("Hello", hello.clone()); diff --git a/library/core/tests/clone.rs b/library/core/tests/clone.rs index c97a87aebce41..e5787d79c3226 100644 --- a/library/core/tests/clone.rs +++ b/library/core/tests/clone.rs @@ -1,3 +1,5 @@ +#![allow(noop_method_call)] + #[test] fn test_borrowed_clone() { let x = 5; diff --git a/library/core/tests/iter/adapters/intersperse.rs b/library/core/tests/iter/adapters/intersperse.rs index 9dbe232e4eec8..b336c03b5adbe 100644 --- a/library/core/tests/iter/adapters/intersperse.rs +++ b/library/core/tests/iter/adapters/intersperse.rs @@ -9,7 +9,7 @@ fn test_intersperse() { assert_eq!(v, vec![1]); let xs = ["a", "", "b", "c"]; - let v: Vec<&str> = xs.iter().map(|x| x.clone()).intersperse(", ").collect(); + let v: Vec<&str> = xs.iter().map(|x| *x).intersperse(", ").collect(); let text: String = v.concat(); assert_eq!(text, "a, , b, c".to_string()); @@ -24,7 +24,7 @@ fn test_intersperse_size_hint() { assert_eq!(iter.size_hint(), (0, Some(0))); let xs = ["a", "", "b", "c"]; - let mut iter = xs.iter().map(|x| x.clone()).intersperse(", "); + let mut iter = xs.iter().map(|x| *x).intersperse(", "); assert_eq!(iter.size_hint(), (7, Some(7))); assert_eq!(iter.next(), Some("a")); From d3b49c2ed2c7bf35e6155ea25acbf657d8224eac Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Thu, 7 Jan 2021 11:24:32 +0100 Subject: [PATCH 30/63] Only allow new lint when not bootstrapping - since beta doesn't know about the lint --- library/core/tests/clone.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/tests/clone.rs b/library/core/tests/clone.rs index e5787d79c3226..2f2aa9a20f912 100644 --- a/library/core/tests/clone.rs +++ b/library/core/tests/clone.rs @@ -1,4 +1,4 @@ -#![allow(noop_method_call)] +#![cfg_attr(not(bootstrap), allow(noop_method_call))] #[test] fn test_borrowed_clone() { From c5ff54cbdb6b0da742110b416cdbcf0ca0ff0fc4 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Thu, 7 Jan 2021 13:13:25 +0100 Subject: [PATCH 31/63] Fix std tests --- library/std/src/collections/hash/map/tests.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/std/src/collections/hash/map/tests.rs b/library/std/src/collections/hash/map/tests.rs index 467968354e25d..819be14222752 100644 --- a/library/std/src/collections/hash/map/tests.rs +++ b/library/std/src/collections/hash/map/tests.rs @@ -774,11 +774,11 @@ fn test_occupied_entry_key() { let key = "hello there"; let value = "value goes here"; assert!(a.is_empty()); - a.insert(key.clone(), value.clone()); + a.insert(key, value); assert_eq!(a.len(), 1); assert_eq!(a[key], value); - match a.entry(key.clone()) { + match a.entry(key) { Vacant(_) => panic!(), Occupied(e) => assert_eq!(key, *e.key()), } @@ -793,11 +793,11 @@ fn test_vacant_entry_key() { let value = "value goes here"; assert!(a.is_empty()); - match a.entry(key.clone()) { + match a.entry(key) { Occupied(_) => panic!(), Vacant(e) => { assert_eq!(key, *e.key()); - e.insert(value.clone()); + e.insert(value); } } assert_eq!(a.len(), 1); From 217c88655b1155796739edbf415e7ce37d30830b Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Fri, 8 Jan 2021 11:37:52 +0100 Subject: [PATCH 32/63] Improve warning --- compiler/rustc_lint/src/noop_method_call.rs | 10 ++++++---- src/test/ui/lint/noop-method-call.rs | 8 ++++---- src/test/ui/lint/noop-method-call.stderr | 16 ++++++++-------- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_lint/src/noop_method_call.rs b/compiler/rustc_lint/src/noop_method_call.rs index dad557128f8b7..b9b5009d9dd95 100644 --- a/compiler/rustc_lint/src/noop_method_call.rs +++ b/compiler/rustc_lint/src/noop_method_call.rs @@ -70,11 +70,13 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall { .iter() .any(|s| cx.tcx.is_diagnostic_item(*s, i.def_id())) { - let span = expr.span; + let expr_span = expr.span; - cx.struct_span_lint(NOOP_METHOD_CALL, span, |lint| { - let message = "call to noop method"; - lint.build(&message).emit() + cx.struct_span_lint(NOOP_METHOD_CALL, expr_span, |lint| { + let message = "call to method that does nothing"; + lint.build(&message) + .span_label(expr_span, "unnecessary method call") + .emit() }); } } diff --git a/src/test/ui/lint/noop-method-call.rs b/src/test/ui/lint/noop-method-call.rs index 4b81e04d3f7f9..b8ff75845bd7d 100644 --- a/src/test/ui/lint/noop-method-call.rs +++ b/src/test/ui/lint/noop-method-call.rs @@ -21,19 +21,19 @@ impl Deref for DerefExample { fn main() { let foo = &Foo(1u32); - let foo_clone: &Foo = foo.clone(); //~ WARNING call to noop method + let foo_clone: &Foo = foo.clone(); //~ WARNING call to method that does nothing [noop_method_call] let bar = &Bar(1u32); let bar_clone: Bar = bar.clone(); let deref = &&DerefExample(12u32); - let derefed: &DerefExample = deref.deref(); //~ WARNING call to noop method + let derefed: &DerefExample = deref.deref(); //~ WARNING call to method that does nothing [noop_method_call] let deref = &DerefExample(12u32); let derefed: &u32 = deref.deref(); let a = &&Foo(1u32); - let borrowed: &Foo = a.borrow(); //~ WARNING call to noop method + let borrowed: &Foo = a.borrow(); //~ WARNING call to method that does nothing [noop_method_call] } fn generic(foo: &Foo) { @@ -41,5 +41,5 @@ fn generic(foo: &Foo) { } fn non_generic(foo: &Foo) { - foo.clone(); //~ WARNING call to noop method + foo.clone(); //~ WARNING call to method that does nothing [noop_method_call] } diff --git a/src/test/ui/lint/noop-method-call.stderr b/src/test/ui/lint/noop-method-call.stderr index 1120adee121b1..f5b766f42333f 100644 --- a/src/test/ui/lint/noop-method-call.stderr +++ b/src/test/ui/lint/noop-method-call.stderr @@ -1,28 +1,28 @@ -warning: call to noop method +warning: call to method that does nothing --> $DIR/noop-method-call.rs:24:32 | LL | let foo_clone: &Foo = foo.clone(); - | ^^^^^^^^^^^ + | ^^^^^^^^^^^ unnecessary method call | = note: `#[warn(noop_method_call)]` on by default -warning: call to noop method +warning: call to method that does nothing --> $DIR/noop-method-call.rs:30:39 | LL | let derefed: &DerefExample = deref.deref(); - | ^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^ unnecessary method call -warning: call to noop method +warning: call to method that does nothing --> $DIR/noop-method-call.rs:36:31 | LL | let borrowed: &Foo = a.borrow(); - | ^^^^^^^^^^ + | ^^^^^^^^^^ unnecessary method call -warning: call to noop method +warning: call to method that does nothing --> $DIR/noop-method-call.rs:44:5 | LL | foo.clone(); - | ^^^^^^^^^^^ + | ^^^^^^^^^^^ unnecessary method call warning: 4 warnings emitted From 16c4afbde4bcc0b2471cd48cf29378d557a8f4ab Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Fri, 8 Jan 2021 12:09:29 +0100 Subject: [PATCH 33/63] Fix tidy errors --- src/test/ui/lint/noop-method-call.rs | 9 ++++++--- src/test/ui/lint/noop-method-call.stderr | 6 +++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/test/ui/lint/noop-method-call.rs b/src/test/ui/lint/noop-method-call.rs index b8ff75845bd7d..9f0ab3960f8dd 100644 --- a/src/test/ui/lint/noop-method-call.rs +++ b/src/test/ui/lint/noop-method-call.rs @@ -21,19 +21,22 @@ impl Deref for DerefExample { fn main() { let foo = &Foo(1u32); - let foo_clone: &Foo = foo.clone(); //~ WARNING call to method that does nothing [noop_method_call] + let foo_clone: &Foo = foo.clone(); + //~^ WARNING call to method that does nothing [noop_method_call] let bar = &Bar(1u32); let bar_clone: Bar = bar.clone(); let deref = &&DerefExample(12u32); - let derefed: &DerefExample = deref.deref(); //~ WARNING call to method that does nothing [noop_method_call] + let derefed: &DerefExample = deref.deref(); + //~^ WARNING call to method that does nothing [noop_method_call] let deref = &DerefExample(12u32); let derefed: &u32 = deref.deref(); let a = &&Foo(1u32); - let borrowed: &Foo = a.borrow(); //~ WARNING call to method that does nothing [noop_method_call] + let borrowed: &Foo = a.borrow(); + //~^ WARNING call to method that does nothing [noop_method_call] } fn generic(foo: &Foo) { diff --git a/src/test/ui/lint/noop-method-call.stderr b/src/test/ui/lint/noop-method-call.stderr index f5b766f42333f..32acf1632336d 100644 --- a/src/test/ui/lint/noop-method-call.stderr +++ b/src/test/ui/lint/noop-method-call.stderr @@ -7,19 +7,19 @@ LL | let foo_clone: &Foo = foo.clone(); = note: `#[warn(noop_method_call)]` on by default warning: call to method that does nothing - --> $DIR/noop-method-call.rs:30:39 + --> $DIR/noop-method-call.rs:31:39 | LL | let derefed: &DerefExample = deref.deref(); | ^^^^^^^^^^^^^ unnecessary method call warning: call to method that does nothing - --> $DIR/noop-method-call.rs:36:31 + --> $DIR/noop-method-call.rs:38:31 | LL | let borrowed: &Foo = a.borrow(); | ^^^^^^^^^^ unnecessary method call warning: call to method that does nothing - --> $DIR/noop-method-call.rs:44:5 + --> $DIR/noop-method-call.rs:47:5 | LL | foo.clone(); | ^^^^^^^^^^^ unnecessary method call From 95e330bd0197a658096dfa1922ec3b3e6a4e6b78 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Mon, 11 Jan 2021 11:47:16 +0100 Subject: [PATCH 34/63] Update error message --- compiler/rustc_lint/src/noop_method_call.rs | 9 ++++++--- src/test/ui/issues/issue-11820.rs | 8 ++++---- src/test/ui/lint/noop-method-call.rs | 9 +++++---- src/test/ui/lint/noop-method-call.stderr | 19 +++++++++++++++---- 4 files changed, 30 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_lint/src/noop_method_call.rs b/compiler/rustc_lint/src/noop_method_call.rs index b9b5009d9dd95..e91dd37d8aaae 100644 --- a/compiler/rustc_lint/src/noop_method_call.rs +++ b/compiler/rustc_lint/src/noop_method_call.rs @@ -39,7 +39,7 @@ declare_lint_pass!(NoopMethodCall => [NOOP_METHOD_CALL]); impl<'tcx> LateLintPass<'tcx> for NoopMethodCall { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { // We only care about method calls - if let ExprKind::MethodCall(..) = expr.kind { + if let ExprKind::MethodCall(call, ..) = expr.kind { // Get the `DefId` only when dealing with an `AssocFn` if let Some((DefKind::AssocFn, did)) = cx.typeck_results().type_dependent_def(expr.hir_id) @@ -55,7 +55,7 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall { } let substs = cx.typeck_results().node_substs(expr.hir_id); - // We can't resolve on types that recursively require monomorphization, + // We can't resolve on types that require monomorphization, // so check that we don't need to perfom substitution if !substs.needs_subst() { let param_env = cx.tcx.param_env(trait_id); @@ -73,9 +73,12 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall { let expr_span = expr.span; cx.struct_span_lint(NOOP_METHOD_CALL, expr_span, |lint| { - let message = "call to method that does nothing"; + let method = &call.ident.name; + let message = format!("call to `.{}()` on a reference in this situation does nothing", &method); lint.build(&message) .span_label(expr_span, "unnecessary method call") + .note("the type the method is being called on and the return type are functionally equivalent.") + .note("therefore, the method call doesn't actually do anything and can be removed.") .emit() }); } diff --git a/src/test/ui/issues/issue-11820.rs b/src/test/ui/issues/issue-11820.rs index 8a26624a05dc9..dc6349b10ee58 100644 --- a/src/test/ui/issues/issue-11820.rs +++ b/src/test/ui/issues/issue-11820.rs @@ -6,9 +6,9 @@ struct NoClone; fn main() { - let rnc = &NoClone; - let rsnc = &Some(NoClone); + let rnc = &NoClone; + let rsnc = &Some(NoClone); - let _: &NoClone = rnc.clone(); - let _: &Option = rsnc.clone(); + let _: &NoClone = rnc.clone(); + let _: &Option = rsnc.clone(); } diff --git a/src/test/ui/lint/noop-method-call.rs b/src/test/ui/lint/noop-method-call.rs index 9f0ab3960f8dd..b8aa55e1e1da6 100644 --- a/src/test/ui/lint/noop-method-call.rs +++ b/src/test/ui/lint/noop-method-call.rs @@ -22,21 +22,21 @@ impl Deref for DerefExample { fn main() { let foo = &Foo(1u32); let foo_clone: &Foo = foo.clone(); - //~^ WARNING call to method that does nothing [noop_method_call] + //~^ WARNING call to `.clone()` on a reference in this situation does nothing [noop_method_call] let bar = &Bar(1u32); let bar_clone: Bar = bar.clone(); let deref = &&DerefExample(12u32); let derefed: &DerefExample = deref.deref(); - //~^ WARNING call to method that does nothing [noop_method_call] + //~^ WARNING call to `.deref()` on a reference in this situation does nothing [noop_method_call] let deref = &DerefExample(12u32); let derefed: &u32 = deref.deref(); let a = &&Foo(1u32); let borrowed: &Foo = a.borrow(); - //~^ WARNING call to method that does nothing [noop_method_call] + //~^ WARNING call to `.borrow()` on a reference in this situation does nothing [noop_method_call] } fn generic(foo: &Foo) { @@ -44,5 +44,6 @@ fn generic(foo: &Foo) { } fn non_generic(foo: &Foo) { - foo.clone(); //~ WARNING call to method that does nothing [noop_method_call] + foo.clone(); + //~^ WARNING call to `.clone()` on a reference in this situation does nothing [noop_method_call] } diff --git a/src/test/ui/lint/noop-method-call.stderr b/src/test/ui/lint/noop-method-call.stderr index 32acf1632336d..f9cc9735d54f6 100644 --- a/src/test/ui/lint/noop-method-call.stderr +++ b/src/test/ui/lint/noop-method-call.stderr @@ -1,28 +1,39 @@ -warning: call to method that does nothing +warning: call to `.clone()` on a reference in this situation does nothing --> $DIR/noop-method-call.rs:24:32 | LL | let foo_clone: &Foo = foo.clone(); | ^^^^^^^^^^^ unnecessary method call | = note: `#[warn(noop_method_call)]` on by default + = note: the type the method is being called on and the return type are functionally equivalent. + = note: therefore, the method call doesn't actually do anything and can be removed. -warning: call to method that does nothing +warning: call to `.deref()` on a reference in this situation does nothing --> $DIR/noop-method-call.rs:31:39 | LL | let derefed: &DerefExample = deref.deref(); | ^^^^^^^^^^^^^ unnecessary method call + | + = note: the type the method is being called on and the return type are functionally equivalent. + = note: therefore, the method call doesn't actually do anything and can be removed. -warning: call to method that does nothing +warning: call to `.borrow()` on a reference in this situation does nothing --> $DIR/noop-method-call.rs:38:31 | LL | let borrowed: &Foo = a.borrow(); | ^^^^^^^^^^ unnecessary method call + | + = note: the type the method is being called on and the return type are functionally equivalent. + = note: therefore, the method call doesn't actually do anything and can be removed. -warning: call to method that does nothing +warning: call to `.clone()` on a reference in this situation does nothing --> $DIR/noop-method-call.rs:47:5 | LL | foo.clone(); | ^^^^^^^^^^^ unnecessary method call + | + = note: the type the method is being called on and the return type are functionally equivalent. + = note: therefore, the method call doesn't actually do anything and can be removed. warning: 4 warnings emitted From 316e9db0cf86852b0150d8f0d475ba9a1dcc4774 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Mon, 11 Jan 2021 19:02:19 +0100 Subject: [PATCH 35/63] Fix tidy error --- src/test/ui/lint/noop-method-call.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/ui/lint/noop-method-call.rs b/src/test/ui/lint/noop-method-call.rs index b8aa55e1e1da6..bc61d619abe82 100644 --- a/src/test/ui/lint/noop-method-call.rs +++ b/src/test/ui/lint/noop-method-call.rs @@ -22,21 +22,21 @@ impl Deref for DerefExample { fn main() { let foo = &Foo(1u32); let foo_clone: &Foo = foo.clone(); - //~^ WARNING call to `.clone()` on a reference in this situation does nothing [noop_method_call] + //~^ WARNING call to `.clone()` on a reference in this situation does nothing let bar = &Bar(1u32); let bar_clone: Bar = bar.clone(); let deref = &&DerefExample(12u32); let derefed: &DerefExample = deref.deref(); - //~^ WARNING call to `.deref()` on a reference in this situation does nothing [noop_method_call] + //~^ WARNING call to `.deref()` on a reference in this situation does nothing let deref = &DerefExample(12u32); let derefed: &u32 = deref.deref(); let a = &&Foo(1u32); let borrowed: &Foo = a.borrow(); - //~^ WARNING call to `.borrow()` on a reference in this situation does nothing [noop_method_call] + //~^ WARNING call to `.borrow()` on a reference in this situation does nothing } fn generic(foo: &Foo) { @@ -45,5 +45,5 @@ fn generic(foo: &Foo) { fn non_generic(foo: &Foo) { foo.clone(); - //~^ WARNING call to `.clone()` on a reference in this situation does nothing [noop_method_call] + //~^ WARNING call to `.clone()` on a reference in this situation does nothing } From 49f32e0c8e78948210654299a8c19b2e0f0cbfa9 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Tue, 12 Jan 2021 14:58:51 +0100 Subject: [PATCH 36/63] Improve error messages --- compiler/rustc_lint/src/noop_method_call.rs | 18 +++++++++---- src/test/ui/lint/noop-method-call.stderr | 28 +++++++++------------ 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_lint/src/noop_method_call.rs b/compiler/rustc_lint/src/noop_method_call.rs index e91dd37d8aaae..1aa4a986cd1fe 100644 --- a/compiler/rustc_lint/src/noop_method_call.rs +++ b/compiler/rustc_lint/src/noop_method_call.rs @@ -39,7 +39,7 @@ declare_lint_pass!(NoopMethodCall => [NOOP_METHOD_CALL]); impl<'tcx> LateLintPass<'tcx> for NoopMethodCall { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { // We only care about method calls - if let ExprKind::MethodCall(call, ..) = expr.kind { + if let ExprKind::MethodCall(call, _, elements, _) = expr.kind { // Get the `DefId` only when dealing with an `AssocFn` if let Some((DefKind::AssocFn, did)) = cx.typeck_results().type_dependent_def(expr.hir_id) @@ -70,15 +70,23 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall { .iter() .any(|s| cx.tcx.is_diagnostic_item(*s, i.def_id())) { + let method = &call.ident.name; + let receiver = &elements[0]; + let receiver_ty = cx.typeck_results().expr_ty(receiver); let expr_span = expr.span; + let note = format!( + "the type `{:?}` which `{}` is being called on is the same as the type returned from `{}`, \ + so the method call does not do anything and can be removed.", + receiver_ty, method, method + ); - cx.struct_span_lint(NOOP_METHOD_CALL, expr_span, |lint| { + let span = expr_span.with_lo(receiver.span.hi()); + cx.struct_span_lint(NOOP_METHOD_CALL, span, |lint| { let method = &call.ident.name; let message = format!("call to `.{}()` on a reference in this situation does nothing", &method); lint.build(&message) - .span_label(expr_span, "unnecessary method call") - .note("the type the method is being called on and the return type are functionally equivalent.") - .note("therefore, the method call doesn't actually do anything and can be removed.") + .span_label(span, "unnecessary method call") + .note(¬e) .emit() }); } diff --git a/src/test/ui/lint/noop-method-call.stderr b/src/test/ui/lint/noop-method-call.stderr index f9cc9735d54f6..7e27bf3abf9f3 100644 --- a/src/test/ui/lint/noop-method-call.stderr +++ b/src/test/ui/lint/noop-method-call.stderr @@ -1,39 +1,35 @@ warning: call to `.clone()` on a reference in this situation does nothing - --> $DIR/noop-method-call.rs:24:32 + --> $DIR/noop-method-call.rs:24:35 | LL | let foo_clone: &Foo = foo.clone(); - | ^^^^^^^^^^^ unnecessary method call + | ^^^^^^^^ unnecessary method call | = note: `#[warn(noop_method_call)]` on by default - = note: the type the method is being called on and the return type are functionally equivalent. - = note: therefore, the method call doesn't actually do anything and can be removed. + = note: the type `&Foo` which `clone` is being called on is the same as the type returned from `clone`, so the method call does not do anything and can be removed. warning: call to `.deref()` on a reference in this situation does nothing - --> $DIR/noop-method-call.rs:31:39 + --> $DIR/noop-method-call.rs:31:44 | LL | let derefed: &DerefExample = deref.deref(); - | ^^^^^^^^^^^^^ unnecessary method call + | ^^^^^^^^ unnecessary method call | - = note: the type the method is being called on and the return type are functionally equivalent. - = note: therefore, the method call doesn't actually do anything and can be removed. + = note: the type `&&DerefExample` which `deref` is being called on is the same as the type returned from `deref`, so the method call does not do anything and can be removed. warning: call to `.borrow()` on a reference in this situation does nothing - --> $DIR/noop-method-call.rs:38:31 + --> $DIR/noop-method-call.rs:38:32 | LL | let borrowed: &Foo = a.borrow(); - | ^^^^^^^^^^ unnecessary method call + | ^^^^^^^^^ unnecessary method call | - = note: the type the method is being called on and the return type are functionally equivalent. - = note: therefore, the method call doesn't actually do anything and can be removed. + = note: the type `&&Foo` which `borrow` is being called on is the same as the type returned from `borrow`, so the method call does not do anything and can be removed. warning: call to `.clone()` on a reference in this situation does nothing - --> $DIR/noop-method-call.rs:47:5 + --> $DIR/noop-method-call.rs:47:8 | LL | foo.clone(); - | ^^^^^^^^^^^ unnecessary method call + | ^^^^^^^^ unnecessary method call | - = note: the type the method is being called on and the return type are functionally equivalent. - = note: therefore, the method call doesn't actually do anything and can be removed. + = note: the type `&Foo` which `clone` is being called on is the same as the type returned from `clone`, so the method call does not do anything and can be removed. warning: 4 warnings emitted From e48670c34a3f5ab3fe9defcc61861999f3f14e93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 12 Jan 2021 18:37:32 -0800 Subject: [PATCH 37/63] Increase accuracy of lint trigger --- compiler/rustc_lint/src/noop_method_call.rs | 58 +++++++++++++-------- src/test/ui/lint/noop-method-call.rs | 3 ++ src/test/ui/lint/noop-method-call.stderr | 10 ++-- 3 files changed, 43 insertions(+), 28 deletions(-) diff --git a/compiler/rustc_lint/src/noop_method_call.rs b/compiler/rustc_lint/src/noop_method_call.rs index 1aa4a986cd1fe..04d87dc959273 100644 --- a/compiler/rustc_lint/src/noop_method_call.rs +++ b/compiler/rustc_lint/src/noop_method_call.rs @@ -62,33 +62,45 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall { // Resolve the trait method instance if let Ok(Some(i)) = ty::Instance::resolve(cx.tcx, param_env, did, substs) { // Check that it implements the noop diagnostic - if [ - sym::noop_method_borrow, - sym::noop_method_clone, - sym::noop_method_deref, + for (s, peel_ref) in [ + (sym::noop_method_borrow, true), + (sym::noop_method_clone, false), + (sym::noop_method_deref, true), ] .iter() - .any(|s| cx.tcx.is_diagnostic_item(*s, i.def_id())) { - let method = &call.ident.name; - let receiver = &elements[0]; - let receiver_ty = cx.typeck_results().expr_ty(receiver); - let expr_span = expr.span; - let note = format!( - "the type `{:?}` which `{}` is being called on is the same as the type returned from `{}`, \ - so the method call does not do anything and can be removed.", - receiver_ty, method, method - ); - - let span = expr_span.with_lo(receiver.span.hi()); - cx.struct_span_lint(NOOP_METHOD_CALL, span, |lint| { + if cx.tcx.is_diagnostic_item(*s, i.def_id()) { let method = &call.ident.name; - let message = format!("call to `.{}()` on a reference in this situation does nothing", &method); - lint.build(&message) - .span_label(span, "unnecessary method call") - .note(¬e) - .emit() - }); + let receiver = &elements[0]; + let receiver_ty = cx.typeck_results().expr_ty(receiver); + let receiver_ty = match receiver_ty.kind() { + // Remove one borrow from the receiver as all the trait methods + // we care about here have a `&self` receiver. + ty::Ref(_, ty, _) if *peel_ref => ty, + _ => receiver_ty, + }; + let expr_ty = cx.typeck_results().expr_ty_adjusted(expr); + if receiver_ty != expr_ty { + return; + } + let expr_span = expr.span; + let note = format!( + "the type `{:?}` which `{}` is being called on is the same as \ + the type returned from `{}`, so the method call does not do \ + anything and can be removed", + receiver_ty, method, method, + ); + + let span = expr_span.with_lo(receiver.span.hi()); + cx.struct_span_lint(NOOP_METHOD_CALL, span, |lint| { + let method = &call.ident.name; + let message = format!("call to `.{}()` on a reference in this situation does nothing", &method); + lint.build(&message) + .span_label(span, "unnecessary method call") + .note(¬e) + .emit() + }); + } } } } diff --git a/src/test/ui/lint/noop-method-call.rs b/src/test/ui/lint/noop-method-call.rs index bc61d619abe82..70363a191e9e9 100644 --- a/src/test/ui/lint/noop-method-call.rs +++ b/src/test/ui/lint/noop-method-call.rs @@ -37,6 +37,9 @@ fn main() { let a = &&Foo(1u32); let borrowed: &Foo = a.borrow(); //~^ WARNING call to `.borrow()` on a reference in this situation does nothing + + let xs = ["a", "b", "c"]; + let _v: Vec<&str> = xs.iter().map(|x| x.clone()).collect(); // ok, but could use `*x` instead } fn generic(foo: &Foo) { diff --git a/src/test/ui/lint/noop-method-call.stderr b/src/test/ui/lint/noop-method-call.stderr index 7e27bf3abf9f3..316c47975e6de 100644 --- a/src/test/ui/lint/noop-method-call.stderr +++ b/src/test/ui/lint/noop-method-call.stderr @@ -5,7 +5,7 @@ LL | let foo_clone: &Foo = foo.clone(); | ^^^^^^^^ unnecessary method call | = note: `#[warn(noop_method_call)]` on by default - = note: the type `&Foo` which `clone` is being called on is the same as the type returned from `clone`, so the method call does not do anything and can be removed. + = note: the type `&Foo` which `clone` is being called on is the same as the type returned from `clone`, so the method call does not do anything and can be removed warning: call to `.deref()` on a reference in this situation does nothing --> $DIR/noop-method-call.rs:31:44 @@ -13,7 +13,7 @@ warning: call to `.deref()` on a reference in this situation does nothing LL | let derefed: &DerefExample = deref.deref(); | ^^^^^^^^ unnecessary method call | - = note: the type `&&DerefExample` which `deref` is being called on is the same as the type returned from `deref`, so the method call does not do anything and can be removed. + = note: the type `&DerefExample` which `deref` is being called on is the same as the type returned from `deref`, so the method call does not do anything and can be removed warning: call to `.borrow()` on a reference in this situation does nothing --> $DIR/noop-method-call.rs:38:32 @@ -21,15 +21,15 @@ warning: call to `.borrow()` on a reference in this situation does nothing LL | let borrowed: &Foo = a.borrow(); | ^^^^^^^^^ unnecessary method call | - = note: the type `&&Foo` which `borrow` is being called on is the same as the type returned from `borrow`, so the method call does not do anything and can be removed. + = note: the type `&Foo` which `borrow` is being called on is the same as the type returned from `borrow`, so the method call does not do anything and can be removed warning: call to `.clone()` on a reference in this situation does nothing - --> $DIR/noop-method-call.rs:47:8 + --> $DIR/noop-method-call.rs:50:8 | LL | foo.clone(); | ^^^^^^^^ unnecessary method call | - = note: the type `&Foo` which `clone` is being called on is the same as the type returned from `clone`, so the method call does not do anything and can be removed. + = note: the type `&Foo` which `clone` is being called on is the same as the type returned from `clone`, so the method call does not do anything and can be removed warning: 4 warnings emitted From 055db16479937d0a567b2318c775a25ab6cb7d99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 13 Jan 2021 10:39:25 -0800 Subject: [PATCH 38/63] Clean up code rightward drift --- compiler/rustc_lint/src/noop_method_call.rs | 144 +++++++++++--------- 1 file changed, 79 insertions(+), 65 deletions(-) diff --git a/compiler/rustc_lint/src/noop_method_call.rs b/compiler/rustc_lint/src/noop_method_call.rs index 04d87dc959273..b4ee1f9157c82 100644 --- a/compiler/rustc_lint/src/noop_method_call.rs +++ b/compiler/rustc_lint/src/noop_method_call.rs @@ -38,73 +38,87 @@ declare_lint_pass!(NoopMethodCall => [NOOP_METHOD_CALL]); impl<'tcx> LateLintPass<'tcx> for NoopMethodCall { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - // We only care about method calls - if let ExprKind::MethodCall(call, _, elements, _) = expr.kind { - // Get the `DefId` only when dealing with an `AssocFn` - if let Some((DefKind::AssocFn, did)) = - cx.typeck_results().type_dependent_def(expr.hir_id) - { - // Check that we're dealing with a trait method - if let Some(trait_id) = cx.tcx.trait_of_item(did) { - // Check we're dealing with one of the traits we care about - if ![sym::Clone, sym::Deref, sym::Borrow] + // We only care about method calls. + let (call, elements) = match expr.kind { + ExprKind::MethodCall(call, _, elements, _) => (call, elements), + _ => return, + }; + // We only care about method calls corresponding to the `Clone`, `Deref` and `Borrow` + // traits and ignore any other method call. + let (trait_id, did) = match cx.typeck_results().type_dependent_def(expr.hir_id) { + // Verify we are dealing with a method/associated function. + Some((DefKind::AssocFn, did)) => match cx.tcx.trait_of_item(did) { + // Check that we're dealing with a trait method for one of the traits we care about. + Some(trait_id) + if [sym::Clone, sym::Deref, sym::Borrow] .iter() - .any(|s| cx.tcx.is_diagnostic_item(*s, trait_id)) - { - return; - } - - let substs = cx.typeck_results().node_substs(expr.hir_id); - // We can't resolve on types that require monomorphization, - // so check that we don't need to perfom substitution - if !substs.needs_subst() { - let param_env = cx.tcx.param_env(trait_id); - // Resolve the trait method instance - if let Ok(Some(i)) = ty::Instance::resolve(cx.tcx, param_env, did, substs) { - // Check that it implements the noop diagnostic - for (s, peel_ref) in [ - (sym::noop_method_borrow, true), - (sym::noop_method_clone, false), - (sym::noop_method_deref, true), - ] - .iter() - { - if cx.tcx.is_diagnostic_item(*s, i.def_id()) { - let method = &call.ident.name; - let receiver = &elements[0]; - let receiver_ty = cx.typeck_results().expr_ty(receiver); - let receiver_ty = match receiver_ty.kind() { - // Remove one borrow from the receiver as all the trait methods - // we care about here have a `&self` receiver. - ty::Ref(_, ty, _) if *peel_ref => ty, - _ => receiver_ty, - }; - let expr_ty = cx.typeck_results().expr_ty_adjusted(expr); - if receiver_ty != expr_ty { - return; - } - let expr_span = expr.span; - let note = format!( - "the type `{:?}` which `{}` is being called on is the same as \ - the type returned from `{}`, so the method call does not do \ - anything and can be removed", - receiver_ty, method, method, - ); - - let span = expr_span.with_lo(receiver.span.hi()); - cx.struct_span_lint(NOOP_METHOD_CALL, span, |lint| { - let method = &call.ident.name; - let message = format!("call to `.{}()` on a reference in this situation does nothing", &method); - lint.build(&message) - .span_label(span, "unnecessary method call") - .note(¬e) - .emit() - }); - } - } - } - } + .any(|s| cx.tcx.is_diagnostic_item(*s, trait_id)) => + { + (trait_id, did) } + _ => return, + }, + _ => return, + }; + let substs = cx.typeck_results().node_substs(expr.hir_id); + if substs.needs_subst() { + // We can't resolve on types that require monomorphization, so we don't handle them if + // we need to perfom substitution. + return; + } + let param_env = cx.tcx.param_env(trait_id); + // Resolve the trait method instance. + let i = match ty::Instance::resolve(cx.tcx, param_env, did, substs) { + Ok(Some(i)) => i, + _ => return, + }; + // (Re)check that it implements the noop diagnostic. + for (s, peel_ref) in [ + (sym::noop_method_borrow, true), + (sym::noop_method_clone, false), + (sym::noop_method_deref, true), + ] + .iter() + { + if cx.tcx.is_diagnostic_item(*s, i.def_id()) { + let method = &call.ident.name; + let receiver = &elements[0]; + let receiver_ty = cx.typeck_results().expr_ty(receiver); + let receiver_ty = match receiver_ty.kind() { + // Remove one borrow from the receiver if appropriate to positively verify that + // the receiver `&self` type and the return type are the same, depending on the + // involved trait being checked. + ty::Ref(_, ty, _) if *peel_ref => ty, + // When it comes to `Clone` we need to check the `receiver_ty` directly. + // FIXME: we must come up with a better strategy for this. + _ => receiver_ty, + }; + let expr_ty = cx.typeck_results().expr_ty_adjusted(expr); + if receiver_ty != expr_ty { + // This lint will only trigger if the receiver type and resulting expression \ + // type are the same, implying that the method call is unnecessary. + return; + } + let expr_span = expr.span; + let note = format!( + "the type `{:?}` which `{}` is being called on is the same as \ + the type returned from `{}`, so the method call does not do \ + anything and can be removed", + receiver_ty, method, method, + ); + + let span = expr_span.with_lo(receiver.span.hi()); + cx.struct_span_lint(NOOP_METHOD_CALL, span, |lint| { + let method = &call.ident.name; + let message = format!( + "call to `.{}()` on a reference in this situation does nothing", + &method, + ); + lint.build(&message) + .span_label(span, "unnecessary method call") + .note(¬e) + .emit() + }); } } } From 4be7052b1a81f8f39d9ad23781e19f30d071f572 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Mon, 18 Jan 2021 14:15:19 +0100 Subject: [PATCH 39/63] Allow noop_method_call in clippy ui test --- src/tools/clippy/tests/ui/unnecessary_clone.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/clippy/tests/ui/unnecessary_clone.rs b/src/tools/clippy/tests/ui/unnecessary_clone.rs index 6770a7fac90fd..ce26634a995d9 100644 --- a/src/tools/clippy/tests/ui/unnecessary_clone.rs +++ b/src/tools/clippy/tests/ui/unnecessary_clone.rs @@ -1,7 +1,7 @@ // does not test any rustfixable lints #![warn(clippy::clone_on_ref_ptr)] -#![allow(unused, clippy::redundant_clone, clippy::unnecessary_wraps)] +#![allow(unused, noop_method_call, clippy::redundant_clone, clippy::unnecessary_wraps)] use std::cell::RefCell; use std::rc::{self, Rc}; From 6bf6652616ca75dde30cbdd021942433ae519730 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Wed, 10 Feb 2021 11:08:00 +0100 Subject: [PATCH 40/63] Move unrelated ui test back to what it was before --- src/test/ui/panic-handler/weak-lang-item.stderr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/ui/panic-handler/weak-lang-item.stderr b/src/test/ui/panic-handler/weak-lang-item.stderr index b7c040c7a850b..68e3e21df3e08 100644 --- a/src/test/ui/panic-handler/weak-lang-item.stderr +++ b/src/test/ui/panic-handler/weak-lang-item.stderr @@ -10,10 +10,10 @@ help: you can use `as` to change the binding name of the import LL | extern crate core as other_core; | -error: `#[panic_handler]` function required, but not found - error: language item required, but not found: `eh_personality` +error: `#[panic_handler]` function required, but not found + error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0259`. From da3995f0ec3085de42dcce9e91dbb5662b2c99d3 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Tue, 16 Feb 2021 15:12:19 +0100 Subject: [PATCH 41/63] Remove lint pass on borrow and deref --- compiler/rustc_lint/src/noop_method_call.rs | 12 +----- compiler/rustc_span/src/symbol.rs | 3 -- library/core/src/borrow.rs | 2 - library/core/src/ops/deref.rs | 2 - src/test/ui/lint/noop-method-call.rs | 46 +++++++-------------- src/test/ui/lint/noop-method-call.stderr | 34 ++++----------- 6 files changed, 25 insertions(+), 74 deletions(-) diff --git a/compiler/rustc_lint/src/noop_method_call.rs b/compiler/rustc_lint/src/noop_method_call.rs index b4ee1f9157c82..335c3c575e766 100644 --- a/compiler/rustc_lint/src/noop_method_call.rs +++ b/compiler/rustc_lint/src/noop_method_call.rs @@ -50,9 +50,7 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall { Some((DefKind::AssocFn, did)) => match cx.tcx.trait_of_item(did) { // Check that we're dealing with a trait method for one of the traits we care about. Some(trait_id) - if [sym::Clone, sym::Deref, sym::Borrow] - .iter() - .any(|s| cx.tcx.is_diagnostic_item(*s, trait_id)) => + if [sym::Clone].iter().any(|s| cx.tcx.is_diagnostic_item(*s, trait_id)) => { (trait_id, did) } @@ -73,13 +71,7 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall { _ => return, }; // (Re)check that it implements the noop diagnostic. - for (s, peel_ref) in [ - (sym::noop_method_borrow, true), - (sym::noop_method_clone, false), - (sym::noop_method_deref, true), - ] - .iter() - { + for (s, peel_ref) in [(sym::noop_method_clone, false)].iter() { if cx.tcx.is_diagnostic_item(*s, i.def_id()) { let method = &call.ident.name; let receiver = &elements[0]; diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 61f9a080a5217..f43b180e06321 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -142,7 +142,6 @@ symbols! { Decodable, Decoder, Default, - Deref, Encodable, Encoder, Eq, @@ -791,9 +790,7 @@ symbols! { none_error, nontemporal_store, nontrapping_dash_fptoint: "nontrapping-fptoint", - noop_method_borrow, noop_method_clone, - noop_method_deref, noreturn, nostack, not, diff --git a/library/core/src/borrow.rs b/library/core/src/borrow.rs index a0cdf681f67e1..c9040cd0a1670 100644 --- a/library/core/src/borrow.rs +++ b/library/core/src/borrow.rs @@ -153,7 +153,6 @@ /// [`HashMap`]: ../../std/collections/struct.HashMap.html /// [`String`]: ../../std/string/struct.String.html #[stable(feature = "rust1", since = "1.0.0")] -#[rustc_diagnostic_item = "Borrow"] pub trait Borrow { /// Immutably borrows from an owned value. /// @@ -220,7 +219,6 @@ impl BorrowMut for T { #[stable(feature = "rust1", since = "1.0.0")] impl Borrow for &T { - #[rustc_diagnostic_item = "noop_method_borrow"] fn borrow(&self) -> &T { &**self } diff --git a/library/core/src/ops/deref.rs b/library/core/src/ops/deref.rs index 10e3ce67448c8..2419771eae212 100644 --- a/library/core/src/ops/deref.rs +++ b/library/core/src/ops/deref.rs @@ -60,7 +60,6 @@ #[doc(alias = "*")] #[doc(alias = "&*")] #[stable(feature = "rust1", since = "1.0.0")] -#[rustc_diagnostic_item = "Deref"] pub trait Deref { /// The resulting type after dereferencing. #[stable(feature = "rust1", since = "1.0.0")] @@ -79,7 +78,6 @@ pub trait Deref { impl Deref for &T { type Target = T; - #[rustc_diagnostic_item = "noop_method_deref"] fn deref(&self) -> &T { *self } diff --git a/src/test/ui/lint/noop-method-call.rs b/src/test/ui/lint/noop-method-call.rs index 70363a191e9e9..8e4b5bf4d12c7 100644 --- a/src/test/ui/lint/noop-method-call.rs +++ b/src/test/ui/lint/noop-method-call.rs @@ -2,51 +2,33 @@ #![allow(unused)] -use std::borrow::Borrow; -use std::ops::Deref; - -struct Foo(T); +struct NonCloneType(T); #[derive(Clone)] -struct Bar(T); - -struct DerefExample(T); - -impl Deref for DerefExample { - type Target = T; - fn deref(&self) -> &Self::Target { - &self.0 - } -} +struct CloneType(T); fn main() { - let foo = &Foo(1u32); - let foo_clone: &Foo = foo.clone(); + let non_clone_type_ref = &NonCloneType(1u32); + let non_clone_type_ref_clone: &NonCloneType = non_clone_type_ref.clone(); //~^ WARNING call to `.clone()` on a reference in this situation does nothing - let bar = &Bar(1u32); - let bar_clone: Bar = bar.clone(); - - let deref = &&DerefExample(12u32); - let derefed: &DerefExample = deref.deref(); - //~^ WARNING call to `.deref()` on a reference in this situation does nothing - - let deref = &DerefExample(12u32); - let derefed: &u32 = deref.deref(); + let clone_type_ref = &CloneType(1u32); + let clone_type_ref_clone: CloneType = clone_type_ref.clone(); - let a = &&Foo(1u32); - let borrowed: &Foo = a.borrow(); - //~^ WARNING call to `.borrow()` on a reference in this situation does nothing + // Calling clone on a double reference doesn't warn since the method call itself + // peels the outer reference off + let clone_type_ref = &&CloneType(1u32); + let clone_type_ref_clone: &CloneType = clone_type_ref.clone(); let xs = ["a", "b", "c"]; let _v: Vec<&str> = xs.iter().map(|x| x.clone()).collect(); // ok, but could use `*x` instead } -fn generic(foo: &Foo) { - foo.clone(); +fn generic(non_clone_type: &NonCloneType) { + non_clone_type.clone(); } -fn non_generic(foo: &Foo) { - foo.clone(); +fn non_generic(non_clone_type: &NonCloneType) { + non_clone_type.clone(); //~^ WARNING call to `.clone()` on a reference in this situation does nothing } diff --git a/src/test/ui/lint/noop-method-call.stderr b/src/test/ui/lint/noop-method-call.stderr index 316c47975e6de..85a67f538407c 100644 --- a/src/test/ui/lint/noop-method-call.stderr +++ b/src/test/ui/lint/noop-method-call.stderr @@ -1,35 +1,19 @@ warning: call to `.clone()` on a reference in this situation does nothing - --> $DIR/noop-method-call.rs:24:35 + --> $DIR/noop-method-call.rs:12:74 | -LL | let foo_clone: &Foo = foo.clone(); - | ^^^^^^^^ unnecessary method call +LL | let non_clone_type_ref_clone: &NonCloneType = non_clone_type_ref.clone(); + | ^^^^^^^^ unnecessary method call | = note: `#[warn(noop_method_call)]` on by default - = note: the type `&Foo` which `clone` is being called on is the same as the type returned from `clone`, so the method call does not do anything and can be removed - -warning: call to `.deref()` on a reference in this situation does nothing - --> $DIR/noop-method-call.rs:31:44 - | -LL | let derefed: &DerefExample = deref.deref(); - | ^^^^^^^^ unnecessary method call - | - = note: the type `&DerefExample` which `deref` is being called on is the same as the type returned from `deref`, so the method call does not do anything and can be removed - -warning: call to `.borrow()` on a reference in this situation does nothing - --> $DIR/noop-method-call.rs:38:32 - | -LL | let borrowed: &Foo = a.borrow(); - | ^^^^^^^^^ unnecessary method call - | - = note: the type `&Foo` which `borrow` is being called on is the same as the type returned from `borrow`, so the method call does not do anything and can be removed + = note: the type `&NonCloneType` which `clone` is being called on is the same as the type returned from `clone`, so the method call does not do anything and can be removed warning: call to `.clone()` on a reference in this situation does nothing - --> $DIR/noop-method-call.rs:50:8 + --> $DIR/noop-method-call.rs:32:19 | -LL | foo.clone(); - | ^^^^^^^^ unnecessary method call +LL | non_clone_type.clone(); + | ^^^^^^^^ unnecessary method call | - = note: the type `&Foo` which `clone` is being called on is the same as the type returned from `clone`, so the method call does not do anything and can be removed + = note: the type `&NonCloneType` which `clone` is being called on is the same as the type returned from `clone`, so the method call does not do anything and can be removed -warning: 4 warnings emitted +warning: 2 warnings emitted From 1999a3147f5ab65cd556d45e631be5c18fbaebf4 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Tue, 16 Feb 2021 22:39:05 +0100 Subject: [PATCH 42/63] Fix borrow and deref --- compiler/rustc_lint/src/noop_method_call.rs | 18 ++++------ compiler/rustc_span/src/symbol.rs | 3 ++ library/core/src/borrow.rs | 2 ++ library/core/src/ops/deref.rs | 2 ++ library/core/tests/clone.rs | 2 -- src/test/ui/issues/issue-11820.rs | 2 -- src/test/ui/lint/noop-method-call.rs | 30 +++++++++++++--- src/test/ui/lint/noop-method-call.stderr | 36 ++++++++++++++----- src/test/ui/underscore-imports/cycle.rs | 1 - .../ui/underscore-imports/macro-expanded.rs | 1 - .../clippy/tests/ui/unnecessary_clone.rs | 2 +- 11 files changed, 67 insertions(+), 32 deletions(-) diff --git a/compiler/rustc_lint/src/noop_method_call.rs b/compiler/rustc_lint/src/noop_method_call.rs index 335c3c575e766..3a1b9c85fd1bd 100644 --- a/compiler/rustc_lint/src/noop_method_call.rs +++ b/compiler/rustc_lint/src/noop_method_call.rs @@ -15,6 +15,7 @@ declare_lint! { /// /// ```rust /// # #![allow(unused)] + /// #![deny(noop_method_call)] /// struct Foo; /// let foo = &Foo; /// let clone: &Foo = foo.clone(); @@ -30,7 +31,7 @@ declare_lint! { /// calling `clone` on a `&T` where `T` does not implement clone, actually doesn't do anything /// as references are copy. This lint detects these calls and warns the user about them. pub NOOP_METHOD_CALL, - Warn, + Allow, "detects the use of well-known noop methods" } @@ -50,7 +51,9 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall { Some((DefKind::AssocFn, did)) => match cx.tcx.trait_of_item(did) { // Check that we're dealing with a trait method for one of the traits we care about. Some(trait_id) - if [sym::Clone].iter().any(|s| cx.tcx.is_diagnostic_item(*s, trait_id)) => + if [sym::Clone, sym::Deref, sym::Borrow] + .iter() + .any(|s| cx.tcx.is_diagnostic_item(*s, trait_id)) => { (trait_id, did) } @@ -71,20 +74,11 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall { _ => return, }; // (Re)check that it implements the noop diagnostic. - for (s, peel_ref) in [(sym::noop_method_clone, false)].iter() { + for s in [sym::noop_method_clone, sym::noop_method_deref, sym::noop_method_borrow].iter() { if cx.tcx.is_diagnostic_item(*s, i.def_id()) { let method = &call.ident.name; let receiver = &elements[0]; let receiver_ty = cx.typeck_results().expr_ty(receiver); - let receiver_ty = match receiver_ty.kind() { - // Remove one borrow from the receiver if appropriate to positively verify that - // the receiver `&self` type and the return type are the same, depending on the - // involved trait being checked. - ty::Ref(_, ty, _) if *peel_ref => ty, - // When it comes to `Clone` we need to check the `receiver_ty` directly. - // FIXME: we must come up with a better strategy for this. - _ => receiver_ty, - }; let expr_ty = cx.typeck_results().expr_ty_adjusted(expr); if receiver_ty != expr_ty { // This lint will only trigger if the receiver type and resulting expression \ diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index f43b180e06321..61f9a080a5217 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -142,6 +142,7 @@ symbols! { Decodable, Decoder, Default, + Deref, Encodable, Encoder, Eq, @@ -790,7 +791,9 @@ symbols! { none_error, nontemporal_store, nontrapping_dash_fptoint: "nontrapping-fptoint", + noop_method_borrow, noop_method_clone, + noop_method_deref, noreturn, nostack, not, diff --git a/library/core/src/borrow.rs b/library/core/src/borrow.rs index c9040cd0a1670..f28be20aaa1e6 100644 --- a/library/core/src/borrow.rs +++ b/library/core/src/borrow.rs @@ -153,6 +153,7 @@ /// [`HashMap`]: ../../std/collections/struct.HashMap.html /// [`String`]: ../../std/string/struct.String.html #[stable(feature = "rust1", since = "1.0.0")] +#[rustc_diagnostic_item = "Borrow"] pub trait Borrow { /// Immutably borrows from an owned value. /// @@ -205,6 +206,7 @@ pub trait BorrowMut: Borrow { #[stable(feature = "rust1", since = "1.0.0")] impl Borrow for T { + #[rustc_diagnostic_item = "noop_method_borrow"] fn borrow(&self) -> &T { self } diff --git a/library/core/src/ops/deref.rs b/library/core/src/ops/deref.rs index 2419771eae212..10e3ce67448c8 100644 --- a/library/core/src/ops/deref.rs +++ b/library/core/src/ops/deref.rs @@ -60,6 +60,7 @@ #[doc(alias = "*")] #[doc(alias = "&*")] #[stable(feature = "rust1", since = "1.0.0")] +#[rustc_diagnostic_item = "Deref"] pub trait Deref { /// The resulting type after dereferencing. #[stable(feature = "rust1", since = "1.0.0")] @@ -78,6 +79,7 @@ pub trait Deref { impl Deref for &T { type Target = T; + #[rustc_diagnostic_item = "noop_method_deref"] fn deref(&self) -> &T { *self } diff --git a/library/core/tests/clone.rs b/library/core/tests/clone.rs index 2f2aa9a20f912..c97a87aebce41 100644 --- a/library/core/tests/clone.rs +++ b/library/core/tests/clone.rs @@ -1,5 +1,3 @@ -#![cfg_attr(not(bootstrap), allow(noop_method_call))] - #[test] fn test_borrowed_clone() { let x = 5; diff --git a/src/test/ui/issues/issue-11820.rs b/src/test/ui/issues/issue-11820.rs index dc6349b10ee58..7ffe9652797cf 100644 --- a/src/test/ui/issues/issue-11820.rs +++ b/src/test/ui/issues/issue-11820.rs @@ -1,8 +1,6 @@ // run-pass // pretty-expanded FIXME #23616 -#![allow(noop_method_call)] - struct NoClone; fn main() { diff --git a/src/test/ui/lint/noop-method-call.rs b/src/test/ui/lint/noop-method-call.rs index 8e4b5bf4d12c7..9870c813572e3 100644 --- a/src/test/ui/lint/noop-method-call.rs +++ b/src/test/ui/lint/noop-method-call.rs @@ -1,15 +1,19 @@ // check-pass #![allow(unused)] +#![warn(noop_method_call)] -struct NonCloneType(T); +use std::borrow::Borrow; +use std::ops::Deref; + +struct PlainType(T); #[derive(Clone)] struct CloneType(T); fn main() { - let non_clone_type_ref = &NonCloneType(1u32); - let non_clone_type_ref_clone: &NonCloneType = non_clone_type_ref.clone(); + let non_clone_type_ref = &PlainType(1u32); + let non_clone_type_ref_clone: &PlainType = non_clone_type_ref.clone(); //~^ WARNING call to `.clone()` on a reference in this situation does nothing let clone_type_ref = &CloneType(1u32); @@ -20,15 +24,31 @@ fn main() { let clone_type_ref = &&CloneType(1u32); let clone_type_ref_clone: &CloneType = clone_type_ref.clone(); + let non_deref_type = &PlainType(1u32); + let non_deref_type_deref: &PlainType = non_deref_type.deref(); + //~^ WARNING call to `.deref()` on a reference in this situation does nothing + + // Dereferencing a &&T does not warn since it has collapsed the double reference + let non_deref_type = &&PlainType(1u32); + let non_deref_type_deref: &PlainType = non_deref_type.deref(); + + let non_borrow_type = &PlainType(1u32); + let non_borrow_type_borrow: &PlainType = non_borrow_type.borrow(); + //~^ WARNING call to `.borrow()` on a reference in this situation does nothing + + // Borrowing a &&T does not warn since it has collapsed the double reference + let non_borrow_type = &&PlainType(1u32); + let non_borrow_type_borrow: &PlainType = non_borrow_type.borrow(); + let xs = ["a", "b", "c"]; let _v: Vec<&str> = xs.iter().map(|x| x.clone()).collect(); // ok, but could use `*x` instead } -fn generic(non_clone_type: &NonCloneType) { +fn generic(non_clone_type: &PlainType) { non_clone_type.clone(); } -fn non_generic(non_clone_type: &NonCloneType) { +fn non_generic(non_clone_type: &PlainType) { non_clone_type.clone(); //~^ WARNING call to `.clone()` on a reference in this situation does nothing } diff --git a/src/test/ui/lint/noop-method-call.stderr b/src/test/ui/lint/noop-method-call.stderr index 85a67f538407c..7f6f96bf1d142 100644 --- a/src/test/ui/lint/noop-method-call.stderr +++ b/src/test/ui/lint/noop-method-call.stderr @@ -1,19 +1,39 @@ warning: call to `.clone()` on a reference in this situation does nothing - --> $DIR/noop-method-call.rs:12:74 + --> $DIR/noop-method-call.rs:16:71 | -LL | let non_clone_type_ref_clone: &NonCloneType = non_clone_type_ref.clone(); - | ^^^^^^^^ unnecessary method call +LL | let non_clone_type_ref_clone: &PlainType = non_clone_type_ref.clone(); + | ^^^^^^^^ unnecessary method call | - = note: `#[warn(noop_method_call)]` on by default - = note: the type `&NonCloneType` which `clone` is being called on is the same as the type returned from `clone`, so the method call does not do anything and can be removed +note: the lint level is defined here + --> $DIR/noop-method-call.rs:4:9 + | +LL | #![warn(noop_method_call)] + | ^^^^^^^^^^^^^^^^ + = note: the type `&PlainType` which `clone` is being called on is the same as the type returned from `clone`, so the method call does not do anything and can be removed + +warning: call to `.deref()` on a reference in this situation does nothing + --> $DIR/noop-method-call.rs:28:63 + | +LL | let non_deref_type_deref: &PlainType = non_deref_type.deref(); + | ^^^^^^^^ unnecessary method call + | + = note: the type `&PlainType` which `deref` is being called on is the same as the type returned from `deref`, so the method call does not do anything and can be removed + +warning: call to `.borrow()` on a reference in this situation does nothing + --> $DIR/noop-method-call.rs:36:66 + | +LL | let non_borrow_type_borrow: &PlainType = non_borrow_type.borrow(); + | ^^^^^^^^^ unnecessary method call + | + = note: the type `&PlainType` which `borrow` is being called on is the same as the type returned from `borrow`, so the method call does not do anything and can be removed warning: call to `.clone()` on a reference in this situation does nothing - --> $DIR/noop-method-call.rs:32:19 + --> $DIR/noop-method-call.rs:52:19 | LL | non_clone_type.clone(); | ^^^^^^^^ unnecessary method call | - = note: the type `&NonCloneType` which `clone` is being called on is the same as the type returned from `clone`, so the method call does not do anything and can be removed + = note: the type `&PlainType` which `clone` is being called on is the same as the type returned from `clone`, so the method call does not do anything and can be removed -warning: 2 warnings emitted +warning: 4 warnings emitted diff --git a/src/test/ui/underscore-imports/cycle.rs b/src/test/ui/underscore-imports/cycle.rs index 987410fa84b20..bacf9b2d5a96a 100644 --- a/src/test/ui/underscore-imports/cycle.rs +++ b/src/test/ui/underscore-imports/cycle.rs @@ -14,6 +14,5 @@ mod y { pub fn main() { use x::*; - #[allow(noop_method_call)] (&0).deref(); } diff --git a/src/test/ui/underscore-imports/macro-expanded.rs b/src/test/ui/underscore-imports/macro-expanded.rs index 55e86e848558d..43f527bc9a408 100644 --- a/src/test/ui/underscore-imports/macro-expanded.rs +++ b/src/test/ui/underscore-imports/macro-expanded.rs @@ -3,7 +3,6 @@ // check-pass #![feature(decl_macro, rustc_attrs)] -#![allow(noop_method_call)] mod x { pub use std::ops::Not as _; diff --git a/src/tools/clippy/tests/ui/unnecessary_clone.rs b/src/tools/clippy/tests/ui/unnecessary_clone.rs index ce26634a995d9..6770a7fac90fd 100644 --- a/src/tools/clippy/tests/ui/unnecessary_clone.rs +++ b/src/tools/clippy/tests/ui/unnecessary_clone.rs @@ -1,7 +1,7 @@ // does not test any rustfixable lints #![warn(clippy::clone_on_ref_ptr)] -#![allow(unused, noop_method_call, clippy::redundant_clone, clippy::unnecessary_wraps)] +#![allow(unused, clippy::redundant_clone, clippy::unnecessary_wraps)] use std::cell::RefCell; use std::rc::{self, Rc}; From 25637b228d68e9bdf2d3ce1ba421cbd115fcb81e Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Wed, 17 Feb 2021 10:06:23 +0100 Subject: [PATCH 43/63] Warn in doc test --- compiler/rustc_lint/src/noop_method_call.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_lint/src/noop_method_call.rs b/compiler/rustc_lint/src/noop_method_call.rs index 3a1b9c85fd1bd..479cc00199f6a 100644 --- a/compiler/rustc_lint/src/noop_method_call.rs +++ b/compiler/rustc_lint/src/noop_method_call.rs @@ -15,7 +15,7 @@ declare_lint! { /// /// ```rust /// # #![allow(unused)] - /// #![deny(noop_method_call)] + /// #![warn(noop_method_call)] /// struct Foo; /// let foo = &Foo; /// let clone: &Foo = foo.clone(); From 9425e304b13e5cfc6f5d36f0a872fc8313da40aa Mon Sep 17 00:00:00 2001 From: Caleb Sander Date: Mon, 28 Dec 2020 15:45:36 -0500 Subject: [PATCH 44/63] Avoid unnecessary Vec construction in BufReader --- library/std/src/io/buffered/bufreader.rs | 7 +++---- library/std/src/lib.rs | 1 + 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/library/std/src/io/buffered/bufreader.rs b/library/std/src/io/buffered/bufreader.rs index 987371f50ec22..152bd34ffbaf2 100644 --- a/library/std/src/io/buffered/bufreader.rs +++ b/library/std/src/io/buffered/bufreader.rs @@ -90,10 +90,9 @@ impl BufReader { #[stable(feature = "rust1", since = "1.0.0")] pub fn with_capacity(capacity: usize, inner: R) -> BufReader { unsafe { - let mut buffer = Vec::with_capacity(capacity); - buffer.set_len(capacity); - inner.initializer().initialize(&mut buffer); - BufReader { inner, buf: buffer.into_boxed_slice(), pos: 0, cap: 0 } + let mut buf = Box::new_uninit_slice(capacity).assume_init(); + inner.initializer().initialize(&mut buf); + BufReader { inner, buf, pos: 0, cap: 0 } } } } diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index ba49dee38e642..f62fb83533ca2 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -289,6 +289,7 @@ #![feature(needs_panic_runtime)] #![feature(negative_impls)] #![feature(never_type)] +#![feature(new_uninit)] #![feature(nll)] #![feature(nonnull_slice_from_raw_parts)] #![feature(once_cell)] From 66a260617a88ed1ad55a46f03c5a90d5ad3004d3 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Fri, 19 Feb 2021 12:15:37 +0100 Subject: [PATCH 45/63] Increment self.len in specialized ZipImpl to avoid underflow in size_hint --- library/core/src/iter/adapters/zip.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/library/core/src/iter/adapters/zip.rs b/library/core/src/iter/adapters/zip.rs index 9d0f4e3618fc5..ce48016afcd50 100644 --- a/library/core/src/iter/adapters/zip.rs +++ b/library/core/src/iter/adapters/zip.rs @@ -200,6 +200,7 @@ where } else if A::MAY_HAVE_SIDE_EFFECT && self.index < self.a.size() { let i = self.index; self.index += 1; + self.len += 1; // match the base implementation's potential side effects // SAFETY: we just checked that `i` < `self.a.len()` unsafe { From 8b9ac4d4155c74db5b317046033ab9c05a09e351 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Fri, 19 Feb 2021 12:16:12 +0100 Subject: [PATCH 46/63] Add test for underflow in specialized Zip's size_hint --- library/core/tests/iter/adapters/zip.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/library/core/tests/iter/adapters/zip.rs b/library/core/tests/iter/adapters/zip.rs index 1fce0951e365e..a597710392952 100644 --- a/library/core/tests/iter/adapters/zip.rs +++ b/library/core/tests/iter/adapters/zip.rs @@ -245,3 +245,23 @@ fn test_double_ended_zip() { assert_eq!(it.next_back(), Some((3, 3))); assert_eq!(it.next(), None); } + +#[test] +fn test_issue_82282() { + fn overflowed_zip(arr: &[i32]) -> impl Iterator { + static UNIT_EMPTY_ARR: [(); 0] = []; + + let mapped = arr.into_iter().map(|i| *i); + let mut zipped = mapped.zip(UNIT_EMPTY_ARR.iter()); + zipped.next(); + zipped + } + + let arr = [1, 2, 3]; + let zip = overflowed_zip(&arr).zip(overflowed_zip(&arr)); + + assert_eq!(zip.size_hint(), (0, Some(0))); + for _ in zip { + panic!(); + } +} From aeb4ea739efb70e0002a4a9c4c7b8027dd0620b3 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Fri, 19 Feb 2021 12:17:48 +0100 Subject: [PATCH 47/63] Remove useless comparison since now self.index <= self.len is an invariant --- library/core/src/iter/adapters/zip.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/iter/adapters/zip.rs b/library/core/src/iter/adapters/zip.rs index ce48016afcd50..817fc2a51e981 100644 --- a/library/core/src/iter/adapters/zip.rs +++ b/library/core/src/iter/adapters/zip.rs @@ -259,7 +259,7 @@ where if sz_a != sz_b { let sz_a = self.a.size(); if A::MAY_HAVE_SIDE_EFFECT && sz_a > self.len { - for _ in 0..sz_a - cmp::max(self.len, self.index) { + for _ in 0..sz_a - self.len { self.a.next_back(); } } From 09cbcdc2c31325ec67047c5b9ce87dee03af62dc Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 4 Mar 2021 15:34:47 +0100 Subject: [PATCH 48/63] Add BTreeMap::try_insert and btree_map::OccupiedError. --- library/alloc/src/collections/btree/map.rs | 36 ++++++++++++++++++- .../alloc/src/collections/btree/map/entry.rs | 21 +++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/library/alloc/src/collections/btree/map.rs b/library/alloc/src/collections/btree/map.rs index 783f88f026b8f..12a7322d8e7ee 100644 --- a/library/alloc/src/collections/btree/map.rs +++ b/library/alloc/src/collections/btree/map.rs @@ -14,7 +14,7 @@ use super::node::{self, marker, ForceResult::*, Handle, NodeRef, Root}; use super::search::SearchResult::*; mod entry; -pub use entry::{Entry, OccupiedEntry, VacantEntry}; +pub use entry::{Entry, OccupiedEntry, OccupiedError, VacantEntry}; use Entry::*; /// Minimum number of elements in nodes that are not a root. @@ -836,6 +836,40 @@ impl BTreeMap { } } + /// Tries to insert a key-value pair into the map, and returns + /// a mutable reference to the value in the entry. + /// + /// If the map already had this key present, nothing is updated, and + /// an error containing the occupied entry and the value is returned. + /// + /// # Examples + /// + /// Basic usage: + /// + /// ``` + /// #![feature(map_try_insert)] + /// + /// use std::collections::BTreeMap; + /// + /// let mut map = BTreeMap::new(); + /// assert_eq!(map.try_insert(37, "a").unwrap(), &"a"); + /// + /// let err = map.try_insert(37, "b").unwrap_err(); + /// assert_eq!(err.entry.key(), &37); + /// assert_eq!(err.entry.get(), &"a"); + /// assert_eq!(err.value, "b"); + /// ``` + #[unstable(feature = "map_try_insert", issue = "none")] + pub fn try_insert(&mut self, key: K, value: V) -> Result<&mut V, OccupiedError<'_, K, V>> + where + K: Ord, + { + match self.entry(key) { + Occupied(entry) => Err(OccupiedError { entry, value }), + Vacant(entry) => Ok(entry.insert(value)), + } + } + /// Removes a key from the map, returning the value at the key if the key /// was previously in the map. /// diff --git a/library/alloc/src/collections/btree/map/entry.rs b/library/alloc/src/collections/btree/map/entry.rs index 941f82a8070a0..bd7114f8a82b7 100644 --- a/library/alloc/src/collections/btree/map/entry.rs +++ b/library/alloc/src/collections/btree/map/entry.rs @@ -71,6 +71,27 @@ impl Debug for OccupiedEntry<'_, K, V> { } } +/// The error returned by [`try_insert`](BTreeMap::try_insert) when the key already exists. +/// +/// Contains the occupied entry, and the value that was not inserted. +#[unstable(feature = "map_try_insert", issue = "none")] +pub struct OccupiedError<'a, K: 'a, V: 'a> { + /// The entry in the map that was already occupied. + pub entry: OccupiedEntry<'a, K, V>, + /// The value which was not inserted, because the entry was already occupied. + pub value: V, +} + +#[unstable(feature = "map_try_insert", issue = "none")] +impl Debug for OccupiedError<'_, K, V> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("OccupiedError") + .field("entry", &self.entry) + .field("value", &self.value) + .finish() + } +} + impl<'a, K: Ord, V> Entry<'a, K, V> { /// Ensures a value is in the entry by inserting the default if empty, and returns /// a mutable reference to the value in the entry. From f6fe24aab36814ee31ad9dd46fbefe1017670ece Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 4 Mar 2021 15:34:47 +0100 Subject: [PATCH 49/63] Add HashMap::try_insert and hash_map::OccupiedError. --- library/std/src/collections/hash/map.rs | 46 +++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/library/std/src/collections/hash/map.rs b/library/std/src/collections/hash/map.rs index 27f7191831d41..4e6aee98647ea 100644 --- a/library/std/src/collections/hash/map.rs +++ b/library/std/src/collections/hash/map.rs @@ -842,6 +842,40 @@ where self.base.insert(k, v) } + /// Tries to insert a key-value pair into the map, and returns + /// a mutable reference to the value in the entry. + /// + /// If the map already had this key present, nothing is updated, and + /// an error containing the occupied entry and the value is returned. + /// + /// # Examples + /// + /// Basic usage: + /// + /// ``` + /// #![feature(map_try_insert)] + /// + /// use std::collections::HashMap; + /// + /// let mut map = HashMap::new(); + /// assert_eq!(map.try_insert(37, "a").unwrap(), &"a"); + /// + /// let err = map.try_insert(37, "b").unwrap_err(); + /// assert_eq!(err.entry.key(), &37); + /// assert_eq!(err.entry.get(), &"a"); + /// assert_eq!(err.value, "b"); + /// ``` + #[unstable(feature = "map_try_insert", issue = "none")] + pub fn try_insert(&mut self, key: K, value: V) -> Result<&mut V, OccupiedError<'_, K, V>> + where + K: Ord, + { + match self.entry(key) { + Occupied(entry) => Err(OccupiedError { entry, value }), + Vacant(entry) => Ok(entry.insert(value)), + } + } + /// Removes a key from the map, returning the value at the key if the key /// was previously in the map. /// @@ -1851,6 +1885,18 @@ impl Debug for VacantEntry<'_, K, V> { } } +/// The error returned by [`try_insert`](HashMap::try_insert) when the key already exists. +/// +/// Contains the occupied entry, and the value that was not inserted. +#[unstable(feature = "map_try_insert", issue = "none")] +#[derive(Debug)] +pub struct OccupiedError<'a, K: 'a, V: 'a> { + /// The entry in the map that was already occupied. + pub entry: OccupiedEntry<'a, K, V>, + /// The value which was not inserted, because the entry was already occupied. + pub value: V, +} + #[stable(feature = "rust1", since = "1.0.0")] impl<'a, K, V, S> IntoIterator for &'a HashMap { type Item = (&'a K, &'a V); From 69d95e232af0fe81de85e1e4a1f8dc73d7b0f16c Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 4 Mar 2021 15:56:38 +0100 Subject: [PATCH 50/63] Improve Debug implementations of OccupiedError. --- library/alloc/src/collections/btree/map/entry.rs | 5 +++-- library/std/src/collections/hash/map.rs | 12 +++++++++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/library/alloc/src/collections/btree/map/entry.rs b/library/alloc/src/collections/btree/map/entry.rs index bd7114f8a82b7..cc508f30a614a 100644 --- a/library/alloc/src/collections/btree/map/entry.rs +++ b/library/alloc/src/collections/btree/map/entry.rs @@ -86,8 +86,9 @@ pub struct OccupiedError<'a, K: 'a, V: 'a> { impl Debug for OccupiedError<'_, K, V> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("OccupiedError") - .field("entry", &self.entry) - .field("value", &self.value) + .field("key", self.entry.key()) + .field("old_value", self.entry.get()) + .field("new_value", &self.value) .finish() } } diff --git a/library/std/src/collections/hash/map.rs b/library/std/src/collections/hash/map.rs index 4e6aee98647ea..7a50b49007cb0 100644 --- a/library/std/src/collections/hash/map.rs +++ b/library/std/src/collections/hash/map.rs @@ -1889,7 +1889,6 @@ impl Debug for VacantEntry<'_, K, V> { /// /// Contains the occupied entry, and the value that was not inserted. #[unstable(feature = "map_try_insert", issue = "none")] -#[derive(Debug)] pub struct OccupiedError<'a, K: 'a, V: 'a> { /// The entry in the map that was already occupied. pub entry: OccupiedEntry<'a, K, V>, @@ -1897,6 +1896,17 @@ pub struct OccupiedError<'a, K: 'a, V: 'a> { pub value: V, } +#[unstable(feature = "map_try_insert", issue = "none")] +impl Debug for OccupiedError<'_, K, V> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("OccupiedError") + .field("key", self.entry.key()) + .field("old_value", self.entry.get()) + .field("new_value", &self.value) + .finish() + } +} + #[stable(feature = "rust1", since = "1.0.0")] impl<'a, K, V, S> IntoIterator for &'a HashMap { type Item = (&'a K, &'a V); From d85d82ab2234cf08e2e86575d0cdebefdb819831 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 4 Mar 2021 15:57:26 +0100 Subject: [PATCH 51/63] Implement Error for OccupiedError. --- .../alloc/src/collections/btree/map/entry.rs | 13 +++++++++++++ library/std/src/collections/hash/map.rs | 13 +++++++++++++ library/std/src/error.rs | 18 ++++++++++++++++++ library/std/src/lib.rs | 1 + 4 files changed, 45 insertions(+) diff --git a/library/alloc/src/collections/btree/map/entry.rs b/library/alloc/src/collections/btree/map/entry.rs index cc508f30a614a..a5ece31d67b62 100644 --- a/library/alloc/src/collections/btree/map/entry.rs +++ b/library/alloc/src/collections/btree/map/entry.rs @@ -93,6 +93,19 @@ impl Debug for OccupiedError<'_, K, V> { } } +#[unstable(feature = "map_try_insert", issue = "none")] +impl<'a, K: Debug + Ord, V: Debug> fmt::Display for OccupiedError<'a, K, V> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "failed to insert {:?}, key {:?} already exists with value {:?}", + self.value, + self.entry.key(), + self.entry.get(), + ) + } +} + impl<'a, K: Ord, V> Entry<'a, K, V> { /// Ensures a value is in the entry by inserting the default if empty, and returns /// a mutable reference to the value in the entry. diff --git a/library/std/src/collections/hash/map.rs b/library/std/src/collections/hash/map.rs index 7a50b49007cb0..a4a7b2566547b 100644 --- a/library/std/src/collections/hash/map.rs +++ b/library/std/src/collections/hash/map.rs @@ -1907,6 +1907,19 @@ impl Debug for OccupiedError<'_, K, V> { } } +#[unstable(feature = "map_try_insert", issue = "none")] +impl<'a, K: Debug, V: Debug> fmt::Display for OccupiedError<'a, K, V> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "failed to insert {:?}, key {:?} already exists with value {:?}", + self.value, + self.entry.key(), + self.entry.get(), + ) + } +} + #[stable(feature = "rust1", since = "1.0.0")] impl<'a, K, V, S> IntoIterator for &'a HashMap { type Item = (&'a K, &'a V); diff --git a/library/std/src/error.rs b/library/std/src/error.rs index 94338c7b04d06..a7f744ce51533 100644 --- a/library/std/src/error.rs +++ b/library/std/src/error.rs @@ -470,6 +470,24 @@ impl Error for char::DecodeUtf16Error { } } +#[unstable(feature = "map_try_insert", issue = "none")] +impl<'a, K: Debug + Ord, V: Debug> Error + for crate::collections::btree_map::OccupiedError<'a, K, V> +{ + #[allow(deprecated)] + fn description(&self) -> &str { + "key already exists" + } +} + +#[unstable(feature = "map_try_insert", issue = "none")] +impl<'a, K: Debug, V: Debug> Error for crate::collections::hash_map::OccupiedError<'a, K, V> { + #[allow(deprecated)] + fn description(&self) -> &str { + "key already exists" + } +} + #[stable(feature = "box_error", since = "1.8.0")] impl Error for Box { #[allow(deprecated, deprecated_in_future)] diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index ba49dee38e642..7e86311bbb00c 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -281,6 +281,7 @@ #![feature(linkage)] #![feature(llvm_asm)] #![feature(log_syntax)] +#![feature(map_try_insert)] #![feature(maybe_uninit_extra)] #![feature(maybe_uninit_ref)] #![feature(maybe_uninit_slice)] From da01455813f8887d7c709f8c37bff9dcbbce34c3 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 4 Mar 2021 16:25:24 +0100 Subject: [PATCH 52/63] Ignore file length tidy warning in hash/map.rs. --- library/std/src/collections/hash/map.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/std/src/collections/hash/map.rs b/library/std/src/collections/hash/map.rs index a4a7b2566547b..24333fb5724cb 100644 --- a/library/std/src/collections/hash/map.rs +++ b/library/std/src/collections/hash/map.rs @@ -1,3 +1,5 @@ +// ignore-tidy-filelength + #[cfg(test)] mod tests; From 1aedb4c3a38b099a127c3fd3815400b3d0098475 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 4 Mar 2021 16:46:41 +0100 Subject: [PATCH 53/63] Remove unnecessary bound from HashMap::try_insert. --- library/std/src/collections/hash/map.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/library/std/src/collections/hash/map.rs b/library/std/src/collections/hash/map.rs index 24333fb5724cb..cd6787cf58812 100644 --- a/library/std/src/collections/hash/map.rs +++ b/library/std/src/collections/hash/map.rs @@ -868,10 +868,7 @@ where /// assert_eq!(err.value, "b"); /// ``` #[unstable(feature = "map_try_insert", issue = "none")] - pub fn try_insert(&mut self, key: K, value: V) -> Result<&mut V, OccupiedError<'_, K, V>> - where - K: Ord, - { + pub fn try_insert(&mut self, key: K, value: V) -> Result<&mut V, OccupiedError<'_, K, V>> { match self.entry(key) { Occupied(entry) => Err(OccupiedError { entry, value }), Vacant(entry) => Ok(entry.insert(value)), From eddd4f05012ed9ebefa6e2e5ca91da4116d30996 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 4 Mar 2021 16:54:28 +0100 Subject: [PATCH 54/63] Add tracking issue for map_try_insert. --- library/alloc/src/collections/btree/map.rs | 2 +- library/alloc/src/collections/btree/map/entry.rs | 6 +++--- library/std/src/collections/hash/map.rs | 8 ++++---- library/std/src/error.rs | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/library/alloc/src/collections/btree/map.rs b/library/alloc/src/collections/btree/map.rs index 12a7322d8e7ee..622983996aa08 100644 --- a/library/alloc/src/collections/btree/map.rs +++ b/library/alloc/src/collections/btree/map.rs @@ -859,7 +859,7 @@ impl BTreeMap { /// assert_eq!(err.entry.get(), &"a"); /// assert_eq!(err.value, "b"); /// ``` - #[unstable(feature = "map_try_insert", issue = "none")] + #[unstable(feature = "map_try_insert", issue = "82766")] pub fn try_insert(&mut self, key: K, value: V) -> Result<&mut V, OccupiedError<'_, K, V>> where K: Ord, diff --git a/library/alloc/src/collections/btree/map/entry.rs b/library/alloc/src/collections/btree/map/entry.rs index a5ece31d67b62..6b30d95977395 100644 --- a/library/alloc/src/collections/btree/map/entry.rs +++ b/library/alloc/src/collections/btree/map/entry.rs @@ -74,7 +74,7 @@ impl Debug for OccupiedEntry<'_, K, V> { /// The error returned by [`try_insert`](BTreeMap::try_insert) when the key already exists. /// /// Contains the occupied entry, and the value that was not inserted. -#[unstable(feature = "map_try_insert", issue = "none")] +#[unstable(feature = "map_try_insert", issue = "82766")] pub struct OccupiedError<'a, K: 'a, V: 'a> { /// The entry in the map that was already occupied. pub entry: OccupiedEntry<'a, K, V>, @@ -82,7 +82,7 @@ pub struct OccupiedError<'a, K: 'a, V: 'a> { pub value: V, } -#[unstable(feature = "map_try_insert", issue = "none")] +#[unstable(feature = "map_try_insert", issue = "82766")] impl Debug for OccupiedError<'_, K, V> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("OccupiedError") @@ -93,7 +93,7 @@ impl Debug for OccupiedError<'_, K, V> { } } -#[unstable(feature = "map_try_insert", issue = "none")] +#[unstable(feature = "map_try_insert", issue = "82766")] impl<'a, K: Debug + Ord, V: Debug> fmt::Display for OccupiedError<'a, K, V> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( diff --git a/library/std/src/collections/hash/map.rs b/library/std/src/collections/hash/map.rs index cd6787cf58812..233afa9238999 100644 --- a/library/std/src/collections/hash/map.rs +++ b/library/std/src/collections/hash/map.rs @@ -867,7 +867,7 @@ where /// assert_eq!(err.entry.get(), &"a"); /// assert_eq!(err.value, "b"); /// ``` - #[unstable(feature = "map_try_insert", issue = "none")] + #[unstable(feature = "map_try_insert", issue = "82766")] pub fn try_insert(&mut self, key: K, value: V) -> Result<&mut V, OccupiedError<'_, K, V>> { match self.entry(key) { Occupied(entry) => Err(OccupiedError { entry, value }), @@ -1887,7 +1887,7 @@ impl Debug for VacantEntry<'_, K, V> { /// The error returned by [`try_insert`](HashMap::try_insert) when the key already exists. /// /// Contains the occupied entry, and the value that was not inserted. -#[unstable(feature = "map_try_insert", issue = "none")] +#[unstable(feature = "map_try_insert", issue = "82766")] pub struct OccupiedError<'a, K: 'a, V: 'a> { /// The entry in the map that was already occupied. pub entry: OccupiedEntry<'a, K, V>, @@ -1895,7 +1895,7 @@ pub struct OccupiedError<'a, K: 'a, V: 'a> { pub value: V, } -#[unstable(feature = "map_try_insert", issue = "none")] +#[unstable(feature = "map_try_insert", issue = "82766")] impl Debug for OccupiedError<'_, K, V> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("OccupiedError") @@ -1906,7 +1906,7 @@ impl Debug for OccupiedError<'_, K, V> { } } -#[unstable(feature = "map_try_insert", issue = "none")] +#[unstable(feature = "map_try_insert", issue = "82766")] impl<'a, K: Debug, V: Debug> fmt::Display for OccupiedError<'a, K, V> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( diff --git a/library/std/src/error.rs b/library/std/src/error.rs index a7f744ce51533..80c35307d52ac 100644 --- a/library/std/src/error.rs +++ b/library/std/src/error.rs @@ -470,7 +470,7 @@ impl Error for char::DecodeUtf16Error { } } -#[unstable(feature = "map_try_insert", issue = "none")] +#[unstable(feature = "map_try_insert", issue = "82766")] impl<'a, K: Debug + Ord, V: Debug> Error for crate::collections::btree_map::OccupiedError<'a, K, V> { @@ -480,7 +480,7 @@ impl<'a, K: Debug + Ord, V: Debug> Error } } -#[unstable(feature = "map_try_insert", issue = "none")] +#[unstable(feature = "map_try_insert", issue = "82766")] impl<'a, K: Debug, V: Debug> Error for crate::collections::hash_map::OccupiedError<'a, K, V> { #[allow(deprecated)] fn description(&self) -> &str { From eb18746bc6c6c5c710ad674873438cbad5894f06 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 4 Mar 2021 17:55:23 +0100 Subject: [PATCH 55/63] Add assert_matches!(expr, pat). --- library/core/src/macros/mod.rs | 52 +++++++++++++++++++++++++ library/core/src/panicking.rs | 69 ++++++++++++++++++++++------------ library/std/src/lib.rs | 5 ++- 3 files changed, 101 insertions(+), 25 deletions(-) diff --git a/library/core/src/macros/mod.rs b/library/core/src/macros/mod.rs index 9a54921f07b49..3e31a9e8910e0 100644 --- a/library/core/src/macros/mod.rs +++ b/library/core/src/macros/mod.rs @@ -110,6 +110,58 @@ macro_rules! assert_ne { }); } +/// Asserts that an expression matches a pattern. +/// +/// On panic, this macro will print the value of the expression with its +/// debug representation. +/// +/// Like [`assert!`], this macro has a second form, where a custom +/// panic message can be provided. +/// +/// # Examples +/// +/// ``` +/// let a = 1u32.checked_add(2); +/// let b = 1u32.checked_sub(2); +/// assert_matches!(a, Some(_)); +/// assert_matches!(b, None); +/// ``` +#[macro_export] +#[unstable(feature = "assert_matches", issue = "none")] +#[allow_internal_unstable(core_panic)] +macro_rules! assert_matches { + ($left:expr, $right:pat $(,)?) => ({ + match &$left { + left_val => { + if let $right = left_val { + // OK + } else { + $crate::panicking::assert_matches_failed( + &*left_val, + $crate::stringify!($right), + $crate::option::Option::None + ); + } + } + } + }); + ($left:expr, $right:expr, $($arg:tt)+) => ({ + match &$left { + left_val => { + if let $right = left_val { + // OK + } else { + $crate::panicking::assert_matches_failed( + &*left_val, + $crate::stringify!($right), + $crate::option::Option::Some($crate::format_args!($($arg)+)) + ); + } + } + } + }); +} + /// Asserts that a boolean expression is `true` at runtime. /// /// This will invoke the [`panic!`] macro if the provided expression cannot be diff --git a/library/core/src/panicking.rs b/library/core/src/panicking.rs index af8a6101392a4..12acf5b4329db 100644 --- a/library/core/src/panicking.rs +++ b/library/core/src/panicking.rs @@ -97,6 +97,7 @@ pub fn panic_fmt(fmt: fmt::Arguments<'_>) -> ! { pub enum AssertKind { Eq, Ne, + Match, } /// Internal function for `assert_eq!` and `assert_ne!` macros @@ -113,32 +114,54 @@ where T: fmt::Debug + ?Sized, U: fmt::Debug + ?Sized, { - #[track_caller] - fn inner( - kind: AssertKind, - left: &dyn fmt::Debug, - right: &dyn fmt::Debug, - args: Option>, - ) -> ! { - let op = match kind { - AssertKind::Eq => "==", - AssertKind::Ne => "!=", - }; - - match args { - Some(args) => panic!( - r#"assertion failed: `(left {} right)` + assert_failed_inner(kind, &left, &right, args) +} + +/// Internal function for `assert_match!` +#[cold] +#[track_caller] +#[doc(hidden)] +pub fn assert_matches_failed( + left: &T, + right: &str, + args: Option>, +) -> ! { + // Use the Display implementation to display the pattern. + struct Pattern<'a>(&'a str); + impl fmt::Debug for Pattern<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(self.0, f) + } + } + assert_failed_inner(AssertKind::Match, &left, &Pattern(right), args); +} + +/// Non-generic version of the above functions, to avoid code bloat. +#[track_caller] +fn assert_failed_inner( + kind: AssertKind, + left: &dyn fmt::Debug, + right: &dyn fmt::Debug, + args: Option>, +) -> ! { + let op = match kind { + AssertKind::Eq => "==", + AssertKind::Ne => "!=", + AssertKind::Match => "matches", + }; + + match args { + Some(args) => panic!( + r#"assertion failed: `(left {} right)` left: `{:?}`, right: `{:?}: {}`"#, - op, left, right, args - ), - None => panic!( - r#"assertion failed: `(left {} right)` + op, left, right, args + ), + None => panic!( + r#"assertion failed: `(left {} right)` left: `{:?}`, right: `{:?}`"#, - op, left, right, - ), - } + op, left, right, + ), } - inner(kind, &left, &right, args) } diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index ba49dee38e642..ddceb492765da 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -228,6 +228,7 @@ #![feature(arbitrary_self_types)] #![feature(array_error_internals)] #![feature(asm)] +#![feature(assert_matches)] #![feature(associated_type_bounds)] #![feature(atomic_mut_ptr)] #![feature(box_syntax)] @@ -550,8 +551,8 @@ pub use std_detect::detect; #[stable(feature = "rust1", since = "1.0.0")] #[allow(deprecated, deprecated_in_future)] pub use core::{ - assert_eq, assert_ne, debug_assert, debug_assert_eq, debug_assert_ne, matches, r#try, todo, - unimplemented, unreachable, write, writeln, + assert_eq, assert_matches, assert_ne, debug_assert, debug_assert_eq, debug_assert_ne, matches, + r#try, todo, unimplemented, unreachable, write, writeln, }; // Re-export built-in macros defined through libcore. From cfce60ea3760acf8537d882fbae4fd1086e2b332 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 4 Mar 2021 18:07:26 +0100 Subject: [PATCH 56/63] Allow for multiple patterns and a guard in assert_matches. --- library/core/src/macros/mod.rs | 46 +++++++++++++++++----------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/library/core/src/macros/mod.rs b/library/core/src/macros/mod.rs index 3e31a9e8910e0..9bde2207fe194 100644 --- a/library/core/src/macros/mod.rs +++ b/library/core/src/macros/mod.rs @@ -110,7 +110,10 @@ macro_rules! assert_ne { }); } -/// Asserts that an expression matches a pattern. +/// Asserts that an expression matches any of the given patterns. +/// +/// Like in a `match` expression, the pattern can be optionally followed by `if` +/// and a guard expression that has access to names bound by the pattern. /// /// On panic, this macro will print the value of the expression with its /// debug representation. @@ -125,38 +128,35 @@ macro_rules! assert_ne { /// let b = 1u32.checked_sub(2); /// assert_matches!(a, Some(_)); /// assert_matches!(b, None); +/// +/// let c = Ok("abc".to_string()); +/// assert_matches!(a, Ok(x) | Err(x) if x.len() < 100); /// ``` #[macro_export] #[unstable(feature = "assert_matches", issue = "none")] #[allow_internal_unstable(core_panic)] macro_rules! assert_matches { - ($left:expr, $right:pat $(,)?) => ({ - match &$left { + ($left:expr, $( $pattern:pat )|+ $( if $guard: expr )? $(,)?) => ({ + match $left { + $( $pattern )|+ $( if $guard )? => {} left_val => { - if let $right = left_val { - // OK - } else { - $crate::panicking::assert_matches_failed( - &*left_val, - $crate::stringify!($right), - $crate::option::Option::None - ); - } + $crate::panicking::assert_matches_failed( + &left_val, + $crate::stringify!($($pattern)|+ $(if $guard)?), + $crate::option::Option::None + ); } } }); - ($left:expr, $right:expr, $($arg:tt)+) => ({ - match &$left { + ($left:expr, $( $pattern:pat )|+ $( if $guard: expr )?, $($arg:tt)+) => ({ + match $left { + $( $pattern )|+ $( if $guard )? => {} left_val => { - if let $right = left_val { - // OK - } else { - $crate::panicking::assert_matches_failed( - &*left_val, - $crate::stringify!($right), - $crate::option::Option::Some($crate::format_args!($($arg)+)) - ); - } + $crate::panicking::assert_matches_failed( + &left_val, + $crate::stringify!($($pattern)|+ $(if $guard)?), + $crate::option::Option::Some($crate::format_args!($($arg)+)) + ); } } }); From 0a8e401188062f0c60c989978352663b1e25e70e Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 4 Mar 2021 18:12:33 +0100 Subject: [PATCH 57/63] Add debug_assert_matches macro. --- library/core/src/macros/mod.rs | 34 ++++++++++++++++++++++++++++++++++ library/std/src/lib.rs | 4 ++-- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/library/core/src/macros/mod.rs b/library/core/src/macros/mod.rs index 9bde2207fe194..1d41d7f64bbfb 100644 --- a/library/core/src/macros/mod.rs +++ b/library/core/src/macros/mod.rs @@ -260,6 +260,40 @@ macro_rules! debug_assert_ne { ($($arg:tt)*) => (if $crate::cfg!(debug_assertions) { $crate::assert_ne!($($arg)*); }) } +/// Asserts that an expression matches any of the given patterns. +/// +/// Like in a `match` expression, the pattern can be optionally followed by `if` +/// and a guard expression that has access to names bound by the pattern. +/// +/// On panic, this macro will print the value of the expression with its +/// debug representation. +/// +/// Unlike [`assert_matches!`], `debug_assert_matches!` statements are only +/// enabled in non optimized builds by default. An optimized build will not +/// execute `debug_assert_matches!` statements unless `-C debug-assertions` is +/// passed to the compiler. This makes `debug_assert_matches!` useful for +/// checks that are too expensive to be present in a release build but may be +/// helpful during development. The result of expanding `debug_assert_matches!` +/// is always type checked. +/// +/// # Examples +/// +/// ``` +/// let a = 1u32.checked_add(2); +/// let b = 1u32.checked_sub(2); +/// debug_assert_matches!(a, Some(_)); +/// debug_assert_matches!(b, None); +/// +/// let c = Ok("abc".to_string()); +/// debug_assert_matches!(a, Ok(x) | Err(x) if x.len() < 100); +/// ``` +#[macro_export] +#[unstable(feature = "assert_matches", issue = "none")] +#[allow_internal_unstable(assert_matches)] +macro_rules! debug_assert_matches { + ($($arg:tt)*) => (if $crate::cfg!(debug_assertions) { $crate::assert_matches!($($arg)*); }) +} + /// Returns whether the given expression matches any of the given patterns. /// /// Like in a `match` expression, the pattern can be optionally followed by `if` diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index ddceb492765da..467f69d45b5d1 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -551,8 +551,8 @@ pub use std_detect::detect; #[stable(feature = "rust1", since = "1.0.0")] #[allow(deprecated, deprecated_in_future)] pub use core::{ - assert_eq, assert_matches, assert_ne, debug_assert, debug_assert_eq, debug_assert_ne, matches, - r#try, todo, unimplemented, unreachable, write, writeln, + assert_eq, assert_matches, assert_ne, debug_assert, debug_assert_eq, debug_assert_matches, + debug_assert_ne, matches, r#try, todo, unimplemented, unreachable, write, writeln, }; // Re-export built-in macros defined through libcore. From 5bd1204fc2a3147d425bc98fc0fefb00c6b4702d Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 4 Mar 2021 18:41:43 +0100 Subject: [PATCH 58/63] Fix assert_matches doc examples. --- library/core/src/macros/mod.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/library/core/src/macros/mod.rs b/library/core/src/macros/mod.rs index 1d41d7f64bbfb..b0b35c6915f99 100644 --- a/library/core/src/macros/mod.rs +++ b/library/core/src/macros/mod.rs @@ -124,13 +124,15 @@ macro_rules! assert_ne { /// # Examples /// /// ``` +/// #![feature(assert_matches)] +/// /// let a = 1u32.checked_add(2); /// let b = 1u32.checked_sub(2); /// assert_matches!(a, Some(_)); /// assert_matches!(b, None); /// /// let c = Ok("abc".to_string()); -/// assert_matches!(a, Ok(x) | Err(x) if x.len() < 100); +/// assert_matches!(c, Ok(x) | Err(x) if x.len() < 100); /// ``` #[macro_export] #[unstable(feature = "assert_matches", issue = "none")] @@ -279,13 +281,15 @@ macro_rules! debug_assert_ne { /// # Examples /// /// ``` +/// #![feature(assert_matches)] +/// /// let a = 1u32.checked_add(2); /// let b = 1u32.checked_sub(2); /// debug_assert_matches!(a, Some(_)); /// debug_assert_matches!(b, None); /// /// let c = Ok("abc".to_string()); -/// debug_assert_matches!(a, Ok(x) | Err(x) if x.len() < 100); +/// debug_assert_matches!(c, Ok(x) | Err(x) if x.len() < 100); /// ``` #[macro_export] #[unstable(feature = "assert_matches", issue = "none")] From a5951d4b238947479b4851eb491f8cc76bf103ee Mon Sep 17 00:00:00 2001 From: Mateusz Gacek <96mateusz.gacek@gmail.com> Date: Thu, 4 Mar 2021 10:09:02 -0800 Subject: [PATCH 59/63] Add diagnostic item to `Default` trait Required to resolve #6562 rust-clippy issue. --- library/core/src/default.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/library/core/src/default.rs b/library/core/src/default.rs index 28ec3279459ab..fd7159d35fa7f 100644 --- a/library/core/src/default.rs +++ b/library/core/src/default.rs @@ -80,6 +80,7 @@ /// bar: f32, /// } /// ``` +#[cfg_attr(not(test), rustc_diagnostic_item = "Default")] #[stable(feature = "rust1", since = "1.0.0")] pub trait Default: Sized { /// Returns the "default value" for a type. From 58d6f80f96a7e53fcafaa9ca42a7a8d6f8fa7444 Mon Sep 17 00:00:00 2001 From: Mateusz Gacek <96mateusz.gacek@gmail.com> Date: Thu, 4 Mar 2021 10:13:53 -0800 Subject: [PATCH 60/63] Fix comment with path to `symbols!` macro --- compiler/rustc_span/src/symbol.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 27bb45bcc8512..63ecc4b36d3ea 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -18,7 +18,7 @@ use crate::{Edition, Span, DUMMY_SP, SESSION_GLOBALS}; #[cfg(test)] mod tests; -// The proc macro code for this is in `src/librustc_macros/src/symbols.rs`. +// The proc macro code for this is in `compiler/rustc_macros/src/symbols.rs`. symbols! { // After modifying this list adjust `is_special`, `is_used_keyword`/`is_unused_keyword`, // this should be rarely necessary though if the keywords are kept in alphabetic order. From f223affd7ae383816a038700d591d78bebd98d46 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 4 Mar 2021 19:36:36 +0100 Subject: [PATCH 61/63] Don't consume the expression in assert_matches!()'s failure case. --- library/core/src/macros/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/core/src/macros/mod.rs b/library/core/src/macros/mod.rs index b0b35c6915f99..fb43433a79465 100644 --- a/library/core/src/macros/mod.rs +++ b/library/core/src/macros/mod.rs @@ -141,9 +141,9 @@ macro_rules! assert_matches { ($left:expr, $( $pattern:pat )|+ $( if $guard: expr )? $(,)?) => ({ match $left { $( $pattern )|+ $( if $guard )? => {} - left_val => { + ref left_val => { $crate::panicking::assert_matches_failed( - &left_val, + left_val, $crate::stringify!($($pattern)|+ $(if $guard)?), $crate::option::Option::None ); @@ -153,9 +153,9 @@ macro_rules! assert_matches { ($left:expr, $( $pattern:pat )|+ $( if $guard: expr )?, $($arg:tt)+) => ({ match $left { $( $pattern )|+ $( if $guard )? => {} - left_val => { + ref left_val => { $crate::panicking::assert_matches_failed( - &left_val, + left_val, $crate::stringify!($($pattern)|+ $(if $guard)?), $crate::option::Option::Some($crate::format_args!($($arg)+)) ); From 80fcdef3b53c43f3d7bace1db3e3ef9ffebd757e Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 4 Mar 2021 21:33:31 +0100 Subject: [PATCH 62/63] Add tracking issue for assert_matches. --- library/core/src/macros/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/core/src/macros/mod.rs b/library/core/src/macros/mod.rs index fb43433a79465..3e70ba81d4997 100644 --- a/library/core/src/macros/mod.rs +++ b/library/core/src/macros/mod.rs @@ -135,7 +135,7 @@ macro_rules! assert_ne { /// assert_matches!(c, Ok(x) | Err(x) if x.len() < 100); /// ``` #[macro_export] -#[unstable(feature = "assert_matches", issue = "none")] +#[unstable(feature = "assert_matches", issue = "82775")] #[allow_internal_unstable(core_panic)] macro_rules! assert_matches { ($left:expr, $( $pattern:pat )|+ $( if $guard: expr )? $(,)?) => ({ @@ -292,7 +292,7 @@ macro_rules! debug_assert_ne { /// debug_assert_matches!(c, Ok(x) | Err(x) if x.len() < 100); /// ``` #[macro_export] -#[unstable(feature = "assert_matches", issue = "none")] +#[unstable(feature = "assert_matches", issue = "82775")] #[allow_internal_unstable(assert_matches)] macro_rules! debug_assert_matches { ($($arg:tt)*) => (if $crate::cfg!(debug_assertions) { $crate::assert_matches!($($arg)*); }) From ad9191545573a2d174ecb325b4f06a808375c56f Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Thu, 4 Mar 2021 17:06:06 -0700 Subject: [PATCH 63/63] Remove unused code from main.js It looks like `lev_distance` was used in a very old version of the function, since it was written but never read, and Blame reports that it was added before the `checkGenerics` function header itself. `convertHTMLToPlaintext` is was removed by 768d5e950953738a54480e530341964838d29da2 --- src/librustdoc/html/static/main.js | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/src/librustdoc/html/static/main.js b/src/librustdoc/html/static/main.js index 89b1362b32b63..c9b8100fd0378 100644 --- a/src/librustdoc/html/static/main.js +++ b/src/librustdoc/html/static/main.js @@ -843,7 +843,6 @@ function defocusSearchBar() { function checkGenerics(obj, val) { // The names match, but we need to be sure that all generics kinda // match as well. - var lev_distance = MAX_LEV_DISTANCE + 1; if (val.generics.length > 0) { if (obj.length > GENERICS_DATA && obj[GENERICS_DATA].length >= val.generics.length) { @@ -866,7 +865,6 @@ function defocusSearchBar() { } if (lev.pos !== -1) { elems.splice(lev.pos, 1); - lev_distance = Math.min(lev.lev, lev_distance); total += lev.lev; done += 1; } else { @@ -2054,24 +2052,6 @@ function defocusSearchBar() { } } - /** - * Convert HTML to plaintext: - * - * * Replace "foo" with "`foo`" - * * Strip all other HTML tags - * - * Used by the dynamic sidebar crate list renderer. - * - * @param {[string]} html [The HTML to convert] - * @return {[string]} [The resulting plaintext] - */ - function convertHTMLToPlaintext(html) { - var x = document.createElement("div"); - x.innerHTML = html.replace('', '`').replace('', '`'); - return x.innerText; - } - - // delayed sidebar rendering. window.initSidebarItems = function(items) { var sidebar = document.getElementsByClassName("sidebar-elems")[0];