From f5e9a1581162f687a228d3e074625e3d9f8d9664 Mon Sep 17 00:00:00 2001 From: Valerii Lashmanov Date: Tue, 6 Jun 2017 22:44:44 -0400 Subject: [PATCH 1/9] template copied --- 0000-heterogeneous-comparisons.md | 47 +++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 0000-heterogeneous-comparisons.md diff --git a/0000-heterogeneous-comparisons.md b/0000-heterogeneous-comparisons.md new file mode 100644 index 00000000000..ef898e3360a --- /dev/null +++ b/0000-heterogeneous-comparisons.md @@ -0,0 +1,47 @@ +- Feature Name: (fill me in with a unique ident, my_awesome_feature) +- Start Date: (fill me in with today's date, YYYY-MM-DD) +- RFC PR: (leave this empty) +- Rust Issue: (leave this empty) + +# Summary +[summary]: #summary + +One para explanation of the feature. + +# Motivation +[motivation]: #motivation + +Why are we doing this? What use cases does it support? What is the expected outcome? + +# Detailed design +[design]: #detailed-design + +This is the bulk of the RFC. Explain the design in enough detail for somebody familiar +with the language to understand, and for somebody familiar with the compiler to implement. +This should get into specifics and corner-cases, and include examples of how the feature is used. + +# How We Teach This +[how-we-teach-this]: #how-we-teach-this + +What names and terminology work best for these concepts and why? +How is this idea best presented—as a continuation of existing Rust patterns, or as a wholly new one? + +Would the acceptance of this proposal change how Rust is taught to new users at any level? +How should this feature be introduced and taught to existing Rust users? + +What additions or changes to the Rust Reference, _The Rust Programming Language_, and/or _Rust by Example_ does it entail? + +# Drawbacks +[drawbacks]: #drawbacks + +Why should we *not* do this? + +# Alternatives +[alternatives]: #alternatives + +What other designs have been considered? What is the impact of not doing this? + +# Unresolved questions +[unresolved]: #unresolved-questions + +What parts of the design are still TBD? From 31d92e41eaf8c0b44a8c2a43328fd308fa552e86 Mon Sep 17 00:00:00 2001 From: Valerii Lashmanov Date: Tue, 6 Jun 2017 22:46:02 -0400 Subject: [PATCH 2/9] all but motivation added --- 0000-heterogeneous-comparisons.md | 36 ++++++++++++++++--------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/0000-heterogeneous-comparisons.md b/0000-heterogeneous-comparisons.md index ef898e3360a..7ddd3f94264 100644 --- a/0000-heterogeneous-comparisons.md +++ b/0000-heterogeneous-comparisons.md @@ -1,47 +1,49 @@ -- Feature Name: (fill me in with a unique ident, my_awesome_feature) -- Start Date: (fill me in with today's date, YYYY-MM-DD) +- Feature Name: heterogeneous_comparisons +- Start Date: 2017-06-06 - RFC PR: (leave this empty) - Rust Issue: (leave this empty) # Summary [summary]: #summary -One para explanation of the feature. +Allow to compare integer values of different signedness and size. # Motivation [motivation]: #motivation -Why are we doing this? What use cases does it support? What is the expected outcome? +Inability to elegantly and safely mix signed and unsigned types is rather frustrating. + +Comparison between values of different integer types is always well-defined. # Detailed design [design]: #detailed-design -This is the bulk of the RFC. Explain the design in enough detail for somebody familiar -with the language to understand, and for somebody familiar with the compiler to implement. -This should get into specifics and corner-cases, and include examples of how the feature is used. +`Ord` and `Eq` traits should be modified to allow `Rhs` type not equal to `Self`. -# How We Teach This -[how-we-teach-this]: #how-we-teach-this +After that `PartialEq`, `Eq`, `PartialOrd` and `Ord` should be implemented for all pairs of signed/unsigend 8/16/32/64/(128) bit integers and `isize`/`usize` variants. -What names and terminology work best for these concepts and why? -How is this idea best presented—as a continuation of existing Rust patterns, or as a wholly new one? +Implementation for signed-singed and unsigned-unsigned pairs should promote values to larger type first, then perform comparison. -Would the acceptance of this proposal change how Rust is taught to new users at any level? -How should this feature be introduced and taught to existing Rust users? +Implementation for signed-unsigned pairs should first check if signed value less than zero. If not, then it should promote both values to signed type with the same size as larger argument type and perform comparison. + +# How We Teach This +[how-we-teach-this]: #how-we-teach-this -What additions or changes to the Rust Reference, _The Rust Programming Language_, and/or _Rust by Example_ does it entail? +No changes I can find or think of. # Drawbacks [drawbacks]: #drawbacks -Why should we *not* do this? +* Correct signed-unsigned comparison requires one more operation than regular comparison. Proposed change hides this performance cost. If user doesn't care about correctness in his particular use case, then cast and comparison is faster. +* The rest of rust math prohibits mixing different types and requires explicit casts. Allowing heterogeneous comparisons (and only comparisons) makes rust math somewhat inconsistent. # Alternatives [alternatives]: #alternatives -What other designs have been considered? What is the impact of not doing this? +* Keep things as is. +* Add generic helper function for cross-type comparisons. # Unresolved questions [unresolved]: #unresolved-questions -What parts of the design are still TBD? +Is `Ord` between float and int values as bad idea as it seems? From 605f3e8490a741690218aec8d89e68fc5a68141e Mon Sep 17 00:00:00 2001 From: Valerii Lashmanov Date: Tue, 6 Jun 2017 23:28:23 -0400 Subject: [PATCH 3/9] better motivation --- 0000-heterogeneous-comparisons.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/0000-heterogeneous-comparisons.md b/0000-heterogeneous-comparisons.md index 7ddd3f94264..9e2032c682a 100644 --- a/0000-heterogeneous-comparisons.md +++ b/0000-heterogeneous-comparisons.md @@ -11,9 +11,11 @@ Allow to compare integer values of different signedness and size. # Motivation [motivation]: #motivation -Inability to elegantly and safely mix signed and unsigned types is rather frustrating. +Right now every comparison between different integral types requires a cast. These casts not only clutter the code, but also encourage writing incorrect code. -Comparison between values of different integer types is always well-defined. +The easiest way to compare signed and unsigned values is to cast unsigned value to signed type. It works most of the time, but it will silently ignore overflows. + +Comparison between values of different integer types is always well-defined. There is only one correct result and only one way to get it. Allowing compiler to perform these comparisons will reduce both code clutter and count of hidden overflow/underflow errors users make. # Detailed design [design]: #detailed-design @@ -34,6 +36,7 @@ No changes I can find or think of. # Drawbacks [drawbacks]: #drawbacks +* It might break some code relying on return type polymorphism. It won't be possible to infer type of the second argument from type of the first one for `Eq` and `Ord`. * Correct signed-unsigned comparison requires one more operation than regular comparison. Proposed change hides this performance cost. If user doesn't care about correctness in his particular use case, then cast and comparison is faster. * The rest of rust math prohibits mixing different types and requires explicit casts. Allowing heterogeneous comparisons (and only comparisons) makes rust math somewhat inconsistent. From d4fd71c0668930917f926d5967e75c0a877dcbeb Mon Sep 17 00:00:00 2001 From: Valerii Lashmanov Date: Tue, 6 Jun 2017 23:33:50 -0400 Subject: [PATCH 4/9] grammar fix --- 0000-heterogeneous-comparisons.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/0000-heterogeneous-comparisons.md b/0000-heterogeneous-comparisons.md index 9e2032c682a..e5b0d065543 100644 --- a/0000-heterogeneous-comparisons.md +++ b/0000-heterogeneous-comparisons.md @@ -11,7 +11,7 @@ Allow to compare integer values of different signedness and size. # Motivation [motivation]: #motivation -Right now every comparison between different integral types requires a cast. These casts not only clutter the code, but also encourage writing incorrect code. +Right now every comparison between different integral types requires a cast. These casts don't only clutter the code, but also encourage writing incorrect code. The easiest way to compare signed and unsigned values is to cast unsigned value to signed type. It works most of the time, but it will silently ignore overflows. From cccd48eef276426f147ea5a63bbadb1661658a42 Mon Sep 17 00:00:00 2001 From: Valerii Lashmanov Date: Tue, 6 Jun 2017 23:40:54 -0400 Subject: [PATCH 5/9] rfc file moved into right dir --- .../0000-heterogeneous-comparisons.md | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename 0000-heterogeneous-comparisons.md => text/0000-heterogeneous-comparisons.md (100%) diff --git a/0000-heterogeneous-comparisons.md b/text/0000-heterogeneous-comparisons.md similarity index 100% rename from 0000-heterogeneous-comparisons.md rename to text/0000-heterogeneous-comparisons.md From 54bef7d1c05ab463e0772bb5ca06518ebef99e91 Mon Sep 17 00:00:00 2001 From: Valerii Lashmanov Date: Wed, 7 Jun 2017 08:16:49 -0400 Subject: [PATCH 6/9] signed->unsigned typo fixed, Ord/Eq mentioned as optional, unresolved questions Ord->PartialOrd typo is fixed --- text/0000-heterogeneous-comparisons.md | 30 +++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/text/0000-heterogeneous-comparisons.md b/text/0000-heterogeneous-comparisons.md index e5b0d065543..50e892d13a1 100644 --- a/text/0000-heterogeneous-comparisons.md +++ b/text/0000-heterogeneous-comparisons.md @@ -20,13 +20,33 @@ Comparison between values of different integer types is always well-defined. The # Detailed design [design]: #detailed-design -`Ord` and `Eq` traits should be modified to allow `Rhs` type not equal to `Self`. - -After that `PartialEq`, `Eq`, `PartialOrd` and `Ord` should be implemented for all pairs of signed/unsigend 8/16/32/64/(128) bit integers and `isize`/`usize` variants. +`PartialEq` and `PartialOrd` should be implemented for all pairs of signed/unsigend 8/16/32/64/(128) bit integers and `isize`/`usize` variants. Implementation for signed-singed and unsigned-unsigned pairs should promote values to larger type first, then perform comparison. -Implementation for signed-unsigned pairs should first check if signed value less than zero. If not, then it should promote both values to signed type with the same size as larger argument type and perform comparison. +Implementation for signed-unsigned pairs should first check if signed value less than zero. If not, then it should promote both values to unsigned type with the same size as larger argument type and perform comparison. + +Example: + +``` +fn less_than(a: i32, b: u16) -> bool { + if a < 0 { + return true; + } else { + return (a as u32) < (b as u32); + } +} +``` + +Optionally `Ord` and `Eq` can be modified to allow `Rhs` type not equal to `Self`: + +``` +pub trait Eq: PartialEq { } + +pub trait Ord: Eq + PartialOrd { + fn cmp(&self, other: &Rhs) -> Ordering; +} +``` # How We Teach This [how-we-teach-this]: #how-we-teach-this @@ -49,4 +69,4 @@ No changes I can find or think of. # Unresolved questions [unresolved]: #unresolved-questions -Is `Ord` between float and int values as bad idea as it seems? +Is `PartialOrd` between float and int values as bad idea as it seems? From ca6b57491fd717bcd6969040fa9969def974ce0e Mon Sep 17 00:00:00 2001 From: Valerii Lashmanov Date: Wed, 7 Jun 2017 10:30:40 -0400 Subject: [PATCH 7/9] widening to signed added as a more efficient way to compare signed/unsigned, more examples --- text/0000-heterogeneous-comparisons.md | 28 ++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/text/0000-heterogeneous-comparisons.md b/text/0000-heterogeneous-comparisons.md index 50e892d13a1..219330c20fe 100644 --- a/text/0000-heterogeneous-comparisons.md +++ b/text/0000-heterogeneous-comparisons.md @@ -24,16 +24,36 @@ Comparison between values of different integer types is always well-defined. The Implementation for signed-singed and unsigned-unsigned pairs should promote values to larger type first, then perform comparison. -Implementation for signed-unsigned pairs should first check if signed value less than zero. If not, then it should promote both values to unsigned type with the same size as larger argument type and perform comparison. +Example: + +``` +fn less_than(a: i8, b: i16) -> bool { + (a as i16) < b +} +``` + +Implementation for signed-unsigned pairs where unsigned type is smaller than machine word size can promote both values to smallest signed type fully covering values of both argument types, then perform comparison. Example: ``` -fn less_than(a: i32, b: u16) -> bool { +fn less_than(a: i8, b: u16) -> bool { + // i32 is the smallest signed type + // which can contain any u16 value + (a as i32) < (b as i32) +} +``` + +Implementation for signed-unsigned pairs where unsigned type is as big as machine word size or larger should first check if signed value less than zero. If not, then it should promote both values to unsigned type with the same size as larger argument type and perform comparison. + +Example (for 32-bit system): + +``` +fn less_than(a: i64, b: u32) -> bool { if a < 0 { - return true; + true } else { - return (a as u32) < (b as u32); + (a as u64) < (b as u64) } } ``` From c6af8799f67f73e9db2e6f0e12ba3f68508bb2e1 Mon Sep 17 00:00:00 2001 From: Valerii Lashmanov Date: Wed, 7 Jun 2017 21:45:42 -0400 Subject: [PATCH 8/9] added note about branching into implementation, explicit branching removed from example, potential security risks mentioned --- text/0000-heterogeneous-comparisons.md | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/text/0000-heterogeneous-comparisons.md b/text/0000-heterogeneous-comparisons.md index 219330c20fe..bbc659f117e 100644 --- a/text/0000-heterogeneous-comparisons.md +++ b/text/0000-heterogeneous-comparisons.md @@ -44,20 +44,17 @@ fn less_than(a: i8, b: u16) -> bool { } ``` -Implementation for signed-unsigned pairs where unsigned type is as big as machine word size or larger should first check if signed value less than zero. If not, then it should promote both values to unsigned type with the same size as larger argument type and perform comparison. +Implementation for signed-unsigned pairs where unsigned type is as big as machine word size or larger should first check if signed value less than zero. If not, then it should promote both values to unsigned type with the same size as larger argument type and perform comparison. For most platforms it should be possible to implement it without actual branching. Example (for 32-bit system): ``` fn less_than(a: i64, b: u32) -> bool { - if a < 0 { - true - } else { - (a as u64) < (b as u64) - } + (a < 0) || ((a as u64) < (b as u64)) } ``` + Optionally `Ord` and `Eq` can be modified to allow `Rhs` type not equal to `Self`: ``` @@ -79,6 +76,7 @@ No changes I can find or think of. * It might break some code relying on return type polymorphism. It won't be possible to infer type of the second argument from type of the first one for `Eq` and `Ord`. * Correct signed-unsigned comparison requires one more operation than regular comparison. Proposed change hides this performance cost. If user doesn't care about correctness in his particular use case, then cast and comparison is faster. * The rest of rust math prohibits mixing different types and requires explicit casts. Allowing heterogeneous comparisons (and only comparisons) makes rust math somewhat inconsistent. +* For some platforms it might be necessary or more efficient to use branching in signed-unsigned comparisons (arm thumb?). Using branching will turn comparison into a non-constant-time operation. Any value-dependant operation in cryptography code is a potential security risk because of timing attacks. On other hand, on some platforms even multiplication is not guaranteed to be a constant-time operation. # Alternatives [alternatives]: #alternatives @@ -89,4 +87,5 @@ No changes I can find or think of. # Unresolved questions [unresolved]: #unresolved-questions -Is `PartialOrd` between float and int values as bad idea as it seems? +* Is `PartialOrd` between float and int values as bad idea as it seems? +* Which platforms might need branching in signed-unsigned comparison implementation? From d2612f5d9b8039b3e5ee5995fc99620fbc9d7459 Mon Sep 17 00:00:00 2001 From: Valerii Lashmanov Date: Wed, 5 Jul 2017 21:31:09 -0400 Subject: [PATCH 9/9] how we teach section updated, short description of potential changes --- text/0000-heterogeneous-comparisons.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/text/0000-heterogeneous-comparisons.md b/text/0000-heterogeneous-comparisons.md index bbc659f117e..00f09723ff1 100644 --- a/text/0000-heterogeneous-comparisons.md +++ b/text/0000-heterogeneous-comparisons.md @@ -68,7 +68,11 @@ pub trait Ord: Eq + PartialOrd { # How We Teach This [how-we-teach-this]: #how-we-teach-this -No changes I can find or think of. +As of now I can't find anything about comparisons in "The Rust Programming Language". The most reasonable place seems to be in chapter "3.2 Data Types", in or just after "Numeric Operations" paragraph (for second edition). + +We just need to mention that Rust allows comparison between different numeric types and that it returns mathematically correct result. Optionally, potential overhead can be mentioned, even though I'm not sure that's the right place to mention it. + +Rust reference seems to describe language itself and doesn't go into details about any traits/operators implementation. I can't find any reasonable place to cover it there. # Drawbacks [drawbacks]: #drawbacks