-
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 |
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
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 13 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) - #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) - #143957 (tidy: check for invalid file names) - #143968 (Add tracing to `InterpCx::fn_abi_of_instance/fn_abi_of_fn_ptr`) - #143990 (Add LocalKey<Cell>::update) r? `@ghost` `@rustbot` modify labels: rollup
Rollup failed: #144017 (comment) |
Why is this problem occurring. Is it because my base commit is too old (7 days ago)? |
No, it's because run-make tests by default will cross-compile. In this case, we don't really care about cross-compile coverage (i.e. this can be a host-only test). Probably just add |
Signed-off-by: xizheyin <xizheyin@smail.nju.edu.cn>
// This test is to check if the warning is emitted when no space | ||
// between `-o` and arg is applied, see issue #142812 | ||
|
||
//@ ignore-cross-compile | ||
use run_make_support::rustc; |
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.
Oh, I see now. I added it on top of the test,
@bors r+ |
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 15 pull requests Successful merges: - #142300 (Disable `tests/run-make/mte-ffi` because no CI runners have MTE extensions enabled) - #143271 (Store the type of each GVN value) - #143293 (fix `-Zsanitizer=kcfi` on `#[naked]` functions) - #143719 (Emit warning when there is no space between `-o` and arg) - #143833 (Ban projecting into SIMD types [MCP838]) - #143846 (pass --gc-sections if -Zexport-executable-symbols is enabled and improve tests) - #143879 (parse `const trait Trait`) - #143891 (Port `#[coverage]` to the new attribute system) - #143967 (constify `Option` methods) - #143985 (rustc_public: de-StableMIR-ize) - #144008 (Fix false positive double negations with macro invocation) - #144010 (Boostrap: add warning on `optimize = false`) - #144034 (tests: Test line number in debuginfo for diverging function calls) - #144049 (rustc-dev-guide subtree update) - #144056 (Copy GCC sources into the build directory even outside CI) r? `@ghost` `@rustbot` modify labels: rollup
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: - #142300 (Disable `tests/run-make/mte-ffi` because no CI runners have MTE extensions enabled) - #143271 (Store the type of each GVN value) - #143293 (fix `-Zsanitizer=kcfi` on `#[naked]` functions) - #143719 (Emit warning when there is no space between `-o` and arg) - #143846 (pass --gc-sections if -Zexport-executable-symbols is enabled and improve tests) - #143891 (Port `#[coverage]` to the new attribute system) - #143967 (constify `Option` methods) - #144008 (Fix false positive double negations with macro invocation) - #144010 (Boostrap: add warning on `optimize = false`) - #144034 (tests: Test line number in debuginfo for diverging function calls) - #144049 (rustc-dev-guide subtree update) - #144056 (Copy GCC sources into the build directory even outside CI) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 12 pull requests Successful merges: - #142300 (Disable `tests/run-make/mte-ffi` because no CI runners have MTE extensions enabled) - #143271 (Store the type of each GVN value) - #143293 (fix `-Zsanitizer=kcfi` on `#[naked]` functions) - #143719 (Emit warning when there is no space between `-o` and arg) - #143846 (pass --gc-sections if -Zexport-executable-symbols is enabled and improve tests) - #143891 (Port `#[coverage]` to the new attribute system) - #143967 (constify `Option` methods) - #144008 (Fix false positive double negations with macro invocation) - #144010 (Boostrap: add warning on `optimize = false`) - #144034 (tests: Test line number in debuginfo for diverging function calls) - #144049 (rustc-dev-guide subtree update) - #144056 (Copy GCC sources into the build directory even outside CI) 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