Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stabilize some Option methods as const #76135

Merged
merged 4 commits into from
Sep 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions library/core/src/option.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ impl<T> Option<T> {
/// ```
#[must_use = "if you intended to assert that this has a value, consider `.unwrap()` instead"]
#[inline]
#[rustc_const_unstable(feature = "const_option", issue = "67441")]
#[rustc_const_stable(feature = "const_option", since = "1.48.0")]
#[stable(feature = "rust1", since = "1.0.0")]
pub const fn is_some(&self) -> bool {
matches!(*self, Some(_))
Expand All @@ -195,7 +195,7 @@ impl<T> Option<T> {
#[must_use = "if you intended to assert that this doesn't have a value, consider \
`.and_then(|| panic!(\"`Option` had a value when expected `None`\"))` instead"]
#[inline]
#[rustc_const_unstable(feature = "const_option", issue = "67441")]
#[rustc_const_stable(feature = "const_option", since = "1.48.0")]
#[stable(feature = "rust1", since = "1.0.0")]
pub const fn is_none(&self) -> bool {
!self.is_some()
Expand Down Expand Up @@ -254,7 +254,7 @@ impl<T> Option<T> {
/// println!("still can print text: {:?}", text);
/// ```
#[inline]
#[rustc_const_unstable(feature = "const_option", issue = "67441")]
#[rustc_const_stable(feature = "const_option", since = "1.48.0")]
#[stable(feature = "rust1", since = "1.0.0")]
pub const fn as_ref(&self) -> Option<&T> {
match *self {
Expand Down
16 changes: 16 additions & 0 deletions library/core/tests/option.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,3 +356,19 @@ fn test_replace() {
assert_eq!(x, Some(3));
assert_eq!(old, None);
}

#[test]
fn option_const() {
// test that the methods of `Option` are usable in a const context

const OPTION: Option<usize> = Some(32);

const REF: Option<&usize> = OPTION.as_ref();
assert_eq!(REF, Some(&32));

const IS_SOME: bool = OPTION.is_some();
assert!(IS_SOME);

const IS_NONE: bool = OPTION.is_none();
assert!(!IS_NONE);
}
14 changes: 0 additions & 14 deletions src/test/ui/consts/const-option.rs

This file was deleted.

78 changes: 16 additions & 62 deletions src/tools/clippy/clippy_lints/src/matches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1440,15 +1440,12 @@ where

mod redundant_pattern_match {
use super::REDUNDANT_PATTERN_MATCHING;
use crate::utils::{in_constant, match_qpath, match_trait_method, paths, snippet, span_lint_and_then};
use crate::utils::{match_qpath, match_trait_method, paths, snippet, span_lint_and_then};
use if_chain::if_chain;
use rustc_ast::ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir::{Arm, Expr, ExprKind, HirId, MatchSource, PatKind, QPath};
use rustc_hir::{Arm, Expr, ExprKind, MatchSource, PatKind, QPath};
use rustc_lint::LateContext;
use rustc_middle::ty;
use rustc_mir::const_eval::is_const_fn;
use rustc_span::source_map::Symbol;

pub fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let ExprKind::Match(op, arms, ref match_source) = &expr.kind {
Expand All @@ -1468,37 +1465,24 @@ mod redundant_pattern_match {
arms: &[Arm<'_>],
keyword: &'static str,
) {
fn find_suggestion(cx: &LateContext<'_>, hir_id: HirId, path: &QPath<'_>) -> Option<&'static str> {
if match_qpath(path, &paths::RESULT_OK) {
return Some("is_ok()");
}
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") {
return Some("is_some()");
}
if match_qpath(path, &paths::OPTION_NONE) && can_suggest(cx, hir_id, sym!(option_type), "is_none") {
return Some("is_none()");
}
None
}

let hir_id = expr.hir_id;
let good_method = match arms[0].pat.kind {
PatKind::TupleStruct(ref path, ref patterns, _) if patterns.len() == 1 => {
if let PatKind::Wild = patterns[0].kind {
find_suggestion(cx, hir_id, path)
if match_qpath(path, &paths::RESULT_OK) {
"is_ok()"
} else if match_qpath(path, &paths::RESULT_ERR) {
"is_err()"
} else if match_qpath(path, &paths::OPTION_SOME) {
"is_some()"
} else {
return;
}
} else {
None
return;
}
},
PatKind::Path(ref path) => find_suggestion(cx, hir_id, path),
_ => None,
};
let good_method = match good_method {
Some(method) => method,
None => return,
PatKind::Path(ref path) if match_qpath(path, &paths::OPTION_NONE) => "is_none()",
_ => return,
};

// check that `while_let_on_iterator` lint does not trigger
Expand Down Expand Up @@ -1547,7 +1531,6 @@ mod redundant_pattern_match {
if arms.len() == 2 {
let node_pair = (&arms[0].pat.kind, &arms[1].pat.kind);

let hir_id = expr.hir_id;
let found_good_method = match node_pair {
(
PatKind::TupleStruct(ref path_left, ref patterns_left, _),
Expand All @@ -1562,8 +1545,6 @@ mod redundant_pattern_match {
&paths::RESULT_ERR,
"is_ok()",
"is_err()",
|| true,
|| true,
)
} else {
None
Expand All @@ -1582,8 +1563,6 @@ mod redundant_pattern_match {
&paths::OPTION_NONE,
"is_some()",
"is_none()",
|| can_suggest(cx, hir_id, sym!(option_type), "is_some"),
|| can_suggest(cx, hir_id, sym!(option_type), "is_none"),
)
} else {
None
Expand Down Expand Up @@ -1616,7 +1595,6 @@ mod redundant_pattern_match {
}
}

#[allow(clippy::too_many_arguments)]
fn find_good_method_for_match<'a>(
arms: &[Arm<'_>],
path_left: &QPath<'_>,
Expand All @@ -1625,8 +1603,6 @@ mod redundant_pattern_match {
expected_right: &[&str],
should_be_left: &'a str,
should_be_right: &'a str,
can_suggest_left: impl Fn() -> bool,
can_suggest_right: impl Fn() -> bool,
) -> Option<&'a str> {
let body_node_pair = if match_qpath(path_left, expected_left) && match_qpath(path_right, expected_right) {
(&(*arms[0].body).kind, &(*arms[1].body).kind)
Expand All @@ -1638,35 +1614,13 @@ mod redundant_pattern_match {

match body_node_pair {
(ExprKind::Lit(ref lit_left), ExprKind::Lit(ref lit_right)) => match (&lit_left.node, &lit_right.node) {
(LitKind::Bool(true), LitKind::Bool(false)) if can_suggest_left() => Some(should_be_left),
(LitKind::Bool(false), LitKind::Bool(true)) if can_suggest_right() => Some(should_be_right),
(LitKind::Bool(true), LitKind::Bool(false)) => Some(should_be_left),
(LitKind::Bool(false), LitKind::Bool(true)) => Some(should_be_right),
_ => None,
},
_ => None,
}
}

fn can_suggest(cx: &LateContext<'_>, hir_id: HirId, diag_item: Symbol, name: &str) -> bool {
if !in_constant(cx, hir_id) {
return true;
}

// Avoid suggesting calls to non-`const fn`s in const contexts, see #5697.
cx.tcx
.get_diagnostic_item(diag_item)
.and_then(|def_id| {
cx.tcx.inherent_impls(def_id).iter().find_map(|imp| {
cx.tcx
.associated_items(*imp)
.in_definition_order()
.find_map(|item| match item.kind {
ty::AssocKind::Fn if item.ident.name.as_str() == name => Some(item.def_id),
_ => None,
})
})
})
.map_or(false, |def_id| is_const_fn(cx.tcx, def_id))
}
}

#[test]
Expand Down
39 changes: 14 additions & 25 deletions src/tools/clippy/tests/ui/redundant_pattern_matching.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ fn main() {
takes_bool(x);

issue5504();
issue5697();
issue6067();

let _ = if gen_opt().is_some() {
Expand Down Expand Up @@ -129,41 +128,31 @@ fn issue5504() {
while m!().is_some() {}
}

// None of these should be linted because none of the suggested methods
// are `const fn` without toggling a feature.
const fn issue5697() {
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,
};
}

// 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.
// However, in Rust 1.48.0 the methods `is_ok` and `is_err` of `Result`, and `is_some` and `is_none`
// of `Option` were stabilized as const, so the following should be linted.
const fn issue6067() {
if Ok::<i32, i32>(42).is_ok() {}

if Err::<i32, i32>(42).is_err() {}

if Some(42).is_some() {}

if None::<()>.is_none() {}

while Ok::<i32, i32>(10).is_ok() {}

while Ok::<i32, i32>(10).is_err() {}

while Some(42).is_some() {}

while None::<()>.is_none() {}

Ok::<i32, i32>(42).is_ok();

Err::<i32, i32>(42).is_err();

Some(42).is_some();

None::<()>.is_none();
}
45 changes: 20 additions & 25 deletions src/tools/clippy/tests/ui/redundant_pattern_matching.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ fn main() {
takes_bool(x);

issue5504();
issue5697();
issue6067();

let _ = if let Some(_) = gen_opt() {
Expand Down Expand Up @@ -150,40 +149,26 @@ fn issue5504() {
while let Some(_) = m!() {}
}

// None of these should be linted because none of the suggested methods
// are `const fn` without toggling a feature.
const fn issue5697() {
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,
};
}

// 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.
// However, in Rust 1.48.0 the methods `is_ok` and `is_err` of `Result`, and `is_some` and `is_none`
// of `Option` were stabilized as const, so the following should be linted.
const fn issue6067() {
if let Ok(_) = Ok::<i32, i32>(42) {}

if let Err(_) = Err::<i32, i32>(42) {}

if let Some(_) = Some(42) {}

if let None = None::<()> {}

while let Ok(_) = Ok::<i32, i32>(10) {}

while let Err(_) = Ok::<i32, i32>(10) {}

while let Some(_) = Some(42) {}

while let None = None::<()> {}

match Ok::<i32, i32>(42) {
Ok(_) => true,
Err(_) => false,
Expand All @@ -193,4 +178,14 @@ const fn issue6067() {
Ok(_) => false,
Err(_) => true,
};

match Some(42) {
Some(_) => true,
None => false,
};

match None::<()> {
Some(_) => false,
None => true,
};
}
Loading