c-variadic: document Clone and Drop instances and require VaArgSafe: Copy#155821
c-variadic: document Clone and Drop instances and require VaArgSafe: Copy#155821rust-bors[bot] merged 4 commits intorust-lang:mainfrom
Clone and Drop instances and require VaArgSafe: Copy#155821Conversation
|
|
2538ff2 to
d143239
Compare
|
This is still unstable, so it doesn't need an FCP. LGTM. @bors r+ rollup |
| /// Corresponds to `va_end` in C. | ||
| #[inline] // Avoid codegen when not used to help backends that don't support VaList. | ||
| fn drop(&mut self) { | ||
| // Call the rust `va_end` intrinsic, which is a no-op and does not map to LLVM `va_end`. |
There was a problem hiding this comment.
this is incorrect on latest nightly, it does call llvm.va_end. see #155697 (comment)
|
de-approving so this can get looked at: |
|
This pull request was unapproved. |
|
huh, that never got updated apparently. It doesn't really matter because it's a no-op, but we should clean that up. |
when you or someone else cleans it up, please put the |
|
It's more complicated than that: The drop instance is for tracking that a |
IMO it's unnecessary to follow the must-be-in-same-function rule, we did talk about that before along with why |
|
That's in line with that I'm proposing: the rust The use of the LLVM |
because a VaList can be cloned, the same c-variadic argument can be read twice and that is only safe if the argument type is copy
it should use the fallback body instead
d143239 to
ac12c69
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
@programmerjake let us know if you think the latest commit unblocks this PR at least |
yeah, now that the comment is accurate I think it's good enough for this PR.
I'm proposing what I thought we had agreed on in the Zulip thread, which is kinda the opposite of what this PR does, though it doesn't matter until we add a target where |
I don't really see an LLVM target ever doing that, but perhaps the gcc backend will need it one day for some obscure target. Even so I suspect at that point that we'd pull @bors r=joshtriplett |
Rollup of 19 pull requests Successful merges: - #155237 (Disentangle AST crates and error crates) - #155249 (Fix: On wasm targets, call `panic_in_cleanup` if panic occurs in cleanup) - #155853 (Use `_mcount` as the mcount symbol name on RISC-V Linux GNU targets) - #155919 (simplify `ast_fragments!`) - #155939 (Add feature gate for view_types experiment) - #155954 (rustdoc: preserve parent doc cfg for `macro_export` macros) - #155974 (add `c_variadic_experimental_arch` feature) - #155991 (Catch unwinds from the global ctxt callback to complete queries profiling data in more cases) - #156003 (Pass Session to optimize_and_codegen_fat_lto) - #153566 (Add suggestion for E0401 on inner const items) - #154610 (Suggest public re-exports when a private module makes an import path inaccessible) - #155523 (Reorganize `tests/ui/issues/` - 02) - #155821 (c-variadic: document `Clone` and `Drop` instances and require `VaArgSafe: Copy`) - #155980 (Move `feature*` methods from `parse` mod to `errors` mod.) - #155987 (Make lifting infallible) - #155988 (tests/run-make/print-cfg: add Android target_env case) - #156000 (Fix ICE when using -Zinstrument-mcount and -Clinker-flavor=lld) - #156002 (Allow to use `Diagnostic` directly in `SharedContext::emit_lint`) - #156015 (rustc-dev-guide subtree update)
tracking issue: #44930
Fixing some things that came up in the stabilization PR
r? tgross35
cc @kpreid