GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — More scheduler and I/O work by brson · Pull Request #5409 · rust-lang/rust

Conversation

@brson

@brson brson commented Mar 16, 2013

Copy link
Copy Markdown
Contributor

r?

There are a lot of commits here, but not all that much substance. Mostly just refactoring.

I started sketching out the beginnings of a very simple I/O API in core::rt::io that represents I/O streams as a single Stream trait instead of Reader / Writer pairs. This seems to be the more common pattern (at least this is how the .NET BCL does it) and it seems to me that separate readers and writers would make duplex streams very awkward. Regardless, I don't intend to go very far down the I/O API design road without some mailing list discussion.

I've also started on the uv bindings for file I/O but haven't gotten very far.

Also hooked up the new scheduler to rust_start and the compiletest driver. 70% of run-pass test cases already pass, but I wouldn't read too much into that.

I also split the direct, low-level uv bindings in two so that the scheduler can have its own set, leaving std::net on its own.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
graydon
graydon reviewed Mar 18, 2013
View reviewed changes
Comment thread src/libcore/rt/io/mod.rs

@graydon graydon Mar 18, 2013

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

A note here, as I've seen a few conditions getting written this way lately: there's seldom a need to have a return value from a condition that tells the raise-site that it should fail; the handler itself can just fail (and indeed, untrapped conditions already fail). There's just as much context (more actually) visible to a backtrace if you let the handler / absence-of-handler itself fail.

Generally the only things that should show up in the return slot of a condition are data to use for recovery-and-continuation: sentinel values or replacement values, things that represent the handler saying "oh, well, use this value instead". If no such data is required or plausible (i.e. if the only reasonable thing one might do to recover is "ignore the error") then just put () in the return slot. If control proceeds past the condition.raise() line, you know a caller trapped the error and wants you to ignore it and continue.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@brson brson Mar 18, 2013

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I see. I'll make this not return a bool.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
brson added 12 commits March 18, 2013 16:59
@brson
core: Cleanup rt::context
e8ddef9
@brson
Give core::rt and std::net their own uvll bindings
d30c758
I intend to do some big refactoring and don't want to deal w/ std just now
@brson
core: Don't use printf in rtdebug!
723d224
The bots were showing a segfault that I can't reproduce. Assuming it's
varargs related so let's not use printf
@brson
core: Simplify uvll bindings and strip out currently-unused bits
54bb722
No more mapping uv structs to Rust structs
@brson
core: Convert some multiline statements to single-line
42cba98
@brson
core: Rename rt::io to rt::rtio
9a075f2
This is an internal interface. I want to use rt::io for public interfaces.
@brson
mk: If NO_REBUILD is set then don't rebuild core/std before testing
5e6dacf
Can make turnaround of testing changes to core/std/syntax much faster.
@brson
core: Add rt::io and start sketching the API
57e85b5
@brson
core: Begin uv file system bindings
7ef54c7
@brson
core: Refactor uv bindings
a882554
I can already see these are going to get massive. Putting them into multiple
files.
@brson
Add a way to run the test suite with the new scheduler
0447034
TESTARGS=--newrt make check-stage1-rpass

Conflicts:
	src/rt/rustrt.def.in
@brson
core: Initialize global state lazily in the Scheduler ctor
5af5766
I don't want any global one-time initalization functions because
that will make embedding harder.
@brson
Merge remote-tracking branch 'brson/rt'
30d4124
Conflicts:
	src/libcore/rt/context.rs
	src/libcore/rt/sched.rs
	src/libcore/rt/thread.rs
	src/libcore/rt/uv.rs
bors added a commit that referenced this pull request Mar 25, 2013
@bors
auto merge of #5409 : brson/rust/rt, r=brson
ef282db
r?

There are a lot of commits here, but not all that much substance. Mostly just refactoring.

