Skip to content
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

Change registered "program name" for -Cllvm-args usage messages #75471

Merged
merged 1 commit into from
Aug 14, 2020

Conversation

richkadel
Copy link
Contributor

While debugging a codegen issue, I tried adding LLVM options with
the rustc -Cllvm-args option, and was confused by the error and usage
messaging.

The LLVM "program name" argument is set to "rustc", and command line
error messages make it look like invalid arguments are "rustc"
arguments, not LLVM.

I changed this argument so error messages and the "-help" usage feedback
is easier to understand and react to. (Clang does something similar.)

r? @wesleywiser

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 12, 2020
@richkadel
Copy link
Contributor Author

---- [ui] ui/unknown-llvm-arg.rs stdout ----
diff of stderr:

-	rustc -Cllvm-args="..." with: Unknown command line argument '-not-a-real-llvm-arg'.  Try: 'rustc -Cllvm-args="..." with --help'
+	rustc -Cllvm-args="..." with: Unknown command line argument '-not-a-real-llvm-arg'.  Try: 'rustc -Cllvm-args="..." with -help'

I generated the stderr file with --bless, and on my local machine, the message includes 2 dashes for "--help", but apparently CI's "llvm" is different. I'll normalize this.

While debugging a codegen issue, I tried adding LLVM options with
the rustc -Cllvm-args option, and was confused by the error and usage
messaging.

The LLVM "program name" argument is set to "rustc", and command line
error messages make it look like invalid arguments are "rustc"
arguments, not LLVM.

I changed this argument so error messages and the "-help" usage feedback
is easier to understand and react to. (Clang does something similar.)
Copy link
Member

@wesleywiser wesleywiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a good idea to me.

cc @rust-lang/wg-codegen

@wesleywiser
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Aug 13, 2020

📌 Commit d4593af has been approved by wesleywiser

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 13, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 14, 2020
Rollup of 12 pull requests

Successful merges:

 - rust-lang#74650 (Correctly parse `{} && false` in tail expression)
 - rust-lang#75319 (Fix ICE rust-lang#75307 in `format`)
 - rust-lang#75417 (Don't spill operands onto the stack in naked functions)
 - rust-lang#75452 (self-profile: Cache more query key strings when doing self-profiling.)
 - rust-lang#75459 (fix LocalInfo doc comment)
 - rust-lang#75462 (Remove unused tcx parameter)
 - rust-lang#75467 (Fix E0741 error code explanation)
 - rust-lang#75471 (Change registered "program name" for -Cllvm-args usage messages)
 - rust-lang#75477 (Expand function pointer docs)
 - rust-lang#75479 (make rustc-docs component available to rustup)
 - rust-lang#75496 (Prioritization WG: Open Zulip topics only for `I-prioritize` issues)
 - rust-lang#75500 (Disable zlib in LLVM on aarch64-apple-darwin)

Failed merges:

r? @ghost
@bors bors merged commit d000fb1 into rust-lang:master Aug 14, 2020
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants