Suggest to specify a target triple when lang item is missing#91820
Merged
bors merged 1 commit intorust-lang:masterfrom Dec 14, 2021
Merged
Suggest to specify a target triple when lang item is missing#91820bors merged 1 commit intorust-lang:masterfrom
bors merged 1 commit intorust-lang:masterfrom
Conversation
davidtwco
requested changes
Dec 13, 2021
Member
davidtwco
left a comment
There was a problem hiding this comment.
I think this is a sensible change, I've left some suggestions for the wording/style but otherwise looks good.
Member
There was a problem hiding this comment.
Suggested change
| .note(&format!("This occurs when a binary crate with `#![no_std]` is compiled for a target that usually gets `{}` from the standard library.", name)) | |
| .note(&format!("this can occur when a binary crate with `#![no_std]` is compiled for a target where `{}` is defined in the standard library.", name)) |
nit: wording change + lowercase diagnostic message
Member
There was a problem hiding this comment.
Suggested change
| r#"If you want to compile for a specific target_triple that doesnt need `{}`, then make sure to run with `--target target_triple` or add the following to the crate's `.cargo/config` file: | |
| "you may be able to compile for a target that doesn't need `{}`, specify a target with `--target` or in `.cargo/config`" |
nit: wording
I don't know if there are other diagnostics where we reference what to do in Cargo, might be worth checking if there is precedent.
Contributor
Author
There was a problem hiding this comment.
I searched for other references to .cargo/config and didnt find any.
But I did stumble across this earlier:
rust/compiler/rustc_metadata/src/locator.rs
Lines 1151 to 1156 in 2ca9333
Which references what to do in rustup.
It also makes the case that its useful even when the user doesnt have rustup because it gives a hint what you would have to do in the equivalent tool.
In this case it hints that you should probably set the target in your build system of choice.
Member
There was a problem hiding this comment.
Suggested change
| tcx.sess.note_without_error("Use `#![feature(default_alloc_error_handler)]` for a default error handler"); | |
| tcx.sess.note_without_error("use `#![feature(default_alloc_error_handler)]` for a default error handler"); |
pre-existing: our diagnostics are typically always lowercase, could you add a commit changing this?
018d5ae to
fae40c5
Compare
Contributor
Author
|
The suggested changes are a big improvement, thanks! |
davidtwco
approved these changes
Dec 13, 2021
Member
|
@bors r+ |
Collaborator
|
📌 Commit fae40c5 has been approved by |
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Dec 14, 2021
… r=davidtwco Suggest to specify a target triple when lang item is missing It is very common for newbies to embedded to hit this confusing error when forgetting to specify the target. Source: me googling this error many times. ## Possible changes * We could possibly restrict the note+help to only be included on eh_personality lang item if that helped reduce false positives, but its also possible doing so would just increase false negatives * Open to any suggestions on rewriting the messages * We could possibly remove the `.cargo/config` alternative to avoid the message getting too noisy but I think its valuable to have as its the correct approach for most embedded projects so that `cargo build` just works. r? rust-lang/diagnostics
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Dec 14, 2021
…askrgr Rollup of 7 pull requests Successful merges: - rust-lang#91529 (add BinaryHeap::try_reserve and BinaryHeap::try_reserve_exact) - rust-lang#91820 (Suggest to specify a target triple when lang item is missing) - rust-lang#91851 (Make `MaybeUninit::zeroed` `const`) - rust-lang#91875 (Use try_normalize_erasing_regions in RevealAllVisitor) - rust-lang#91887 (Remove `in_band_lifetimes` from `rustc_const_eval`) - rust-lang#91892 (Fix HashStable implementation on InferTy) - rust-lang#91893 (Remove `in_band_lifetimes` from `rustc_hir`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
It is very common for newbies to embedded to hit this confusing error when forgetting to specify the target.
Source: me googling this error many times.
Possible changes
.cargo/configalternative to avoid the message getting too noisy but I think its valuable to have as its the correct approach for most embedded projects so thatcargo buildjust works.r? rust-lang/diagnostics