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

Make BinderFlags public #30223

Open
jaredpar opened this issue Sep 28, 2018 · 5 comments
Open

Make BinderFlags public #30223

jaredpar opened this issue Sep 28, 2018 · 5 comments
Assignees
Milestone

Comments

@jaredpar
Copy link
Member

The compiler currently has an IVT into the interactive layer and soon to the EditorFeatures.Wpf layer. This violates our principle of keeping the editor and compiler portions communicating via the public API surface area only. We should fix this ASAP to ensure that the internal usage doesn't spread any further.

@gafter
Copy link
Member

gafter commented Oct 2, 2018

@jaredpar said elsewhere:

Looking the only relevant API that is exposed is BinderFlags and only in this code (assuming a similar problem for VB)

            return new CSharpCompilationOptions(
                OutputKind.DynamicallyLinkedLibrary,
                scriptClassName: name,
                allowUnsafe: true,
                xmlReferenceResolver: null, // no support for permission set and doc includes in interactive
                usings: imports,
                sourceReferenceResolver: sourceReferenceResolver,
                metadataReferenceResolver: metadataReferenceResolver,
                assemblyIdentityComparer: DesktopAssemblyIdentityComparer.Default
                ).WithTopLevelBinderFlags(BinderFlags.IgnoreCorLibraryDuplicatedTypes);

I think we should fix this before the IVT spreads. I see two options here:

  1. Make BinderFlags public
  2. Make this specific option, or a subset of BinderFlags options, public

Currently leaning towards #2. This seems really like the only option we’d want to reasonably expose here.

@gafter
Copy link
Member

gafter commented Oct 2, 2018

I agree. I propose we add a (bool) compilation option specifically for “compilation at runtime” that would do this.

@jaredpar
Copy link
Member Author

jaredpar commented Oct 3, 2018

"compilation at runtime" is the scenario though, not the behavior. Shouldn't the flag express the behavior here? Was thinking something along the lines of "PreferNonCorLibraryTypes".

@tmat
Copy link
Member

tmat commented Oct 4, 2018

We should revisit if we still need this. I think this was a problem in early .NET Core FX. Might not be a problem anymore for 2.0+.

@jaredpar
Copy link
Member Author

jaredpar commented Oct 5, 2018

@tmat what would I need to validate this? Just delete it, run tests and if they pass we're good to go?

@jinujoseph jinujoseph modified the milestones: 16.0, 16.3 Jun 9, 2019
@jcouv jcouv modified the milestones: 16.3, Compiler.Next Jul 11, 2019
@jaredpar jaredpar modified the milestones: Compiler.Next, Backlog Sep 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants