Skip to content

Conversation

@dsyme
Copy link
Contributor

@dsyme dsyme commented Mar 8, 2019

Proof of concept for RFC FS-1068 Open static classes.

Details are on the RFC, but for example:

> open System.Math;;
> Min(1.0, 2.0);;
val it : float = 1.0

> Min(2.0, 1.0);;
val it : float = 1.0

> open System.Math;;
> Min(2.0, 1.0);;
val it : float = 1.0

TODO:

  • Resolve how static classes are defined in F#
  • Respect and test for RequireQualifiedNameAttribute(true) on a static class, that prevents it being opened, or at least gives a warning, as for F# modules.

@7sharp9
Copy link
Contributor

7sharp9 commented Mar 8, 2019

The design has an interaction with the F#-only feature "static" extension members

Does that mean this will not work with C# extensions?

@dsyme
Copy link
Contributor Author

dsyme commented Mar 9, 2019

The design has an interaction with the F#-only feature "static" extension members

Does that mean this will not work with C# extensions?

No - C# doesn't have static extension methods (though they've been talking about "extension everything" like F#).

@dsyme
Copy link
Contributor Author

dsyme commented Mar 10, 2019

I've noticed this failure before - I think we have some kind of glitch in our binary writer on an edge case:

    13>FSC : error FS2014: A problem occurred writing the binary '/Users/vsts/agent/2.148.0/work/1/s/fcs/../artifacts/obj/fcs/net45/FSharp.Compiler.Service.dll': EmitZUntaggedIndex: too big for small address or simple index [/Users/vsts/agent/2.148.0/work/1/s/fcs/FSharp.Compiler.Service/FSharp.Compiler.Service.fsproj]

@dsyme
Copy link
Contributor Author

dsyme commented Mar 11, 2019

-> Couldn't get package details for package FSharp.Compiler.Service 2.0.0.6 on https://www.nuget.org/api/v2.
-> All possible sources are already blacklisted. 
	 - 1_https://www.nuget.org/api/v2/Packages(Id='fsharp.compiler.service',Version='2.0.0.6')
	 - 1_https://www.nuget.org/api/v2/Packages(Id='fsharp.compiler.service',Version='2.0.0.6')
	 - 1_https://www.nuget.org/api/v2/Packages(Id='FSharp.Compiler.Service',Version='2.0.0.6')
	 - 1_https://www.nuget.org/api/v2/Packages(Id='FSharp.Compiler.Service',Version='2.0.0.6')
	 - 2_https://www.nuget.org/api/v2/Packages?$filter=(Id eq 'FSharp.Compiler.Service') and (NormalizedVersion eq '2.0.0.6')
	 - 2_https://www.nuget.org/api/v2/Packages?$filter=(tolower(Id) eq 'fsharp.compiler.service') and (NormalizedVersion eq '2.0.0.6')
	 - 2_https://www.nuget.org/api/v2/Packages?$filter=(Id eq 'FSharp.Compiler.Service') and (Version eq '2.0.0.6')
	 - 2_https://www.nuget.org/api/v2/Packages?$filter=(tolower(Id) eq 'fsharp.compiler.service') and (Version eq '2.0.0.6')
	 - 2_https://www.nuget.org/api/v2/Packages?$filter=(tolower(Id) eq 'fsharp.compiler.service') and (Version eq '2.0.0.6')

Seems related to fsprojects/Paket#2640

@dsyme dsyme closed this Mar 11, 2019
@dsyme dsyme reopened this Mar 11, 2019
@dsyme dsyme reopened this Mar 12, 2019
@isaacabraham
Copy link
Contributor

Great to see this :-)

Quick question:

If multiple APIs are open'd with conflicting method sets then severe usability problems will occur.

Is this not the same issue that we have today if you open two modules with the same let-bound values that have the same name and type? Or is this something more "severe"?

@cartermp
Copy link
Contributor

It's the same issue, just exacerbated since there are now two ways to get the issue.

@chkn
Copy link
Contributor

chkn commented Mar 13, 2019

Is this not the same issue that we have today if you open two modules with the same let-bound values that have the same name and type? Or is this something more "severe"?

Is that really an issue? I thought previously opened declarations are simply shadowed by the new declarations when you open the new module? Isn't that basically how Operators.Checked works?

Personally, I'd like to see static classes and modules unified so that module Foo is just a shorthand for declaring a static class (that's basically what it compiles to anyway). This would bring features like open to static classes, but would also bring class-like features to modules, such as being able to use them in typeof or as generic arguments.

@isaacabraham
Copy link
Contributor

@cartermp ok - to me then this is nothing to worry about then. The flipside is that it unifies the end-user experience in terms of consistency - you can now open static classes just like modules (which ironically are compiled into static classes).

@chkn I have no problem with this RFC (indeed, I created the original one some years ago ;-)

@7sharp9
Copy link
Contributor

7sharp9 commented Mar 14, 2019

Don't forget in F# there are no nested types so modules and types each have their place in the current scheme. I would be happy to just have open capability, I don't think we need another way of making modules.

@dsyme
Copy link
Contributor Author

dsyme commented Mar 22, 2019

The usual flaky test:

Failed   TypeProvider.Disposal.SmokeTest1
Error Message:
   Check4b, countCreations() = countDisposals(), iteration 3
  Expected: True
  But was:  False

@dsyme dsyme closed this Mar 22, 2019
@dsyme dsyme reopened this Mar 22, 2019
@dsyme
Copy link
Contributor Author

dsyme commented Mar 22, 2019

Odd unexpected failure after integrating:

Exception thrown: ("DL9", "NoClosingBrace,NextDefinition", "", DotCompleteExpected ("(*D0*)","Length")) 
  FAILURE on systematic test: ("DL9", "NoClosingBrace,NextDefinition", "", DotCompleteExpected ("(*D0*)","Length")) 
 
 
 fileContents = <<<
 module Test
 let abbbbc = [| 1 |]
 let aaaaaa = 0
 
 let x = query { for bbbb in abbbbc(*D0*) do 
                 join cccc in abbbbc(*D1*) on (bbbb(*D11*) = cccc(*D12*)
  
 let nextDefinition () = 1
 >>>
 Making progress, run 600 so far...
 Making progress, run 650 so far...
 Making progress, run 700 so far...
 Making progress, run 750 so far...
 Making progress, run 800 so far...
 820 TESTS, 819 SUCCESS, 1 FAILURE, %99.88 success rate
 EXTRA OBSERVED FAILURES:  
 [
    ("DL9", "NoClosingBrace,NextDefinition", "", DotCompleteExpected ("(*D0*)","Length")) 
 ]

@dsyme
Copy link
Contributor Author

dsyme commented May 22, 2019

Closed in favour of #6807 to use a feature branch

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.

6 participants