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

VaList::copy should not require a mutable ref #57311

Merged
merged 1 commit into from Jan 5, 2019

Conversation

Projects
None yet
4 participants
@dlrobertson
Copy link
Contributor

dlrobertson commented Jan 3, 2019

VaList::copy does not need to take a mutable reference. The va_copy
intrinsic takes a immutable reference.

VaList::copy should not require a mutable ref
VaList::copy does not need to take a mutable reference. The va_copy
intrinsic takes a immutable reference.
@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Jan 3, 2019

r? @KodrAus

(rust_highfive has picked a reviewer for you, use r? to override)

@KodrAus

This comment has been minimized.

Copy link

KodrAus commented Jan 4, 2019

Thanks @dlrobertson! I'm not familiar with this code. As the original author, is there any forwards compatibility hazards you can think of that could make us prefer &mut over & here?

@dlrobertson

This comment has been minimized.

Copy link
Contributor

dlrobertson commented Jan 4, 2019

@KodrAus not that I can think of. The current use of &mut was a mistake by me in the original implementation. The RFC indicates that this is the correct signature too https://github.com/rust-lang/rfcs/blob/master/text/2137-variadic.md#guide-level-explanation

@KodrAus

This comment has been minimized.

Copy link

KodrAus commented Jan 4, 2019

Thanks!

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 4, 2019

📌 Commit 917985e has been approved by KodrAus

kennytm added a commit to kennytm/rust that referenced this pull request Jan 5, 2019

Rollup merge of rust-lang#57311 - dlrobertson:fix_valist_copy, r=KodrAus
VaList::copy should not require a mutable ref

`VaList::copy` does not need to take a mutable reference. The `va_copy`
intrinsic takes a immutable reference.

bors added a commit that referenced this pull request Jan 5, 2019

Auto merge of #57354 - kennytm:rollup, r=kennytm
Rollup of 17 pull requests

Successful merges:

 - #57219 (Remove some unused code)
 - #57229 (Fix #56806 by using `delay_span_bug` in object safety layout sanity checks)
 - #57233 (Rename and fix nolink-with-link-args test)
 - #57238 (Fix backtraces for inlined functions on Windows)
 - #57249 (Fix broken links to second edition TRPL.)
 - #57267 (src/jemalloc is gone, remove its mention from COPYRIGHT)
 - #57273 (Update the stdsimd submodule)
 - #57278 (Add Clippy to config.toml.example)
 - #57295 (Fix 'be be' constructs)
 - #57311 (VaList::copy should not require a mutable ref)
 - #57312 (`const fn` is no longer coming soon (const keyword docs))
 - #57313 (Improve Box<T> -> Pin<Box<T>> conversion)
 - #57314 (Fix repeated word typos)
 - #57326 (Doc rewording, use the same name `writer`)
 - #57338 (rustdoc: force binary filename for compiled doctests)
 - #57342 (librustc_mir: Make qualify_min_const_fn module public)
 - #57343 (Calculate privacy access only via query)

Failed merges:

 - #57340 (Use correct tracking issue for c_variadic)

r? @ghost

@bors bors merged commit 917985e into rust-lang:master Jan 5, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@dlrobertson dlrobertson deleted the dlrobertson:fix_valist_copy branch Jan 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment