yannickl
Make precendence of basic math operators more explicit.
This caused me some grief with recent update to Xcode 11 GM. Somehow, Swift (5.1) started re-ordering some of DynamicColor's math expressions to use + and - overloads I had created to handle mixing CGFloat and Int without the need to cast. This was possible because DynamicColor code and mine are in the same module. Strangely, this did not occur before Swift 5.1
The change in Swift's behavior caused major breakage in my app colors, because the math expressions became incorrect -- for example, let r = hueToRGB(m1: m1, m2: m2, h: h + 1 / 3) would compute h + 1 before / 3.
Notwithstanding the dumbness of my operator overloads (!), I think DynamicColor should make these expressions more explicit to shield future problems from occurring.
Using that same example, change:
let r = hueToRGB(m1: m1, m2: m2, h: h + 1 / 3)
to
let r = hueToRGB(m1: m1, m2: m2, h: h + (1.0 / 3.0))
And of course, other places where similar precedence problems may occur.
I would be happy to create the pull request for this as soon as I have a moment. Else, if someone else does I'd be happy to review it.
Thanks for doing this @yannickl
I took at look at the commit and would make a small suggestion for improvement. There a few places where integers are used in expressions that returns floats. For example: https://github.com/yannickl/DynamicColor/blob/master/Sources/HSL.swift#L112 (there may be other places like the Lab file for one..)
In my testing the presence of these integers was still causing the precedence (and my + overload) to misfire, even with the parenthesis. E.g. h + (1 / 3) was still being interpreted as (h + 1) / 3. This may be a Swift bug, but probably better to be safe than sorry.
In my testing the presence of these integers was still causing the precedence (and my + overload) to misfire, even with the parenthesis. E.g. h + (1 / 3) was still being interpreted as (h + 1) / 3. This may be a Swift bug, but probably better to be safe than sorry.
This is very strange, how your operator overload can affect the parenthesis precedence?! Moreover here the type inference should treat these constants as CGFloat so how it can affect precedence?
I'm not following the Swift language evolution anymore so I don't know what is the root cause. :/
When I was troubleshooting this problem in Xcode I tried your latest version first h + (1 / 3) and set a breakpoint in my overload and it hit with the backtrace coming from that line in DynamicColor. This also seemed VERY strange to me. After changing the code to h + (1.0 / 3.0), the breakpoint was no longer hit. 🤷♂
I will update the code to reflect this, but it does not look like a robust fix (by default the decimal notation represent a Double instead of a Int, not a CGFloat). :/
Oh yeah, I didn't think about that. I mean conceivably a person could overload operators to elide Double to CGFloat conversions and potentially the same bug would occur. It's super edge case, but possible.
What I really need to do is reproduce the bug and report it on https://bugs.swift.org/.