Skip to content

[RFC FS-1093] Additional type directed conversions - #10884

Merged
dsyme merged 78 commits into
mainfrom
feature/auto-widen
Aug 13, 2021
Merged

[RFC FS-1093] Additional type directed conversions#10884
dsyme merged 78 commits into
mainfrom
feature/auto-widen

Conversation

@dsyme

@dsymedsyme commented Jan 15, 2021

Copy link
Copy Markdown
Contributor

Implementation of RFC https://github.com/fsharp/fslang-design/blob/master/FSharp-6.0/FS-1093-additional-conversions.md

This primarily loosens the auto-widening rules in the presence of explicit type annotations or other "known" type information, e.g. to allow things like:

let x: obj = if true then 1 else 3.0

It also adds int32 --> int64 conversions and a limited form of op_Implicit support.

In this PR the rule is:

  • When processing an expression the "overall type" is usually labelled as "MustConvertTo" the known type, instead of "MustEqual"

  • This is propagated over if/then/else and other control structures. So if if then A then B must convert to T, then both A and B must convert to T, but A and B may have different types.

  • For leaf expressions, we often have immediate knowledge of both the source and MustConvertTo types. If a unification between the two would have failed then subsumption and adhoc conversions can be tried instead.

The adhoc conversions enabled are in the RFC.

