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

Add a test for the case that impl function is poisoned #4950

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

Conversation

bricknerb
Copy link
Contributor

This adds missing coverage.
Part of #4622.

Comment on lines 23 to 28
class C2 {
// CHECK:STDERR: fail_impl_function_poisoned.carbon:[[@LINE+3]]:14: error: name used before it was declared [NameUseBeforeDecl]
// CHECK:STDERR: var v: F;
// CHECK:STDERR: ^
var v: F;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

It's probably best to use an example here that would otherwise be valid if possible, and it's not valid to declare things in the impl that don't correspond to names in the interface. How about something like this:

interface I {
  fn A(x: Self);
  fn B();
}

class B {
  impl as I {
    fn A(x: B);
    fn B();
  }
}

Longer term, I think the plan is for name lookup for the impl scope to look in the facet (B as I in this case), so it would find the function name B from the interface here instead of the class name. It's invalid either way, though, and seems good to test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.
Interestingly, because B is poisoned, we also get ImplMissingFunction error.

@bricknerb bricknerb requested a review from zygoloid February 15, 2025 03:23
@bricknerb bricknerb enabled auto-merge February 15, 2025 03:24
@jonmeow jonmeow removed their request for review February 18, 2025 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants