Move UV to its own crate. by alexcrichton · Pull Request #10058 · rust-lang/rust

alexcrichton · 2013-10-24T23:51:41Z

This is one of the final steps needed to complete #9128. It still needs a little bit of polish before closing that issue, but it's in a pretty much "done" state now.

The idea here is that the entire event loop implementation using libuv is now housed in librustuv as a completely separate library. This library is then injected (via extern mod rustv) into executable builds (similarly to how libstd is injected, tunable via #[no_uv]) to bring in the "rust blessed event loop implementation."

Codegen-wise, there is a new event_loop_factory language item which is tagged on a function with 0 arguments returning ~EventLoop. This function's symbol is then inserted into the crate map for an executable crate, and if there is no definition of the event_loop_factory language item then the value is null.

What this means is that embedding rust as a library in another language just got a little harder. Libraries don't have crate maps, which means that there's no way to find the event loop implementation to spin up the runtime. That being said, it's always possible to build the runtime manually. This request also makes more runtime components public which should probably be public anyway. This new public-ness should allow custom scheduler setups everywhere regardless of whether you follow the rt::startpath.

alexcrichton · 2013-10-24T23:53:35Z

I also wanted to mention that we should not be linking libuv into librustrt right now, but rather we should statically link it to this librustuv crate and prevent the symbols from being exposed. We're currently "really lucky" in that the -lrustrt is propagated all over the place so the symbols can all get resolved, but this situation is not ideal.

I don't believe that our linking model is currently well suited to express functionality like this, but it's also not a high-priority issue.

alexcrichton · 2013-10-25T18:08:09Z

One change which I've now snuck in here is removing the weak_linkage attribute. From what I've seen, this cannot be done in a platform-agnostic manner (at least not as implemented), and certainly not by "just using" the extern_weak linkage from LLVM.

The only use case for this was the crate map, and it turned out that the crate map needed even more special treatment than just the weak_linkage attribute. The problem that I ran into was that rust_crate_map_toplevel was declared at the start of trans as a global (when compiling libstd as a --test binary), but then when it came around to translating the static which is a weak reference to the crate map, it would redeclare a global with the same name (which LLVM would silently rewrite to rust_crate_map_XXXX). Linkage then failed because the weak global of rust_crate_map_XXXX was never resolved and things started dying.

I'm personally in favor of this issue because weak_linkage barely works today, and it really only works on linux. On OSX you've got to pass some weird linker flags, and then on windows nothing works at all so we have to manually resort to a dynamic lookup of a symbol. If we want to support the idea of "weak linkage", then I think this is going to need some more language and runtime support than just using LLVM's ExternWeak linkage.

Primarily this makes the Scheduler and all of its related interfaces public. The reason for doing this is that currently any extern event loops had no access to the scheduler at all. This allows third-party event loops to manipulate the scheduler, along with allowing the uv event loop to live inside of its own crate.

brson · 2013-10-28T23:53:35Z

The #[event_loop_factory] lang item implies that multiple crates that implement the event loop can't be used interchangeably (because we'd be linking in two event loop factory lang items). In other words, if librustev implements the event loop and also defines the lang item, then a user can't use the default libuv for most schedulers then also opt in to one scheduler using libev. That's a contrived example, and I don't think this will be a problem for a long time, so not a big deal now. Just pointing it out.

brson · 2013-10-28T23:59:27Z

The compiler doesn't appear to use the 'event_loop' lang item. Is it really needed?

brson · 2013-10-29T00:09:47Z

This is one of the final steps needed to complete #9128. It still needs a little bit of polish before closing that issue, but it's in a pretty much "done" state now. The idea here is that the entire event loop implementation using libuv is now housed in `librustuv` as a completely separate library. This library is then injected (via `extern mod rustv`) into executable builds (similarly to how libstd is injected, tunable via `#[no_uv]`) to bring in the "rust blessed event loop implementation." Codegen-wise, there is a new `event_loop_factory` language item which is tagged on a function with 0 arguments returning `~EventLoop`. This function's symbol is then inserted into the crate map for an executable crate, and if there is no definition of the `event_loop_factory` language item then the value is null. What this means is that embedding rust as a library in another language just got a little harder. Libraries don't have crate maps, which means that there's no way to find the event loop implementation to spin up the runtime. That being said, it's always possible to build the runtime manually. This request also makes more runtime components public which should probably be public anyway. This new public-ness should allow custom scheduler setups everywhere regardless of whether you follow the `rt::start `path.

There are a few reasons that this is a desirable move to take: 1. Proof of concept that a third party event loop is possible 2. Clear separation of responsibility between rt::io and the uv-backend 3. Enforce in the future that the event loop is "pluggable" and replacable Here's a quick summary of the points of this pull request which make this possible: * Two new lang items were introduced: event_loop, and event_loop_factory. The idea of a "factory" is to define a function which can be called with no arguments and will return the new event loop as a trait object. This factory is emitted to the crate map when building an executable. The factory doesn't have to exist, and when it doesn't then an empty slot is in the crate map and a basic event loop with no I/O support is provided to the runtime. * When building an executable, then the rustuv crate will be linked by default (providing a default implementation of the event loop) via a similar method to injecting a dependency on libstd. This is currently the only location where the rustuv crate is ever linked. * There is a new #[no_uv] attribute (implied by #[no_std]) which denies implicitly linking to rustuv by default Closes rust-lang#5019

This is one of the final steps needed to complete #9128. It still needs a little bit of polish before closing that issue, but it's in a pretty much "done" state now. The idea here is that the entire event loop implementation using libuv is now housed in `librustuv` as a completely separate library. This library is then injected (via `extern mod rustv`) into executable builds (similarly to how libstd is injected, tunable via `#[no_uv]`) to bring in the "rust blessed event loop implementation." Codegen-wise, there is a new `event_loop_factory` language item which is tagged on a function with 0 arguments returning `~EventLoop`. This function's symbol is then inserted into the crate map for an executable crate, and if there is no definition of the `event_loop_factory` language item then the value is null. What this means is that embedding rust as a library in another language just got a little harder. Libraries don't have crate maps, which means that there's no way to find the event loop implementation to spin up the runtime. That being said, it's always possible to build the runtime manually. This request also makes more runtime components public which should probably be public anyway. This new public-ness should allow custom scheduler setups everywhere regardless of whether you follow the `rt::start `path.

There used to be a logical bug where IncrementVisitor would completely stop checking an expression/block after seeing a continue statement. This led to issue rust-lang#10058 where a variable incremented (or otherwise modified) after any continue statement would still be considered incremented only once. The solution is to continue scanning the expression after seeing a `continue` statement, but increment self.depth so that the Visitor thinks that the rest of the loop is within a conditional.

…Frednet fix logic in IncrementVisitor There used to be a logical bug where IncrementVisitor would completely stop checking an expression/block after seeing a continue statement. I am a little unsure of whether my fix to `IncrementVisitor` is logically sound (I hope it makes sense). Let me know what you think, and thanks in advance for the review! fixes rust-lang#10058 --- changelog: FP: [`explicit_counter_loop`]: No longer ignores counter changes after `continue` expressions [rust-lang#10094](rust-lang/rust-clippy#10094)

Should fix rust-lang#10090, rust-lang#10046, rust-lang#10179. This is only a workaround, but the proper fix requires some bigger refactoring (also related to fixing rust-lang#10058), and this at least prevents the crash.

10212: fix: Avoid type inference panic on bitslice methods r=flodiebold a=flodiebold Should fix rust-lang#10090, fix rust-lang#10046, and fix rust-lang#10179. This is only a workaround, but the proper fix requires some bigger refactoring (also related to fixing rust-lang#10058), and this at least prevents the crash. Co-authored-by: Florian Diebold <flodiebold@gmail.com>

- don't return the receiver type from method resolution; instead just return the autorefs/autoderefs that happened and repeat them. This ensures all the effects like trait obligations and whatever we learned about type variables from derefing them are actually applied. Also, it allows us to get rid of `decanonicalize_ty`, which was just wrong in principle. - Autoderef itself now directly works with an inference table. Sadly this has the effect of making it harder to use as an iterator, often requiring manual `while let` loops. (rustc works around this by using inner mutability in the inference context, so that things like unifying types don't require a unique reference.) - We now record the adjustments (autoref/deref) for method receivers and index expressions, which we didn't before. - Removed the redundant crate parameter from method resolution, since the trait_env contains the crate as well. - in the HIR API, the methods now take a scope to determine the trait env. `Type` carries a trait env, but I think that's probably a bad decision because it's easy to create it with the wrong env, e.g. by using `Adt::ty`. This mostly didn't matter so far because `iterate_method_candidates` took a crate parameter and ignored `self.krate`, but the trait env would still have been wrong in those cases, which I think would give some wrong results in some edge cases. Fixes rust-lang#10058.

