swiftlang / swift

The Swift Programming Language
https://swift.org
Apache License 2.0
67.38k stars 10.34k forks source link

[SR-3953] Unary sign operator on integer literal gives "ambiguous use of operator" error #46538

Open swift-ci opened 7 years ago

swift-ci commented 7 years ago
Previous ID SR-3953
Radar None
Original Reporter chowell (JIRA User)
Type Bug
Environment macOS 10.12.3, MacBook Pro (Retina, 15-inch, Mid 2014) Swift version 3.1-dev (LLVM 3887fe8485, Clang f9a0f935a5, Swift 59037e9fdc)
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 0 | |Component/s | Compiler | |Labels | Bug, TypeChecker | |Assignee | @moiseev | |Priority | Medium | md5: 9859694b560d4f584e18ea58a30be0d0

Issue Description:

Swift will produce "ambiguous use of operator" errors from trivial expressions of a unary sign operator on an integer literal. Similar expressions using floating-point literals are unaffected. This is not a recent bug; it's also present in the Swift 3.0.2 and Swift 2.3 releases.

Here are some examples. First, the trivial input of a unary + on an integer literal:

+1

gives the output:

<stdin>:1:1: error: ambiguous use of operator '+'
+1
^
Swift.+:1:20: note: found this candidate
prefix public func +(x: Float) -> Float
                   ^
Swift.+:1:20: note: found this candidate
prefix public func +(x: Double) -> Double
                   ^
Swift.+:1:20: note: found this candidate
prefix public func +(x: Float80) -> Float80
                   ^

While an integer literal has no predefined type, it defaults to Int in the absence of further information. Yet from examining the constraint solver's debugging output, the generic operator

public prefix func + <T : SignedNumber>(x: T) -> T

which should satisfy this, doesn't seem to be actually checked by the solver as a potential solution, even though the solver lists it among the potential overloads of unary +.

This is inconsistent with Swift's treatment of

+1.0

which gives an unambiguous result, as expected.

The input:

-1

doesn't have this problem, but only because the minus sign is treated as part of the literal. If we use parentheses to force the use of the unary - operator:

-(1)

Swift again gives "ambiguous use" errors:

<stdin>:1:1: error: ambiguous use of operator '-'
-(1)
^
Swift.-:1:20: note: found this candidate
prefix public func -(x: Float) -> Float
                   ^
Swift.-:1:20: note: found this candidate
prefix public func -(x: Double) -> Double
                   ^
Swift.-:1:20: note: found this candidate
prefix public func -(x: Float80) -> Float80
                   ^

Again, the generic operators:

public prefix func - <T : SignedNumber>(x: T) -> T
public prefix func - <T: SignedArithmetic>(x: T) -> T

are apparently never checked by the constraint solver, even though it lists them among the overloads of unary -.

By constrast, the input:

-(1.0)

is unambiguous, as expected.

Incidentally, this bug appears to be "Case 1" in SR-2459, and I've found two references to what seems to be the same bug on Stack Overflow: one from over a year ago, and one from just this morning.

swift-ci commented 7 years ago

Comment by Colin Douglas Howell (JIRA)

Found out why the generic operators are never considered. Thanks to commit 752828465f86f83107f93e44018b7cb87baa952d (named "Type checker performance hack"), if a disjunction constraint is a binding of an overloaded operator with both generic and non-generic versions, this constraint will be short-circuited before any of the generic versions are checked, as long as any of the non-generic versions can produce a solution. In this case, the non-generic floating-point versions of unary + and - are preferred, and the generic ones (both floating-point and integer) are ignored.

I'm not sure what the preferred way to solve this would be. One obvious solution would be to add non-generic integer versions of unary + and - to the standard library, but I suspect such a change would be forbidden for Swift 3. Perhaps it might make more sense to refine the mechanism for short-circuiting disjunctions.

(Pinging @DougGregor because the commit I mentioned was yours and this code in general is your area.)

DougGregor commented 7 years ago

Yeah, I knew this hack was unprincipled when I did it back in 2014. [SE-0091](https://github.com/apple/swift-evolution/blob/master/proposals/0091-improving-operators-in-protocols.md) is intended to provide a path forward here, but it isn't implemented yet.

swift-ci commented 7 years ago

Comment by Colin Douglas Howell (JIRA)

Is there any possibility for a quicker workaround before SE-0091 is implemented?

As a short-term fix, would it be acceptable to add non-generic integer versions of unary + and - to the standard library, or would that be a language-breaking change? Most other operators seem to already have such versions.

(I'm trying to think about other possibilities as well, but I'm not sure if they are reasonable.)

DougGregor commented 7 years ago

Yeah, that might be a reasonable workaround. @moiseev, what do you think about adding concrete versions of unary +/- for integers?

ee21ea02-9d7a-4385-8c3c-ad21e8e490a8 commented 7 years ago

In the new-integer-protocols branch, where the unary + is declared on the Arithmetic protocol (or Number or whatever it ends up being called) and thus is applied to all the number-like things. No ambiguity is reported in that case.
As for this particular problem, I don't mind at all adding an overload.
It is not clear to me, however, why we would need an overload for -? It seems to work fine.

ee21ea02-9d7a-4385-8c3c-ad21e8e490a8 commented 7 years ago

After having discussed it with @DougGregor we decided it is too risky to do it for Swift 3.1 now, but provide a solution in Swift 4.

swift-ci commented 7 years ago

Comment by Colin Douglas Howell (JIRA)

- works fine in the usual case (such as -1) because it's not treated as an operator at all, but as part of the literal. To get the operator and see the ambiguity, you have to use parentheses (such as -(1)). Obviously that would be a much less common thing for a user to do.

That said, I understand your decision.

ee21ea02-9d7a-4385-8c3c-ad21e8e490a8 commented 7 years ago

Ah. Right. Thanks!

swift-ci commented 7 years ago

Comment by Colin Douglas Howell (JIRA)

Let me mention another possibility I had been thinking about, just to see what you think. I understand it may not be reasonable, and even if it is worth considering, it is probably too late for Swift 3.1. I just want to bounce the idea off you rather than letting it remain locked in my head.

I understand that when the constraint solver first generates its constraints, it optimizes them before starting the main constraint-solving process. The optimizer walks through the expression, and in so doing, it checks disjunctions with operator overloads to see if the parameter type of any of the overloads exactly matches the type of the argument, including the default type of literals. (I'm thinking of favorMatchingUnaryOperators() (and its counterparts for binary operators and other overloaded expressions), favorCallOverloads(), and isFavoredParamAndArg() in CSGen.cpp.) Any of the overloads which match are copied to the head of the disjunction's overload list and are marked as favored.

What's interesting is that this happens after the operator overloads are sorted, but well before the solving process where short-circuiting of disjunctions occurs. So it would seem to be a way to bypass that short circuiting...except that the process of matching overload parameter and argument types requires an exact match, which obviously won't happen for a generic overload.

What I'm wondering is whether it would be feasible to add an extra step to this overload-favoring process, after the overloads are first checked for exact parameter-argument type matches. If and only if none are found, do a second check, except now only test those overloads which are generic and whose parameter type is constrained by a simple protocol requirement; check these for conformance of the argument type to the protocol specified for the parameter. If the argument conforms, mark the overload as favored in the same way as before.

Could I actually make this work? Or am I missing something that would make it unworkable or a bad idea?

ee21ea02-9d7a-4385-8c3c-ad21e8e490a8 commented 7 years ago

/cc @DougGregor and perhaps @rudkx for the question above.