[SR-5762] Clarify documentation for `FixedWidthInteger.unsafe*(_:)`

[swift-ci]

Previous ID	SR-5762
Radar	None
Original Reporter	Jnosh (JIRA User)
Type	Improvement
Status	In Progress
Resolution

Additional Detail from JIRA

| | | |------------------|-----------------| |Votes | 0 | |Component/s | Standard Library | |Labels | Improvement | |Assignee | @natecook1000 | |Priority | Medium | md5: 8bd2abc8b1aa6bb05ea490f82609bf8b

Issue Description:

The documentation for the FixedWidthInteger.unsafe*(_:) group of methods (unsafeAdding, unsafeSubtracting, unsafeMultiplied, unsafeDivided) states:

If an arithmetic overflow occurs, the behavior is undefined. Use this function only to avoid the cost of overflow checking when you are sure that the operation won’t overflow.

I recently had a short conversation with @belkadan on Twitter (Link, Link) about these and and he mentioned that it would probably be best not to describe this as undefined behaviour (given the associated meaning from C-family compilers, I assume; at least that's where my mind went upon reading the documentation).

Personally, I think it might also be helpful to explicitly state that these are not equivalent to the respective &\+, &-, &* operators. At least I naively assumed them to perform the same operation.

Although this might be less necessary with the current documentation for &\+, &-, &* since it explicitly differs from the documentation for FixedWidthInteger.unsafe*. At the time I was looking at older documentation for &\+, &-, &* which was still very sparse.

[belkadan]

cc @natecook1000

[natecook1000]

Thanks Jordan, sorry to miss the Twitter cc!

[swift-ci]

Comment by Janosch Hildebrand (JIRA)

I had a bit of a go at this myself; In case it is helpful here is what I came up with, inspired mostly by the documentation for precondition and unsafelyUnwrapped:

  /// Returns the sum of this value and the given value without checking for
  /// arithmetic overflow.
  ///
  /// If an arithmetic overflow occurs, the behavior is undefined. Use this
  /// function only to avoid the cost of overflow checking when you are sure
  /// that the operation won't overflow.
  /// Use this function only to avoid the cost of overflow checking when you are
  /// certain that the operation won't overflow. In optimized builds (`-O`) the
  /// compiler is free to assume that overflow won't occur and failure to
  /// satisfy that assumption is a serious programming error.
  ///
  /// In debug builds (`-Onone`) a check is still performed and a runtime error
  /// triggered if the operation overflows.
  ///
  /// - Parameter rhs: The value to add to this value.
  /// - Returns: The sum of this value and `rhs`.

Not sure if this is going in the direction you had in mind. I'm also not really happy that it ultimately punts on describing the consequences (an approach I borrowed from precondition).
However I don't know how to concisely describe it without landing back on 'undefined behaviour'.

I assume the semantics of this in Swift is somewhere along the lines of LLVM undef/poison? Which, from what I understand is defined to be more limited than classic C "can erase your hard drive" undefined behaviour but I don't know of any term of art for it?

I bring this up also because there seem to be a few other cases of documentation that mention 'undefined behaviour', at least some of which probably have similar semantics as this case. unsafelyUnwrapped for example? So if you're thinking of moving away from using 'undefined behaviour' in the documentation in general, some adequate replacement terminology would be helpful...

Anyway, not sure if any of that is particularly constructive so feel free to just ignore me :-)

[belkadan]

Maybe something like "the compiler is free to optimize your code assuming that overflow will never occur, which could lead to statements unexpectedly being executed or skipped"?

[swift-ci]

Comment by Janosch Hildebrand (JIRA)

I like that a lot. That should also help people not familiar with 'undefined behaviour' and should be adaptable to most if not all places where 'undefined behaviour' is currently used.

[belkadan]

The other piece I didn't include was "memory corruption", if you want to stir that in, Nate.