-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Don't add argc
and argv
arguments to main
on WASI.
#65576
Conversation
Add a target setting to allow targets to specify whether the generated `main` function should be passed `argc` and `argv` arguments. Set it to false on wasm32-wasi, since WASI's `args::args()` calls into the WASI APIs itself. This will allow the WASI toolchain to avoid linking and running command-line argument initialization code when the arguments aren't actually needed.
r? @cramertj (rust_highfive has picked a reviewer for you, use r? to override) |
@bors: r+ |
📌 Commit b25e323 has been approved by |
@@ -414,8 +414,11 @@ pub fn maybe_create_entry_wrapper<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(cx: &' | |||
rust_main_def_id: DefId, | |||
use_start_lang_item: bool, | |||
) { | |||
let llfty = | |||
cx.type_func(&[cx.type_int(), cx.type_ptr_to(cx.type_i8p())], cx.type_int()); | |||
let llfty = if cx.sess().target.target.options.main_needs_argc_argv { |
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 think this wants a comment.
let param_argv = bx.get_param(1); | ||
let arg_argc = bx.intcast(param_argc, cx.type_isize(), true); | ||
let arg_argv = param_argv; | ||
let (arg_argc, arg_argv) = if cx.sess().target.target.options.main_needs_argc_argv { |
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.
The function is already large; this bit could be extracted into its own (+ appropriate doc comment).
let arg_argc = bx.intcast(param_argc, cx.type_isize(), true); | ||
let arg_argv = param_argv; | ||
let (arg_argc, arg_argv) = if cx.sess().target.target.options.main_needs_argc_argv { | ||
// Params from native main() used as args for rust start function |
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.
// Params from native main() used as args for rust start function | |
// Params from native `main()` used as args for rust start function |
let arg_argv = param_argv; | ||
(arg_argc, arg_argv) | ||
} else { | ||
// The Rust start function doesn't need argc and argv, so just pass zeros. |
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.
// The Rust start function doesn't need argc and argv, so just pass zeros. | |
// The Rust start function doesn't need `argc` and `argv`, so just pass zeros. |
@sunfishcode I noticed this was r+ed now -- if this PR is merged before, can you follow up to address my review comments? |
…=alexcrichton Don't add `argc` and `argv` arguments to `main` on WASI. Add a target setting to allow targets to specify whether the generated `main` function should be passed `argc` and `argv` arguments. Set it to false on wasm32-wasi, since WASI's `args::args()` calls into the WASI APIs itself. This will allow the WASI toolchain to avoid linking and running command-line argument initialization code when the arguments aren't actually needed.
Rollup of 6 pull requests Successful merges: - #65174 (Fix zero-size uninitialized boxes) - #65252 (expand: Simplify expansion of derives) - #65485 (Suppress ICE when validators disagree on `LiveDrop`s in presence of `&mut`) - #65542 (Refer to "associated functions" instead of "static methods") - #65545 (More symbol cleanups) - #65576 (Don't add `argc` and `argv` arguments to `main` on WASI.) Failed merges: r? @ghost
@Centril The PR was indeed merged; I'll submit a new PR to address your feedback. |
This makes a few code cleanups to follow up on the review comments in rust-lang#65576.
Code cleanups following up on rust-lang#65576. This makes a few code cleanups to follow up on the review comments in rust-lang#65576. r? @Centril
Code cleanups following up on rust-lang#65576. This makes a few code cleanups to follow up on the review comments in rust-lang#65576. r? @Centril
Rollup of 14 pull requests Successful merges: - #64145 (Target-feature documented as unsafe) - #65007 (Mention keyword closing policy) - #65417 (Add more coherence tests) - #65507 (Fix test style in unused parentheses lint test) - #65591 (Add long error explanation for E0588) - #65617 (Fix WASI sleep impl) - #65656 (Add option to disable keyboard shortcuts in docs) - #65678 (Add long error explanation for E0728) - #65681 (Code cleanups following up on #65576.) - #65686 (refactor and move `maybe_append` ) - #65688 (Add some tests for fixed ICEs) - #65689 (bring back some Debug instances for Miri) - #65695 (self-profiling: Remove module names from some event-ids in codegen backend.) - #65706 (Add missing space in librustdoc) Failed merges: r? @ghost
Add a target setting to allow targets to specify whether the generated
main
function should be passedargc
andargv
arguments. Set itto false on wasm32-wasi, since WASI's
args::args()
calls into theWASI APIs itself. This will allow the WASI toolchain to avoid linking
and running command-line argument initialization code when the arguments
aren't actually needed.