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

Wrapping<_> implements ShX<usize> but wrapping_shX takes u32 #34809

Open
jethrogb opened this issue Jul 14, 2016 · 4 comments
Open

Wrapping<_> implements ShX<usize> but wrapping_shX takes u32 #34809

jethrogb opened this issue Jul 14, 2016 · 4 comments
Labels
C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@jethrogb
Copy link
Contributor

jethrogb commented Jul 14, 2016

This is mostly just slightly confusing.

impl ShX for Wrapping<_> (the code refers to #23545, I'm not sure how this is related).

wrapping_shX

@sfackler sfackler added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jul 14, 2016
@nagisa
Copy link
Member

nagisa commented Jul 14, 2016

The same way Shr/Shl are implemented for all primitive integrals with all primitive integral types for RHS, Shr/Shl should be implemented ∀ Wrapping<T>, where T: primitive with all primitive integral types for RHS.

@Mark-Simulacrum
Copy link
Member

Does anyone know why this is? It was added by @aturon, and it seems it was intended to be implemented for more types: #23549 (comment). I suspect that this is a side-effect of #23545 since it causes inference regressions, but I'm uncertain (and that seems odd, since I'd expect most/all uses of shifting operations to have the types fairly well defined explicitly...).

@Mark-Simulacrum Mark-Simulacrum added C-feature-request Category: A feature request, i.e: not implemented / a PR. C-bug Category: This is a bug. and removed C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Jul 25, 2017
@clarfonthey
Copy link
Contributor

I ran into this problem when working on #50465; it appears that neither Wrapping(1u32) >> 2 nor Wrapping(1u32) >> Wrapping(2) work today without type ascription.

IMHO, it makes the most sense to get rid of the Shr<usize> and Shl<usize> impls and use Wrapping on both sides, but that would be a breaking change. I assume that there's no way to deprecate trait impls either…

Ideally, we'd want to implement all of the necessary impls for Wrapping >> Wrapping and Wrapping << Wrapping, using all combinations of integer types like the non-wrapping versions, then somehow deprecate the current, lonely usize impl. Ideally, we could outright remove it, but I doubt that'd happen any time soon.

@Enselic
Copy link
Member

Enselic commented Nov 18, 2023

Triage: I believe this issue can be closed after the disabled impls here have been enabled, which is requires #23545 to be fixed first. Please correct me if that is wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. 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

6 participants