Skip to content

Incr. comp. dep-node for traits, tests #33998

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 6 commits into from
Jun 4, 2016

Conversation

nikomatsakis
Copy link
Contributor

Introduce new tests and also make dep-node for trait selection a bit more selective.

Fixes #33850

r? @michaelwoerister

These tests reveal that the edges are in some cases
too strict.
To handle the general case, we include a vector of def-ids, so that we
can account for things like `(Foo, Bar)` which references both `Foo` and
`Bar`. This means it is not Copy, so re-jigger some APIs to use
borrowing more intelligently.
This way we distinguish, in particular, `Foo: Sized`
and `Bar: Sized`, which fixes rust-lang#33850.
@nikomatsakis nikomatsakis changed the title Incr comp dep node trait Incr. comp. dep-node for traits, tests Jun 1, 2016
TraitSelect(ref d) => op(d).map(TraitSelect),
TraitSelect(ref d, ref type_ds) => {
let d = try_opt!(op(d));
let type_ds = try_opt!(type_ds.iter().map(|d| op(d)).collect());
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why this works. Doesn't the argument to try_opt!() have to be an Option of something? But collect() will return a Vec?

Copy link
Contributor

Choose a reason for hiding this comment

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

You can collect an iterator yielding Options into a single Option

Copy link
Member

Choose a reason for hiding this comment

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

@jonas-schievink Oh, that's interesting. Thanks for enlightening me :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can collect an iterator yielding Options into a single Option

best feature ever

@michaelwoerister
Copy link
Member

LGTM! r=me with the typo fixed. I'll leave it up to you, if you want to get rid of the avoidable allocation in DepTask::drop. It's not occurring in a tight loop exactly.

@nikomatsakis
Copy link
Contributor Author

@bors r=mw

@bors
Copy link
Collaborator

bors commented Jun 4, 2016

📌 Commit e548c46 has been approved by mw

@bors
Copy link
Collaborator

bors commented Jun 4, 2016

⌛ Testing commit e548c46 with merge 382ab92...

bors added a commit that referenced this pull request Jun 4, 2016
Incr. comp. dep-node for traits, tests

Introduce new tests and also make dep-node for trait selection a bit more selective.

Fixes #33850

r? @michaelwoerister
@bors bors merged commit e548c46 into rust-lang:master Jun 4, 2016
@nikomatsakis nikomatsakis deleted the incr-comp-dep-node-trait branch October 3, 2016 14:55
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants