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

Builtin bounds for closures and traits #7248

Closed
wants to merge 9 commits into from

Conversation

bblum
Copy link
Contributor

@bblum bblum commented Jun 20, 2013

  • Kind-check closure bounds (a few cases are missing; I think for each one that missing it is conservative rather than unsound).
  • Parse, type-check, and 1/2 kind-check trait bounds.

This is pretty much all I can do before a snapshot.

The next step is to change the way boxed traits kind-check, which will involve lifting one restriction in kind.rs and imposing several others in type_contents. To do that I will need to actually add bounds to traits in unrelated code, which requires them to parse to begin with, which requires a snapshot.

r? @nikomatsakis @pcwalton

@nikomatsakis
Copy link
Contributor

Reading.

@@ -360,7 +360,7 @@ fn visit_expr(expr: @ast::expr, (rcx, v): (@mut Rcx, rvt)) {
// explaining how it goes about doing that.
let target_ty = rcx.resolve_node_type(expr.id);
match ty::get(target_ty).sty {
ty::ty_trait(_, _, ty::RegionTraitStore(trait_region), _) => {
ty::ty_trait(_, _, ty::RegionTraitStore(trait_region), _, _) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

In reality, what should happen here is that we should check the region bound no matter the store, and also check the region from the store as we do now. This is what causes #5723, because for an @Object or ~Object we never check any region bounds at all.

Copy link
Contributor

Choose a reason for hiding this comment

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

It might be that this fix should wait for another PR though since it will implicate the I/O library a bit I guess.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm already having to change the way with_bytes_reader is "implicated" in some more recent commits. I've made it use unsafe transmute and tagged it with a bunch of fixmes saying it should use an &Trait instead.

@bblum
Copy link
Contributor Author

bblum commented Jun 21, 2013

the extra commit just fixes a macro test

@bblum bblum closed this Jun 23, 2013
bors added a commit that referenced this pull request Jun 23, 2013
Fixed a merge conflict, some tests, some bitrotting, etc., from #7248.
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.

5 participants