Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 18 additions & 13 deletions src/fsharp/NameResolution.fs
Original file line number Diff line number Diff line change
Expand Up @@ -845,7 +845,6 @@ let AtMostOneResult m res =
match res with
| Exception err -> raze err
| Result [] -> raze (Error(FSComp.SR.nrInvalidModuleExprType(),m))
| Result [res] -> success res
| Result (res :: _) -> success res

//-------------------------------------------------------------------------
Expand Down Expand Up @@ -1976,25 +1975,31 @@ let rec ResolveExprLongIdentInModuleOrNamespace (ncenv:NameResolver) nenv (typeN
| Some vspec when IsValAccessible ad (mkNestedValRef modref vspec) ->
success(resInfo,Item.Value (mkNestedValRef modref vspec),rest)
| _->
match TryFindTypeWithUnionCase modref id with
| Some tycon when IsTyconReprAccessible ncenv.amap m ad (modref.NestedTyconRef tycon) ->
let ucref = mkUnionCaseRef (modref.NestedTyconRef tycon) id.idText
let showDeprecated = HasFSharpAttribute ncenv.g ncenv.g.attrib_RequireQualifiedAccessAttribute tycon.Attribs
let ucinfo = FreshenUnionCaseRef ncenv m ucref
success (resInfo,Item.UnionCase(ucinfo,showDeprecated),rest)
| _ ->
match mty.ExceptionDefinitionsByDemangledName.TryFind(id.idText) with
| Some excon when IsTyconReprAccessible ncenv.amap m ad (modref.NestedTyconRef excon) ->
success (resInfo,Item.ExnCase (modref.NestedTyconRef excon),rest)
| _ ->

// Something in a type?
// Something in a discriminated union without RequireQualifiedAccess attribute?
let unionSearch,hasRequireQualifiedAccessAttribute =
match TryFindTypeWithUnionCase modref id with
| Some tycon when IsTyconReprAccessible ncenv.amap m ad (modref.NestedTyconRef tycon) ->
let ucref = mkUnionCaseRef (modref.NestedTyconRef tycon) id.idText
let ucinfo = FreshenUnionCaseRef ncenv m ucref
let hasRequireQualifiedAccessAttribute = HasFSharpAttribute ncenv.g ncenv.g.attrib_RequireQualifiedAccessAttribute tycon.Attribs
success [resInfo,Item.UnionCase(ucinfo,hasRequireQualifiedAccessAttribute),rest],hasRequireQualifiedAccessAttribute
| _ -> NoResultsOrUsefulErrors,false

match unionSearch with
| Result (res :: _) when not hasRequireQualifiedAccessAttribute -> success res
| _ ->

// Something in a type?
let tyconSearch =
let tcrefs = LookupTypeNameInEntityMaybeHaveArity (ncenv.amap, id.idRange, ad, id.idText, (if isNil rest then typeNameResInfo.StaticArgsInfo else TypeNameResolutionStaticArgsInfo.Indefinite), modref)
let tcrefs = tcrefs |> List.map (fun tcref -> (resInfo,tcref))
if not (isNil rest) then
let tcrefs = CheckForTypeLegitimacyAndMultipleGenericTypeAmbiguities (tcrefs, TypeNameResolutionInfo (ResolveTypeNamesToTypeRefs,TypeNameResolutionStaticArgsInfo.Indefinite), PermitDirectReferenceToGeneratedType.No, unionRanges m id.idRange)
ResolveLongIdentInTyconRefs ncenv nenv LookupKind.Expr (depth+1) m ad rest typeNameResInfo id.idRange tcrefs
ResolveLongIdentInTyconRefs ncenv nenv LookupKind.Expr (depth+1) m ad rest typeNameResInfo id.idRange tcrefs
// Check if we've got some explicit type arguments
else
let tcrefs = CheckForTypeLegitimacyAndMultipleGenericTypeAmbiguities (tcrefs, typeNameResInfo, PermitDirectReferenceToGeneratedType.No, unionRanges m id.idRange)
Expand All @@ -2012,7 +2017,7 @@ let rec ResolveExprLongIdentInModuleOrNamespace (ncenv:NameResolver) nenv (typeN

match tyconSearch with
| Result (res :: _) -> success res
| _ ->
| _ ->

// Something in a sub-namespace or sub-module
let moduleSearch =
Expand All @@ -2027,7 +2032,7 @@ let rec ResolveExprLongIdentInModuleOrNamespace (ncenv:NameResolver) nenv (typeN
else
NoResultsOrUsefulErrors

match tyconSearch +++ moduleSearch with
match tyconSearch +++ moduleSearch +++ unionSearch with
| Result [] ->
let predictedPossibleTypes =
modref.ModuleOrNamespaceType.AllEntities
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// #Conformance #TypeInference #Attributes
// Verify the RequireQualifiedAccess attribute works on unions

module A =
[<RequireQualifiedAccess>]
type U = | C

type C() =
static member M() = ()

let x = A.C.M()
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm trying to understand what this test adds. Today it gives a warning and then doesn't compile. What does it give after this PR? Does it just compile?

Likewise, what happens after this PR without the RQA attribute (and with the last line adjusted to let x : A.C = A.C)? Today that compiles without warning? But it feels to me that after this change then it won't compile any more? I haven't tried yet, just wondering.

Adding these test cases would be good, and also adding test cases for union types where the type name is the same as the union case name, e.g. type C = | C

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes pre-PR this is giving an error:

image

Copy link
Contributor

Choose a reason for hiding this comment

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

How about this case:

Likewise, what happens after this PR without the RQA attribute (and with the last line adjusted to let x : A.C = A.C)? Today that compiles without warning? But it feels to me that after this change then it won't compile any more? I haven't tried yet, just wondering.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see also #1512 (comment)

Copy link
Contributor Author

@forki forki Nov 23, 2016

Choose a reason for hiding this comment

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

as already described. this is breaking and #1814 show that this case builds in current master. It will break after this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

@forki We need to make this change non-breaking. It seems to me you should first search non-RQA unions (emitting no warning), then type names, then RQA unions (emitting the warning)?

Copy link
Contributor Author

@forki forki Nov 23, 2016

Choose a reason for hiding this comment

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

Like 398931e ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's now backwards compatible

Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// #Conformance #TypeInference #Attributes

module Module =
type R = { a: int } with static member New = { a = 1 }
type Choice = | R of R
open Module

let record1 = R.New
let choice1 v =
match v with
| R r -> r

let newChoice = R { a = 1}

let choice2 v =
match v with
| Module.R r -> r

let newChoice2 = Module.R { a = 1}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// #Conformance #TypeInference #Attributes
// Verify the access works on unions without RQA

module A =
type U = | C

type C() =
static member M() = ()

let x:A.U = A.C
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// #Conformance #TypeInference #Attributes
// Verify the access works on unions without RQA

module A =
type U = | C

type C() =
static member M() = ()

let x = A.C
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// #Conformance #TypeInference #Attributes
// Verify the access works on unions where type name is case name

module A =
type C =
| B
| C

let x = A.C
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// #Conformance #TypeInference #Attributes
// Verify the access works on unions where type name is case name

module A =
[<RequireQualifiedAccess>]
type C =
| B
| C

let x = A.C
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
SOURCE=OnRecord.fs # OnRecord.fs
SOURCE=E_OnRecord.fs # E_OnRecord.fs

SOURCE=OnRecordVsUnion.fs # OnRecordVsUnion.fs
SOURCE=OnRecordVsUnion2.fs # OnRecordVsUnion2.fs
SOURCE=OnDiscriminatedUnion.fs # OnDiscriminatedUnion.fs
SOURCE=E_OnDiscriminatedUnion.fs # E_OnDiscriminatedUnion.fs

SOURCE=OnRecordVsUnion_NoRQA.fs # OnRecordVsUnion_NoRQA.fs
SOURCE=OnRecordVsUnion_NoRQA2.fs # OnRecordVsUnion_NoRQA2.fs
SOURCE=OnUnionWithCaseOfSameName.fs # OnUnionWithCaseOfSameName.fs
SOURCE=OnUnionWithCaseOfSameName2.fs # OnUnionWithCaseOfSameName2.fs