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

Function anyhow::Ok breaks Ok pattern matching when wildcard importing #201

Closed
N3xed opened this issue Nov 27, 2021 · 5 comments
Closed

Comments

@N3xed
Copy link

N3xed commented Nov 27, 2021

When wildcard importing anyhow (use anyhow::*), the recently added function anyhow::Ok breaks pattern-matching because it shadows Result::Ok and thus causes build failures:

error[E0532]: expected tuple struct or tuple variant, found function `Ok`
   --> src\utils.rs:199:13
    |
199 |             Ok(result) => {
    |             ^^ not a tuple struct or tuple variant
    |
   ::: src\python.rs:12:23
    |
12  |     let version_str = cmd_output!(PYTHON, "--version")
    |                       -------------------------------- in this macro invocation
    |

In my view, this is a breaking change.

It's quite convenient to import all of anyhow at once, so I'd suggest moving this function into a module. Or it would need a new major version, but this may be overkill for such a small change.

@dtolnay
Copy link
Owner

dtolnay commented Nov 27, 2021

Cross-crate glob imports are never supposed to be used for exactly this reason. Please see https://doc.rust-lang.org/1.56.0/cargo/reference/semver.html#minor-adding-new-public-items.

This is not considered a major change because conventionally glob imports are a known forwards-compatibility hazard. Glob imports of items from external crates should be avoided.

@N3xed
Copy link
Author

N3xed commented Nov 27, 2021

I've learnt something new, thanks.

@asayers
Copy link

asayers commented Nov 29, 2021

glob imports are a known forwards-compatibility hazard

True, but in practice I think this change is going to cause confusion. It will break people's code, and most won't think to check the source for #[doc(hidden)] items. I wouldn't be surprised if rust/rust receives bug reports from people mistaking this for a typechecker bug.

Why not include fn Ok() in the docs?

@kali
Copy link

kali commented Nov 29, 2021

Please reconsider this move. You're basically saying it's ok to punish people from having been lazy and wrote a harmless-looking global include. Aggravated by the fact the function is hidden from the documentation and out of convention style.
I've seen how weird the error generated by rustc, this is gonna make a lot of people waste a lot of time.

I have fixed my repos. But please, please, think of all these teams pulling their hair out right now and be nice to them. Please revert and yank this.

@dtolnay
Copy link
Owner

dtolnay commented Nov 29, 2021

Folks, this crate is made for me to use, in the way that I want to use it. If you want to use glob imports your options are:

  • don't use this crate;
  • send a rustc PR to make it handle pattern matches on tuple variants even with a similarly named fn imported;
  • send a rustc PR to improve its error messages.

Repository owner locked and limited conversation to collaborators Nov 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants