Skip to content
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

Various cleanups to repr and intrinsics #8414

Closed
wants to merge 4 commits into from

Conversation

alexcrichton
Copy link
Member

The commit messages explain it pretty well. The main impetus was for removing @io::Writer and replacing it with &mut io::Writer in repr (only marginally worked). This should allow ifmt! using %? to perform fewer allocations!

Otherwise, I just started rampaging through intrinsics to remove as many as I could.

@alexcrichton
Copy link
Member Author

And definitely credit to @jensnockert for his work in #5785 inspiring moving out the intrinsics!

@graydon
Copy link
Contributor

graydon commented Aug 9, 2013

Does &Writer work well enough to do this?! If so (woooo!) then &Visitor should be next.

@alexcrichton
Copy link
Member Author

I don't think &Writer can work because the write method on it takes &mut self. This code fails to compile:

use std::rt::io;

fn foo(r: &io::Writer) {
    r.write([])
}

fn main() {}

with

foo.rs:4:4: 4:5 error: cannot borrow immutable argument as mutable
foo.rs:4     r.write([])
             ^
error: aborting due to previous error

@Visitor could get interesting to remove, no reason it shouldn't be though!

At the same time, this updates the TyVisitor to use a mutable self because it's
probably going to be mutating state as it goes along anyway.
@alexcrichton
Copy link
Member Author

This has stayed open too long, splitting up into smaller pull requests and reopening

flip1995 pushed a commit to flip1995/rust that referenced this pull request Mar 14, 2022
…=oli-obk

Optimize `redundant_clone`

Fixes rust-lang#8412

changelog: none
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.

2 participants