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

[WIP] Check more things for well-formedness #25701

Closed
wants to merge 4 commits into from

Conversation

arielb1
Copy link
Contributor

@arielb1 arielb1 commented May 22, 2015

Makes wf check more bounds (hopefully all of them).

Fixes #21748
Fixes #25692
Fixes #25388

Because it makes the rules stricter, this is technically a
[breaking-change]

@rust-highfive
Copy link
Collaborator

r? @nrc

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive assigned nikomatsakis and unassigned nrc May 22, 2015
@arielb1 arielb1 changed the title [WIP] wf improvements [WIP] Make the well-formedness check check more rules May 22, 2015
Because we don't check the well-formedness of region relationships of
an impl's on the Self-type, or infer region relationss from them, many impls
now have to specify their region relations, making this a [breaking-change].

The most common pattern I found that is now broken is that

    impl<'a, T> IntoIterator for &'a Collection<T> {
        type Item = &'a T;
        type IntoIter = ...;
    }

now has to specify its `T: 'a` bound.
They don't do anything ATM.

This is a
[breaking-change]
@arielb1 arielb1 changed the title [WIP] Make the well-formedness check check more rules Check more things for well-formedness May 23, 2015
@arielb1
Copy link
Contributor Author

arielb1 commented May 23, 2015

r? @nikomatsakis

@bors
Copy link
Contributor

bors commented May 24, 2015

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

@nikomatsakis
Copy link
Contributor

Hmm, I think this PR deserves a bit of thought. First, I've never been happy with the wf code, and this PR makes me less happy. It seems like we should be adding the knowledge that (e.g.) (A,B) requires that A:Sized and B:Sized into implicator.rs, not wf.rs, but honestly I have to re-read the code to remember how they interact (and I suspect there is overlap and they should be simplified).

Second, I do think we should consider just when/where to enforce WF and trait requirements (and where to infer region relationships). The current policy is that we enforce all WF/trait requirements "everywhere" except that it is haphazardly enforced; but it's not totally clear to me that this is the best policy. Certainly the changes here seem ergonomically costly. I have to mull this a bit to form my opinion. (Note that enforcing WF in struct definitions, for example, was done deliberately in order to allow us to obviate things like T:'a from appearing on lots of fns.)

In any case, certainly this change is breaking enough that we would want to start considering the steps and mitigation measures from (pending) RFC rust-lang/rfcs#1122.

cc @rust-lang/lang

@nikomatsakis
Copy link
Contributor

ps @arielb1 just to be clear, thanks very much for the PR! I'm not intending those previous comments as a critique of the PR per se, just pointing out that we want to think about these changes and approach them the right way.

@arielb1 arielb1 changed the title Check more things for well-formedness [WIP] Check more things for well-formedness Jun 2, 2015
@jroesch
Copy link
Member

jroesch commented Jun 4, 2015

@nikomatsakis I think it would be good to clean or clarify the relationship between wf.rs and implicator.rs, a couple of the inconsistencies @kyledewey and I found around the checking of trait requirements seemed to be related.

@alexcrichton
Copy link
Member

Closing due to inactivity, but feel free to reopen with a rebase!

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.

[[T]] is allowed in structs Check types in method signatures &[Trait] should probably not be allowed
7 participants