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

Fix cross-ingot inherent method resolution #1051

Merged
merged 3 commits into from
Jan 28, 2025

Conversation

sbillig
Copy link
Collaborator

@sbillig sbillig commented Jan 27, 2025

This wasn't working before:

use core::Option // where `core` is a different ingot from this file

fn f() {
  let x = Option::Some(10) // failed to resolve `Option::Some` variant constructor
  x.unwrap() // failed to resolve `unwrap` method on `Option` type
  ...
}
  • When a function call expression "callee" path resolves to a tuple enum variant, we now immediately resolve to that variant constructor fn, instead of using select_method_candidate_for_path to find the enum constructor (which was searching the current ingot only and thus failing to resolve).
  • When resolving a method call, we now search for impl blocks in the ingot in which the receiver type is defined instead of in the current ingot.

I incidentally added some stuff to library/core to test this with realistic code.

@sbillig sbillig force-pushed the enum-tuple-ctor-fix branch from 1dd1ec5 to 2015de6 Compare January 28, 2025 05:46
@sbillig sbillig force-pushed the enum-tuple-ctor-fix branch from 2015de6 to ef331cc Compare January 28, 2025 05:54
@sbillig sbillig marked this pull request as ready for review January 28, 2025 05:55
@sbillig sbillig marked this pull request as draft January 28, 2025 05:55
@sbillig sbillig marked this pull request as ready for review January 28, 2025 20:57
@sbillig sbillig requested a review from micahscopes January 28, 2025 21:01
@micahscopes micahscopes merged commit 5be9878 into ethereum:master Jan 28, 2025
6 checks passed
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.

2 participants