RFC: Scoped attributes for checked arithmetic

[glaebhoerl]

Pretty

[idupree]

Just to clarify: the below loop would be allowed to print "hello" any number of times from 127 to infinity (inclusive), and may print "hello" a different number of times on different runs of the program, and may print "hedgehogs" but may not print "penguins"?

fn main() {
    let mut i: i8 = 1;

    while i > 0 {
        println!("hello");
        i = i + 1;
        if i != i { println!("penguins"); }
        if i + 1 != i + 1 { println!("hedgehogs"); }
    }
}

(Thus, for example, LLVM's undef could not be used to represent the value of 100i8 + 100, because undef can be two different values in different usage points. http://llvm.org/releases/3.4/docs/LangRef.html#undefined-values : "two ‘undef‘ operands are not necessarily the same. This can be surprising to people (and also matches C semantics) where they assume that “X^X” is always zero, even if X is undefined. This isn’t true for a number of reasons, but the short answer is that an ‘undef‘ “variable” can arbitrarily change its value over its “live range”.")

[glaebhoerl]

Yes, I think that's right. Or in any case, if it precludes undef that's a good thing. :)

Presumably this is clear to you, but for anyone else reading:

• The idea is that this is very rarely a program you intentionally want to write. If you do want to write it, you should explicitly use .wrapping_add(1) instead of + 1. And if you write it accidentally, turning on checks will catch it.
• If the compiler saw 100i8 + 100, it would be free to represent it as fail!("overflow"), to issue a warning, or possibly even an error.
• The goal is not to introduce unpredictability or to allow compiler optimizations, only to keep programmers honest. I would already be very surprised if, on an actual implementation, the above program did anything other than print "hello" 127 times, and I would be shocked out of my skin if it ever actually printed hedgehogs. But it doesn't make sense to explicitly define the semantics of overflow with checks off as wrapping when the whole point is that overflow shouldn't happen. Turning overflow_checks on or off should only produce an observable difference in the behavior of the program, beyond its runtime, if the program has an overflow bug.
• In C's case, the program would be allowed to do anything, from nothing at all, to printing "LIONS, TIGERS, AND BEARS", to cat /dev/urandom > /dev/sda, to initiating the nuclear launch sequence.

[idupree]

Sounds good :)

I would be pleased if we could tighten the definition of what is allowed even more, without limiting what our desired implementations are allowed to do (such as an As-if-Infinitely-Ranged checker). I haven't thought of a good way though. For example, this is too restrictive: "On any given program run, given bit patterns X and Y, X + Y evaluates to the same value [or the same lack of a value, e.g., fail!()] everywhere X and Y are added in the program." I suppose a stern warning to compiler authors will do.

