Revert "Auto merge of #6568 - Jarcho:redundant_pattern_matching, r=flip1995"

This reverts commit 7f2068c, reversing
changes made to 1e0a3ff.

Author: flip1995

clippy_lints/src/matches.rs

@@ -424,12 +424,7 @@ declare_clippy_lint! {
     /// **Why is this bad?** It's more concise and clear to just use the proper
     /// utility function
     ///
-    /// **Known problems:** This will change the drop order for the matched type. Both `if let` and
-    /// `while let` will drop the value at the end of the block, both `if` and `while` will drop the
-    /// value before entering the block. For most types this change will not matter, but for a few
-    /// types this will not be an acceptable change (e.g. locks). See the
-    /// [reference](https://doc.rust-lang.org/reference/destructors.html#drop-scopes) for more about
-    /// drop order.
+    /// **Known problems:** None.
     ///
     /// **Example:**
     ///
@@ -1707,206 +1702,55 @@ where
 mod redundant_pattern_match {
     use super::REDUNDANT_PATTERN_MATCHING;
     use clippy_utils::diagnostics::span_lint_and_then;
-    use clippy_utils::source::{snippet, snippet_with_applicability};
-    use clippy_utils::ty::{implements_trait, is_type_diagnostic_item, is_type_lang_item, match_type};
+    use clippy_utils::source::snippet;
     use clippy_utils::{is_lang_ctor, is_qpath_def_path, is_trait_method, paths};
     use if_chain::if_chain;
     use rustc_ast::ast::LitKind;
     use rustc_errors::Applicability;
     use rustc_hir::LangItem::{OptionNone, OptionSome, PollPending, PollReady, ResultErr, ResultOk};
-    use rustc_hir::{
-        intravisit::{walk_expr, ErasedMap, NestedVisitorMap, Visitor},
-        Arm, Block, Expr, ExprKind, LangItem, MatchSource, Node, PatKind, QPath,
-    };
+    use rustc_hir::{Arm, Expr, ExprKind, MatchSource, PatKind, QPath};
     use rustc_lint::LateContext;
-    use rustc_middle::ty::{self, subst::GenericArgKind, Ty};
     use rustc_span::sym;
 
     pub fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
         if let ExprKind::Match(op, arms, ref match_source) = &expr.kind {
             match match_source {
                 MatchSource::Normal => find_sugg_for_match(cx, expr, op, arms),
-                MatchSource::IfLetDesugar { contains_else_clause } => {
-                    find_sugg_for_if_let(cx, expr, op, &arms[0], "if", *contains_else_clause)
-                },
-                MatchSource::WhileLetDesugar => find_sugg_for_if_let(cx, expr, op, &arms[0], "while", false),
+                MatchSource::IfLetDesugar { .. } => find_sugg_for_if_let(cx, expr, op, arms, "if"),
+                MatchSource::WhileLetDesugar => find_sugg_for_if_let(cx, expr, op, arms, "while"),
                 _ => {},
             }
         }
     }
 
-    /// Checks if the drop order for a type matters. Some std types implement drop solely to
-    /// deallocate memory. For these types, and composites containing them, changing the drop order
-    /// won't result in any observable side effects.
-    fn type_needs_ordered_drop(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
-        if !ty.needs_drop(cx.tcx, cx.param_env) {
-            false
-        } else if !cx
-            .tcx
-            .lang_items()
-            .drop_trait()
-            .map_or(false, |id| implements_trait(cx, ty, id, &[]))
-        {
-            // This type doesn't implement drop, so no side effects here.
-            // Check if any component type has any.
-            match ty.kind() {
-                ty::Tuple(_) => ty.tuple_fields().any(|ty| type_needs_ordered_drop(cx, ty)),
-                ty::Array(ty, _) => type_needs_ordered_drop(cx, ty),
-                ty::Adt(adt, subs) => adt
-                    .all_fields()
-                    .map(|f| f.ty(cx.tcx, subs))
-                    .any(|ty| type_needs_ordered_drop(cx, ty)),
-                _ => true,
-            }
-        }
-        // Check for std types which implement drop, but only for memory allocation.
-        else if is_type_diagnostic_item(cx, ty, sym::vec_type)
-            || is_type_lang_item(cx, ty, LangItem::OwnedBox)
-            || is_type_diagnostic_item(cx, ty, sym::Rc)
-            || is_type_diagnostic_item(cx, ty, sym::Arc)
-            || is_type_diagnostic_item(cx, ty, sym::cstring_type)
-            || match_type(cx, ty, &paths::BTREEMAP)
-            || match_type(cx, ty, &paths::LINKED_LIST)
-            || match_type(cx, ty, &paths::WEAK_RC)
-            || match_type(cx, ty, &paths::WEAK_ARC)
-        {
-            // Check all of the generic arguments.
-            if let ty::Adt(_, subs) = ty.kind() {
-                subs.types().any(|ty| type_needs_ordered_drop(cx, ty))
-            } else {
-                true
-            }
-        } else {
-            true
-        }
-    }
-
-    // Extract the generic arguments out of a type
-    fn try_get_generic_ty(ty: Ty<'_>, index: usize) -> Option<Ty<'_>> {
-        if_chain! {
-            if let ty::Adt(_, subs) = ty.kind();
-            if let Some(sub) = subs.get(index);
-            if let GenericArgKind::Type(sub_ty) = sub.unpack();
-            then {
-                Some(sub_ty)
-            } else {
-                None
-            }
-        }
-    }
-
-    // Checks if there are any temporaries created in the given expression for which drop order
-    // matters.
-    fn temporaries_need_ordered_drop(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
-        struct V<'a, 'tcx> {
-            cx: &'a LateContext<'tcx>,
-            res: bool,
-        }
-        impl<'a, 'tcx> Visitor<'tcx> for V<'a, 'tcx> {
-            type Map = ErasedMap<'tcx>;
-            fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
-                NestedVisitorMap::None
-            }
-
-            fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
-                match expr.kind {
-                    // Taking the reference of a value leaves a temporary
-                    // e.g. In `&String::new()` the string is a temporary value.
-                    // Remaining fields are temporary values
-                    // e.g. In `(String::new(), 0).1` the string is a temporary value.
-                    ExprKind::AddrOf(_, _, expr) | ExprKind::Field(expr, _) => {
-                        if !matches!(expr.kind, ExprKind::Path(_)) {
-                            if type_needs_ordered_drop(self.cx, self.cx.typeck_results().expr_ty(expr)) {
-                                self.res = true;
-                            } else {
-                                self.visit_expr(expr);
-                            }
-                        }
-                    },
-                    // the base type is alway taken by reference.
-                    // e.g. In `(vec![0])[0]` the vector is a temporary value.
-                    ExprKind::Index(base, index) => {
-                        if !matches!(base.kind, ExprKind::Path(_)) {
-                            if type_needs_ordered_drop(self.cx, self.cx.typeck_results().expr_ty(base)) {
-                                self.res = true;
-                            } else {
-                                self.visit_expr(base);
-                            }
-                        }
-                        self.visit_expr(index);
-                    },
-                    // Method calls can take self by reference.
-                    // e.g. In `String::new().len()` the string is a temporary value.
-                    ExprKind::MethodCall(_, _, [self_arg, args @ ..], _) => {
-                        if !matches!(self_arg.kind, ExprKind::Path(_)) {
-                            let self_by_ref = self
-                                .cx
-                                .typeck_results()
-                                .type_dependent_def_id(expr.hir_id)
-                                .map_or(false, |id| self.cx.tcx.fn_sig(id).skip_binder().inputs()[0].is_ref());
-                            if self_by_ref
-                                && type_needs_ordered_drop(self.cx, self.cx.typeck_results().expr_ty(self_arg))
-                            {
-                                self.res = true;
-                            } else {
-                                self.visit_expr(self_arg)
-                            }
-                        }
-                        args.iter().for_each(|arg| self.visit_expr(arg));
-                    },
-                    // Either explicitly drops values, or changes control flow.
-                    ExprKind::DropTemps(_)
-                    | ExprKind::Ret(_)
-                    | ExprKind::Break(..)
-                    | ExprKind::Yield(..)
-                    | ExprKind::Block(Block { expr: None, .. }, _)
-                    | ExprKind::Loop(..) => (),
-
-                    // Only consider the final expression.
-                    ExprKind::Block(Block { expr: Some(expr), .. }, _) => self.visit_expr(expr),
-
-                    _ => walk_expr(self, expr),
-                }
-            }
-        }
-
-        let mut v = V { cx, res: false };
-        v.visit_expr(expr);
-        v.res
-    }
-
     fn find_sugg_for_if_let<'tcx>(
         cx: &LateContext<'tcx>,
         expr: &'tcx Expr<'_>,