This also allows (the removal of upcast coercions like asExpr on this line](https://github.com/dotnet/fsharp/blob/main/src/fsharp/FSharp.Core/Linq.fs#L282), e.g. in favour of one explicit type annotation.

A longer list of things enabled is below and in the RFC

letx1:obj =(1,2)letx2:obj * obj =(1,2)letx3:(obj * obj) =(letx=1inlety=2in(x,y))letx4:struct (obj * obj)=struct(1,2)letx5:(unit -> obj) =(fun()->1)letx6:(unit -> obj * obj) =(fun()->(1,2))letx7():obj * obj =(1,2)letx8(s:string):obj = s
typeR={mutable F1:(obj * obj)}letannotations=let_:obj =(1,2)let_:obj * obj =(1,2)// structure through letlet_:(obj * obj) =(letx=1inlety=2in(x,y))// structure through let reclet_:(obj * obj) =(let recf x = x in(3,4.0))// structure through sequencelet_:(obj * obj) =();(3,4.0)// struct tuplelet_:struct (obj * obj)=struct(1,2)// record (both field and overall result)// let _ : obj = { F1 = (1, 2) } // TODO// record (both field and overall result){ F1 =(1,2uy)}.F1 <-(3.0,4)// anon recordlet_:{| A: obj |}={| A =1|}// lambda returnlet_:(unit -> obj) =(fun()->1)// function lambda returnlet_:(int -> obj) =(function1->1|2->3.0|_->4uy)let_:(unit -> obj * obj) =(fun()->(1,2))// constants(1:> System.IComparable)|> ignore
let_:System.IComparable =1// array constantslet_:System.Array =[|1us |]let_:System.Array =[|1I |]let_:System.IComparable =1I
// propertylet_:System.IComparable<string>= System.String.Empty // methodlet_:System.IComparable<string>= System.String.Format("")let_:obj = System.String.Format("")let_:System.IComparable = System.String.Format("")// array constantslet_:obj[]=[|1|]let_:(obj * obj)[]=[|(1,1)|]let_:(obj * obj)[]=[|("abc",1)|]let_:(string * obj)[]=[|("abc",1)|]let_:(string * obj)[]=[|("abc",1);("abc",3.0)|]let_:(string * obj)[]=[| Unchecked.defaultof<_>;("abc",3.0)|]let_:struct (string * obj)[]=[| Unchecked.defaultof<_>;struct("abc",3.0)|]let_:obj =1let_:obj =""let_:obj ={new System.ICloneable withmemberx.Clone()= obj()}let_:obj =""let_:obj = string ""let_:obj = id ""// conditionallet_:obj =iftruethen1else3.0let_:obj =(iftruethen1else3.0)let_:obj =(iftruethen1eliftruethen2uy else3.0)// try-withlet_:obj =try1with_->3.0// try-finallylet_:obj =try1finally()// matchlet_:obj =matchtruewith_->1|_->3.0()letf1():obj =1letf2():obj =iftruethen1else3.0

@dsyme

Copy link
Copy Markdown
ContributorAuthor

Test failures (all expected, at a first glance)

2021-01-16T17:04:27.7873329Z Conformance\Expressions\Type-relatedExpressions (E_rigidtypeannotation02.fs) -- failed
2021-01-16T17:04:27.7904419Z Conformance\Expressions\Type-relatedExpressions (E_rigidtypeannotation02b.fs) -- failed
2021-01-16T17:04:30.5798506Z Conformance\InferenceProcedures\ConstraintSolving (E_NoImplicitDowncast01.fs) -- failed
2021-01-16T17:06:47.8831078Z Conformance\ObjectOrientedTypeDefinitions\ClassTypes\Misc (E_ExplicitConstructor.fs) -- failed
2021-01-16T17:09:35.2129707Z Conformance\TypesAndTypeConstraints\CheckingSyntacticTypes (DefaultConstructorConstraint02.fs) -- failed

@charlesroddie

Copy link
Copy Markdown
Contributor

This would implement fsharp/fslang-suggestions#849

@dsymedsyme changed the title Experiment: feature/auto-widenWIP: feature/auto-widenJan 18, 2021
@dsyme
dsyme marked this pull request as draft January 18, 2021 16:18
@dsyme

Copy link
Copy Markdown
ContributorAuthor

I set up auto-flow from main to this branch here: https://github.com/dotnet/roslyn-tools/blob/master/src/GitHubCreateMergePRs/config.xml

@dsyme

Copy link
Copy Markdown
ContributorAuthor

RFC is here: https://github.com/fsharp/fslang-design/blob/master/RFCs/FS-1093-additional-conversions.md

Don Symeand others added 4 commits January 21, 2021 14:50
* [main] Update dependencies from dotnet/arcade (#10913)
Microsoft.DotNet.Arcade.Sdk
From Version 6.0.0-beta.21068.2 -> To Version 6.0.0-beta.21069.2
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
* Refactor everything but the type provider tests in signature help testing (#10908)
Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Phillip Carter <pcarter@fastmail.com>
@Swoorup

Swoorup commented Jan 22, 2021

Copy link
Copy Markdown
Contributor

Testing this branch along with anonymous union.
Looks like records and DUs fail to widen.

typeDU= DU ofintlet_:obj = DU(0)//error FS0001: This expression was expected to have type ...typeR={ a:int }let_:obj ={ a =0}//error FS0001: This expression was expected to have type ...let_:obj ={|a=1|}// works[<Measure>]typeidtypeUserId= int<id>letid:UserId =0<id>let_:obj = id // works

Edit: just noticed your notes on record which is todo😛

@Happypig375

Copy link
Copy Markdown
Member

But we can still disable automatic upcasting at override returns (and go on the error path before this PR) and emit a covariant return instead?

@Happypig375

Happypig375 commented Jul 26, 2021

Copy link
Copy Markdown
Member

Like

type[<AbstractClass>]A()=abstractA:unit->objtypeB()=
inherits A()overrideA()=()// Covariant returntypeC()=
inherits A()overrideA()= Unchecked.defaultof<_>// Infer obj since type inference kicks in and no other type is knowntypeD()=
inherits A()overrideA()=():>_// Infer obj type for type hole

@dsyme

Copy link
Copy Markdown
ContributorAuthor

But we can still disable automatic upcasting at override returns (and go on the error path before this PR) and emit a covariant return instead?

I don't think so, because existing code already relies on the eager application of the non-covariant type information.

To be honest, co-variant returns are just unlikely to ever make it into F#. Remember F# strongly biases against class/interface hierarchies as a modelling technique (though this stuff does have its uses).

If we ever add them, I'm pretty certain we would want strong type information for updated types for the slots.

type [<AbstractClass>] A() =
abstract A: unit -> obj
type B() =
inherits A()
override A() : string = "a"

@dsyme

Copy link
Copy Markdown
ContributorAuthor

@KevinRansom This is now ready for preview.

@KevinRansomKevinRansom left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a few things

Comment threadtests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.Tests.fsproj Outdated
Comment threadtests/FSharp.Test.Utilities/FSharp.Test.Utilities.fsproj
Comment threadtests/fsharp/tests.fs
@KevinRansom

Copy link
Copy Markdown
Contributor

I'll fix the test issues

@dsyme

Copy link
Copy Markdown
ContributorAuthor

I'll fix the test issues

@KevinRansom thanks!

@dsyme

Copy link
Copy Markdown
ContributorAuthor

@KevinRansom all ready I think

Sign up for freeto 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.

10 participants

@dsyme@charlesroddie@Swoorup@brettfo@Happypig375@KevinRansom@baronfel@dotnet-bot@cartermp@vzarytovskii