-
-
Notifications
You must be signed in to change notification settings - Fork 22.8k
Remove implicit conversions from math types to String
, to avoid accidental conversions
#107295
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
Conversation
…ental conversions.
String
explicit, to avoid accidental conversionsString
, to avoid accidental conversions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code looks good to me, and I agree it's a good idea to make these explicit.
Change looks well worth making. Incidentally, I don't know if this would work too, but in a similar library recently I put all these string conversion functions into the Now that it is explicit, would an alternative be a constructor on
|
This is possible, whether they're explicit or implicit. The main question is which of the types the string conversion binds more tightly to. I'd say |
Yeah, it's a glass half full / half empty kind of argument, and how you prefer dependencies. |
Thanks! |
We've agreed in a previous core meeting that implicit conversions should be reduced in the codebase. As a costly conversion, implicit conversion to
String
is especially undesirable. This PR makes one small step towards this goal.As an example, the bug fixed by the above PR (#107289) was only possible because
Color
had an implicit conversion toString
. This PR makes this conversion explicit (among others), preventing similar bugs from happening again in the future.