Introduce type-safe Flags and FlagArray to no longer use raw ints#186
Merged
elect86 merged 3 commits intokotlin-graphics:1.89.2from Apr 13, 2023
Merged
Introduce type-safe Flags and FlagArray to no longer use raw ints#186elect86 merged 3 commits intokotlin-graphics:1.89.2from
elect86 merged 3 commits intokotlin-graphics:1.89.2from
Conversation
kyay10
changed the title
- Remove Flag.eq and Flag.notEq and instead override equals - Introduce val emptyFlags - Make Flag's Self type parameter covariant - Use kotlin.internal.NoInfer for has, hasnt, and contains to ensure that they aren't called on flags that can't possibly have those sub-flags. - Replace assertions on flag parameters with type-safe sealed subclasses
Collaborator
|
Awesome, thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is quite a big refactoring...
Building on the previous work of using a
Flaginterface, this PR changes all theXFlagstype aliases from beingIntto beingFlag<XFlags>. That, combined with aprivate value class Flagsand avalue class FlagArray, allows using type-safe flag combinations everywhere, while still having the ever-usefuland,or,wo,has, etc methods.While implementing this, I genuinely found multiple places where the code was using the wrong enum type of Flag or was checking if flags of totally separate types were equal. It was hence helpful in catching those bugs and remedying them quickly. This also removed a lot of unnecessary range checks we were doing on ints.
The only downside of using this PR is that checking flags for equality using
==and!=is broken. This is because you can't overrideequalsin enums nor invalue classes. For now, I addedinfix fun eq/notEqto mitigate that problem. However, a more intrusive solution (that will fix==checks) is to convert all the Flag enums tosealed classes (This is simply an IDE intention and so it's very automatic) with a base class that hasequalsoverridden, and changevalue class Flagsto a non-value class. Note that in Kotlin 1.9.0value classes will be able to overrideequals. I'm not sure whether the currenteq/notEqapproach is too finicky, but it surely is less error-prone than comparing rawInts.Finally, this PR also introduces a useful
MutableReferencetype that extends KMutableProperty0. That type might be the beginning of relying less on global mutable vars like_band friends, and instead createdMutableReferences locally., but that shall come in another PR.