Skip to content

Remove Neg from BaseNum #114

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

Merged
merged 1 commit into from
Apr 4, 2015
Merged

Remove Neg from BaseNum #114

merged 1 commit into from
Apr 4, 2015

Conversation

andersk
Copy link
Contributor

@andersk andersk commented Apr 3, 2015

As per rust-lang/rust#23945, Neg is no longer implemented for unsigned types.

As per rust-lang/rust#23945, Neg is no longer
implemented for unsigned types.

Signed-off-by: Anders Kaseorg <[email protected]>
@milibopp
Copy link
Collaborator

milibopp commented Apr 4, 2015

I think we should discuss in that case, what BaseNum is supposed to mean. See also #113.

cc @sebcrozet

@tomaka
Copy link
Contributor

tomaka commented Apr 4, 2015

There are only two possibilities:

  • Remove the implementation of BaseNum on unsigned types, which means that you won't be able to have a Mat4<u16> for example.
  • Allow Mat4<u16>, but forbid all operations that may need to change the sign of the content of the matrix. That's what this PR does.

@milibopp
Copy link
Collaborator

milibopp commented Apr 4, 2015

I am perfectly aware of that. The question is whether u8, u16 etc. are supposed to be BaseNum. I tend to think that they ought to be as well, since that has been the case before.

But this still changes the abstract definition of BaseNum. Personally, I think that the algebraic definitions here should be more fine-grained anyhow. I just can't really judge, what exactly BaseNum is supposed to entail.

Anyway, to be practical I could probably merge this to get nalgebra to compile. Then we can think about proper abstractions later, see #32.

milibopp added a commit that referenced this pull request Apr 4, 2015
Remove Neg from BaseNum
@milibopp milibopp merged commit 49abb42 into dimforge:master Apr 4, 2015
@tomaka
Copy link
Contributor

tomaka commented Apr 4, 2015

Ahhh, yes. Could you publish it too?
The version on crates.io hasn't compiled for 3-4 days.

@milibopp
Copy link
Collaborator

milibopp commented Apr 4, 2015

I'm in the process of fixing everything right now.

@andersk andersk deleted the neg branch April 4, 2015 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants