-
Notifications
You must be signed in to change notification settings - Fork 144
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make precendence of basic math operators more explicit. #51
Comments
Hey @davidbjames, I will update the project to fix that. :) |
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: In my testing the presence of these integers was still causing the precedence (and my |
This is very strange, how your operator overload can affect the parenthesis precedence?! Moreover here the type inference should treat these constants as 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 |
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/. |
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 mixingCGFloat
andInt
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.1The 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 computeh + 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.
The text was updated successfully, but these errors were encountered: