Skip to content

move concurrent stuff from libextra to libsync #11939

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 5, 2014
Merged

move concurrent stuff from libextra to libsync #11939

merged 1 commit into from
Feb 5, 2014

Conversation

jeremyletang
Copy link
Contributor

This time everything should be okay, No break due to a failed merge or rebase...

Sorry for the abuse of pull request.

So this move extra::sync, extra::arc, extra::future, extra::comm and extra::task_pool to libsync.

alexcrichton
alexcrichton reviewed Jan 30, 2014
View reviewed changes
src/librustdoc/html/render.rs
use extra::arc::Arc;
use extra::json::ToJson;
use sync::arc::Arc;
use sync::json::ToJson;
Copy link
Member

Choose a reason for hiding this comment

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

You probably still want extra::json

@alexcrichton
Copy link
Member

This looks great, thanks!

I'm curious what others think about libsync and reexports. As implemented, it looks like all the inner modules have remained public, so you can access sync::Future and sync::future::Future. I would prefer to make all the inner modules private and force usage through the reexports themselves.

@jeremyletang
Copy link
Contributor Author

I'm okay with you to make the inner modules private. Should i do this?

@alexcrichton
Copy link
Member

Sure, let's try that out and see how it goes.

@alexcrichton
Copy link
Member

Could you also squash the commits together? Otherwise this looks good to me!

@brson
Copy link
Contributor

brson commented Jan 31, 2014

@alexcrichton I think we should do the pub reexports of private items, and we should keep looking for places to apply this pattern. Might want to talk about it in the style guide.

@jeremyletang
Copy link
Contributor Author

@alexcrichton i have not see this before but one test failed, i have made the change.

@alexcrichton
Copy link
Member

Needs another rebase (sorry!)

@jeremyletang
Copy link
Contributor Author

No problem! it's okay.

@jeremyletang
Copy link
Contributor Author

@alexcrichton some test failed on compile-fail, i've updated this test.

I have a question, i run make check-stage2 to test these change and i've no error. This command doesn't run the tests for compile-fail?

@alexcrichton
Copy link
Member

I'm actually not entirely sure what subset of the tests make check-stage2 is running. The bots run make check which I would guess is a superset of what's run at check-stage2.

@jeremyletang
Copy link
Contributor Author

oh, when i used make check-stage2 i have only one test who failed, phase-syntax-link-does-resolve.rs, and i was thinking that all the other tests pass and not that the tests stop after a pool of test if there is an error.

But with make check this test don't fail, and i can see the others error.

I have ignored the tests in guide-task.md, because i can't find a way to specify a given extern mod to rustdoc other than extra.

@alexcrichton
Copy link
Member

Hm, I'd rather not throw the tasks guide under the bus just yet. I thought that it was still at least somewhat useful.

I think that references to sync::module will need to be updated (they should just be sync::Type). This should involve just touching up the documentation in a few places to point to somewhere different (all the content is still the same).

For the tests, the test runner injects fn main() {} if it doesn't find it, so you can inject the sync crate like so:

# extern mod sync;

use sync::FooBar;

# fn main() {
FooBar::do_something_awesome();
# }

@jeremyletang
Copy link
Contributor Author

It's updated to test the examples.

alexcrichton
alexcrichton reviewed Feb 3, 2014
View reviewed changes
doc/guide-tasks.md
[`sync::comm`]: sync/index.html
[`sync::sync`]: sync/index.html
[`sync::Arc`]: sync/index.html
[`sync::Future`]: sync/index.html
Copy link
Member

Choose a reason for hiding this comment

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

Could this link to sync/struct.Future.html (for the specific pages)?

@alexcrichton
Copy link
Member

Thanks for keeping the code examples up to date! It's definitely much appreciated.

@jeremyletang
Copy link
Contributor Author

Should i specify for all reexported structs? or only Future?

@alexcrichton
Copy link
Member

I was thinking for all of the specifically linked-to structs.

@jeremyletang
Copy link
Contributor Author

@alexcrichton i've update the reference in guid-task.md, and updated lib sync with your changes to extra::sync. it finally should be okay i think !

@jeremyletang
Copy link
Contributor Author

@alexcrichton oh sorry for the merge error in the gitignore ! It should be good now !

@jeremyletang
Copy link
Contributor Author

The merge of libuuid make conflit, so i've update the pull request.

@alexcrichton
Copy link
Member

alas, there appears to be another conflict!

@jeremyletang
Copy link
Contributor Author

It should be okay !

@alexcrichton
Copy link
Member

Ah, it looks like this will also need to update src/etc/licenseck.py with the updated location of mpsc_intrusive.rs

@jeremyletang
Copy link
Contributor Author

ah! i ask for this in irc yesterday, but without answer i was thinking bors have a special directive to ignore this kind of error.
So it's updated.

@jeremyletang
Copy link
Contributor Author

@alexcrichton ahah the add of CowArc have break the tests again, i've just update it.

bors added a commit that referenced this pull request Feb 5, 2014
@bors
auto merge of #11939 : JeremyLetang/rust/move-libsync, r=alexcrichton
2bf575c 
This time everything should be okay, No break due to a failed merge or rebase...

Sorry for the abuse of pull request.

So this move extra::sync, extra::arc, extra::future, extra::comm and extra::task_pool to libsync.
@bors bors closed this Feb 5, 2014
@bors bors merged commit dd21a51 into rust-lang:master Feb 5, 2014
flip1995 pushed a commit to flip1995/rust that referenced this pull request Dec 28, 2023
@bors
Auto merge of rust-lang#11977 - y21:is_const_evaluatable_ice, r=Manis…
fff484d 
…hearth

don't visit nested bodies in `is_const_evaluatable`

Fixes rust-lang#11939

This ICE happened in `if_let_some_else_none`, but the root problem is in one of the utils that it uses.
It is (was) possible for `is_const_evalutable` to visit nested bodies which would lead to it trying to get the type of one of the expressions with the wrong typeck table, which won't have the type stored.

Notably, for the expression `Bytes::from_static(&[0; 256 * 1024]);` in the linked issue, the array length is an anonymous const in which type checking happens on its own, so we can't use the typeck table of the enclosing function in there.

Visiting nested bodies is also not needed for checking whether an expression can be const, so I think it's safe to ignore just ignore them altogether.

changelog: Fix ICE when checking for constness in nested bodies
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jeremyletang @alexcrichton @brson @bors