Skip to content

fix: Insert type vars in function arguments #14897

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

Merged
merged 1 commit into from
May 26, 2023
Merged

Conversation

HKalbasi
Copy link
Member

follow up #14891

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 26, 2023
Comment on lines +1684 to +1688
let coercion_target = self.insert_type_vars(coercion_target);
if self.coerce(Some(arg), &ty, &coercion_target).is_err() {
Copy link
Member Author

Choose a reason for hiding this comment

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

Is this correct? I think we shouldn't emit type mismatch in this case:

fn foo(x: Vec<Unknown>) {
}
foo(vec![1, 2, 3]);

So this is correct. But it feels like it was intentionally the way it was. Maybe it has performance consideration?

Copy link
Contributor

@lowr lowr May 26, 2023

Choose a reason for hiding this comment

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

Yeah I think this is correct thing to do. Can you comment that we insert type vars to avoid reporting that kind of type mismatches?

Also, can you add a test that does emit type mismatch when we fail to coerce even after this change? Something like

fn foo(x: Vec<Unknown>) {}
fn test() {
    foo(42);
      //^^ type mismatch here
}

@HKalbasi
Copy link
Member Author

@bors r+

@bors
Copy link
Contributor

bors commented May 26, 2023

📌 Commit c21d09f has been approved by HKalbasi

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented May 26, 2023

⌛ Testing commit c21d09f with merge 2fd9260...

@bors
Copy link
Contributor

bors commented May 26, 2023

☀️ Test successful - checks-actions
Approved by: HKalbasi
Pushing 2fd9260 to master...

@bors bors merged commit 2fd9260 into rust-lang:master May 26, 2023
@lnicola lnicola changed the title Insert type vars in function arguments fix: Insert type vars in function arguments May 27, 2023
@lnicola
Copy link
Member

lnicola commented May 27, 2023

This fixed the type mismatches regression on webrender.

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants