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

Do not eagerly reject inference vars when trying to resolve method calls. #126316

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Jun 12, 2024

Reintroduces the changes reverted in #126258 and fixes the regression in the old solver.

The new solver will still fail on

pub trait Parser<E> {
    fn parse(&self) -> E;
}

impl<E, T: Fn() -> E> Parser<E> for T {
    fn parse(&self) -> E {
        self()
    }
}

pub fn recursive_fn<E>() -> impl Parser<E> {
    move || recursive_fn().parse()
    //[next]~^ ERROR: type annotations needed
}

because while type checking the method call .parse(), when looking for candidates, we end up not finding any, as the only information we have is that there's an unresolved AliasTy(opaque, infer_var) floating around while looking for a method call on infer_var.

This obviously seems fixable by looking at said obligation, but I'd prefer to tackle that separately (I got half an impl locally, but there are multiple paths forward that we should discuss)

r? @lcnr @compiler-errors

I'm going to split out the effect-free cleanups and the inference changes to separate PRs, so we can review them in isolation, but with the goal of enabling this PR to land

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 12, 2024
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

compiler-errors added a commit to compiler-errors/rust that referenced this pull request Jun 12, 2024
…r-errors

Avoid a bunch of booleans in favor of Result<(), ErrorGuaranteed> as that more robustly proves that an error has been emitted

pulled out of rust-lang#126316

This PR cannot have any effect on compilation.
All it does is shift a `Ty::new_misc_error` to a `span_delayed_bug` and preserve the `ErrorGuaranteed` in all other cases
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 12, 2024
Rollup merge of rust-lang#126317 - oli-obk:recursive_rpit4, r=compiler-errors

Avoid a bunch of booleans in favor of Result<(), ErrorGuaranteed> as that more robustly proves that an error has been emitted

pulled out of rust-lang#126316

This PR cannot have any effect on compilation.
All it does is shift a `Ty::new_misc_error` to a `span_delayed_bug` and preserve the `ErrorGuaranteed` in all other cases
@bors
Copy link
Contributor

bors commented Jun 12, 2024

☔ The latest upstream changes (presumably #126345) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Jun 13, 2024

☔ The latest upstream changes (presumably #126374) made this pull request unmergeable. Please resolve the merge conflicts.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Jun 14, 2024

☔ The latest upstream changes (presumably #126462) made this pull request unmergeable. Please resolve the merge conflicts.

@oli-obk
Copy link
Contributor Author

oli-obk commented Jun 14, 2024

@rustbot ready the component PRs have landed

@rust-log-analyzer

This comment has been minimized.

Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

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

want to take some time to look at when exactly we don't error if there's an auto-deref step resulting in an infer var, will get to this next week hopefully

@bors
Copy link
Contributor

bors commented Jun 16, 2024

☔ The latest upstream changes (presumably #126540) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Sep 6, 2024

☔ The latest upstream changes (presumably #121848) made this pull request unmergeable. Please resolve the merge conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants