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

Separate out predicates from ty::Generics and ty::TraitDef #22182

Merged
merged 12 commits into from
Feb 12, 2015

Conversation

nikomatsakis
Copy link
Contributor

This resolves a number of bugs that trigger stack overflows or other cyclic errors (#20220, #20551, at least). I plan to do some follow-up PRs addressing further problems and also simplifying / restructuring collect, but I wanted to get something landed for now to unblock #20551 in particular.

r? @nick29581 (it is based on work that you started)
f? @jroesch (also based on your branch)

@nikomatsakis
Copy link
Contributor Author

Oh, shoot, tidy failures. I'll patch those.

// ICEs trying to fetch the generics early in the pipeline. This
// is kind of a hacky workaround in that -Z verbose is required to
// avoid those ICEs.
let generics = get_generics();
Copy link
Member

Choose a reason for hiding this comment

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

👍 it is better than an ICE and having to remove all the debugging :)

@jroesch
Copy link
Member

jroesch commented Feb 11, 2015

I read it over and it looks good. I'm assuming you are waiting to touch up and land the rest of the changes from my branch? I'm sure there are issues to be ironed out with it still.

@nikomatsakis
Copy link
Contributor Author

@jroesch yes once this lands I plan to rebase your branch on top of it (as well as some other unrelated cleanup I'd like to do).

@@ -233,6 +233,14 @@ pub fn get_trait_def<'tcx>(tcx: &ty::ctxt<'tcx>, def: ast::DefId) -> ty::TraitDe
decoder::get_trait_def(&*cdata, def.node, tcx)
}

pub fn get_predicates<'tcx>(tcx: &ty::ctxt<'tcx>, def: ast::DefId)
-> ty::GenericPredicates<'tcx>
Copy link
Member

Choose a reason for hiding this comment

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

indent

@nikomatsakis
Copy link
Contributor Author

Indentation nits addressed.


So as you can see, in general translating types requires knowing the
trait hierarchy. But this gets a bit tricky because translating the
trait hierarchy requires convering the types that appear in trait
Copy link
Member

Choose a reason for hiding this comment

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

typo 'convering'

@nrc
Copy link
Member

nrc commented Feb 12, 2015

@bors r+ b4a0ffabe15385cf8e6b86fcaa9fc3925af383fa

…nto a separate map, `tcx.predicates`, that is used for both traits and other kinds of items. Also use two newtypes to distinguish

instantiated predicates from the raw, unsubstituted predicates extracted from the map.
and predicates. Try to document how things work. More cleanup is
needed here but I had to draw the line somewhere gosh darn it.
@nikomatsakis
Copy link
Contributor Author

Rebased, nits addressed.

@nikomatsakis
Copy link
Contributor Author

@bors r+ 53b6ef5

fetch trait definitions. This allows is to be used early in the compiler
without triggering ICEs. Also make -Z verbose less horrifyingly ugly.
and the type appearing in the trait would (previously) trigger an
error message. The code is now accepted. No reported issue that I am
aware of.
@nikomatsakis
Copy link
Contributor Author

@bors r+ a25ed22

bors added a commit that referenced this pull request Feb 12, 2015
This resolves a number of bugs that trigger stack overflows or other cyclic errors.

r? @nick29581 (it is based on work that you started)
f? @jroesch (also based on your branch)
@bors
Copy link
Contributor

bors commented Feb 12, 2015

⌛ Testing commit a25ed22 with merge 39b463f...

@bors bors merged commit a25ed22 into rust-lang:master Feb 12, 2015
@goffrie goffrie mentioned this pull request Feb 13, 2015
bors added a commit that referenced this pull request Feb 16, 2015
…felix

Implement rules described in rust-lang/rfcs#599.

Fixes #22211.

~~Based atop PR #22182, so the first few commits (up to and including "Pacify the mercilous nrc") have already been reviewed.~~
@nikomatsakis nikomatsakis deleted the cycles-in-collect branch March 30, 2016 16:12
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.

4 participants