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

Better error message when specializing generic abstract type with unit #1377

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 25 additions & 7 deletions src/fsharp/CompileOps.fs
Expand Up @@ -524,6 +524,7 @@ let TypeTestUnnecessaryE() = DeclareResourceString("TypeTestUnnecessary","")
let OverrideDoesntOverride1E() = DeclareResourceString("OverrideDoesntOverride1","%s")
let OverrideDoesntOverride2E() = DeclareResourceString("OverrideDoesntOverride2","%s")
let OverrideDoesntOverride3E() = DeclareResourceString("OverrideDoesntOverride3","%s")
let OverrideDoesntOverride4E() = DeclareResourceString("OverrideDoesntOverride4","%s")
let UnionCaseWrongArgumentsE() = DeclareResourceString("UnionCaseWrongArguments","%d%d")
let UnionPatternsBindDifferentNamesE() = DeclareResourceString("UnionPatternsBindDifferentNames","")
let RequiredButNotSpecifiedE() = DeclareResourceString("RequiredButNotSpecified","%s%s%s")
Expand Down Expand Up @@ -1144,15 +1145,32 @@ let OutputPhasedErrorR (os:System.Text.StringBuilder) (err:PhasedError) =
Printf.bprintf os "%s" msg
| OverrideDoesntOverride(denv,impl,minfoVirtOpt,g,amap,m) ->
let sig1 = DispatchSlotChecking.FormatOverride denv impl
begin match minfoVirtOpt with
match minfoVirtOpt with
| None ->
os.Append(OverrideDoesntOverride1E().Format sig1) |> ignore
| Some minfoVirt ->
os.Append(OverrideDoesntOverride2E().Format sig1) |> ignore
let sig2 = DispatchSlotChecking.FormatMethInfoSig g amap m denv minfoVirt
if sig1 <> sig2 then
os.Append(OverrideDoesntOverride3E().Format sig2) |> ignore
end
| Some minfoVirt ->
// https://github.com/Microsoft/visualfsharp/issues/35
// Improve error message when attempting to override generic return type with unit:
// we need to check if unit was used as a type argument
let rec hasUnitTType_app (types: TType list) =
match types with
| TType_app (maybeUnit, []) :: ts ->
match maybeUnit.TypeAbbrev with
| Some ttype when Tastops.isUnitTy g ttype -> true
| _ -> hasUnitTType_app ts
| _ :: ts -> hasUnitTType_app ts
| [] -> false

match minfoVirt.EnclosingType with
Copy link
Contributor

@forki forki Jul 25, 2016

Choose a reason for hiding this comment

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

most similar pattern matches call striptypEqns - maybe @dsyme can tell if it's needed here

| TType_app (t, types) when t.IsFSharpInterfaceTycon && hasUnitTType_app types ->
// match abstract member with 'unit' passed as generic argument
os.Append(OverrideDoesntOverride4E().Format sig1) |> ignore
| _ ->
os.Append(OverrideDoesntOverride2E().Format sig1) |> ignore
let sig2 = DispatchSlotChecking.FormatMethInfoSig g amap m denv minfoVirt
if sig1 <> sig2 then
os.Append(OverrideDoesntOverride3E().Format sig2) |> ignore

| UnionCaseWrongArguments (_,n1,n2,_) ->
os.Append(UnionCaseWrongArgumentsE().Format n2 n1) |> ignore
| UnionPatternsBindDifferentNames _ ->
Expand Down
3 changes: 3 additions & 0 deletions src/fsharp/FSStrings.resx
Expand Up @@ -867,6 +867,9 @@
<data name="OverrideDoesntOverride3" xml:space="preserve">
<value> The required signature is '{0}'.</value>
</data>
<data name="OverrideDoesntOverride4" xml:space="preserve">
<value>The member '{0}' is specialized with 'unit' but 'unit' can't be used as return type of an abstract method parameterized on return type.</value>
</data>
<data name="UnionCaseWrongArguments" xml:space="preserve">
<value>This constructor is applied to {0} argument(s) but expects {1}</value>
</data>
Expand Down
@@ -0,0 +1,9 @@
// #UnitGenericAbstractType
//<Expects status="success"></Expects>

type Foo<'t> =
abstract member Bar : 't -> int

type Bar() =
interface Foo<unit> with
member x.Bar _ = 1
Expand Up @@ -6,3 +6,4 @@
SOURCE=E_LazyInType02.fs SCFLAGS="--test:ErrorRanges" # E_LazyInType02.fs
SOURCE=MultipleConstraints01.fs # MultipleConstraints01.fs
SOURCE=ValueTypesWithConstraints01.fs # ValueTypesWithConstraints01.fs
SOURCE=UnitSpecialization.fs # UnitSpecialization.fs
@@ -0,0 +1,9 @@
// #ErrorMessages #UnitGenericAbstractType
//<Expects status="error" span="(7,21)" id="FS0017">The member 'Apply : int -> unit' is specialized with 'unit' but 'unit' can't be used as return type of an abstract method parameterized on return type\.</Expects>
type EDF<'S> =
abstract member Apply : int -> 'S
type SomeEDF () =
interface EDF<unit> with
member this.Apply d =
// [ERROR] The member 'Apply' does not have the correct type to override the corresponding abstract method.
()
@@ -0,0 +1 @@
SOURCE=E_UnitGenericAbstractType1.fs # E_UnitGenericAbstractType1
1 change: 1 addition & 0 deletions tests/fsharpqa/Source/test.lst
Expand Up @@ -260,6 +260,7 @@ Misc01 Libraries\Core\Reflection
Misc01 Libraries\Core\Unchecked
Misc01 Warnings
Misc01 ErrorMessages\NameResolution
Misc01 ErrorMessages\UnitGenericAbstractType

Misc02 Libraries\Portable
Misc02 Misc
Expand Down