-
Notifications
You must be signed in to change notification settings - Fork 13.3k
hir-analysis: make a helpful note #108337
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
r? @cjgillot (rustbot has picked a reviewer for you, use r? to override) |
|
@@ -5,6 +5,12 @@ LL | #[track_caller] | |||
| ^^^^^^^^^^^^^^^ | |||
LL | fn main() { | |||
| --------- `main` function is not allowed to be `#[track_caller]` | |||
| | |||
help: remove the annotation |
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 don't think this being a help note is the best choice.
Preferably it would be a suggestion. If not, then a label is better so that it renders inline.
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 made an attempt at what you meant
☔ The latest upstream changes (presumably #103042) made this pull request unmergeable. Please resolve the merge conflicts. |
a7e5447
to
98805b3
Compare
@@ -128,4 +128,4 @@ hir_analysis_auto_deref_reached_recursion_limit = reached the recursion limit wh | |||
.help = consider increasing the recursion limit by adding a `#![recursion_limit = "{$suggested_limit}"]` attribute to your crate (`{$crate_name}`) | |||
|
|||
hir_analysis_track_caller_on_main = `main` function is not allowed to be `#[track_caller]` | |||
.label = `main` function is not allowed to be `#[track_caller]` | |||
.label = remove this annotation |
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.
For future reference, you can make another label with .label2 = ..
or some other name like .removal_label = ..
. It's probably not best practice to remove existing labels unless they're not useful. I left more comments below for how to make a suggestion, though.
#[primary_span] | ||
pub span: Span, | ||
#[label] | ||
pub annotated: Span, |
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.
#[primary_span]
#[suggestion(applicability = "maybe-incorrect", code = "")]
pub span: Span,
#[label]
pub annotated: Span,
Then make sure the ftl file has a .suggestion = ...
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 made a new commit, thanks.
What does "maybe-incorrect"
and "machine-applicable"
mean?
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.
They correspond to the enum variants in https://doc.rust-lang.org/nightly/nightly-rustc/rustc_errors/enum.Applicability.html
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.
Please squash this into one commit. Then I can approve.
pub span: Span, | ||
#[label] | ||
#[label(hir_analysis_track_caller_on_main)] |
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 can be turned back into just [label]
.
Then you can just add ".label = main
function is not allowed to be #[track_caller]
" back to the ftl file.
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 did it so because it's the exact same text, so did not want to repeat it.
38c880f
to
570f752
Compare
@bors r+ rollup |
📌 Commit 570f75251b7fb4a0ebc5af46b70c40faaaaf0f4f has been approved by It is now in the queue for this repository. |
☔ The latest upstream changes (presumably #108450) made this pull request unmergeable. Please resolve the merge conflicts. |
570f752
to
e5d1fcd
Compare
@bors r+ rollup |
…mpiler-errors Rollup of 7 pull requests Successful merges: - rust-lang#105736 (Test that the compiler/library builds with validate-mir) - rust-lang#107291 ([breaking change] Remove a rustdoc back compat warning) - rust-lang#107675 (Implement -Zlink-directives=yes/no) - rust-lang#107848 (Split `x setup` sub-actions to CLI arguments) - rust-lang#107911 (Add check for invalid #[macro_export] arguments) - rust-lang#108229 ([107049] Recognise top level keys in config.toml.example) - rust-lang#108333 (Make object bound candidates sound in the new trait solver) Failed merges: - rust-lang#108337 (hir-analysis: make a helpful note) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#107941 (Treat `str` as containing `[u8]` for auto trait purposes) - rust-lang#108299 (Require `literal`s for some `(u)int_impl!` parameters) - rust-lang#108337 (hir-analysis: make a helpful note) - rust-lang#108379 (Add `ErrorGuaranteed` to `hir::{Expr,Ty}Kind::Err` variants) - rust-lang#108418 (Replace parse_[sth]_expr with parse_expr_[sth] function names) - rust-lang#108424 (rustc_infer: Consolidate obligation elaboration de-duplication) - rust-lang#108475 (Fix `VecDeque::shrink_to` and add tests.) - rust-lang#108482 (statically guarantee that current error codes are documented) - rust-lang#108484 (Remove `from` lang item) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
No description provided.