11550: Refactor autoderef/method resolution r=flodiebold a=flodiebold - don't return the receiver type from method resolution; instead just return the autorefs/autoderefs that happened and repeat them. This ensures all the effects like trait obligations and whatever we learned about type variables from derefing them are actually applied. Also, it allows us to get rid of `decanonicalize_ty`, which was just wrong in principle. - Autoderef itself now directly works with an inference table. Sadly this has the effect of making it harder to use as an iterator, often requiring manual `while let` loops. (rustc works around this by using inner mutability in the inference context, so that things like unifying types don't require a unique reference.) - We now record the adjustments (autoref/deref) for method receivers and index expressions, which we didn't before. - Removed the redundant crate parameter from method resolution, since the trait_env contains the crate as well. - in the HIR API, the methods now take a scope to determine the trait env. `Type` carries a trait env, but I think that's probably a bad decision because it's easy to create it with the wrong env, e.g. by using `Adt::ty`. This mostly didn't matter so far because `iterate_method_candidates` took a crate parameter and ignored `self.krate`, but the trait env would still have been wrong in those cases, which I think would give some wrong results in some edge cases. Fixes rust-lang#10058. Co-authored-by: Florian Diebold <flodiebold@gmail.com>

brson reviewed Oct 29, 2013
View reviewed changes

Comment thread src/libstd/rt/mod.rs

brson Oct 29, 2013

Copy link
Copy Markdown

Contributor

Choose a reason for hiding this comment

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

Neat!

bors closed this Oct 29, 2013

bors merged commit 201cab8 into rust-lang:master Oct 29, 2013

alexcrichton deleted the uv-crate branch October 29, 2013 18:23

Navigation Menu

Repository navigation

Move UV to its own crate. - #10058

Move UV to its own crate.#10058
bors merged 2 commits into
rust-lang:masterfrom
alexcrichton:uv-crate

alexcrichton commented Oct 24, 2013

Uh oh!

alexcrichton commented Oct 24, 2013

Uh oh!

alexcrichton commented Oct 25, 2013

Uh oh!

brson commented Oct 28, 2013

Uh oh!

brson commented Oct 28, 2013

Uh oh!

brson Oct 29, 2013

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

alexcrichton commented Oct 24, 2013

Uh oh!

alexcrichton commented Oct 24, 2013

Uh oh!

alexcrichton commented Oct 25, 2013

Uh oh!

brson commented Oct 28, 2013

Uh oh!

brson commented Oct 28, 2013

Uh oh!

brson Oct 29, 2013

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants