Adds toSet to create sets from iterables

[beef331]

Replicates the toHashSet proc for normal sets.

[beef331]

Thank you for all your comments/insight, the latest commit should check all of the boxes, atleast as far as I know.

[beef331]

I'm inept with the rebase :D

[beef331]

There we go this should be fine now, thanks.

[timotheecour]

actually, why do we even need the static: doAssert E is SetElement ?
you'll still get an error when attempting var result: set[E] if say, E is int64.

[beef331]

Well the main reason is to give a slightly more informative error instead of just set is to large

[timotheecour]

then we should fix the errmsg in the compiler, where it belongs (which would benefit other code):

nim> var z: set[int]
Error: set is too large

=>

Error: set is too large, got `int` but expected ...

[timotheecour]

LGTM, thanks! I restarted CI, you're hitting windows flaky test timotheecour#411 (EDIT: reported it as #16317)

[timotheecour]

now you're hitting a different CI issue I haven't seen before (timotheecour#441); I restarted failing job

[beef331]

Leave it to me to hit issues no one has hit before.

[timotheecour]

just bikeshedding but another candidate name would be:
std/builtinsets to avoid confusion with existing (and poorly named) std/sets

I'm ok with either, though

[beef331]

Do you mean builtinsets or builtinsetutils? The former may cause confusion with where set[T] comes from.

[timotheecour]

The former may cause confusion with where set[T] comes from.

well by definition they're builtin ;-) ; builtinsetutils is a bit of a mouthful... maybe setutils is good enough

[Araq]

So ... rather than {'a', 'b', 'c'} you want to write toSet ['a', 'b', 'c']? Doesn't seem useful to me. What's the use case? And now don't say "generic programming", that is not a use case.

[timotheecour]

rather than {'a', 'b', 'c'} you want to write toSet ['a', 'b', 'c']?

intended use case is not for cases where input is a litteral (in which case you'd write {'a', 'b', 'c'} directly), just like intended use case for toSeq (or toHashSet, etc) is not for cases where input is a literal. Instead, it's for cases where input is some iterable, which includes strings/seq/array/iterator call etc.

examples:

let input: string = getInput()
let uniqueLetters = input.toSet # or: callFn(input.toSet)

I've often needed this myself as well, it fits well with the rest of stdlib:

• toSeq
• toHashSet
• toPackedSet
• toIntSet
  etc...

to the point that not having toSet seems like a weird omission

[ee7]

Some nitpicks.

[timotheecour]

@beef331 you can click "Resolve Conversation" on each item that was resolved (unless there's a disagreement) :)

[timotheecour]

hitting #16317 again; I've restarted failing tests

[timotheecour]

I restarted again;
you're hitting #16317 again (which is recently very flaky) and also #16315 (which has been fixed already)

This is why flaky tests really need to be fixed..

[Araq]

Needs a changelog entry and maybe (!) a ```--define:nimHasSetUtils`` (edit compiler/condsysm.nim to add it).

[timotheecour]

Needs a changelog

indeed

and maybe (!) a ```--define:nimHasSetUtils`` (edit compiler/condsysm.nim to add it).

this hasn't been needed for other new modules, and in fact will be obsoleted by whichModule timotheecour#376 once I get to it (it's not hard); plus, condsysm.nim is usually about the nim binary, not the stdlib; the distinction matters because of --lib flag etc

[timotheecour]

iteratable doesn't seem enligh; should be iterable

[timotheecour]

merged; previous CI was green and last commit just changed changelog.