Skip to content

Conversation

@dsyme
Copy link
Contributor

@dsyme dsyme commented Jan 31, 2022

While working on #11521 with @vzarytovskii we've identified that the F# signature data metadata resource blob contains Too Much Information.

In particular it contains

  1. the metadata for "private" declarations in implementation files that don't have signature files
  2. the metadata for "internal" declarations when there are no InternalsVisibleTo in the assembly

This means the reference assembly feature will not be quite as effective as we would like , as reference assemblies will get updated even when private implementation details change.

However, this fix to reduce the information on the signature bloc is of a different nature to the rest of the work on reference assemblies and is in a sense completely independent from a logical/correctness point of view. It also has some subtle edge cases, and thus we think this fix is best made separately, and perhaps released under preview or else have a command line option to disable it.

The reference assembly feature is still highly useful without this fix as there are plenty of cases where it's still useful without this.

@dsyme
Copy link
Contributor Author

dsyme commented Feb 18, 2022

@vzarytovskii I'm closing this for now, as it's not critical for us. However we may want to return to this

@dsyme
Copy link
Contributor Author

dsyme commented Feb 18, 2022

Tracked in #12746

@dsyme dsyme closed this Feb 18, 2022

type InterfacePublic = interface end

type private InterfacePrivate = interface end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dsyme is my understanding correct here that this should be kept only when there is an InternalsVisibleTo present on the assembly level?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess, it should be kept if a public type inherits it.

We should verify what C# emits into refassembly in this case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Internal interfaces appear to be present in C#:

https://sharplab.io/#v2:DYLgZgzgNAJiDUAfALgTwA4FMAE6BOAlgG4CGyOAkgHbl5gkDGmACoaedgLzYE2Z2McmKjCA

image

Even without any InternalsVisibleTo attributes.


type InterfacePublic = interface end

type private InterfacePrivate = interface end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dsyme is my understanding correct here that this should be kept only when there is an InternalsVisibleTo present on the assembly level?


val canAccessFromSomewhere: Accessibility -> bool

val canAccessFromSomewhereOutside: 'T -> Accessibility -> bool
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dsyme what was the train of thought with this first generic parameter?
I'm currently only able to deduce that the top-level Entity has the attribute inside the .Attribs property.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants