Skip to content

Move UV to its own crate. #10058

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

Merged
merged 2 commits into from
Oct 29, 2013
Merged

Move UV to its own crate. #10058

merged 2 commits into from
Oct 29, 2013

Conversation

alexcrichton
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Contributor

brson commented Oct 28, 2013

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
Copy link
Contributor

brson commented Oct 28, 2013

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

Some(factory) => return factory()
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Neat!

bors added a commit that referenced this pull request Oct 29, 2013
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
bors added a commit that referenced this pull request Oct 29, 2013
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.
@bors bors closed this Oct 29, 2013
@bors bors merged commit 201cab8 into rust-lang:master Oct 29, 2013
@alexcrichton alexcrichton deleted the uv-crate branch October 29, 2013 18:23
flip1995 pushed a commit to flip1995/rust that referenced this pull request Dec 17, 2022
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.
flip1995 pushed a commit to flip1995/rust that referenced this pull request Dec 17, 2022
…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)
<!-- changelog_checked -->
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