Skip to content
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

RFLX.RFLX_Types.Operators package #1126

Closed
jklmnn opened this issue Jul 28, 2022 · 3 comments · Fixed by #1180
Closed

RFLX.RFLX_Types.Operators package #1126

jklmnn opened this issue Jul 28, 2022 · 3 comments · Fixed by #1180
Assignees
Labels
generator Related to generator package (SPARK code generation) refactoring small Effort of one person-day or less

Comments

@jklmnn
Copy link
Member

jklmnn commented Jul 28, 2022

The operators for the types defined in RFLX.RFLX_Types are currently also defined in this package. As the types in this package are only subtypes the operators are not primitive functions of these types and therefore not visible by using use type. To use these operators the whole package has to be used. Additionally the use type is still required for the default operators of these types. The reason is that they're defined in the package that provides the types for the generic types implementation. It seems there is no way around using use and use type.

To avoid withing the whole RFLX_Types package I suggest to implement the operators in RFLX.RFLX_Types.Operators so only this package has to be used completely. This avoids all the visibility and ambiguity issues that come by using RFLX_Types.
To ensure backwards compatibility one could also think of only renaming the operators in the Operators child package.

@jklmnn jklmnn added generator Related to generator package (SPARK code generation) small Effort of one person-day or less labels Jul 28, 2022
@jklmnn jklmnn added this to To do in RecordFlux 0.6 via automation Jul 28, 2022
@treiher
Copy link
Collaborator

treiher commented Jul 28, 2022

To avoid withing the whole RFLX_Types package I suggest to implement the operators in RFLX.RFLX_Types.Operators so only this package has to be used completely. This avoids all the visibility and ambiguity issues that come by using RFLX_Types.

Sounds reasonable to me.

To ensure backwards compatibility one could also think of only renaming the operators in the Operators child package.

What exactly do you mean with backward compatibility here?

@jklmnn
Copy link
Member Author

jklmnn commented Jul 28, 2022

If we move the operator definitions to the child package I think we will get errors in the currently generated code as it doesn't know about the operators and therefor cannot use the types as intended. So we would have to add with RFLX.RFLX_Types.Operators; use RFLX.RFLX_Types.Operators to all units that make use of the operators. The same applies to any external code that uses the operators.
We should also keep the abstract operators in the original package as otherwise afaict if you don't use the child package you can just use them as the compiler doesn't know they're overridden.

@treiher
Copy link
Collaborator

treiher commented Jul 28, 2022

I think that would be a good thing. I don't see a good reason to allow the old/default operator behavior.

@senier senier removed this from To do in RecordFlux 0.6 Aug 23, 2022
@senier senier added this to To do in RecordFlux Future via automation Aug 23, 2022
@senier senier added this to To do in RecordFlux 0.7 via automation Aug 30, 2022
@senier senier removed this from Medium in RecordFlux Future Aug 30, 2022
@jklmnn jklmnn self-assigned this Sep 6, 2022
@jklmnn jklmnn moved this from To do to Design in RecordFlux 0.7 Sep 6, 2022
@senier senier moved this from Design to Implementation in RecordFlux 0.7 Sep 6, 2022
jklmnn added a commit that referenced this issue Sep 6, 2022
jklmnn added a commit that referenced this issue Sep 7, 2022
jklmnn added a commit that referenced this issue Sep 7, 2022
jklmnn added a commit that referenced this issue Sep 7, 2022
@senier senier moved this from Implementation to Review in RecordFlux 0.7 Sep 15, 2022
jklmnn added a commit that referenced this issue Sep 22, 2022
jklmnn added a commit that referenced this issue Sep 22, 2022
jklmnn added a commit that referenced this issue Sep 23, 2022
jklmnn added a commit that referenced this issue Sep 23, 2022
@senier senier removed this from Review in RecordFlux 0.7 Sep 29, 2022
@senier senier added this to To do in RecordFlux 0.7.1 via automation Sep 29, 2022
@senier senier moved this from To do to Review in RecordFlux 0.7.1 Oct 4, 2022
jklmnn added a commit that referenced this issue Oct 4, 2022
jklmnn added a commit that referenced this issue Oct 4, 2022
jklmnn added a commit that referenced this issue Oct 5, 2022
jklmnn added a commit that referenced this issue Oct 5, 2022
jklmnn added a commit that referenced this issue Oct 6, 2022
jklmnn added a commit that referenced this issue Oct 6, 2022
jklmnn added a commit that referenced this issue Oct 6, 2022
jklmnn added a commit that referenced this issue Oct 6, 2022
RecordFlux 0.7.1 automation moved this from Review to Done Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
generator Related to generator package (SPARK code generation) refactoring small Effort of one person-day or less
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

3 participants