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

Inference worse in method signature using associated type than with new generic type parameter eq-constrained to the associated type #45462

Open
scottmcm opened this issue Oct 23, 2017 · 1 comment
Open

Inference worse in method signature using associated type than with new generic type parameter eq-constrained to the associated type #45462

scottmcm opened this issue Oct 23, 2017 · 1 comment
Labels
A-associated-items Area: Associated items (types, constants & functions) A-inference Area: Type inference C-enhancement Category: An issue proposing an enhancement or a PR with one. T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@scottmcm
Copy link
Member

With this signature

    fn try_fold<F, T: Try>(self, init: T::Ok, mut f: F) -> T where
        Self: Sized, F: FnMut(T::Ok, Self::Item) -> T

This code

    let a = [1, 2, 3];
    let sum = a.iter().try_fold(0i8, |acc, &x| acc.checked_add(x));

Gets error[E0619]: the type of this value must be known in this context.

But it works fine with the seemingly-equivalent signature

    fn try_fold<F, U, T: Try<Ok=U>>(self, init: U, mut f: F) -> T where
        Self: Sized, F: FnMut(U, Self::Item) -> T

Since acc and init must have the same type in both cases, shouldn't both signatures work?

Full repro: https://play.rust-lang.org/?gist=f8d2baeae64b17344e1cf27673a2e905&version=nightly
@scottmcm scottmcm changed the title Inference worse in method signature using associated type than with new generic type parameter eq-constrained to the generic type Inference worse in method signature using associated type than with new generic type parameter eq-constrained to the associated type Oct 23, 2017
@TimNN TimNN added A-associated-items Area: Associated items (types, constants & functions) A-inference Area: Type inference C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Oct 24, 2017
This was referenced Oct 28, 2017
Closed
Merged
bors added a commit that referenced this issue Nov 17, 2017
@bors
Auto merge of #45595 - scottmcm:iter-try-fold, r=dtolnay
b32267f 
Short-circuiting internal iteration with Iterator::try_fold & try_rfold

These are the core methods in terms of which the other methods (`fold`, `all`, `any`, `find`, `position`, `nth`, ...) can be implemented, allowing Iterator implementors to get the full goodness of internal iteration by only overriding one method (per direction).

Based off the `Try` trait, so works with both `Result` and `Option` (:tada: #42526).  The `try_fold` rustdoc examples use `Option` and the `try_rfold` ones use `Result`.

AKA continuing in the vein of PRs #44682 & #44856 for more of `Iterator`.

New bench following the pattern from the latter of those:
```
test iter::bench_take_while_chain_ref_sum          ... bench:   1,130,843 ns/iter (+/- 25,110)
test iter::bench_take_while_chain_sum              ... bench:     362,530 ns/iter (+/- 391)
```

I also ran the benches without the `fold` & `rfold` overrides to test their new default impls, with basically no change.  I left them there, though, to take advantage of existing overrides and because `AlwaysOk` has some sub-optimality due to #43278 (which 45225 should fix).

If you're wondering why there are three type parameters, see issue #45462

Thanks for @bluss for the [original IRLO thread](https://internals.rust-lang.org/t/pre-rfc-fold-ok-is-composable-internal-iteration/4434) and the rfold PR and to @cuviper for adding so many folds, [encouraging me](#45379 (comment)) to make this PR, and finding a catastrophic bug in a pre-review.
@SimonSapin
Copy link
Contributor

To me this looks like a bug or limitation in our current inference implementation. The two signatures should be equivalent.

@scottmcm scottmcm added the T-types Relevant to the types team, which will review and decide on the PR/issue. label Dec 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-associated-items Area: Associated items (types, constants & functions) A-inference Area: Type inference C-enhancement Category: An issue proposing an enhancement or a PR with one. T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@SimonSapin @TimNN @scottmcm