-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Emit warning when there is no space between -o
and arg
#143719
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
base: master
Are you sure you want to change the base?
Conversation
This PR modifies cc @jieyouxu |
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.
This seems reasonable, though I can imagine this might be a bit annoying. I'll defer to @wesleywiser for a second opinion.
r? @wesleywiser (re. #142812) |
|
Ah right. I'll wait a bit in case Wesley has feedback. |
// To avoid confusion, emit warning if no space | ||
// between `-o` and arg, e.g.`-optimize`, is applied, see issue #142812 | ||
if let Some(name) = matches.opt_str("o") { | ||
if args.iter().any(|arg| arg.starts_with("-o") && !arg.starts_with("-o=") && arg.ne("-o")) { | ||
early_dcx.early_warn(format!("option `-o {}` is applied", name)); | ||
} | ||
} |
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.
Suggestion: if we're going to emit this warning, can you also include a bit more context on why this is warned on in the first place and how users can address this warning? I.e. for the wording, I suggest
warning: option `-o` has no space between flag name and value, which can be confusing
help: option `-o {}` is applied instead of a flag named `o{}`
help: if you meant to specify output filename `{}`, write `-o {}` instead
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.
Suggestion: also, maybe remark this case in the rustc book on compiler flags.
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.
Good suggestions, I will finish this later.
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.
You might want to hold off on changing this for a moment, still discussing formulation of this warning in the linked thread
Asking for some more opinions about if warning on cf. #t-compiler > Warn on `-ofilename`/`-Lpath` without space @rustbot label: -S-waiting-on-review +S-waiting-on-team |
Now, it prints like this $ rustc +stage1 src/main.rs -optimize
warning: option `-o` has no space between flag name and value, which can be confusing
note: option `-o ptimize` is applied instead of a flag named `optimize` to specify output filename `ptimize`
$ rustc +stage1 src/main.rs -out-dir
warning: option `-o` has no space between flag name and value, which can be confusing
note: option `-o ut-dir` is applied instead of a flag named `out-dir` to specify output filename `ut-dir`
note: Do you mean `--out-dir`?
$ rustc +stage1 src/main.rs -overflow-checks
warning: option `-o` has no space between flag name and value, which can be confusing
note: option `-o verflow-checks` is applied instead of a flag named `overflow-checks` to specify output filename `verflow-checks`
note: Do you mean `-C overflow_checks`?
$ rustc +stage1 src/main.rs -overflow_checks
warning: option `-o` has no space between flag name and value, which can be confusing
note: option `-o verflow_checks` is applied instead of a flag named `overflow_checks` to specify output filename `verflow_checks`
note: Do you mean `-C overflow_checks`?
$ rustc +stage1 src/main.rs -o3
warning: option `-o` has no space between flag name and value, which can be confusing
note: option `-o 3` is applied instead of a flag named `o3` to specify output filename `3`
|
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.
@rustbot ready
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.
Thanks, some thoughts. Mostly to make this even more conservative.
for option in optgroups { | ||
let prefix = String::from("--"); | ||
if starts_with_ignoring_separators(option.long_name(), suspect) { | ||
return Some(prefix + option.long_name()); | ||
} | ||
} |
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.
Suggestion: I would also not bother with e.g. --oprint
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.
Currently, -oprint
won't emit a warning, do you mean we should emit a warning?
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.
Hm, I'm not sure what I meant by this, I meant we should not.
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.
Yes, that's the way it is now. it match oprint
which does not exist in rustc option group.
@rustbot author |
Reminder, once the PR becomes ready for a review, use |
Signed-off-by: xizheyin <[email protected]>
Done :) @rustbot ready |
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.
Thanks. We'll have to see if even this ends up too annoying in practice, but IMO this is sufficiently conservative.
You can r=me once PR CI is green. |
Emit warning when there is no space between `-o` and arg Closes rust-lang#142812 `getopt` doesn't seem to have an API to check this, so we have to check the args manually. r? compiler
Emit warning when there is no space between `-o` and arg Closes rust-lang#142812 `getopt` doesn't seem to have an API to check this, so we have to check the args manually. r? compiler
Emit warning when there is no space between `-o` and arg Closes rust-lang#142812 `getopt` doesn't seem to have an API to check this, so we have to check the args manually. r? compiler
Emit warning when there is no space between `-o` and arg Closes rust-lang#142812 `getopt` doesn't seem to have an API to check this, so we have to check the args manually. r? compiler
Emit warning when there is no space between `-o` and arg Closes rust-lang#142812 `getopt` doesn't seem to have an API to check this, so we have to check the args manually. r? compiler
Rollup of 12 pull requests Successful merges: - #142936 (rustdoc-json: Structured attributes) - #143355 (wrapping shift: remove first bitmask and table) - #143448 (remote-test-client: Exit code `128 + <signal-number>` instead of `3`) - #143692 (miri: fix out-of-bounds error for ptrs with negative offsets) - #143719 (Emit warning when there is no space between `-o` and arg) - #143738 (Move several float tests to floats/mod.rs) - #143891 (Port `#[coverage]` to the new attribute system) - #143920 (Make more of codegen_llvm safe) - #143921 (Constify `Index` traits) - #143939 (Add 0323pin as maintainer of NetBSD targets, fix link to pkgsrc-wip and explain.) - #143948 (Update mdbook to 0.4.52) - #143968 (Add tracing to `InterpCx::fn_abi_of_instance/fn_abi_of_fn_ptr`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 12 pull requests Successful merges: - #142936 (rustdoc-json: Structured attributes) - #143355 (wrapping shift: remove first bitmask and table) - #143448 (remote-test-client: Exit code `128 + <signal-number>` instead of `3`) - #143692 (miri: fix out-of-bounds error for ptrs with negative offsets) - #143719 (Emit warning when there is no space between `-o` and arg) - #143738 (Move several float tests to floats/mod.rs) - #143891 (Port `#[coverage]` to the new attribute system) - #143920 (Make more of codegen_llvm safe) - #143921 (Constify `Index` traits) - #143939 (Add 0323pin as maintainer of NetBSD targets, fix link to pkgsrc-wip and explain.) - #143948 (Update mdbook to 0.4.52) - #143968 (Add tracing to `InterpCx::fn_abi_of_instance/fn_abi_of_fn_ptr`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 12 pull requests Successful merges: - #142936 (rustdoc-json: Structured attributes) - #143355 (wrapping shift: remove first bitmask and table) - #143448 (remote-test-client: Exit code `128 + <signal-number>` instead of `3`) - #143692 (miri: fix out-of-bounds error for ptrs with negative offsets) - #143719 (Emit warning when there is no space between `-o` and arg) - #143738 (Move several float tests to floats/mod.rs) - #143891 (Port `#[coverage]` to the new attribute system) - #143920 (Make more of codegen_llvm safe) - #143921 (Constify `Index` traits) - #143939 (Add 0323pin as maintainer of NetBSD targets, fix link to pkgsrc-wip and explain.) - #143948 (Update mdbook to 0.4.52) - #143968 (Add tracing to `InterpCx::fn_abi_of_instance/fn_abi_of_fn_ptr`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 12 pull requests Successful merges: - #142936 (rustdoc-json: Structured attributes) - #143355 (wrapping shift: remove first bitmask and table) - #143448 (remote-test-client: Exit code `128 + <signal-number>` instead of `3`) - #143692 (miri: fix out-of-bounds error for ptrs with negative offsets) - #143719 (Emit warning when there is no space between `-o` and arg) - #143738 (Move several float tests to floats/mod.rs) - #143891 (Port `#[coverage]` to the new attribute system) - #143920 (Make more of codegen_llvm safe) - #143921 (Constify `Index` traits) - #143939 (Add 0323pin as maintainer of NetBSD targets, fix link to pkgsrc-wip and explain.) - #143948 (Update mdbook to 0.4.52) - #143968 (Add tracing to `InterpCx::fn_abi_of_instance/fn_abi_of_fn_ptr`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 12 pull requests Successful merges: - #142936 (rustdoc-json: Structured attributes) - #143355 (wrapping shift: remove first bitmask and table) - #143448 (remote-test-client: Exit code `128 + <signal-number>` instead of `3`) - #143692 (miri: fix out-of-bounds error for ptrs with negative offsets) - #143719 (Emit warning when there is no space between `-o` and arg) - #143738 (Move several float tests to floats/mod.rs) - #143891 (Port `#[coverage]` to the new attribute system) - #143920 (Make more of codegen_llvm safe) - #143921 (Constify `Index` traits) - #143939 (Add 0323pin as maintainer of NetBSD targets, fix link to pkgsrc-wip and explain.) - #143948 (Update mdbook to 0.4.52) - #143968 (Add tracing to `InterpCx::fn_abi_of_instance/fn_abi_of_fn_ptr`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 12 pull requests Successful merges: - #142936 (rustdoc-json: Structured attributes) - #143355 (wrapping shift: remove first bitmask and table) - #143448 (remote-test-client: Exit code `128 + <signal-number>` instead of `3`) - #143692 (miri: fix out-of-bounds error for ptrs with negative offsets) - #143719 (Emit warning when there is no space between `-o` and arg) - #143738 (Move several float tests to floats/mod.rs) - #143891 (Port `#[coverage]` to the new attribute system) - #143920 (Make more of codegen_llvm safe) - #143921 (Constify `Index` traits) - #143939 (Add 0323pin as maintainer of NetBSD targets, fix link to pkgsrc-wip and explain.) - #143948 (Update mdbook to 0.4.52) - #143968 (Add tracing to `InterpCx::fn_abi_of_instance/fn_abi_of_fn_ptr`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 12 pull requests Successful merges: - #142936 (rustdoc-json: Structured attributes) - #143355 (wrapping shift: remove first bitmask and table) - #143448 (remote-test-client: Exit code `128 + <signal-number>` instead of `3`) - #143692 (miri: fix out-of-bounds error for ptrs with negative offsets) - #143719 (Emit warning when there is no space between `-o` and arg) - #143738 (Move several float tests to floats/mod.rs) - #143891 (Port `#[coverage]` to the new attribute system) - #143920 (Make more of codegen_llvm safe) - #143921 (Constify `Index` traits) - #143939 (Add 0323pin as maintainer of NetBSD targets, fix link to pkgsrc-wip and explain.) - #143948 (Update mdbook to 0.4.52) - #143968 (Add tracing to `InterpCx::fn_abi_of_instance/fn_abi_of_fn_ptr`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 12 pull requests Successful merges: - #142936 (rustdoc-json: Structured attributes) - #143355 (wrapping shift: remove first bitmask and table) - #143448 (remote-test-client: Exit code `128 + <signal-number>` instead of `3`) - #143692 (miri: fix out-of-bounds error for ptrs with negative offsets) - #143719 (Emit warning when there is no space between `-o` and arg) - #143738 (Move several float tests to floats/mod.rs) - #143891 (Port `#[coverage]` to the new attribute system) - #143920 (Make more of codegen_llvm safe) - #143921 (Constify `Index` traits) - #143939 (Add 0323pin as maintainer of NetBSD targets, fix link to pkgsrc-wip and explain.) - #143948 (Update mdbook to 0.4.52) - #143968 (Add tracing to `InterpCx::fn_abi_of_instance/fn_abi_of_fn_ptr`) r? `@ghost` `@rustbot` modify labels: rollup
Closes #142812
getopt
doesn't seem to have an API to check this, so we have to check the args manually.r? compiler