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

Make primitive integers always be Displayable and Debuggable. #36

Closed
wants to merge 4 commits into from
Closed

Make primitive integers always be Displayable and Debuggable. #36

wants to merge 4 commits into from

Conversation

ltratt
Copy link

@ltratt ltratt commented Feb 17, 2018

I have quite a lot of code which ends up looking like:

fn <F: PrimInt + Debug + Display>(v: F) {
  println!("{}", v};
}

AFAICS there isn't any fundamental need to force users to specify Debug and Display manually, since every primitive integer type implements those traits. This simple PR means that I can now happily do:

fn <F: PrimInt> (v: F) {
  println!("{}", v};
}

and get the same effect.

@ltratt
Copy link
Author

ltratt commented Feb 17, 2018

The second commit is a hack to deal with the (I-should-have-seen-this-coming) fact that when compiled with no_std, Display and Debug aren't available. In the long run, I guess that trait aliases will be a nicer fix: we can use cfg to select the appropriate one depending on whether std/no_std is specified.

@cuviper
Copy link
Member

cuviper commented Feb 17, 2018

It could import the traits from core for #[no_std] support, rather than toggling this entirely.

But unfortunately, adding constraints is a breaking change for external implementations of the trait. These do exist -- for instance extprim implements PrimInt for its 128-bit types. And while they do have Debug and Display already too, we can't know whether this is true for all existing implementations. There could be some internal newtype where someone didn't bother with display, for instance. (That would be a lot of work, but... it's possible.)

I suggest simply adding your own local trait combining these is the easier way forward.

@ltratt
Copy link
Author

ltratt commented Feb 17, 2018

As a naive user, I naively expected that PrimInt would be debuggable and displayable. Not that naive users expectations are always a guide to good design though!

I agree that it's a breaking change. But I also agree with you that it probably won't break anything in practise, because I suspect neither of us can fathom the idea of a number you can't print out :) Perhaps on the next API bump it would not be unreasonable to add such constraints, perhaps Debug at a minimum?

That said, I understand your POV, and I won't be offended if you want to close the PR.

@cuviper
Copy link
Member

cuviper commented Feb 17, 2018

I think with a semver bump, I'd be happy adding both. However, I have no plans to bump any time soon. Even the recent 0.2 bump came with no actual API changes, just feature changes to support #[no_std], and 0.1.43 reexporting it all to stay in sync.

Thanks for understanding!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants