-
Notifications
You must be signed in to change notification settings - Fork 831
Allow use of multi interface instantiations #18
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
Conversation
|
I've updated the link on the status page to point to this one, the PR on codeplex can be closed |
|
With this update, the additional issue I found is still present, and now the original tests you added also aren't passing either. I double checked and undid your 2 fix commits, and at least the original tests started working again, so I'm pretty sure my environment is correct. |
|
Don, write it out 100 times: |
|
OK, adjsted once again. I'm really sorry about the screw up, and very, very glad for your diligence here. I've run the "Conformance" section of the fsharpqa tests, adjusted the code for the mistakes, and re-run the failing tests. Will now attempt to run all of "fsharp" and "fsharpqa" |
|
:) :) |
findIndex/tryFindIndex and a fix to Concat when there are side effects
…groups Failing test with groups

Copied from matching PR on codeplex, retargeted to fsharp4 branch with fix included
This implements the F# 4.0+ language feature allow inheritance from C# types implementing multiple instantiations of the same interface
F# 2.0-3.1 had an overly draconian restrictions where you could not inherit from any .NET type that contains multiple instantiations of the same interface type (for the full interface type set in the whole hierarchy). This made it impossible to use certain C# types at all. We've gradually seen more of these C# types in practice
This change allows inheritance from such types in type declarations and object expressions.
Testing has been added by adjusting and extending the existing fsharpqa tests
An error message has been simplified to remove "inherits"
Note, the change still keeps the restriction that an F# type can itself only implement one instantiation of a generic interface type. For example
type C() =
interface I
interface I
is not allowed. This is partly because of the way interface implementation methods are named in compiled IL (only the prefix "I" is added), and partly because the equivalent object expression form can inculde type unknowns, e.g.
and we don't want to support this kind of inference, and equally don't want a non-orthogonality between object expressions and class definitions. As a workaround you can always add an extra inherit each type you wish to introduce new interface instantiations, e.g.
type C() =
interface I with ...
type D() =
inherit C()
interface I with ...
This is ready for review (pending full test run to see if any "fsharp" tests need adjusting)
vladima Nov 28, 2014 : LGTM!
tomasp: Dec 02, 2014 I do not understand all the implications here, but the change looks good to me. Does this just let you inherit from C# classes, or can you also implement C# interfaces directly? For example, can I do:
... or is this not allowed? If it is allowed, then having this in the tests would be nice.
dsyme Dec 02, 2014 Hi @TomAsp - it turns out that you could already do that particular object-expression in F# 3.x, so it's inn the test cases already
latkin Jan 05, 2015 - F# 4.0 triage -- This PR is approved in principle for F# 4.0. If remaining issues can be addressed by 1/16 it will be included in F# 4.0. Otherwise it will be considered for the next release. I have not yet reviewed this one, will try to get to it ASAP.
latkin 2 days ago tests\fsharpqa\Source\Conformance\Expressions\DataExpressions\ObjectExpressions\ObjExprWithSameInterface01.fs This previously expected a compile error, which (as expected) now goes away. However, running the code through FSI now causes an internal error, and compiling with FSC succeeds but results in an unverifiable binary.
dsyme Wow, good catch, thanks. Am looking into it now.
dsyme Now fixed
Avatar
dsyme added changesets to this pull request