Auto merge of rust-lang#6077 - ebroto:revert_or_fun_call_const, r=mat…

…thiaskrgr

Revert: or_fun_call should lint calls to `const fn`s with no args

The changes in rust-lang#5889 and rust-lang#5984 were done under the incorrect assumption that a `const fn` with no args was guaranteed to be evaluated at compile time.  A `const fn` is only guaranteed to be evaluated at compile time if it's inside a const context (the initializer of a `const` or a `static`).

See this [zulip conversation](https://rust-lang.zulipchat.com/#narrow/stream/122651-general/topic/Common.20misconception.3A.20.60const.20fn.60.20and.20its.20effect.20on.20codegen/near/208059113) for more details on this common misconception.

Given that none of the linted methods by `or_fun_call` can be called in const contexts, the lint should make no exceptions.

changelog: [`or_fun_call`] lints again calls to `const fn` with no args

clippy_lints/src/utils/mod.rs

@@ -46,7 +46,6 @@ use rustc_lint::{LateContext, Level, Lint, LintContext};
 use rustc_middle::hir::map::Map;
 use rustc_middle::ty::subst::{GenericArg, GenericArgKind};
 use rustc_middle::ty::{self, layout::IntegerExt, Ty, TyCtxt, TypeFoldable};
-use rustc_mir::const_eval;
 use rustc_span::hygiene::{ExpnKind, MacroKind};
 use rustc_span::source_map::original_sp;
 use rustc_span::symbol::{self, kw, Symbol};
@@ -883,19 +882,11 @@ pub fn is_copy<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
 
 /// Checks if an expression is constructing a tuple-like enum variant or struct
 pub fn is_ctor_or_promotable_const_function(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
-    fn has_no_arguments(cx: &LateContext<'_>, def_id: DefId) -> bool {
-        cx.tcx.fn_sig(def_id).skip_binder().inputs().is_empty()
-    }
-
     if let ExprKind::Call(ref fun, _) = expr.kind {
         if let ExprKind::Path(ref qp) = fun.kind {
             let res = cx.qpath_res(qp, fun.hir_id);
             return match res {
                 def::Res::Def(DefKind::Variant | DefKind::Ctor(..), ..) => true,
-                // FIXME: check the constness of the arguments, see https://github.com/rust-lang/rust-clippy/pull/5682#issuecomment-638681210
-                def::Res::Def(DefKind::Fn | DefKind::AssocFn, def_id) if has_no_arguments(cx, def_id) => {
-                    const_eval::is_const_fn(cx.tcx, def_id)
-                },
                 def::Res::Def(_, def_id) => cx.tcx.is_promotable_const_fn(def_id),
                 _ => false,
             };

tests/ui/or_fun_call.fixed

@@ -58,6 +58,12 @@ fn or_fun_call() {
     let without_default = Some(Foo);
     without_default.unwrap_or_else(Foo::new);
 
+    let mut map = HashMap::<u64, String>::new();
+    map.entry(42).or_insert_with(String::new);
+
+    let mut btree = BTreeMap::<u64, String>::new();
+    btree.entry(42).or_insert_with(String::new);
+
     let stringy = Some(String::from(""));
     let _ = stringy.unwrap_or_else(|| "".to_owned());
 
@@ -110,23 +116,4 @@ fn f() -> Option<()> {
     Some(())
 }
 
-// Issue 5886 - const fn (with no arguments)
-pub fn skip_const_fn_with_no_args() {
-    const fn foo() -> Option<i32> {
-        Some(42)
-    }
-    let _ = None.or(foo());
-
-    // See issue #5693.
-    let mut map = std::collections::HashMap::new();
-    map.insert(1, vec![1]);
-    map.entry(1).or_insert(vec![]);
-
-    let mut map = HashMap::<u64, String>::new();
-    map.entry(42).or_insert(String::new());
-
-    let mut btree = BTreeMap::<u64, String>::new();
-    btree.entry(42).or_insert(String::new());
-}
-
 fn main() {}

tests/ui/or_fun_call.rs

@@ -58,6 +58,12 @@ fn or_fun_call() {
     let without_default = Some(Foo);
     without_default.unwrap_or(Foo::new());
 
+    let mut map = HashMap::<u64, String>::new();
+    map.entry(42).or_insert(String::new());
+
+    let mut btree = BTreeMap::<u64, String>::new();
+    btree.entry(42).or_insert(String::new());
+
     let stringy = Some(String::from(""));
     let _ = stringy.unwrap_or("".to_owned());
 
@@ -110,23 +116,4 @@ fn f() -> Option<()> {
     Some(())
 }
 
-// Issue 5886 - const fn (with no arguments)
-pub fn skip_const_fn_with_no_args() {
-    const fn foo() -> Option<i32> {
-        Some(42)
-    }
-    let _ = None.or(foo());
-
-    // See issue #5693.
-    let mut map = std::collections::HashMap::new();
-    map.insert(1, vec![1]);
-    map.entry(1).or_insert(vec![]);
-
-    let mut map = HashMap::<u64, String>::new();
-    map.entry(42).or_insert(String::new());
-
-    let mut btree = BTreeMap::<u64, String>::new();
-    btree.entry(42).or_insert(String::new());
-}
-
 fn main() {}

tests/ui/or_fun_call.stderr

@@ -60,23 +60,35 @@ error: use of `unwrap_or` followed by a function call
 LL |     without_default.unwrap_or(Foo::new());
    |                     ^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(Foo::new)`
 
+error: use of `or_insert` followed by a function call
+  --> $DIR/or_fun_call.rs:62:19
+   |
+LL |     map.entry(42).or_insert(String::new());
+   |                   ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(String::new)`
+
+error: use of `or_insert` followed by a function call
+  --> $DIR/or_fun_call.rs:65:21
+   |
+LL |     btree.entry(42).or_insert(String::new());
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_insert_with(String::new)`
+
 error: use of `unwrap_or` followed by a function call
-  --> $DIR/or_fun_call.rs:62:21
+  --> $DIR/or_fun_call.rs:68:21
    |
 LL |     let _ = stringy.unwrap_or("".to_owned());
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| "".to_owned())`
 
 error: use of `or` followed by a function call
-  --> $DIR/or_fun_call.rs:87:35
+  --> $DIR/or_fun_call.rs:93:35
    |
 LL |     let _ = Some("a".to_string()).or(Some("b".to_string()));
    |                                   ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_else(|| Some("b".to_string()))`
 
 error: use of `or` followed by a function call
-  --> $DIR/or_fun_call.rs:91:10
+  --> $DIR/or_fun_call.rs:97:10
    |
 LL |         .or(Some(Bar(b, Duration::from_secs(2))));
    |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `or_else(|| Some(Bar(b, Duration::from_secs(2))))`
 
-error: aborting due to 13 previous errors
+error: aborting due to 15 previous errors