(Also, for anyone who doesn't know: Though it's undesirable here, LLVM's undef is not quite as bad as undefined behavior. "[T]he LLVM optimizer generally takes much less liberty with undefined behavior than it could." ... "[In Clang/LLVM,] Arithmetic that operates on undefined values is considered to produce a undefined value instead of producing undefined behavior. The distinction is that undefined values can't format your hard drive or produce other undesirable effects." http://blog.llvm.org/2011/05/what-every-c-programmer-should-know_21.html )

[Valloric]

Thank you for writing this up! It appears to accurately reflect the current thinking from the mailing list.

Personally, I like this proposal a lot. I think we should follow C# and make the default not check for overflow. The perf hit is too large to have it on all the time. People who always want the check can easily turn it on on a crate-level basis.

[cgaebel]

I am in favor of having overflow check for the operators off by default. People have a mental model for how fast or slow + is, and that generally doesn't include the cost of a branch.

[1fish2]

Good work!

• Note that % (and the rem method) is a 5th wrappable arithmetic operator.
• Please show an example of how a program would ask for wrapping arithmetic.
• AIUI, the reason that unchecked overflow returns an undefined result in this proposal is to encourage programmers to explicitly ask for wrapping when they need it rather than just turning off overflow checking. But it's not obvious that it'll accomplish that goal unless maybe a language runtime actually returns different values at different times.

[glaebhoerl]

@1fish2

• Good point.
• They would call the wrapping_* methods from the Wrapping* traits whose definitions were listed. I believe how calling a method works is obvious enough to not require a demonstration.
• I'd be fine if, in practice, implementations just returned the wrapped result when checks are off basically always, as long as the language spec explicitly disavows this guarantee. The language pedants helpful people on Stack Overflow and the possibility of --force-overflow-checks should be enough to keep people straight. If the idea were to encourage unpredictable results in practice as a deterrent, then I think the cure is worse than the disease.

[glaebhoerl]

One minor variation could be whether to have separate WrappingAdd, WrappingSub, etc. traits, as in the RFC, or just a single WrappingNum trait with all five (or however many) of the methods. As far as I'm concerned, the purpose of these is only to be implemented by the built-in types, and from that perspective a single trait is simpler, more convenient, and doesn't have any obvious drawback. Does anyone see an advantage of having separate traits?

(I went with separate traits when writing the RFC because I just based them on the existing Checked traits, and didn't even think of the alternative. For the built-in Add etc. traits it makes sense to separate them because they control operator overloading, and may be implemented on anything from strings to matrices, but that's not the case here.)

[glaebhoerl]

(Merged ops into WrappingOps trait and added rem.)

[arielb1]

I don't really like overflow returning "unspecified results" - then it could potentially "explode" within an unsafe block.

A better way to deal with this is to call overflow a "strict conformance violation"(Bikeshed) and require that high-quality code be "strictly conforming" - with maybe a compiler option to add checks (or, if you're feeling evil, add an option to let the compiler C-style-assume that the program is - but this would make all code unsafe - so...).

Additionally, I don't think overflow attributes are the right way to deal with this problem - a 3n-type solution (signed integers, unsigned integers, elements of Z/2^nZ) would probably be the best (I prefer still having "overflow checking" on unsigned ints, and I think "must be >=0" ints are horrible - the type-system exists for a reason).

[glaebhoerl]

First, I don't really like overflow returning "unspecified results" - then it could potentially "explode" within an unsafe block.

So can today's integer types. In fact, as far as I'm aware, it's happened.

I think a better way to deal with this is to call overflow a "strict conformance violation"(Bikeshed) and require that high-quality code be "strictly conforming" - with maybe a compiler option to add checks

Isn't this the same idea under a different cloak?

Additionally, I don't think overflow attributes are the right way to deal with this problem - a 3n-type solution (signed integers, unsigned integers, elements of Z/2^nZ) would probably be the best

Not sure I understand this right - by "signed integers" and "unsigned integers" are you implying unbounded ones, while "elements of Z/2^nZ" are the existing fixed-size ones? If so, we do have BigInt and BigUint. I think they should be made more accessible, but that's for a different RFC.

[ftxqxd]

How can % overflow? AFAIK the result is always ≤ the absolute value of either argument.

Maybe the #[overflow_checks] attribute could be changed to something like #[overflow(default|wrap|saturate|fail)], with default being undefined behaviour. This way the Wrapping<T> type is no longer needed, since one can just mark a function/block/whatever as #[overflow(wrap)].

The behaviour of out-of-range literals should also be discussed—would something like -1u64 error inside #[overflow_checks(on)], whereas simply warn (as it does today) inside #[overflow_checks(off)]?

[sfackler]

INT_MIN / -1 and therefore INT_MIN % -1 does not have an expressible result.

[bill-myers]

Actually, that's just because LLVM specifies it to be undefined so that it is possible to use a division and remainder instruction to compute it (e.g. on x86 idiv is used for that, and it triggers an exception for overflow).

Mathematically, modulus can never overflow and INT_MIN % -1 has value 0.

Not sure if it makes sense for Rust to adopt LLVM's rule or if it should just generate a check for this condition (division is already very slow, so the branch is not that bad).

[1fish2]

I mentioned % because Swift has 5 operators for "when you specifically want an overflow condition to truncate the number of available bits" including "Overflow division" &/ and "Overflow remainder" &%. [They ought to be called "Wrapping operators" or some such. They do not overflow.]
https://developer.apple.com/library/prerelease/mac/documentation/Swift/Conceptual/Swift_Programming_Language/AdvancedOperators.html

It says &/ and &% "return a value of zero if you divide by zero" instead of causing an error.

Maybe the #[overflow_checks] attribute could be changed to something like #[overflow(default|wrap|saturate|fail)], with default being undefined behaviour.

Yes. This looks great but may I suggest changing default to undefined to tell it like it is?

#[overflow(undefined|wrap|saturate|fail)]

[arielb1]

glaebhoerl:

By "exploding", I mean that a calculation within safe code overflows, and interacts badly with an inlined unsafe function or macro. Of course unsafe code needs to be careful with integers - that's why you shouldn't write lots of it.

The thing is that I don't think attributes are the right solution to the problem - the thing we want is to ensure the code does not have unintentional integer overflows during testing, but still have well-defined safe behaviour in production (because safe code shouldn't be able to behave in a non-well-defined manner). Attributes, as a per-code-block option, aren't particularly supportive of this.

About uN/iN/zN - again, I don't think that attributes are the right way to handle intentional integer wrapping (if its intentional then its not overflow) - wrapping arithmetic is not the same thing as integer arithmetic.

My proposal was to have "non-strictly-conforming-overflow" uN and iN types + a wrapping zN type (maybe the uN and zN types could be merged C-style - but I like unsigned types and prefer that code use them without being forced to use wraparound arithmetic).

This means that addition would go like (a as z32) + 2 (or (b as z32) + (c as z32)), which is slightly ugly (maybe allow zN+int addition)? Another proposal is to put wraparound arithmetic via ugly intristics wrapping_add(i, j). Actually, that may not be as ugly as I thought.

Thinking of it, I think the best solution is "strict semantics" + intristics for quotient ring arithmetic (ring_add, ring_sub, ...) - I think "ring_XYZ" would be good names - they're short, and if you don't know what a ring is you probably shouldn't be using them.

[glaebhoerl]

Maybe the #[overflow_checks] attribute could be changed to something like #[overflow(default|wrap|saturate|fail)], with default being undefined behaviour.

Yes. This looks great but may I suggest changing default to undefined to tell it like it is?

#[overflow(undefined|wrap|saturate|fail)]

The reason I don't think this is the way to go is that the official semantics of a type shouldn't magically change based on context.

The way it's written in the RFC, the official semantics of arithmetic on the built-in integers are specified such that overflow is a program error, but because of regrettable performance constraints, it's left up to the implementation whether to return an unspecified result, or to terminate normal execution in some fashion. And the implementation, in this case rustc, implements this by checking for overflow at runtime in overflow_checks(on) blocks, and returning an unspecified result, but almost certainly wrapping, in off blocks.

If overflow is normal behavior, and e.g. wraparound is required, then different operations, e.g. wrapping_add should be used.

For more background on the meaning of "unspecified result" see this mini-conversation with @idupree.

---

@P1start

The behaviour of out-of-range literals should also be discussed—would something like -1u64 error inside #[overflow_checks(on)], whereas simply warn (as it does today) inside #[overflow_checks(off)]?

I think it should behave the same in both instances: it's a program error either way, and the attribute is only a runtime thing the programmer decides based on whether or not the performance cost of checking is acceptable. Even in the off case, the compiler could represent it as fail!("overflow"), because it doesn't involve adding runtime overflow checks. It could also emit a warning or error at compile time (again, irrespective of the attribute). I would personally lean towards a warning, on the basis that errors should be for things that are guaranteed to be caught, but overflow can only be caught at compile time in limited circumstances. But I wouldn't be opposed to an error either.

---

@bill-myers (% overflow)

Not sure if it makes sense for Rust to adopt LLVM's rule or if it should just generate a check for this condition (division is already very slow, so the branch is not that bad).

I don't really mind either way. I'll update the RFC if there's a consensus around either option. (Or this change can be proposed in a separate RFC - I assume we currently do whatever LLVM does.)

---

@arielb1

By "exploding", I mean that a calculation within safe code overflows, and interacts badly with an inlined unsafe function or macro. Of course unsafe code needs to be careful with integers - that's why you shouldn't write lots of it.

If code isn't safe for all possible inputs, then it's not safe. Therefore, if it's a function, it has to be an unsafe fn. If it's an unsafe fn, it can only be used inside unsafe blocks. So this is not a separate case from "you have to be careful with unsafe blocks".

Anyways, my previous remark still holds. Currently the integer types are defined to wrap around on overflow. Outside of a few specialized cases like hash functions and checksums, this doesn't make any sense, and programs aren't prepared to handle it. If an integer overflows, it wraps around, and if it's used in an unsafe block, it may explode. I'm lead to believe that this has already happened in some instances. This RFC is an improvement in the respect that you can enable overflow checks for that code where you couldn't before.

again, I don't think that attributes are the right way to handle intentional integer wrapping (if its intentional then its not overflow) - wrapping arithmetic is not the same thing as integer arithmetic.

I agree, and this is why the RFC has a WrappingOps trait for when wrapping arithmetic is explicitly intended.

Another proposal is to put wraparound arithmetic via ugly intristics wrapping_add(i, j). Actually, that may not be as ugly as I thought.

Yes, the RFC specifies i.wrapping_add(j).

I encourage you to read the mailing list thread, in case you haven't, and maybe give the RFC another look, because many of the things you wrote have already been discussed (several times), and in some cases addressed.

[1fish2]

@glaebhoerl Agreed. Sorry for my brain fart about #[overflow(...)]. You put it very well and clearly:

The reason I don't think this is the way to go is that the official semantics of a type shouldn't magically change based on context.

The way it's written in the RFC, the official semantics of arithmetic on the built-in integers are specified such that overflow is a program error, but because of regrettable performance constraints, it's left up to the implementation whether to return an unspecified result, or to terminate normal execution in some fashion. And the implementation, in this case rustc, implements this by checking for overflow at runtime in overflow_checks(on) blocks, and returning an unspecified result, but almost certainly wrapping, in off blocks.

[1fish2]

I think this operator list should include as conversion to an integer type. Otherwise computing a value in one numeric type then converting would accidentally bypass the overflow check.

#161 (comment)

[glaebhoerl]

Good idea, I'll add that shortly.

[glaebhoerl]

I've updated the RFC with points from the discussion here.

@idupree @bill-myers: I've lifted parts of your comments into the text wholesale. I hope this is OK!

[nikomatsakis]

I just want to say that this is a wonderfully written RFC and it was a joy to read.

[glaebhoerl]

Thank you!

[idupree]

@glaebhoerl Using my writing is okay with me!

[rpjohnst]

This doesn't have a lot of precedent in Rust, but using attributes on types is another solution (similar to . Much like C's unsigned, there could be checked, saturating, wrapping, etc. This should make things less confusing (don't have to choose among so many types).

API compatibility hazards could be avoided by letting the types be easily converted between each other- there would be relatively little danger or unsafety compared to the proposed solution, and purposeful uses of different styles of arithmetic would be more obvious than the current situation.

[huonw]

@alexchandel

First, "the actual performance of checked arithmetic is wholly irrelevant to the content of this proposal" is misleading and false, since checked arithmetic necessarily has terrible performance, as argued above.

It is clarified in the next sentence:

It will be relevant to the decisions programmers make when employing the tools this proposal proposes to supply them with, but to the tools themselves it is not.

The language can provide tools for detecting overflow independently of the performance of overflow detection.

Second, for me, making integer overflow "undefined" runs contrary to very idea of Rust.

Could you expand on this in more detail? What part of your idea of Rust is it violating?

I use overflow often, and having to opt in would be inelegant and yield ugly code.

This RFC acknowledges that many/most people do not want overflows, or only want overflows in small pieces of their code (e.g. it is likely true that the only place in rustc where overflow is desired is hashing computations and possibly constant evaluation, and everywhere else overflow would be incorrect).

In any case, it seems to me that the features as written in this RFC do not lead to either of those things, since one can just add #![overflow_checks(off)] to whole crates.

[aidancully]

Coming to this late, but I've deliberately used wrapping operations in places besides those named. An embedded RTOS can record the current time as the number of timer ticks that have occurred since startup. When the timer ticks every millisecond (a very common configuration), and this number is stored in a u32, this will wrap about once every 50 days. If you want to know the number of milliseconds that have elapsed since an event, the operation current_time() - event_time will result in the correct value when using wrapping operations, even around the 50-day boundary condition. (We don't perform the calculation in its bare form, shown here, we prefer to use wrapping functions for safety, and b/c of signed / unsigned conversion gotchas. But still, the function is much more efficient with wrapping operations.)

This is just one example of where I've relied on wrapping arithmetic, though they basically all take this form: wrapping operations are useful for working with deltas against monotonically increasing values, where the range of the value can be expressed as [0..2^x), and overflow is likely.

[aidancully]

For what it's worth, I'd second rpjohnst's comment above - this feels like a more natural fit in the type system than in specific expressions. For example, when working with hashing operations where wrapping operations are desirable, it is a mistake to use a non-wrapping operation. I'd never implement something like this, but consider the following:

struct HashCalculator {
    mult: i32;
    add: i32;
}

impl HashCalculator {
    fn perform_mult(&self, input: i32) -> i32 {
        input * self.mult
    }
    fn perform_add(&self, input: i32) -> i32 {
        #[overflow_checks(off)]
        { input + self.add }
    }
    fn perform_op(&self, input: 32) -> i32 {
        self.perform_add(self.perform_mult(input))
    }
}

There are too many places to keep track of which type of operation should be performed, it's more bug-prone than if we could record the attribute against a newtype HashValue type.

[glaebhoerl]

@aidancully Thanks, more use cases for wrapping arithmetic are definitely good to know about. However, this didn't make me change my opinion about any of the decisions in the RFC - was it intended to? The fact that wrapping operations work well for deltas as you describe is a nifty but nontrivial fact: you have to think about what wrapping will do, and whether or not it will do what you want, so the fact that wrapping is explicitly desired here should be reflected in the source, whether as a .wrapping_sub() call or a Wrapping<u32> type (both of which are described in the RFC). For the hashing example, you could use mult: Wrapping<i32>, add: Wrapping<i32> and (possibly after adding casts for the fn arguments as well) it will presumably do what you want. Again as laid out in the RFC, #[overflow_checks(off)] is not intended to be a way to request wrapping semantics (for that, use explicit wrapping_* calls or Wrapping<T> types), it is strictly a performance decision. ("Overflow is still wrong, but in this hot loop, we can't spare the cycles to check for it.")

[aidancully]

@glaebhoerl Sorry, I misread the RFC earlier - I didn't notice the Wrapping<?> types. If I had noticed them, I wouldn't have made the point the same way.

That said, I haven't seen (and have a hard time imagining) a use-case where wrapping operations should sometimes be used against a particular variable, while bounds-checking operations should be used against the same variable at other times. For example, I can't imagine why you'd ever do something like:

fn foo(a: i32) -> i32 {
    // using both wrapping and bounds-checking operations at once.
    a.wrapping_mul(1001) + 45
}

It seems to me that, for a given variable, it should always use the same type of arithmetic operation (where by "type of operation" I mean, saturating, bounds-checking, or what-have-you), and that it's generally nonsensical for that variable to use different types of operation in different places. At the least, this is likely the most common case - it would almost always be an error for the same variable to use, say, a saturating operation in one place and a wrapping operation in another. As such, I think the choice of what type of operations one wants to use belongs in the type system, rather than by multiplying the number of operators that one can choose from when using a single type. (That is, I'd like to continue using + to mean addition, with the specific type of addition being chosen by the variable type, than to be forced to choose between +, or wrapping_add, or saturating_add, with all forms valid in the same code.) I'd like the code snippet in this comment to fail to compile, and I consider it beneficial that vec<int> and vec<Wrapping<int>> are incompatible.

