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

Add public methods to supress visibility checks for compilation #11149

Closed
jkotas opened this issue May 6, 2016 · 15 comments
Closed

Add public methods to supress visibility checks for compilation #11149

jkotas opened this issue May 6, 2016 · 15 comments
Labels
Area-Compilers Concept-API This issue involves adding, removing, clarification, or modification of an API. Feature Request
Milestone

Comments

@jkotas
Copy link
Member

jkotas commented May 6, 2016

The ability to suppress visibility checks was proven to be useful for auto-generated code like interop or serialization. It requires manipulating internal Roslyn details via private reflection today. There should be a public API to achieve it instead.

See dotnet/orleans#1729 for example

@jkotas
Copy link
Member Author

jkotas commented May 6, 2016

@sergeybykov
Copy link

👍 cc @ReubenBond.

If would really help also if we could suppress visibility check on a per-type in addition to a per-assembly basis.

@tmat
Copy link
Member

tmat commented May 6, 2016

Why do you need per type?

@sergeybykov
Copy link

Why do you need per type?

Today we inject generated code back into the original solution, so that we only produce a single assembly and can leverage IntelliSense. If we could mark only codegen'ed types for suppression, other application types in the same assembly wouldn't be adversely affected.

@gafter gafter added Concept-API This issue involves adding, removing, clarification, or modification of an API. Area-Compilers Feature Request labels May 7, 2016
@ReubenBond
Copy link
Member

This feature would be immensely useful for serializers and other nuts & bolts libraries which involve code generation, for example, ORMs and RPC frameworks.

@galvesribeiro
Copy link
Member

Is there any progress made on that issue @jkotas ?

Other folks on Roslyn channel mentioned that there a new tooling and new scenarios being created for codegeneration that could make our lifes easier in Orleans... Is there anything we can have a look?

@jkotas
Copy link
Member Author

jkotas commented Jun 23, 2016

@galvesribeiro Check #5561

IMHO, I am not sure whether it would make your life in Orleans easier because of it works only for static compilation from sources case. It does not work for the dynamic case where you create Orleans proxies on the fly. You would need to maintain two disjoint solutions for these.

@gafter gafter added this to the Unknown milestone Aug 10, 2016
@gafter
Copy link
Member

gafter commented Aug 10, 2016

I'm actually more comfortable the way you do it (poking into Roslyn internals using reflection). This is a request to make the compiler stop obeying parts of the language specification, and... well... it is the job of the compiler to obey the language specification.

@gafter gafter added the Resolution-Won't Fix A real bug, but Triage feels that the issue is not impactful enough to spend time on. label Aug 10, 2016
@gafter gafter self-assigned this Aug 10, 2016
@galvesribeiro
Copy link
Member

@gafter I dont agree with you. If that is the case, remove [InternalsVisibleTo()] from the framework, remove the ability to acess internal types from reflection, because all this bypass the language specification. What we are asking here is not to break the language but to give the power at least while generating code to skip the acessibility check so we can generate code for it.

@jaredpar
Copy link
Member

@gafter the compiler already allows removing visibility checks at at API level for the expression evaluator. What is being asked here is not for the compiler command line to allow it but for the API experience to do so.

@jcouv jcouv removed the Resolution-Won't Fix A real bug, but Triage feels that the issue is not impactful enough to spend time on. label Dec 15, 2017
@mjsabby
Copy link
Contributor

mjsabby commented Aug 22, 2018

This would be really good to have for the aforementioned reasons. Any chance we can plumb this through to a public API?

@gafter gafter removed their assignment Jul 2, 2020
@ReubenBond
Copy link
Member

ReubenBond commented Dec 19, 2020

If Source Generators could emit code which had greater access to type internals, that would be very useful for serialization libraries.

For example, if we could emit code to get/set private/readonly/initonly fields and properties without reflection or runtime helpers (such as proposed in dotnet/runtime#45152), we could write high-performance, straight-forward code.

Currently, serialization libraries choose between several options:

  1. Throw an error if the user creates a type with private/init-only/get-only/readonly data members
  2. Use reflection to bypass accessibility checks
  3. Emit IL which bypasses those checks
  4. Make the user create partial types and generate additional constructors and methods which access those members within the language rules

Option 1 coerces the user into avoiding language safety features (implementation hiding, immutable data), and may lead to buggier user code.

2 & 3 work today but are slow, potentially fragile, and are not AOT-friendly.

4 Is onerous for developers and prevents interop scenarios such as generating C# code external to foreign C# or F# assemblies. It is also overly restrictive in what code can be generated. For example, a generated deserialization ctor cannot be a generic method, and therefore cannot accept generic parameters (eg, Reader<TInputBuffer>). Not to mention the user's types with foreign members. There are ways around some of these limitations, but they are not ideal for performance: eg, generate a generic deserialization method which passes a surrogate type to a non-generic ctor.

I would like a mechanism to bend the accessibility rules for generated C# code in an opt-in fashion. For example, this could be enabled by adding a special attribute to generated types/methods which would otherwise be violating these checks. Consider [UnsafeSuppressAccessibilityChecks]. If we wanted to reduce the room for error further, we could add a Type argument to the attribute: [UnsafeSuppressAccessibilityChecks(typeof(MySerializedType<>))].

@TwentyFourMinutes
Copy link

I fully agree with @ReubenBond, Source Generators are truly awesome and we can finally move away from Reflection, Linq Expression and IL Generators to a more maintainable and less error prone solution. As it currently stands, we have to make 'ugly' workarounds to make things work like previously. This especially applies to ORMs and all other types of mappers.

Of course one might say that such attributes or abilities might be a bad idea and brake rules and that it should better be hidden through the reflection APIs. Anyhow, we already have the IgnoresAccessChecksToAttribute which is recognized and respected by the CLR. So IMHO I do not see any issue there.

@ReubenBond
Copy link
Member

Could we consider this in the .NET 7 timeframe? Please see my comment above for more info: #11149 (comment)

As Source Generators are becoming more popular (in part due to a push for more AOT compilation), I believe this feature becomes more desirable.

@jaredpar
Copy link
Member

Closing as this is not planned for any release in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Concept-API This issue involves adding, removing, clarification, or modification of an API. Feature Request
Projects
None yet
Development

No branches or pull requests

10 participants