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

refactor(dag): CommitSet::from_iter() -> collect::<CommitSet>() #376

Merged
merged 1 commit into from
Apr 27, 2022

Conversation

claytonrcarter
Copy link
Collaborator

Oops, I wrote CommitSet::from_iter(foo.collect()) but it should be just foo.collect(). Anyways, you can leave this as is, and I'll update this site and the other uses in the codebase to use .collect() later.

Is this what you had in mind over at #366 (comment)?

Out of curiosity, do you generally prefer let ... : CommitSet = ... .collect() or let ... = ... collect::<CommitSet>()? Or does it depend on the context? My understanding ATM is that they're roughly equivalent ways of nudging the compiler towards the desired type, right?

@arxanas
Copy link
Owner

arxanas commented Apr 27, 2022

Thanks for cleaning this up!

My understanding ATM is that they're roughly equivalent ways of nudging the compiler towards the desired type, right?

Yep, they're pretty much the same. Generally, I prefer let x: CommitSet = (...).collect(); just because there's less line noise (as ::<> can be a bit hard to read). I would say that Rust is actually missing the relevant feature, which is the ability to annotate the type of an arbitrary expression, and not just a pattern binding. Using the same : syntax, it might hypothetically look like this:

let x = foo
    .iter()
    .collect(): CommitSet;

I would probably prefer to do it that way if the feature existed.

@arxanas arxanas merged commit 45fcbb0 into arxanas:master Apr 27, 2022
@martinvonz
Copy link
Collaborator

I would say that Rust is actually missing the relevant feature, which is the ability to annotate the type of an arbitrary expression, and not just a pattern binding. Using the same : syntax, it might hypothetically look like this:

let x = foo
    .iter()
    .collect(): CommitSet;

I would probably prefer to do it that way if the feature existed.

It's might be less hypothetical than you think: rust-lang/rust#23416. At least I think that feature will allow you to write what you wanted.

@arxanas
Copy link
Owner

arxanas commented Apr 27, 2022

@martinvonz I knew the feature was in in RFC (after all, OCaml has it), but given that it's been 7+ years, I don't have any hope for it coming out anytime soon 😅.

@martinvonz
Copy link
Collaborator

@martinvonz I knew the feature was in in RFC (after all, OCaml has it), but given that it's been 7+ years, I don't have any hope for it coming out anytime soon 😅.

Wow, I didn't realize it had been there for that long (or even very long at all)

@claytonrcarter claytonrcarter deleted the refactor-commitset-from-iter branch July 1, 2022 01:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants