-
Notifications
You must be signed in to change notification settings - Fork 145
Create FS-1333-record-union-constructor-accessibility.md #816
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
base: main
Are you sure you want to change the base?
Conversation
| **Alternate design**: Should the constructor accessibility be applied instead, even if it is less restrictive than the representation accessibility? | ||
| It does seem strange to be able to construct a value using field names, and then to be unable to access said fields. | ||
| Reasonable use cases would be welcome in support of this variation. | ||
|
|
||
| **Design question**: If we do keep the design as specified here, should there be a warning, or even an error, | ||
| when the constructor accessibility is less restrictive than the representation accessibility? | ||
| The constructor accessibility will essentially be ignored. |
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.
IL allows that, but I would tend towards a warning if this scenario is detected in user code.
|
|
||
| * What happens when previous versions of the F# compiler encounter this design addition in compiled binaries? | ||
|
|
||
| This must be checked: what does the current F# compiler do when it encounters a compiled record or union with (from its point of view) inconsistent accessibility specifications? |
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.
Lets assume a scenario of a Library.fs compiled with F# vNext and using this new feature.
App.fs will consume this library, its user will however use an older SDK/Compiler.
The older compiler does not read metadata about F# from IL, but rather from the binary resources inside the .dll.
This is where the complications begin:
- No matter what we do, we cannot affect the metadata import for the "old" compiler.
- If there is a change in the metadata (the "pickled" representation), the old compiler will fail. This can be a deliberate and intentional failure. It can also be solved like with fullness, via a separate binary stream which the old compiler does not read at all
- But if older compilers do not read this information, it will mean they bypass it at type check time.
- Depending on the IL codegen, bypass at type check time can then either lead to AccessViolation at runtime, or just silent ignore of the
privatemodifier at runtime (both are bad options)
For safety reasons, we would want the old compiler to report an error and suggest the user to update SDK.
Ideally only when user attempts to actually violate the access modifier, but that might be technically difficult to accomplish.
There are some tricks (C# did use them in the past), like emitting the constructor member with an attribute saying it is Obsolete (with IsError=true) for older compilers. And new compiler has code to ignore that.
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.
That being said, the approach to it is more of an implementation detail, but rather an important one.
The RFC-level decision should be:
- Fail old compilers and force update?
- or let old compilers bypass type checking this - if yes, either bypass runtime as well, or accept access violation at runtime.
Click “Files changed” → “⋯” → “View file” for the rendered RFC.