-
Notifications
You must be signed in to change notification settings - Fork 790
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
support .NET Standard 2.0 in F# Interactive and type providers #3307
Changes from 5 commits
87ec42b
3b9c664
8acd825
3f17ab6
07b636e
1507206
4416324
0b7f13d
62e432e
0512011
ec488d7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1036,10 +1036,10 @@ | |
<value>Interface implementations should be given on the initial declaration of a type.</value> | ||
</data> | ||
<data name="UnresolvedReferenceNoRange" xml:space="preserve"> | ||
<value>A required assembly reference is missing. You must add a reference to assembly '{0}'.</value> | ||
<value>The type '{0}' is required here and is unavailable. You must add a reference to assembly '{1}'.</value> | ||
</data> | ||
<data name="UnresolvedPathReferenceNoRange" xml:space="preserve"> | ||
<value>The type referenced through '{0}' is defined in an assembly that is not referenced. You must add a reference to assembly '{1}'.</value> | ||
<value>A reference to the type '{0}' in assembly '{1}' was found, but the type could not be found in that assembly.</value> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Has this broken the codefixer that looks for this compile error to automatically add references? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wow, amazing code review catch. I had no idea we were matching on messages like this. By the look of this code it's ok, since the order of
In fact the adjustment to |
||
</data> | ||
<data name="HashIncludeNotAllowedInNonScript" xml:space="preserve"> | ||
<value>#I directives may only occur in F# script files (extensions .fsx or .fsscript). Either move this code to a script file, add a '-I' compiler option for this reference or delimit the directive with delimit it with '#if INTERACTIVE'/'#endif'.</value> | ||
|
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.
This is not really right and the existing code is wrong too ...
This forces us to co-locate System.ValueTuple.dll with FSharp.Core.dll which is not right. Especially since dotnet cli hosting can locate assemblies in a central shared cache rather than app local.
The existing code assumes that ValueTuple was loaded from an assembly named System.ValueTuple.dll which is also not right. Since from dotnet 4.7 it exists in mscorlib, probably System.Private.Corlib.dll on coreclr.
I guess I will have to revisit this code at some point, but we need to do better long term.
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.
OK, yes, this is wrong when running the compiler or F# Interactive in .NET Core without --noframework etc.
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.
And it won't work on the dotnet cli. Since we don't co-locate system.valuetuple.dll