-        op: &'tcx Expr<'tcx>,
-        arm: &Arm<'_>,
+        op: &Expr<'_>,
+        arms: &[Arm<'_>],
         keyword: &'static str,
-        has_else: bool,
     ) {
         // also look inside refs
-        let mut kind = &arm.pat.kind;
+        let mut kind = &arms[0].pat.kind;
         // if we have &None for example, peel it so we can detect "if let None = x"
         if let PatKind::Ref(inner, _mutability) = kind {
             kind = &inner.kind;
         }
-        let op_ty = cx.typeck_results().expr_ty(op);
-        // Determine which function should be used, and the type contained by the corresponding
-        // variant.
-        let (good_method, inner_ty) = match kind {
+        let good_method = match kind {
             PatKind::TupleStruct(ref path, [sub_pat], _) => {
                 if let PatKind::Wild = sub_pat.kind {
                     if is_lang_ctor(cx, path, ResultOk) {
-                        ("is_ok()", try_get_generic_ty(op_ty, 0).unwrap_or(op_ty))
+                        "is_ok()"
                     } else if is_lang_ctor(cx, path, ResultErr) {
-                        ("is_err()", try_get_generic_ty(op_ty, 1).unwrap_or(op_ty))
+                        "is_err()"
                     } else if is_lang_ctor(cx, path, OptionSome) {
-                        ("is_some()", op_ty)
+                        "is_some()"
                     } else if is_lang_ctor(cx, path, PollReady) {
-                        ("is_ready()", op_ty)
+                        "is_ready()"
                     } else if is_qpath_def_path(cx, path, sub_pat.hir_id, &paths::IPADDR_V4) {
-                        ("is_ipv4()", op_ty)
+                        "is_ipv4()"
                     } else if is_qpath_def_path(cx, path, sub_pat.hir_id, &paths::IPADDR_V6) {
-                        ("is_ipv6()", op_ty)
+                        "is_ipv6()"
                     } else {
                         return;
                     }
@@ -1915,36 +1759,17 @@ mod redundant_pattern_match {
                 }
             },
             PatKind::Path(ref path) => {
-                let method = if is_lang_ctor(cx, path, OptionNone) {
+                if is_lang_ctor(cx, path, OptionNone) {
                     "is_none()"
                 } else if is_lang_ctor(cx, path, PollPending) {
                     "is_pending()"
                 } else {
                     return;
-                };
-                // `None` and `Pending` don't have an inner type.
-                (method, cx.tcx.types.unit)
+                }
             },
             _ => return,
         };
 
-        // If this is the last expression in a block or there is an else clause then the whole
-        // type needs to be considered, not just the inner type of the branch being matched on.
-        // Note the last expression in a block is dropped after all local bindings.
-        let check_ty = if has_else
-            || (keyword == "if" && matches!(cx.tcx.hir().parent_iter(expr.hir_id).next(), Some((_, Node::Block(..)))))
-        {
-            op_ty
-        } else {
-            inner_ty
-        };
-
-        // All temporaries created in the scrutinee expression are dropped at the same time as the
-        // scrutinee would be, so they have to be considered as well.
-        // e.g. in `if let Some(x) = foo.lock().unwrap().baz.as_ref() { .. }` the lock will be held
-        // for the duration if body.
-        let needs_drop = type_needs_ordered_drop(cx, check_ty) || temporaries_need_ordered_drop(cx, op);
-
         // check that `while_let_on_iterator` lint does not trigger
         if_chain! {
             if keyword == "while";
@@ -1963,7 +1788,7 @@ mod redundant_pattern_match {
         span_lint_and_then(
             cx,
             REDUNDANT_PATTERN_MATCHING,
-            arm.pat.span,
+            arms[0].pat.span,
             &format!("redundant pattern matching, consider using `{}`", good_method),
             |diag| {
                 // while let ... = ... { ... }
@@ -1977,20 +1802,12 @@ mod redundant_pattern_match {
                 // while let ... = ... { ... }
                 // ^^^^^^^^^^^^^^^^^^^
                 let span = expr_span.until(op_span.shrink_to_hi());
-
-                let mut app = if needs_drop {
-                    Applicability::MaybeIncorrect
-                } else {
-                    Applicability::MachineApplicable
-                };
-                let sugg = snippet_with_applicability(cx, op_span, "_", &mut app);
-
-                diag.span_suggestion(span, "try this", format!("{} {}.{}", keyword, sugg, good_method), app);
-
-                if needs_drop {
-                    diag.note("this will change drop order of the result, as well as all temporaries");
-                    diag.note("add `#[allow(clippy::redundant_pattern_matching)]` if this is important");
-                }
+                diag.span_suggestion(
+                    span,
+                    "try this",
+                    format!("{} {}.{}", keyword, snippet(cx, op_span, "_"), good_method),
+                    Applicability::MachineApplicable, // snippet
+                );
             },
         );
     }