{"action":"created","issue":{"url":"https://api.github.com/repos/rust-lang/rust/issues/54041","repository_url":"https://api.github.com/repos/rust-lang/rust","labels_url":"https://api.github.com/repos/rust-lang/rust/issues/54041/labels{/name}","comments_url":"https://api.github.com/repos/rust-lang/rust/issues/54041/comments","events_url":"https://api.github.com/repos/rust-lang/rust/issues/54041/events","html_url":"https://github.com/rust-lang/rust/issues/54041","id":358188390,"node_id":"MDU6SXNzdWUzNTgxODgzOTA=","number":54041,"title":"Warn on function call in `unwrap_or(..)` parameter","user":{"login":"mqudsi","id":606923,"node_id":"MDQ6VXNlcjYwNjkyMw==","avatar_url":"https://avatars3.githubusercontent.com/u/606923?v=4","gravatar_id":"","url":"https://api.github.com/users/mqudsi","html_url":"https://github.com/mqudsi","followers_url":"https://api.github.com/users/mqudsi/followers","following_url":"https://api.github.com/users/mqudsi/following{/other_user}","gists_url":"https://api.github.com/users/mqudsi/gists{/gist_id}","starred_url":"https://api.github.com/users/mqudsi/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mqudsi/subscriptions","organizations_url":"https://api.github.com/users/mqudsi/orgs","repos_url":"https://api.github.com/users/mqudsi/repos","events_url":"https://api.github.com/users/mqudsi/events{/privacy}","received_events_url":"https://api.github.com/users/mqudsi/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-09-07T19:40:35Z","updated_at":"2018-09-07T20:14:20Z","closed_at":null,"author_association":"NONE","body":"I believe the `Result::unwrap_or(..)` has a high potential for causing problems or unexpected behavior to users without previous intimate familiarity with lower-level languages, as it can cause unexpected side effects to take place.\r\n\r\nFor example, it is completely plausible that someone could write the following and be surprised by the output:\r\n\r\n```rust\r\nfn get_alternative() -> i32 {\r\n    eprintln!(\"Forced to use alternative approach!\");\r\n    return 42;\r\n}\r\n\r\nfn main() {\r\n    let r: Result<_, !> = Ok(7);\r\n    println!(\"{}\", r.unwrap_or(get_alternative()));\r\n}\r\n```\r\n\r\nWhere `get_alternative()` could have even more serious side effects than an incorrect error message. \r\n\r\nWhile `unwrap_or_else(|...| ...)` exists, I believe this is not sufficient to prevent this class of error from happening. The documentation on the reasons for the existence of `unwrap_or_else` vs `unwrap_or` (apart their names being complete artifacts due to rust's strict no overloading policy, only further adding to the confusion) does not stress the importance of this aspect of their differences.\r\n\r\nFurthermore, given the sheer abundance of `Option` and `Result` return values in the Rust standard library, this has the potential to significantly optimize runtimes if people are doing something like this:\r\n\r\n```rust\r\nsome_value.map(|x| x.foo.unwrap_or(fallback())).unwrap_or(fallback)\r\n```\r\n\r\nWhere `fallback` is called twice to initialize just one value, when it doesn't even need to be called _at all_ in the normal case.\r\n\r\nI can't think of a good story for _wanting_ the nested function calls in a `unwrap_or(...)` call to be executed regardless of whether the unwrap succeeded or failed, and if someone really wants it, the inconvenience of having them silence the warning with a #`[allow(unwrap_or_fn_call)]` is, imho, not a big deal compared to the entire class of errors implementing this warning would provide.\r\n\r\n (If it were up to me, I would have originally translated both `unwrap_or` and `unwrap_or_else` to a `match` statement in pre-compilation and forced delayed execution of nested function calls.)\r\n\r\n**Proposal**\r\n\r\nCompiling code containing something along the lines of the following:\r\n```rust\r\n    let foo = bar.unwrap_or(get_alternative());\r\n```\r\n\r\nWould generate ~an error~ a warning message:\r\n\r\n> Nested function calls in Result::unwrap_or expressions are executed unconditionally. Did you mean to use Result::unwrap_or_else(...) instead?"},"comment":{"url":"https://api.github.com/repos/rust-lang/rust/issues/comments/419553385","html_url":"https://github.com/rust-lang/rust/issues/54041#issuecomment-419553385","issue_url":"https://api.github.com/repos/rust-lang/rust/issues/54041","id":419553385,"node_id":"MDEyOklzc3VlQ29tbWVudDQxOTU1MzM4NQ==","user":{"login":"euclio","id":1372438,"node_id":"MDQ6VXNlcjEzNzI0Mzg=","avatar_url":"https://avatars0.githubusercontent.com/u/1372438?v=4","gravatar_id":"","url":"https://api.github.com/users/euclio","html_url":"https://github.com/euclio","followers_url":"https://api.github.com/users/euclio/followers","following_url":"https://api.github.com/users/euclio/following{/other_user}","gists_url":"https://api.github.com/users/euclio/gists{/gist_id}","starred_url":"https://api.github.com/users/euclio/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/euclio/subscriptions","organizations_url":"https://api.github.com/users/euclio/orgs","repos_url":"https://api.github.com/users/euclio/repos","events_url":"https://api.github.com/users/euclio/events{/privacy}","received_events_url":"https://api.github.com/users/euclio/received_events","type":"User","site_admin":false},"created_at":"2018-09-07T20:14:20Z","updated_at":"2018-09-07T20:14:20Z","author_association":"CONTRIBUTOR","body":"There is already a [clippy lint](https://rust-lang-nursery.github.io/rust-clippy/master/index.html#or_fun_call) for this exact scenario."}}
{"actor":{"display_login":"euclio"}}