Skip to content

Implement Ord, PartialOrd, Eq, PartialEq by hand#470

Merged
bvssvni merged 2 commits intoPistonDevelopers:masterfrom
TyOverby:partialfix
Jul 4, 2014
Merged

Implement Ord, PartialOrd, Eq, PartialEq by hand#470
bvssvni merged 2 commits intoPistonDevelopers:masterfrom
TyOverby:partialfix

Conversation

@TyOverby
Copy link
Contributor

@TyOverby TyOverby commented Jul 3, 2014

There is a bug in rustc that causes code explosion when auto-implementing PartialOrd. rust-lang/rust#15375

Until those issues are fixed, I've implemented them by hand.

Closes #467

@TyOverby
Copy link
Contributor Author

TyOverby commented Jul 3, 2014

The travis build is failing due to an issue with Cargo.

@gmorenz
Copy link
Contributor

gmorenz commented Jul 3, 2014

This is an issue with Cargo not piston, isn't it?

The PR compiles and runs on my machine, I have the latest nightly installed.

@TyOverby
Copy link
Contributor Author

TyOverby commented Jul 3, 2014

@gmorenz: For some reason, I keep switching those words up. You were right; I've edited my comment.

@indiv0
Copy link
Member

indiv0 commented Jul 3, 2014

The hammer.rs issue has been fixed. Next steps for this PR?

@gmorenz
Copy link
Contributor

gmorenz commented Jul 3, 2014

I think it can be merged... I don't think I am supposed to be merging pull requests unfortunately, though I technical have the ability to. If you need to build against this (or any non merged pull request) in the meantime you can check out the PR locally, and use [dependencies.piston] git = "file:///location/of/repo" in your projects Cargo.toml.

@indiv0
Copy link
Member

indiv0 commented Jul 4, 2014

Trying to build with this PR, I get the following error:

projects/rust-ssb [ cargo build --verbose                      master * ] 8:44 PM
   Compiling piston v0.0.0 (file:/home/indiv0/projects/rust-ssb/lib/piston)
       Fresh codegen v0.0.1 (https://github.com/AngryLawyer/rust-sdl2)
       Fresh image v0.0.0 (https://github.com/PistonDevelopers/rust-image)
       Fresh graphics v0.0.0 (https://github.com/PistonDevelopers/rust-graphics)
       Fresh gl v0.0.1 (https://github.com/bjz/gl-rs)
       Fresh sdl2 v0.0.1 (https://github.com/AngryLawyer/rust-sdl2)
       Fresh opengl_graphics v0.0.0 (https://github.com/PistonDevelopers/opengl_graphics)
Could not execute process `rustc src/lib.rs --crate-type lib --out-dir /media/projects/rust-ssb/target/deps -L /media/projects/rust-ssb/target/deps -L /media/projects/rust-ssb/target/deps` (status=101)
--- stderr
src/keyboard.rs:266:1: 271:2 error: not all trait methods implemented, missing: `partial_cmp`
src/keyboard.rs:266 impl PartialOrd for Key {
src/keyboard.rs:267     #[inline(always)]
src/keyboard.rs:268     fn lt(&self, rhs: &Key) -> bool {
src/keyboard.rs:269         self.code() < rhs.code()
src/keyboard.rs:270     }
src/keyboard.rs:271 }
src/event.rs:13:17: 13:30 error: type `keyboard::Key` does not implement any method in scope named `partial_cmp`
src/event.rs:13     KeyReleased(keyboard::Key),
                                ^~~~~~~~~~~~~
src/event.rs:8:30: 8:40 note: in expansion of #[deriving(PartialOrd)]
src/event.rs:13:17: 13:30 note: expansion site
src/event.rs:15:16: 15:29 error: type `keyboard::Key` does not implement any method in scope named `partial_cmp`
src/event.rs:15     KeyPressed(keyboard::Key),
                               ^~~~~~~~~~~~~
src/event.rs:8:30: 8:40 note: in expansion of #[deriving(PartialOrd)]
src/event.rs:15:16: 15:29 note: expansion site
error: aborting due to 3 previous errors


Caused by:
  Could not execute process `rustc src/lib.rs --crate-type lib --out-dir /media/projects/rust-ssb/target/deps -L /media/projects/rust-ssb/target/deps -L /media/projects/rust-ssb/target/deps` (status=101)
--- stderr
src/keyboard.rs:266:1: 271:2 error: not all trait methods implemented, missing: `partial_cmp`
src/keyboard.rs:266 impl PartialOrd for Key {
src/keyboard.rs:267     #[inline(always)]
src/keyboard.rs:268     fn lt(&self, rhs: &Key) -> bool {
src/keyboard.rs:269         self.code() < rhs.code()
src/keyboard.rs:270     }
src/keyboard.rs:271 }
src/event.rs:13:17: 13:30 error: type `keyboard::Key` does not implement any method in scope named `partial_cmp`
src/event.rs:13     KeyReleased(keyboard::Key),
                                ^~~~~~~~~~~~~
src/event.rs:8:30: 8:40 note: in expansion of #[deriving(PartialOrd)]
src/event.rs:13:17: 13:30 note: expansion site
src/event.rs:15:16: 15:29 error: type `keyboard::Key` does not implement any method in scope named `partial_cmp`
src/event.rs:15     KeyPressed(keyboard::Key),
                               ^~~~~~~~~~~~~
src/event.rs:8:30: 8:40 note: in expansion of #[deriving(PartialOrd)]
src/event.rs:15:16: 15:29 note: expansion site
error: aborting due to 3 previous errors

@indiv0
Copy link
Member

indiv0 commented Jul 4, 2014

So partial_cmp needs to be implemented for Keyboard as well.

@gmorenz
Copy link
Contributor

gmorenz commented Jul 4, 2014

Your error message doesn't seem to be coming from a version of piston including this pull request, e.g. on line 266 you have impl PartialOrd for key but this pull request has a blank line.

For reference the following sequence of commands successfully builds piston from scratch with this PR for me:

git clone https://github.com/pistondevelopers/piston
cd piston
git fetch origin pull/470/head:partialfix
git checkout partialfix
cargo build

@indiv0
Copy link
Member

indiv0 commented Jul 4, 2014

Yeah it worked with your instructions. My bad :P

bvssvni added a commit that referenced this pull request Jul 4, 2014
@bvssvni
Merge pull request #470 from TyOverby/partialfix
dbf29f2 
Implement Ord, PartialOrd, Eq, PartialEq by hand
@bvssvni bvssvni merged commit dbf29f2 into PistonDevelopers:master Jul 4, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Piston Freezes During Compilation with Cargo Build

4 participants

@TyOverby @gmorenz @indiv0 @bvssvni