If we have what I think is the uncommon case, and different types of operations are necessary against the same underlying variable, then the desired type of operation can be chosen via type-casts: let i:Wrapping<i32> = 3i; ((i + 5) as Saturating<i32>) * 100 (or something like that).

Thanks

[glaebhoerl]

It seems to me that, for a given variable, it should always use the same type of arithmetic operation (where by "type of operation" I mean, saturating, bounds-checking, or what-have-you), and that it's generally nonsensical for that variable to use different types of operation in different places.

This sounds plausible, but do you perhaps have a theoretical explanation of why this is the case (if it is)?

The other issue is that the LHS and RHS operands would often be different types. What should happen then? Should the overflow mode also "infect" the types of the function's arguments? Or should the function body cast them appropriately? IIRC Jerry Morrison (@1fish2) had some good arguments and examples regarding this subject in the original mailing list discussion (linked from the text).

[aidancully]

This sounds plausible, but do you perhaps have a theoretical explanation of why this is the case (if it is)?

I don't have a hard-and-fast rationale. Heuristically, can you (or anyone) think of an instance where that argument doesn't hold? Hash calculations should always use wrapping operations, array indices should always use bounds-checked operations, some graphics operations (lighting calculation, perhaps?) should always use saturating operations (can't get a brighter color in common 24-bit RGB representation than 0xFFFFFF, or darker than 0x000000). The difficulty in coming up with cases where we'd reasonably mix types of operations is (to me) a strong indication that, in general, if types of operations are mixed, it's likely to be a bug, and the compiler can likely catch it. If it isn't a bug, then there's still an escape hatch available by type-casting intermediates to use other types of operation.

For a soft theoretical explanation, I would say that any variable is a representation of some domain concept. It's the domain concept which dictates what type of arithmetic is appropriate. For example, that a hash value is a number calculated through arithmetic operations is essentially an implementation detail. The concept is that we want a function mapping a large domain to a small range, with very low chance of collision. In this case, arithmetic operations are just a convenient way of defining such a function. We don't particularly care about how much sense the operations make in isolation (like, why would someone in general want to multiply a number by some arbitrary constant, then add another arbitrary constant? the reason we do so here is related to how the hash calculation function does not make sense outside of hash calculation), but we do care that the range is not artificially constrained in our calculation operation. Wrapping operations are always the correct choice for hash calculation, Similarly, for an RGB value, our domain dictates that an individual color can't be less than 0, nor greater than 255, so saturating operations seem more appropriate. For representing time since system boot, it's important that the time value keep increasing, so supporting wrapping is necessary. In all of these cases, it's the domain concept (hash value, color intensity, time since boot) that dictates what type of arithmetic operation to use... And representing the operations that are valid against a domain concept is what a type system is for.

Sorry to be so wordy... I hope that was helpful.

The other issue is that the LHS and RHS operands would often be different types. What should happen then? Should the overflow mode also "infect" the types of the function's arguments? Or should the function body cast them appropriately? IIRC Jerry Morrison (@1fish2) had some good arguments and examples regarding this subject in the original mailing list discussion (linked from the text).

I'd compare it to :u32 + :u8. That is, promotion to the proper form to make these expressions make sense is theoretically possible (C has a lot of rules for cases like this), but Rust currently punts, and asks programmers to write well-typed expressions in the first place. I'd probably default to the same approach: compilation error if the LHS and RHS disagree.

[1fish2]

... any variable is a representation of some domain concept. It's the domain concept which dictates what type of arithmetic is appropriate.

Let's take an example from the Effective Java section on how to write a good hash method (for a PhoneNumber class):

@Override public int hashCode() {
    int result = 17;
    result = 31 * result + areaCode;
    result = 31 * result + prefix;
    result = 31 * result + lineNumber;
    return result;
}

These arithmetic operations are meant to be wrapping operations, so in Rust you might declare result as Wrapping<i32>.

But areaCode, prefix, lineNumber, 17, and 31 are not wrapping integers. These operations all have mixed type operands. How is the compiler to know whether to use the modular arithmetic of result or the non-modular arithmetic of areaCode?

After computing a hash, what do you do with it? Generally you pass it around, compare it for equality, and index into a hash table hashTable[hashCode % numberOfElements]. Computing the hash table index is modulo the numberOfElements, and whatever hash table indexing you might decide to do, it should not wraparound modulo 2³². Indexing is always array-bounds-checked, and all the other operations would be wrong if they overflowed.

So the hashing domain concept doesn't suffice to make the hash computation wrap-around, nor does it fit the other things you do with a hash value.

[1fish2]

(BTW I consider the term "overflow" to include underflow. I.e. compute an out-of-range value.)

[aidancully]

@1fish2, I actually think there are three domain concepts at work, in the example, and each concept should support different operations:

1. Telephone number, which must be used by a
2. Hash calculation (which requires Hash intermediate values), which results in a
3. Hash value.
   (Optional) 4. The hash value may need to be converted into another form (like array index, say) by the caller.

What arithmetic operations are natural against a telephone number? In general, I'd say the answer is none, it's nonsensical to write area_code * 3 (for example). (Aside: what about including numeric types that don't support arithmetic?) So the telephone number first must be translated into the hash calculation domain, in which wrapping operations are allowed. In rust, that'd be something like:

fn calculate_hash(tel: &TelephoneNumber) -> HashResult {
    let mut result: Wrapping<u32> = 17;
    result = 31 * result + TelephoneNumber.area_code as Wrapping<u32>;
    result = 31 * result + TelephoneNumber.prefix as Wrapping<u32>;
    result = 31 * result + TelephoneNumber.line_number as Wrapping<u32>;
    result as HashResult
}

I see that you argued against this form in the email @glaebhoerl referenced from the RFC, arguing that this way to write the function is more bug-prone (at least in Java) than it is to use an explicit wrapping operator. That might be true in Java (though I'm not yet convinced so), but I'm pretty convinced that using the type system would be more robust in rust: It is always a mistake to use a non-wrapping operation when computing a hash. Keeping the operation selection in the type system prevents incorrectly using a bounds-checking or (heaven-forbid) saturating operation when calculating the hash. It is harder to write buggy code when the type system prevents you from doing so. The program should fail to type-check if you attempt to use the wrong form of number in calculating the operation.

I don't consider it that onerous to typecast the telephone number's input fields to use them as hash intermediate values... That sort of thing will be necessary any time you're hashing something that isn't a number (like, say, a string-slice name), and believe it or not, I kind of like the parallelism of requiring a similar conversion to hash calculation domain no matter the input type. In any event, it's certainly a price I'm willing to pay for increased safety.

[glaebhoerl]

@aidancully @1fish2 Thanks for the discussion, this is somewhat persuasive to me. It's certainly good design to use newtypes to distinguish values with equal representations but different meanings, and I can see how this can extend to overflow semantics as well.

That said, it still doesn't make me think that the design in the RFC should be changed in any way. A single Wrapping<T> type seems better than a whole parallel family of unrelated types i8w, i16w, and so on; and building it on top of a WrappingOps trait implemented for all the usual integer types also seems sensible. Both options are then available, and if someone considers Wrapping<T> a better choice for their design, they are free to use it. I don't have the feeling that using individual .wrapping_add(), etc. operations is such a universally bad idea that it should be discouraged to the extent of refusing to provide them at all.

[aidancully]

I know I started reading the wrong version of the RFC at first (I have to apologize, I'm still getting used to github after doing almost entirely closed-source development for a very long time)... I mostly don't object to the technical decisions in the RFC (though I have different biases, and probably wouldn't have made the same decisions), but I will say that text in the RFC like this:

Note that this is only for potential convenience. The type-based approach has the drawback that e.g. Vec<int> and Vec<Wrapping<int>> are incompatible types. The recommendation is to not use Vec<Wrapping<int>>, but to use Vec<int> and the wrapping_* methods directly, instead.

(describing the intention behind Wrapping<T> types) rubs me the wrong way, and is certainly the part I'm responding to most at this point. To me, it's a benefit that Vec<int> and Vec<Wrapping<int>> are incompatible, as I can't imagine that the ints contained by these two Vecs could be used interchangeably. That is, my recommendation would almost always be to use the Wrapping<T> types. And I'd also investigate adding Saturating<T> (for saturating arithmetic) and Value<T> (for disabling operators) as well, and perhaps even a Checked<T> and Unchecked<T> to make an explicit choice to use or not-use bounds-checked arithmetic (and, I suppose, preventing optimizing the checks out in optimized builds). I don't want to pretend that my opinion matters more than it does, obviously I'm new here. But if the recommendation were changed to prefer using wrapping types over wrapping operators, then that closes a lot of the gap for me (and actually makes scoped attributes mostly, if not entirely, unnecessary). And if the additonal "operator types" were added (Saturating<T>, Checked<T>, etc.), then I'd be happy.

[glaebhoerl]

Yeah, that's basically what I was trying to say: some of the motivating text, "filler" might turn out different if I were to write it again, but the decisions themselves still feel sound. What the actual text under the Wrapping<T> section in this RFC says is basically irrelevant to how people are actually going to use it in practice, so I'm not really bothered by it one way or the other. :)

(I think a SaturatingOps trait and Saturating<T> type on the same model as the existing wrapping ones seem like they would be straightforward to do; the reason wrapping is "special" is that it's existing functionality which must continue to be supported, whereas saturation, and others, would be mostly new. So I think they can be left to a followup RFC.)

[thestinger]

To me, it's a benefit that Vec and Vec<Wrapping> are incompatible

It's a major performance loss for safe code.

[aidancully]

@thestinger, why is it a performance loss? My assumption is that Wrapping<i32> will be represented identically with i32, so that multiple physical Vec implementations aren't technically necessary.

[thestinger]

Vec<T> and Vec<U<T>> aren't the same type. Getting Vec<U> from Vec<T> from Vec<U<T>> is O(n). It's a problem in general, not just with vectors.

[thestinger]

Rust already has a lot of friction, and stuff like this is going to make it a lot worse. The interest of system programmers dies out more and more as changes like opt-in Copy keep landing.

[1fish2]

@aidancully I'm with you on thinking in terms of domain concepts but skeptical that static types will help manage those differences. Operations happen to the values in transition between domains. (I once tried using types to track dimensional units. It didn't work at all.)

Anyway, go ahead and implement Saturating<N> et al and test them out. If they prove useful, you can publish a crate or a pull request.

The relevant point for this RFC is: When we want numeric wraparound, we must tell the compiler. Do not rely on it to ignore overflow by doing wraparound. We could use wraparound types, methods, or operators. One should selectively turn off overflow checks (that's what the scoped attributes are for) for performance, not to request wraparound behavior.

What arithmetic operations are natural against a telephone number?

That's a fun question. There's at least equals and hash code for comparison and table lookup. Also div/mod to convert to a string and as part of testing if an areaCode is a toll-free number. number + 1 for war-driving.

[1fish2]

To say that in a less tangled way: To get numeric wraparound, use wraparound types, methods, or operators. To turn off overflow checking for performance, use scoped attributes.

[aidancully]

@thestinger, AFAICT, you didn't actually address why the performance would be worse. At the most, your argument reads like you may end up with duplicated code (if Vec<i32> uses a different concrete implementation than Vec<Wrapping<i32>>), but if i32 is represented the same way as Wrapping<i32>, then why wouldn't the performance of each copy of the Vec code be exactly the same? What am I missing?

I'm also skeptical that comparing for type representational equality is that computationally expensive... (Incidentally, what is the "n" in your O(n) for comparing Vec<U<T>> and Vec<T>?) I'm assuming Wrapping<i32> would be declared as a newtype from i32, and since Vec<T> doesn't rely on any properties of T, it'd be reasonable for the compiler to simplify Wrapping<i32> to i32 for purposes of code-generation. If this is more difficult than I give credit for (which I could believe), it's still something that could maybe be enabled at high optimization levels?

And, FWIW, I work in embedded systems programming. What's attractive to me about Rust is that it makes a lot of the abstractions and safety measures developed for higher-level languages finally available for use in this domain. I don't claim to speak for anyone but myself, but my interest actually increases as more high-level ideas are made usable in this domain (where "usable", to me, requires things like performance being easy to reason about, and maintaining a reasonably clear relationship between generated object code and input source).

@1fish2, yes, I agree with that summary, that's where I am now, too. Thank you!

[vadimcn]

To me, it's a benefit that Vec and Vec> are incompatible

It's a major performance loss for safe code.
...
Vec and Vec<U> aren't the same type. Getting Vec from Vec from Vec<U> is O(n). It's a problem in general, not just with vectors.

Can we rely on Wrapping<u32> having the same representation as u32?

If so, we could address your concerns by providing functions in the library that re-borrow &mut [Wrapping<u32>] as &mut [u32] and vice-versa. They'd use unsafe{} internally, of course.

[Thiez]

I think if Wrapping<u32> has #[repr(C)] it should have the same representation as u32. Without #[repr(C)] you have no business making any assumptions about the representation. In this case it would be rather unlikely for it to be different, but you should not rely on it and transmute between them.

[Ericson2314]

IIRC, there have been proposals for more newtyping support, which would allow one to do let a: Vec<Wrapping<int>> = ...; a as Vec<int>. Associated types make this difficult, but its still possible.

[nikomatsakis]

Closing in favor of #560.