From 518f1ccb728aa03665e51710c12973a74cc98df5 Mon Sep 17 00:00:00 2001 From: CDirkx Date: Mon, 31 Aug 2020 02:43:17 +0200 Subject: [PATCH 1/5] Stabilize some Result methods as const Stabilize the following methods of `Result` as const: - `is_ok` - `is_err` - `as_ref` Possible because of stabilization of #49146 (Allow if and match in constants). --- library/core/src/lib.rs | 1 - library/core/src/result.rs | 6 +++--- src/test/ui/consts/const-result.rs | 12 ++++++++++++ 3 files changed, 15 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/consts/const-result.rs diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs index aef82a5aec5d0..290540829e03c 100644 --- a/library/core/src/lib.rs +++ b/library/core/src/lib.rs @@ -87,7 +87,6 @@ #![feature(const_ptr_offset)] #![feature(const_ptr_offset_from)] #![feature(const_raw_ptr_comparison)] -#![feature(const_result)] #![feature(const_slice_from_raw_parts)] #![feature(const_slice_ptr_len)] #![feature(const_size_of_val)] diff --git a/library/core/src/result.rs b/library/core/src/result.rs index ce0fc628e1114..5cec183c237d7 100644 --- a/library/core/src/result.rs +++ b/library/core/src/result.rs @@ -273,7 +273,7 @@ impl Result { /// assert_eq!(x.is_ok(), false); /// ``` #[must_use = "if you intended to assert that this is ok, consider `.unwrap()` instead"] - #[rustc_const_unstable(feature = "const_result", issue = "67520")] + #[rustc_const_stable(feature = "const_result", since = "1.48.0")] #[inline] #[stable(feature = "rust1", since = "1.0.0")] pub const fn is_ok(&self) -> bool { @@ -294,7 +294,7 @@ impl Result { /// assert_eq!(x.is_err(), true); /// ``` #[must_use = "if you intended to assert that this is err, consider `.unwrap_err()` instead"] - #[rustc_const_unstable(feature = "const_result", issue = "67520")] + #[rustc_const_stable(feature = "const_result", since = "1.48.0")] #[inline] #[stable(feature = "rust1", since = "1.0.0")] pub const fn is_err(&self) -> bool { @@ -438,7 +438,7 @@ impl Result { /// assert_eq!(x.as_ref(), Err(&"Error")); /// ``` #[inline] - #[rustc_const_unstable(feature = "const_result", issue = "67520")] + #[rustc_const_stable(feature = "const_result", since = "1.48.0")] #[stable(feature = "rust1", since = "1.0.0")] pub const fn as_ref(&self) -> Result<&T, &E> { match *self { diff --git a/src/test/ui/consts/const-result.rs b/src/test/ui/consts/const-result.rs new file mode 100644 index 0000000000000..e548d3e79ff80 --- /dev/null +++ b/src/test/ui/consts/const-result.rs @@ -0,0 +1,12 @@ +// run-pass + +fn main() { + const X: Result = Ok(32); + const Y: Result<&i32, &bool> = X.as_ref(); + + const IS_OK: bool = X.is_ok(); + assert!(IS_OK); + + const IS_ERR: bool = Y.is_err(); + assert!(!IS_ERR) +} From 787b2707a77fa1a98f512a4905662a1a16daf13c Mon Sep 17 00:00:00 2001 From: Christiaan Dirkx Date: Fri, 4 Sep 2020 00:24:34 +0200 Subject: [PATCH 2/5] Move const tests for `Result` to `library\core` Part of #76268 --- library/core/tests/result.rs | 16 ++++++++++++++++ src/test/ui/consts/const-result.rs | 12 ------------ 2 files changed, 16 insertions(+), 12 deletions(-) delete mode 100644 src/test/ui/consts/const-result.rs diff --git a/library/core/tests/result.rs b/library/core/tests/result.rs index caa2d916cd7a8..d3c650669dab3 100644 --- a/library/core/tests/result.rs +++ b/library/core/tests/result.rs @@ -305,3 +305,19 @@ fn test_result_as_deref_mut() { let expected_result = Result::Err::<&mut u32, &mut Vec>(&mut expected_vec); assert_eq!(mut_err.as_deref_mut(), expected_result); } + +#[test] +fn result_const() { + // test that the methods of `Result` are usable in a const context + + const RESULT: Result = Ok(32); + + const REF: Result<&usize, &bool> = RESULT.as_ref(); + assert_eq!(REF, Ok(&32)); + + const IS_OK: bool = RESULT.is_ok(); + assert!(IS_OK); + + const IS_ERR: bool = RESULT.is_err(); + assert!(!IS_ERR) +} diff --git a/src/test/ui/consts/const-result.rs b/src/test/ui/consts/const-result.rs deleted file mode 100644 index e548d3e79ff80..0000000000000 --- a/src/test/ui/consts/const-result.rs +++ /dev/null @@ -1,12 +0,0 @@ -// run-pass - -fn main() { - const X: Result = Ok(32); - const Y: Result<&i32, &bool> = X.as_ref(); - - const IS_OK: bool = X.is_ok(); - assert!(IS_OK); - - const IS_ERR: bool = Y.is_err(); - assert!(!IS_ERR) -} From ab4fa215b91491be0a7d2c3630efce7a6edb971d Mon Sep 17 00:00:00 2001 From: Christiaan Dirkx Date: Sun, 20 Sep 2020 03:32:36 +0200 Subject: [PATCH 3/5] Update Clippy testcases Update the test `redundant_pattern_matching`: check if `is_ok` and `is_err` are suggested within const contexts. Also removes the `redundant_pattern_matching_const_result` test, as it is no longer needed. --- .../tests/ui/redundant_pattern_matching.fixed | 35 +++++------ .../tests/ui/redundant_pattern_matching.rs | 41 +++++++------ .../ui/redundant_pattern_matching.stderr | 60 ++++++++++++++++--- ...undant_pattern_matching_const_result.fixed | 44 -------------- ...redundant_pattern_matching_const_result.rs | 50 ---------------- ...ndant_pattern_matching_const_result.stderr | 46 -------------- 6 files changed, 93 insertions(+), 183 deletions(-) delete mode 100644 src/tools/clippy/tests/ui/redundant_pattern_matching_const_result.fixed delete mode 100644 src/tools/clippy/tests/ui/redundant_pattern_matching_const_result.rs delete mode 100644 src/tools/clippy/tests/ui/redundant_pattern_matching_const_result.stderr diff --git a/src/tools/clippy/tests/ui/redundant_pattern_matching.fixed b/src/tools/clippy/tests/ui/redundant_pattern_matching.fixed index adbff8af8d9ca..08bfe7c78d38b 100644 --- a/src/tools/clippy/tests/ui/redundant_pattern_matching.fixed +++ b/src/tools/clippy/tests/ui/redundant_pattern_matching.fixed @@ -77,6 +77,7 @@ fn main() { issue5504(); issue5697(); + issue6067(); let _ = if gen_opt().is_some() { 1 @@ -131,31 +132,14 @@ fn issue5504() { // None of these should be linted because none of the suggested methods // are `const fn` without toggling a feature. const fn issue5697() { - if let Ok(_) = Ok::(42) {} - - if let Err(_) = Err::(42) {} - if let Some(_) = Some(42) {} if let None = None::<()> {} - while let Ok(_) = Ok::(10) {} - - while let Err(_) = Ok::(10) {} - while let Some(_) = Some(42) {} while let None = None::<()> {} - match Ok::(42) { - Ok(_) => true, - Err(_) => false, - }; - - match Err::(42) { - Ok(_) => false, - Err(_) => true, - }; match Some(42) { Some(_) => true, None => false, @@ -166,3 +150,20 @@ const fn issue5697() { None => true, }; } + +// Methods that are unstable const should not be suggested within a const context, see issue #5697. +// However, in Rust 1.48.0 the methods `is_ok` and `is_err` of `Result` were stabilized as const, +// so the following should be linted. +const fn issue6067() { + if Ok::(42).is_ok() {} + + if Err::(42).is_err() {} + + while Ok::(10).is_ok() {} + + while Ok::(10).is_err() {} + + Ok::(42).is_ok(); + + Err::(42).is_err(); +} diff --git a/src/tools/clippy/tests/ui/redundant_pattern_matching.rs b/src/tools/clippy/tests/ui/redundant_pattern_matching.rs index 4c2870e7803cb..c0660c6ac3947 100644 --- a/src/tools/clippy/tests/ui/redundant_pattern_matching.rs +++ b/src/tools/clippy/tests/ui/redundant_pattern_matching.rs @@ -98,6 +98,7 @@ fn main() { issue5504(); issue5697(); + issue6067(); let _ = if let Some(_) = gen_opt() { 1 @@ -152,31 +153,14 @@ fn issue5504() { // None of these should be linted because none of the suggested methods // are `const fn` without toggling a feature. const fn issue5697() { - if let Ok(_) = Ok::(42) {} - - if let Err(_) = Err::(42) {} - if let Some(_) = Some(42) {} if let None = None::<()> {} - while let Ok(_) = Ok::(10) {} - - while let Err(_) = Ok::(10) {} - while let Some(_) = Some(42) {} while let None = None::<()> {} - match Ok::(42) { - Ok(_) => true, - Err(_) => false, - }; - - match Err::(42) { - Ok(_) => false, - Err(_) => true, - }; match Some(42) { Some(_) => true, None => false, @@ -187,3 +171,26 @@ const fn issue5697() { None => true, }; } + +// Methods that are unstable const should not be suggested within a const context, see issue #5697. +// However, in Rust 1.48.0 the methods `is_ok` and `is_err` of `Result` were stabilized as const, +// so the following should be linted. +const fn issue6067() { + if let Ok(_) = Ok::(42) {} + + if let Err(_) = Err::(42) {} + + while let Ok(_) = Ok::(10) {} + + while let Err(_) = Ok::(10) {} + + match Ok::(42) { + Ok(_) => true, + Err(_) => false, + }; + + match Err::(42) { + Ok(_) => false, + Err(_) => true, + }; +} diff --git a/src/tools/clippy/tests/ui/redundant_pattern_matching.stderr b/src/tools/clippy/tests/ui/redundant_pattern_matching.stderr index d3c9ceaa3d7c1..efd2f9903ec9c 100644 --- a/src/tools/clippy/tests/ui/redundant_pattern_matching.stderr +++ b/src/tools/clippy/tests/ui/redundant_pattern_matching.stderr @@ -149,52 +149,94 @@ LL | let x = if let Some(_) = opt { true } else { false }; | -------^^^^^^^------ help: try this: `if opt.is_some()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:102:20 + --> $DIR/redundant_pattern_matching.rs:103:20 | LL | let _ = if let Some(_) = gen_opt() { | -------^^^^^^^------------ help: try this: `if gen_opt().is_some()` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching.rs:104:19 + --> $DIR/redundant_pattern_matching.rs:105:19 | LL | } else if let None = gen_opt() { | -------^^^^------------ help: try this: `if gen_opt().is_none()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:106:19 + --> $DIR/redundant_pattern_matching.rs:107:19 | LL | } else if let Ok(_) = gen_res() { | -------^^^^^------------ help: try this: `if gen_res().is_ok()` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching.rs:108:19 + --> $DIR/redundant_pattern_matching.rs:109:19 | LL | } else if let Err(_) = gen_res() { | -------^^^^^^------------ help: try this: `if gen_res().is_err()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:141:19 + --> $DIR/redundant_pattern_matching.rs:142:19 | LL | while let Some(_) = r#try!(result_opt()) {} | ----------^^^^^^^----------------------- help: try this: `while r#try!(result_opt()).is_some()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:142:16 + --> $DIR/redundant_pattern_matching.rs:143:16 | LL | if let Some(_) = r#try!(result_opt()) {} | -------^^^^^^^----------------------- help: try this: `if r#try!(result_opt()).is_some()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:148:12 + --> $DIR/redundant_pattern_matching.rs:149:12 | LL | if let Some(_) = m!() {} | -------^^^^^^^------- help: try this: `if m!().is_some()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:149:15 + --> $DIR/redundant_pattern_matching.rs:150:15 | LL | while let Some(_) = m!() {} | ----------^^^^^^^------- help: try this: `while m!().is_some()` -error: aborting due to 29 previous errors +error: redundant pattern matching, consider using `is_ok()` + --> $DIR/redundant_pattern_matching.rs:179:12 + | +LL | if let Ok(_) = Ok::(42) {} + | -------^^^^^--------------------- help: try this: `if Ok::(42).is_ok()` + +error: redundant pattern matching, consider using `is_err()` + --> $DIR/redundant_pattern_matching.rs:181:12 + | +LL | if let Err(_) = Err::(42) {} + | -------^^^^^^---------------------- help: try this: `if Err::(42).is_err()` + +error: redundant pattern matching, consider using `is_ok()` + --> $DIR/redundant_pattern_matching.rs:183:15 + | +LL | while let Ok(_) = Ok::(10) {} + | ----------^^^^^--------------------- help: try this: `while Ok::(10).is_ok()` + +error: redundant pattern matching, consider using `is_err()` + --> $DIR/redundant_pattern_matching.rs:185:15 + | +LL | while let Err(_) = Ok::(10) {} + | ----------^^^^^^--------------------- help: try this: `while Ok::(10).is_err()` + +error: redundant pattern matching, consider using `is_ok()` + --> $DIR/redundant_pattern_matching.rs:187:5 + | +LL | / match Ok::(42) { +LL | | Ok(_) => true, +LL | | Err(_) => false, +LL | | }; + | |_____^ help: try this: `Ok::(42).is_ok()` + +error: redundant pattern matching, consider using `is_err()` + --> $DIR/redundant_pattern_matching.rs:192:5 + | +LL | / match Err::(42) { +LL | | Ok(_) => false, +LL | | Err(_) => true, +LL | | }; + | |_____^ help: try this: `Err::(42).is_err()` + +error: aborting due to 35 previous errors diff --git a/src/tools/clippy/tests/ui/redundant_pattern_matching_const_result.fixed b/src/tools/clippy/tests/ui/redundant_pattern_matching_const_result.fixed deleted file mode 100644 index de3fe00d5fa68..0000000000000 --- a/src/tools/clippy/tests/ui/redundant_pattern_matching_const_result.fixed +++ /dev/null @@ -1,44 +0,0 @@ -// run-rustfix - -#![feature(const_result)] -#![warn(clippy::redundant_pattern_matching)] -#![allow(clippy::match_like_matches_macro, unused)] - -// Test that results are linted with the feature enabled. - -const fn issue_5697() { - if Ok::(42).is_ok() {} - - if Err::(42).is_err() {} - - while Ok::(10).is_ok() {} - - while Ok::(10).is_err() {} - - Ok::(42).is_ok(); - - Err::(42).is_err(); - - // These should not be linted until `const_option` is implemented. - // See https://github.com/rust-lang/rust/issues/67441 - - if let Some(_) = Some(42) {} - - if let None = None::<()> {} - - while let Some(_) = Some(42) {} - - while let None = None::<()> {} - - match Some(42) { - Some(_) => true, - None => false, - }; - - match None::<()> { - Some(_) => false, - None => true, - }; -} - -fn main() {} diff --git a/src/tools/clippy/tests/ui/redundant_pattern_matching_const_result.rs b/src/tools/clippy/tests/ui/redundant_pattern_matching_const_result.rs deleted file mode 100644 index b77969d53d92d..0000000000000 --- a/src/tools/clippy/tests/ui/redundant_pattern_matching_const_result.rs +++ /dev/null @@ -1,50 +0,0 @@ -// run-rustfix - -#![feature(const_result)] -#![warn(clippy::redundant_pattern_matching)] -#![allow(clippy::match_like_matches_macro, unused)] - -// Test that results are linted with the feature enabled. - -const fn issue_5697() { - if let Ok(_) = Ok::(42) {} - - if let Err(_) = Err::(42) {} - - while let Ok(_) = Ok::(10) {} - - while let Err(_) = Ok::(10) {} - - match Ok::(42) { - Ok(_) => true, - Err(_) => false, - }; - - match Err::(42) { - Ok(_) => false, - Err(_) => true, - }; - - // These should not be linted until `const_option` is implemented. - // See https://github.com/rust-lang/rust/issues/67441 - - if let Some(_) = Some(42) {} - - if let None = None::<()> {} - - while let Some(_) = Some(42) {} - - while let None = None::<()> {} - - match Some(42) { - Some(_) => true, - None => false, - }; - - match None::<()> { - Some(_) => false, - None => true, - }; -} - -fn main() {} diff --git a/src/tools/clippy/tests/ui/redundant_pattern_matching_const_result.stderr b/src/tools/clippy/tests/ui/redundant_pattern_matching_const_result.stderr deleted file mode 100644 index 8ecd72158d33c..0000000000000 --- a/src/tools/clippy/tests/ui/redundant_pattern_matching_const_result.stderr +++ /dev/null @@ -1,46 +0,0 @@ -error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching_const_result.rs:10:12 - | -LL | if let Ok(_) = Ok::(42) {} - | -------^^^^^--------------------- help: try this: `if Ok::(42).is_ok()` - | - = note: `-D clippy::redundant-pattern-matching` implied by `-D warnings` - -error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching_const_result.rs:12:12 - | -LL | if let Err(_) = Err::(42) {} - | -------^^^^^^---------------------- help: try this: `if Err::(42).is_err()` - -error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching_const_result.rs:14:15 - | -LL | while let Ok(_) = Ok::(10) {} - | ----------^^^^^--------------------- help: try this: `while Ok::(10).is_ok()` - -error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching_const_result.rs:16:15 - | -LL | while let Err(_) = Ok::(10) {} - | ----------^^^^^^--------------------- help: try this: `while Ok::(10).is_err()` - -error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching_const_result.rs:18:5 - | -LL | / match Ok::(42) { -LL | | Ok(_) => true, -LL | | Err(_) => false, -LL | | }; - | |_____^ help: try this: `Ok::(42).is_ok()` - -error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching_const_result.rs:23:5 - | -LL | / match Err::(42) { -LL | | Ok(_) => false, -LL | | Err(_) => true, -LL | | }; - | |_____^ help: try this: `Err::(42).is_err()` - -error: aborting due to 6 previous errors - From a59d48064931ab860fb065e4ef3a2f5cc4276f96 Mon Sep 17 00:00:00 2001 From: Christiaan Dirkx Date: Sun, 20 Sep 2020 10:46:30 +0200 Subject: [PATCH 4/5] Remove `can_suggest` check for `is_ok` and `is_err`. `is_ok` and `is_err` are stabilized as const and can thus always be suggested. --- src/tools/clippy/clippy_lints/src/matches.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/tools/clippy/clippy_lints/src/matches.rs b/src/tools/clippy/clippy_lints/src/matches.rs index be879dfe28d70..d20662f1ef948 100644 --- a/src/tools/clippy/clippy_lints/src/matches.rs +++ b/src/tools/clippy/clippy_lints/src/matches.rs @@ -1469,10 +1469,10 @@ mod redundant_pattern_match { keyword: &'static str, ) { fn find_suggestion(cx: &LateContext<'_>, hir_id: HirId, path: &QPath<'_>) -> Option<&'static str> { - if match_qpath(path, &paths::RESULT_OK) && can_suggest(cx, hir_id, sym!(result_type), "is_ok") { + if match_qpath(path, &paths::RESULT_OK) { return Some("is_ok()"); } - if match_qpath(path, &paths::RESULT_ERR) && can_suggest(cx, hir_id, sym!(result_type), "is_err") { + if match_qpath(path, &paths::RESULT_ERR) { return Some("is_err()"); } if match_qpath(path, &paths::OPTION_SOME) && can_suggest(cx, hir_id, sym!(option_type), "is_some") { @@ -1562,8 +1562,8 @@ mod redundant_pattern_match { &paths::RESULT_ERR, "is_ok()", "is_err()", - || can_suggest(cx, hir_id, sym!(result_type), "is_ok"), - || can_suggest(cx, hir_id, sym!(result_type), "is_err"), + || true, + || true ) } else { None From bf70e21a7e48008a8c1d82c3b0376e574f9bc91b Mon Sep 17 00:00:00 2001 From: CDirkx Date: Sun, 20 Sep 2020 12:21:23 +0200 Subject: [PATCH 5/5] Update src/tools/clippy/clippy_lints/src/matches.rs Co-authored-by: Ralf Jung --- src/tools/clippy/clippy_lints/src/matches.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/clippy/clippy_lints/src/matches.rs b/src/tools/clippy/clippy_lints/src/matches.rs index d20662f1ef948..e88ff070fb58e 100644 --- a/src/tools/clippy/clippy_lints/src/matches.rs +++ b/src/tools/clippy/clippy_lints/src/matches.rs @@ -1563,7 +1563,7 @@ mod redundant_pattern_match { "is_ok()", "is_err()", || true, - || true + || true, ) } else { None