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

Obviously overlapping blanket impls are accepted #157

Closed
CheaterCodes opened this issue Nov 13, 2023 · 3 comments · Fixed by #167
Closed

Obviously overlapping blanket impls are accepted #157

CheaterCodes opened this issue Nov 13, 2023 · 3 comments · Fixed by #167

Comments

@CheaterCodes
Copy link

In the hope of progressing rust-lang/rust#20400, I stumbled across rust-lang/rfcs#1672.
Given this comment, I decided to properly investigate a-mir-formality.

As it turns out, the current overlap checks are very generous. I don't know whether this is intentional and I just misunderstood the purpose of this tool, or if it's an oversight, or simply incompleteness. In any case, the following test is currently accepted by a-mir-formality, and I personally think it should (obviously) not be:

[
    crate core {
        trait Foo { }

        trait Bar { }

        impl<ty T> Foo for T { }
        
        impl<ty T> Foo for T where T: Bar { }
    }
]

I also pushed this test blanket_overlap with the expected error output to a branch on my fork:
https://github.com/CheaterCodes/a-mir-formality/tree/overlap_test

It appears that currently, a-mir-formality is not considering that a downstream type may implement both of the above traits, instead it just determines that may_be_remote is provable false (which it isn't, if I understand this code correctly).

If my understanding so far is correct, I'll happily look into this more closely, but it seems like fairly major effort would be required.

@compiler-errors
Copy link
Member

For the record, : is outlives syntax. See #147.

I think the trait bound is expressed Bar(T), though reference other tests to make sure.

@lqd
Copy link
Member

lqd commented Nov 13, 2023

Coherence is also currently being worked on according to https://hackmd.io/gw4UQcN8Rm-MGqFFF0M6jA#Tracking-issue-for-a-mir-formality

@CheaterCodes
Copy link
Author

For the record, : is outlives syntax. See #147.

I think the trait bound is expressed Bar(T), though reference other tests to make sure.

I'm not sure if this is true. Other tests use the same syntax, and the grammar also suggests that this is correct:

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 a pull request may close this issue.

3 participants