I started sketching out the beginnings of a very simple I/O API in `core::rt::io` that represents I/O streams as a single `Stream` trait instead of `Reader` / `Writer` pairs. This seems to be the more common pattern (at least this is how the .NET BCL does it) and it seems to me that separate readers and writers would make duplex streams very awkward. Regardless, I don't intend to go very far down the I/O API design road without some mailing list discussion.

I've also started on the uv bindings for file I/O but haven't gotten very far.

Also hooked up the new scheduler to `rust_start` and the compiletest driver. 70% of run-pass test cases already pass, but I wouldn't read too much into that.

I also split the direct, low-level uv bindings in two so that the scheduler can have its own set, leaving `std::net` on its own.
@bors bors closed this Mar 25, 2013
oli-obk pushed a commit to oli-obk/rust that referenced this pull request May 2, 2020
@flip1995
Rollup merge of rust-lang#5409 - dtolnay:letunit, r=flip1995
935b45d
Downgrade let_unit_value to pedantic

Given that the false positive in rust-lang#1502 is marked E-hard and I don't have much hope of it getting fixed, I think it would be wise to disable this lint by default. I have had to suppress this lint in every substantial codebase (\>100k line) I have worked in. Any time this lint is being triggered, it's always the false positive case.

The motivation for this lint is documented as:

> A unit value cannot usefully be used anywhere. So binding one is kind of pointless.

with this example:

> ```rust
> let x = {
>     1;
> };
> ```

Sure, but the author would find this out via an unused_variable warning or from `x` not being the type that they need further down. If there ends up being a type error on `x`, clippy's advice isn't going to help get the code compiling because it can only run if the code already compiles.

changelog: Remove let_unit_value from default set of enabled lints
oli-obk pushed a commit to oli-obk/rust that referenced this pull request May 2, 2020
@bors
Auto merge of rust-lang#5438 - flip1995:rollup-pi762oy, r=flip1995
f8308c8
Rollup of 11 pull requests

Successful merges:

 - rust-lang#5406 (Fix update_lints)
 - rust-lang#5409 (Downgrade let_unit_value to pedantic)
 - rust-lang#5410 (Downgrade trivially_copy_pass_by_ref to pedantic)
 - rust-lang#5412 (Downgrade inefficient_to_string to pedantic)
 - rust-lang#5415 (Add new lint for `Result<T, E>.map_or(None, Some(T))`)
 - rust-lang#5417 (Update doc links and mentioned names in docs)
 - rust-lang#5419 (Downgrade unreadable_literal to pedantic)
 - rust-lang#5420 (Downgrade new_ret_no_self to pedantic)
 - rust-lang#5422 (CONTRIBUTING.md: fix broken triage link)
 - rust-lang#5424 (Incorrect suspicious_op_assign_impl)
 - rust-lang#5425 (Ehance opt_as_ref_deref lint.)

Failed merges:

 - rust-lang#5345 (Add lint for float in array comparison)
 - rust-lang#5411 (Downgrade implicit_hasher to pedantic)
 - rust-lang#5428 (Move cognitive_complexity to nursery)

r? @ghost

changelog: rollup
U007D pushed a commit to U007D/rust-mos that referenced this pull request Aug 21, 2026
@bors @cpud36
Merge rust-lang#7535
ac59584
7535: Extract function assist r=cpud36 a=cpud36

This PR adds `extract function/method` assist. closes rust-lang#5409.

# Supported features
Assist should support extracting from expressions(`1`, `2 + 2`, `loop { }`) and from a series of statements, e.g.:
```rust
foo();
$0bar();
baz();$0
quix();
```
Assist also supports extracting parameters, like:
```rust
fn foo() -> i32 {
  let n = 1;
  $0n + 1$0
}

// -
fn foo() -> i32 {
  let n = 1;
  fun_name(n)
}

fn fun_name(n: i32) -> i32 {
  n + 1
}
```

Extracting methods also generally works.

