[SR-5964] dividedReportingOverflow may fail to compile for calculations that overflow

[martinr448]

Previous ID	SR-5964
Radar	None
Original Reporter	@martinr448
Type	Bug

Attachment: Download

Environment

macOS 10.12.6 Xcode 9.0 (9A235) Apple Swift version 4.0 (swiftlang-900.0.65 clang-900.0.37) Target: x86_64-apple-macosx10.9

Additional Detail from JIRA

| | | |------------------|-----------------| |Votes | 1 | |Component/s | Compiler, Standard Library | |Labels | Bug | |Assignee | @moiseev | |Priority | Medium | md5: 3e39aeb0aec9075d1c78c3f6037e42b8

relates to:

• SR-3535 divideWithOverflow gives Overflow error with edge case

Issue Description:

The documentation of

public func dividedReportingOverflow(by rhs: Self) -> (partialValue: Self, overflow: Bool)

from the FixedWidthInteger protocol states that overflows are indicated by a flag in the return value, and that dividing by zero is not an error.

However, it may fail to even compile in cases where the compiler already detects an overflow or division by zero:

// main1.swift
do {
    let minusOne = -1
    let r1 = Int.min.dividedReportingOverflow(by: minusOne)
    print(r1)

    let zero = 0
    let r2 = Int.min.dividedReportingOverflow(by: zero)
    print(r2)
}

$ swiftc main1.swift 
main1.swift:7:19: error: division by zero
        let r2 = Int.min.dividedReportingOverflow(by: zero)
                         ^
main1.swift:3:19: error: division '-9223372036854775808 / -1' results in an overflow
        let r1 = Int.min.dividedReportingOverflow(by: minusOne)

If the same code is on top-level then it compiles and runs as expected:

// main2.swift
let minusOne = -1
let r1 = Int.min.dividedReportingOverflow(by: minusOne)
print(r1)

let zero = 0
let r2 = Int.min.dividedReportingOverflow(by: zero)
print(r2)

$ swiftc main2.swift && ./main2 
(partialValue: -9223372036854775808, overflow: true)
(partialValue: -9223372036854775808, overflow: true)

[belkadan]

I'm not sure what the desired behavior here is. There's no point in statically generating an overflow because you already know the answer, right?

@moiseev, what do you think?

[ee21ea02-9d7a-4385-8c3c-ad21e8e490a8]

The problem is that the reportingOverflow and regular functions are dispatch through the same function, and we do want to get the static overflow checks in case of regular ones.

I'm on the fence about it, honestly. From where I stand, yes, it is a behavior that is inconsistent with the documentation in a subtle way, but it only behaves this way in rare cases where constant folding can figure out the concrete values, and as the description shows, it is even not the case for top-level... So anything non-trivial (as in, pretty much anything not-using literals) will compile just fine.

Regardless, this is another manifestation of a known problem that we're going to address (see my first paragraph).

Thanks for reporting it @martinr448!

[ee21ea02-9d7a-4385-8c3c-ad21e8e490a8]

Related to: \rdar://problem/29937936\

[swift-ci]

Comment by Peter W A Wood (JIRA)

I tried Int8.min.dividedReportingOverflow(by: int8(-1)) in an iOS playground. It returns a tuple with partialValue of -128 and overflow of true. This is different from the behaviour in an Xcode playground. I don't know if the difference is due to the different playgrounds or the compiler.

I first noticed this "edge case" issue when I was generating some tests to run in another language. I switched from Swift to C and found the same issue. Then I tried dividing the minimum integer value by -1 using C on both Intel and ARM processors. (The first in Xcode, the second using gcc on a Raspberry Pi). On Intel the division caused an overflow error but not on ARM.

I thought it best to mention this in case it is relevant to the issue.

[ee21ea02-9d7a-4385-8c3c-ad21e8e490a8]

/cc @stephentyrone on the above Intel vs ARM issue.

[stephentyrone]

Constant folding only gets smarter, so while this may effect only a few cases today, it will likely effect more cases in the future. We should be able to make this do what it says on the tin, and (I think) we all know that's ultimately the correct behavior.

It sounds like there's two issues here–getting a static error and the Int8 behavior which is just giving the wrong answer entirely, right?

[ee21ea02-9d7a-4385-8c3c-ad21e8e490a8]

Yes, the static error is clear, the overflow on different architectures is not, at least not to me =)

[swift-ci]

Comment by Andrew Bennett (JIRA)

+1 on this. The documentation states:

Dividing by zero is not an error when using this method. For a value x, the result of x.dividedReportingOverflow(by: 0) is (x, true).

However there is an error "Division by zero", at compile time.

A satisfactory resolution, to me, would be to optimise out the call and replace it with (x, true) when overflow is statically detected.

If it helps find a satisfactory conclusion, I wanted to write a test to ensure that my custom FixedWidthInteger implementation returns (x, true) when passed zero. This is a situation where you would not want a compile-time error.

[swift-ci]

Comment by Steffan Andrews (JIRA)

+1 as well.

I echo bnut (JIRA User)'s observation and sentiment. Despite the documentation's claim that dividing by 0 should return `(x, true)`, the compiler throws an error.

This is still occurring in an Xcode 12.3 GM Swift 5.3 playground.

[NevinBR]

It is not clear to me why “Division by zero” is a compile-time error at all.

At most it should be a warning.

After all, we don’t raise compile-time errors for other code which is guaranteed to trap at runtime, such as fatalError().

If the programmer writes code that always traps, they clearly want it to trap at runtime if execution reaches that point.