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

Use List.exists for contains #1518

Merged
merged 3 commits into from
Sep 11, 2016
Merged

Use List.exists for contains #1518

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
025c35a
Use List.exists for contains
forki Sep 5, 2016
dc41206
ListSet.contains is List.exists
forki Sep 8, 2016
3dacd0c
Revert "ListSet.contains is List.exists"
forki Sep 8, 2016
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
2 changes: 1 addition & 1 deletion src/fsharp/autobox.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ open Microsoft.FSharp.Compiler.TypeRelations
// Decide the set of mutable locals to promote to heap-allocated reference cells

type cenv =
{ g: TcGlobals;
{ g: TcGlobals
amap: Import.ImportMap }

/// Find all the mutable locals that escape a method, function or lambda expression
Expand Down
7 changes: 2 additions & 5 deletions src/fsharp/lib.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,14 +178,11 @@ module ListAssoc =
//------------------------------------------------------------------------

module ListSet =
(* NOTE: O(n)! *)
let rec contains f x l =
match l with
| [] -> false
| x'::t -> f x x' || contains f x t
let inline contains f x l = List.exists (f x) l
Copy link
Member

Choose a reason for hiding this comment

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

@forki
This API look like exists rather than contains. Is it not misnamed? Perhaps we should name it exists and fix up the places where we use it by the wrong name.

exists: https://msdn.microsoft.com/en-us/visualfsharpdocs/conceptual/list.exists%5b't%5d-function-%5bfsharp%5d
contains: https://msdn.microsoft.com/en-us/visualfsharpdocs/conceptual/list.contains%5b't%5d-function-%5bfsharp%5d

Kevin

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point. done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mhm after thinking about it. All the other listset functions have the first parameter f which is the projection. In this sense it makes sense to revert my last commit and keep it named contains.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The real question is: can we use a different data structure!?

It needs to be a set that allows to pass in own equality semantics.


(* NOTE: O(n)! *)
let insert f x l = if contains f x l then l else x::l

let unionFavourRight f l1 l2 =
match l1, l2 with
| _, [] -> l1
Expand Down