Assist allows referencing outer variables, both mutably and immutably, and handles handles access to variables local to extracted function:
```rust
fn foo() {
  let mut n = 1;
  let mut m = 2;
  let mut moved_v = Vec::new();
  let mut ref_mut_v = Vec::new();
  $0
  n += 1;
  let k = 1;
  moved_v.push(n);
  let r = &mut m;
  ref_mut_v.push(*r);
  let h = 3;
  $0
  n = ref_mut_v.len() + k;
  n -= h + m;
}

// -
fn foo() {
  let mut n = 1;
  let mut m = 2;
  let mut moved_v = Vec::new();
  let mut ref_mut_v = Vec::new();
 
  let (k, h) =  fun_name(&mut n, moved_v, &mut m, &mut ref_mut_v);

  n = ref_mut_v.len() + k;
  n -= h + m;
}

fn fun_name(n: &mut i32, mut moved_v: Vec<i32>, m: &mut i32, ref_mut_v: &mut Vec<i32>) -> (i32, i32) {
  *n += 1;
  let k = 1;
  moved_v.push(*n);
  let r = m;
  ref_mut_v.push(*r);
  let h = 3;
  (k, h)
}
```

So we handle both input and output paramters

# Showcase

![extract_cursor_in_range_3](https://user-images.githubusercontent.com/4218373/106980190-c9870800-6770-11eb-83d9-3d36b2550ff6.gif)
![fill_match_arms_discard_wildcard](https://user-images.githubusercontent.com/4218373/106980197-cbe96200-6770-11eb-96b0-14c27894fac0.gif)
![ide_db_helpers_handle_kind](https://user-images.githubusercontent.com/4218373/106980201-cdb32580-6770-11eb-9e6e-6ac8155d65ac.gif)
![ide_db_imports_location_local_query](https://user-images.githubusercontent.com/4218373/106980205-cf7ce900-6770-11eb-8516-653c8fcca807.gif)

# Working with non-`Copy` types

Consider the following example:
```rust
fn foo() {
  let v = Vec::new();
  $0
  let n = v.len();
  $0
  let is_empty = v.is_empty();
}
```
`v` must be a parameter to extracted function. 
The question is, what type should it have.
It could be `v: Vec<i32>`, or `v: &Vec<i32>`. 
The former is incorrect for `Vec<i32>`, but the later is silly for `i32`.

To resolve this we need to know if the type implements `Copy` trait.

I didn't find any api available from assists to query this. 
`hir_ty::method_resolution::implements` seems relevant, but is isn't publicly re-exported from `hir`.

# Star(`*`) token and pointer dereference

If I understand correctly, in order to create expression like `*p`, one should use `ast::make::expr_prefix(T![*], ...)`, which
in turn calls `token(T![*])`.

`token` does not have star in `tokens::SOURCE_FILE`, so this panics.
I had to add `*` to `SOURCE_FILE` to make it work.

Correct me if this is not intended way to do this.

# Lowering access `value -> mut ref -> shared ref`

Consider the following example:
```rust
fn foo() {
  let v = Vec::new();
  $0 let n = v.len(); $0
}
```
`v` is not used after extracted function body, so both `v: &Vec<i32>` and `v: Vec<i32>` would work.
Currently the later would be chosen.

We can however check the body of extracted function and conclude that `v: &Vec<i32>` is sufficient.
Using `v: &Vec<i32>`(that is a minimal required access level) might be a better default.
I am unsure.

# Cleanup
The assist seems to be reasonably handling most of common cases.
If there are no concerns with code it produces(i.e. with test cases), I will start cleaning up

[edit]
added showcase


Co-authored-by: Vladyslav Katasonov <[email protected]>
RalfJung added a commit to RalfJung/rust that referenced this pull request Oct 18, 2026
@RalfJung
Merge pull request rust-lang#5409 from eduardosm/pavg
ace3012
miri: reimplement `pavg` intrinsics
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@brson @graydon @bors
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.