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

regression: Sign is ambiguous #135672

Closed
BoxyUwU opened this issue Jan 18, 2025 · 7 comments
Closed

regression: Sign is ambiguous #135672

BoxyUwU opened this issue Jan 18, 2025 · 7 comments
Labels
regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@BoxyUwU
Copy link
Member

BoxyUwU commented Jan 18, 2025

[INFO] [stdout] error[E0659]: `Sign` is ambiguous
[INFO] [stdout]    --> src/Core/cipher_aes_gcm_siv.rs:114:24
[INFO] [stdout]     |
[INFO] [stdout] 114 |         let mut hmac = Sign::new(plaintext.to_vec(), passphrase, Operation::Sign, SignType::Sha512);
[INFO] [stdout]     |                        ^^^^ ambiguous name
[INFO] [stdout]     |
[INFO] [stdout]     = note: ambiguous because of multiple glob imports of a name in the same module

note: there were some other (knock on?) errors in this crate see logs for more detail

@BoxyUwU BoxyUwU added regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 18, 2025
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jan 18, 2025
@theemathas
Copy link
Contributor

theemathas commented Jan 18, 2025

Presumably regressed in #118159, which added std::fmt::Sign as an unstable type.

@BoxyUwU BoxyUwU added T-libs Relevant to the library team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 18, 2025
@apiraino
Copy link
Contributor

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-high

@rustbot rustbot added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jan 20, 2025
@theemathas
Copy link
Contributor

theemathas commented Jan 21, 2025

As it turns out, glob imports have been causing unstable items to affect stable code for a long time now. For example, the following code has been failing to compile since version 1.1.0, when std::ops::CoercedUnsized was added as an unstable trait.

use foo::*;
use std::ops::*;

mod foo {
    pub struct CoerceUnsized;
}

fn main() {
    let x: CoerceUnsized;
}

@jieyouxu jieyouxu added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jan 21, 2025
@Urgau Urgau added the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Jan 21, 2025
@Noratrieb
Copy link
Member

This is the crate: https://crates.io/crates/crypt_guard

Not that many downloads (but also not none). Looks maintained so they would probably make a fix release when we tell them. Not on the team, but I'd lean towards accepting the breakage, just the usual glob import brittleness.

@Amanieu Amanieu removed the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Jan 21, 2025
@Amanieu
Copy link
Member

Amanieu commented Jan 21, 2025

We discussed this in the libs-api team and decided that this is acceptable breakage as per our policy with glob imports. The crate author should still be notified about the upcoming breaking change via an issue.

Additionally, we think that the lint proposed by @tgross35 in rust-lang/rust-clippy#13961 should be a built-in lint. You almost never want to glob import from the standard library except for a few prelude modules.

@Noratrieb
Copy link
Member

I've notified them in mm9942/crypt_guard#9

@mm9942
Copy link

mm9942 commented Jan 22, 2025

Thanks for the infos! I'll look it up soon and update the crate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

9 participants