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.

Sign up for GitHub

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

Jump to bottom

[WIP] fix for #960 #16476

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

[WIP] fix for #960 #16476

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e0574e8
suggested message clarifying usage of augmentation attributes for iss…
smoothdeveloper Dec 30, 2023
e4a6676
adding a test case that checks two new error messages (augInvalidAttr…
smoothdeveloper Jan 19, 2024
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
107 changes: 73 additions & 34 deletions src/Compiler/Checking/AugmentWithHashCompare.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1004,15 +1004,20 @@ let canBeAugmentedWithCompare g (tycon: Tycon) =
tycon.IsUnionTycon || tycon.IsRecordTycon || isTrueFSharpStructTycon g tycon

let getAugmentationAttribs g (tycon: Tycon) =
let hasAttrAndName (attr: BuiltinAttribInfo) =
match TryFindFSharpBoolAttribute g attr tycon.Attribs with
| Some b -> Some(struct(b, attr.TyconRef.DisplayName))
| None -> None

canBeAugmentedWithEquals g tycon,
canBeAugmentedWithCompare g tycon,
TryFindFSharpBoolAttribute g g.attrib_NoEqualityAttribute tycon.Attribs,
TryFindFSharpBoolAttribute g g.attrib_CustomEqualityAttribute tycon.Attribs,
TryFindFSharpBoolAttribute g g.attrib_ReferenceEqualityAttribute tycon.Attribs,
TryFindFSharpBoolAttribute g g.attrib_StructuralEqualityAttribute tycon.Attribs,
TryFindFSharpBoolAttribute g g.attrib_NoComparisonAttribute tycon.Attribs,
TryFindFSharpBoolAttribute g g.attrib_CustomComparisonAttribute tycon.Attribs,
TryFindFSharpBoolAttribute g g.attrib_StructuralComparisonAttribute tycon.Attribs
hasAttrAndName g.attrib_NoEqualityAttribute,
hasAttrAndName g.attrib_CustomEqualityAttribute,
hasAttrAndName g.attrib_ReferenceEqualityAttribute,
hasAttrAndName g.attrib_StructuralEqualityAttribute,
hasAttrAndName g.attrib_NoComparisonAttribute,
hasAttrAndName g.attrib_CustomComparisonAttribute,
hasAttrAndName g.attrib_StructuralComparisonAttribute

let CheckAugmentationAttribs isImplementation g amap (tycon: Tycon) =
let m = tycon.Range
Expand All @@ -1026,63 +1031,97 @@ let CheckAugmentationAttribs isImplementation g amap (tycon: Tycon) =
| _, _, None, None, None, None, None, None, None

// [<CustomEquality; CustomComparison>] on union/record/struct
| true, _, None, Some true, None, None, None, Some true, None
| true, _, None, Some (true,_), None, None, None, Some (true,_), None

// [<CustomEquality; NoComparison>] on union/record/struct
| true, _, None, Some true, None, None, Some true, None, None -> ()
| true, _, None, Some (true,_), None, None, Some (true,_), None, None -> ()

// [<ReferenceEquality; NoComparison>] on union/record/struct
| true, _, None, None, Some true, None, Some true, None, None
| true, _, None, None, Some (true,_), None, Some (true,_), None, None

// [<ReferenceEquality>] on union/record/struct
| true, _, None, None, Some true, None, None, None, None ->
| true, _, None, None, Some (true,_), None, None, None, None ->
if isTrueFSharpStructTycon g tycon then
errorR (Error(FSComp.SR.augNoRefEqualsOnStruct (), m))
else
()

// [<StructuralEquality; StructuralComparison>] on union/record/struct
| true, true, None, None, None, Some true, None, None, Some true
| true, true, None, None, None, Some (true,_), None, None, Some (true,_)

// [<StructuralEquality; NoComparison>]
| true, _, None, None, None, Some true, Some true, None, None
| true, _, None, None, None, Some (true,_), Some (true,_), None, None

// [<StructuralEquality; CustomComparison>]
| true, _, None, None, None, Some true, None, Some true, None
| true, _, None, None, None, Some (true,_), None, Some (true,_), None

// [<NoComparison>] on anything
| _, _, None, None, None, None, Some true, None, None
| _, _, None, None, None, None, Some (true,_), None, None

// [<NoEquality; NoComparison>] on anything
| _, _, Some true, None, None, None, Some true, None, None -> ()
| _, _, Some (true,_), None, None, None, Some (true,_), None, None -> ()

// THESE ARE THE ERROR CASES

// [<NoEquality; ...>]
| _, _, Some true, _, _, _, None, _, _ -> errorR (Error(FSComp.SR.augNoEqualityNeedsNoComparison (), m))
| _, _, Some (true,_), _, _, _, None, _, _ -> errorR (Error(FSComp.SR.augNoEqualityNeedsNoComparison (), m))

// [<StructuralComparison(_)>]
| true, true, _, _, _, None, _, _, Some true -> errorR (Error(FSComp.SR.augStructCompNeedsStructEquality (), m))
| true, true, _, _, _, None, _, _, Some (true,_) -> errorR (Error(FSComp.SR.augStructCompNeedsStructEquality (), m))
// [<StructuralEquality(_)>]
| true, _, _, _, _, Some true, None, _, None -> errorR (Error(FSComp.SR.augStructEqNeedsNoCompOrStructComp (), m))
| true, _, _, _, _, Some (true,_), None, _, None -> errorR (Error(FSComp.SR.augStructEqNeedsNoCompOrStructComp (), m))

// [<StructuralEquality(_)>]
| true, _, _, Some true, _, _, None, None, _ -> errorR (Error(FSComp.SR.augCustomEqNeedsNoCompOrCustomComp (), m))
| true, _, _, Some (true,_), _, _, None, None, _ -> errorR (Error(FSComp.SR.augCustomEqNeedsNoCompOrCustomComp (), m))

// [<ReferenceEquality; StructuralEquality>]
| true, _, _, _, Some true, Some true, _, _, _
| true, _, _, _, Some (true,_), Some (true,_), _, _, _

// [<ReferenceEquality; StructuralComparison(_) >]
| true, _, _, _, Some true, _, _, _, Some true -> errorR (Error(FSComp.SR.augTypeCantHaveRefEqAndStructAttrs (), m))
| true, _, _, _, Some (true,_), _, _, _, Some (true,_) -> errorR (Error(FSComp.SR.augTypeCantHaveRefEqAndStructAttrs (), m))

// non augmented type, [<ReferenceEquality; ... >]
// non augmented type, [<StructuralEquality; ... >]
// non augmented type, [<StructuralComparison(_); ... >]
| false, _, _, _, Some true, _, _, _, _
| false, _, _, _, _, Some true, _, _, _
| false, _, _, _, _, _, _, _, Some true -> errorR (Error(FSComp.SR.augOnlyCertainTypesCanHaveAttrs (), m))
| false, _, _, _, Some (true,_), _, _, _, _
| false, _, _, _, _, Some (true,_), _, _, _
| false, _, _, _, _, _, _, _, Some (true,_) -> errorR (Error(FSComp.SR.augOnlyCertainTypesCanHaveAttrs (), m))
// All other cases
| _ -> errorR (Error(FSComp.SR.augInvalidAttrs (), m))
| _ ->
if tycon.IsFSharpObjectModelTycon then

let enumAttributeNames =
[
match attribs with
| _,_,_,Some a,_,_,_,_,_ -> a
| _ -> ()
match attribs with
| _,_,_,_,Some a,_,_,_,_ -> a
| _ -> ()
match attribs with
| _,_,_,_,_,Some a,_,_,_ -> a
| _ -> ()
match attribs with
| _,_,_,_,_,_,Some a,_,_ -> a
| _ -> ()
match attribs with
| _,_,_,_,_,_,_,Some a,_ -> a
| _ -> ()
match attribs with
| _,_,_,_,_,_,_,_,Some a -> a
| _ -> ()
]
|> List.map (fun struct(_,b) -> b)
match enumAttributeNames with
| [] ->
assert false
errorR (Error(FSComp.SR.augInvalidAttrs (), m))
| [attrName] -> errorR (Error(FSComp.SR.augInvalidAttrForFSharpObjectModelType attrName, m))
| attrs ->
let names = attrs |> String.concat ", "
errorR (Error(FSComp.SR.augInvalidAttrsForFSharpObjectModelType names, m))
else
errorR (Error(FSComp.SR.augInvalidAttrs (), m))

let hasNominalInterface tcref =
let ty = generalizedTyconRef g (mkLocalTyconRef tycon)
Expand All @@ -1103,21 +1142,21 @@ let CheckAugmentationAttribs isImplementation g amap (tycon: Tycon) =

match attribs with
// [<NoEquality>] + any equality semantics
| _, _, Some true, _, _, _, _, _, _ when (hasExplicitEquals || hasExplicitGenericEquals) ->
| _, _, Some (true,_), _, _, _, _, _, _ when (hasExplicitEquals || hasExplicitGenericEquals) ->
warning (Error(FSComp.SR.augNoEqNeedsNoObjEquals (), m))
// [<NoComparison>] + any comparison semantics
| _, _, _, _, _, _, Some true, _, _ when (hasExplicitICompare || hasExplicitIGenericCompare) ->
| _, _, _, _, _, _, Some (true,_), _, _ when (hasExplicitICompare || hasExplicitIGenericCompare) ->
warning (Error(FSComp.SR.augNoCompCantImpIComp (), m))

// [<CustomEquality>] + no explicit override Object.Equals + no explicit IStructuralEquatable
| _, _, _, Some true, _, _, _, _, _ when isImplementation && not hasExplicitEquals && not hasExplicitGenericEquals ->
| _, _, _, Some (true,_), _, _, _, _, _ when isImplementation && not hasExplicitEquals && not hasExplicitGenericEquals ->
errorR (Error(FSComp.SR.augCustomEqNeedsObjEquals (), m))
// [<CustomComparison>] + no explicit IComparable + no explicit IStructuralComparable
| _, _, _, _, _, _, _, Some true, _ when isImplementation && not hasExplicitICompare && not hasExplicitIGenericCompare ->
| _, _, _, _, _, _, _, Some (true,_), _ when isImplementation && not hasExplicitICompare && not hasExplicitIGenericCompare ->
errorR (Error(FSComp.SR.augCustomCompareNeedsIComp (), m))

// [<ReferenceEquality>] + any equality semantics
| _, _, _, _, Some true, _, _, _, _ when (hasExplicitEquals || hasExplicitIGenericCompare) ->
| _, _, _, _, Some (true,_), _, _, _, _ when (hasExplicitEquals || hasExplicitIGenericCompare) ->
errorR (Error(FSComp.SR.augRefEqCantHaveObjEquals (), m))

| _ -> ()
Expand All @@ -1132,9 +1171,9 @@ let TyconIsCandidateForAugmentationWithCompare (g: TcGlobals) (tycon: Tycon) =
// [< >]
| true, true, None, None, None, None, None, None, None
// [<StructuralEquality; StructuralComparison>]
| true, true, None, None, None, Some true, None, None, Some true
| true, true, None, None, None, Some (true,_), None, None, Some (true,_)
// [<StructuralComparison>]
| true, true, None, None, None, None, None, None, Some true -> true
| true, true, None, None, None, None, None, None, Some (true,_) -> true
// other cases
| _ -> false

Expand All @@ -1151,7 +1190,7 @@ let TyconIsCandidateForAugmentationWithEquals (g: TcGlobals) (tycon: Tycon) =
| true, _, None, None, None, None, _, _, _
// [<StructuralEquality; _ >]
// [<StructuralEquality; StructuralComparison>]
| true, _, None, None, None, Some true, _, _, _ -> true
| true, _, None, None, None, Some (true,_), _, _, _ -> true
// other cases
| _ -> false

Expand Down
2 changes: 2 additions & 0 deletions src/Compiler/FSComp.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,8 @@ astDeprecatedIndexerNotation,"This indexer notation has been removed from the F#
386,augNoEqNeedsNoObjEquals,"A type with attribute 'NoEquality' should not usually have an explicit implementation of 'Object.Equals(obj)'. Disable this warning if this is intentional for interoperability purposes"
386,augNoCompCantImpIComp,"A type with attribute 'NoComparison' should not usually have an explicit implementation of 'System.IComparable', 'System.IComparable<_>' or 'System.Collections.IStructuralComparable'. Disable this warning if this is intentional for interoperability purposes"
387,augCustomEqNeedsNoCompOrCustomComp,"The 'CustomEquality' attribute must be used in conjunction with the 'NoComparison' or 'CustomComparison' attributes"
388,augInvalidAttrsForFSharpObjectModelType,"The attributes %s are only meant for discriminated unions or record types."
388,augInvalidAttrForFSharpObjectModelType,"The attribute %s is only meant for discriminated unions or record types."
forPositionalSpecifiersNotPermitted,"Positional specifiers are not permitted in format strings"
forMissingFormatSpecifier,"Missing format specifier"
forFlagSetTwice,"'%s' flag set twice"
Expand Down
10 changes: 10 additions & 0 deletions src/Compiler/xlf/FSComp.txt.cs.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Compiler/xlf/FSComp.txt.de.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Compiler/xlf/FSComp.txt.es.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Compiler/xlf/FSComp.txt.fr.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Compiler/xlf/FSComp.txt.it.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Compiler/xlf/FSComp.txt.ja.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.