-
Notifications
You must be signed in to change notification settings - Fork 13.8k
Fix target list of link_section
#147418
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
Fix target list of link_section
#147418
Conversation
Signed-off-by: Jonathan Brouwer <jonathantbrouwer@gmail.com>
Some changes occurred in compiler/rustc_attr_parsing |
This comment was marked as outdated.
This comment was marked as outdated.
@rustbot label beta-nominated |
Actually, would like to add a regression test for this |
Reminder, once the PR becomes ready for a review, use |
This comment was marked as resolved.
This comment was marked as resolved.
I commented that above already, that's why it's waiting on author :) |
Oh sorry, I managed to read every other comment but missed that specific comment 😂 |
|
| | ||
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! | ||
= help: `#[link_section]` can be applied to statics and functions | ||
= help: `#[link_section]` can be applied to functions and statics |
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.
maybe we should sort this list in the future, so this doesn't happen
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.
better for incremental too actually hehe
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.
yea this seems like something we missed during migration. Looking good, apart from link name on traits which I agree is weird |
@bors r- I guess the beta backport first should be accepted |
r=me when it does |
Rollup of 7 pull requests Successful merges: - #145943 (stdlib docs: document lifetime extension for `format_args!`'s arguments) - #147243 (cmse: disallow `impl Trait` in `cmse-nonsecure-entry` return types) - #147402 ([rustdoc] Don't serialize & deserialize data that doesn't go OTW) - #147418 (Fix target list of `link_section`) - #147429 (Print tip for human error format in runtest) - #147441 (Fix comments error for Provenance impls) - #147442 (c-variadic: fix thir-print for `...` without a pattern) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #147418 - JonathanBrouwer:link_section_targets, r=jdonszelmann Fix target list of `link_section` Fixes #147411 (comment) r? `@jdonszelmann` `@rustbot` beta-nominate T-compiler
…hanBrouwer sort attribute targets for more consistent error messages In this PR I noticed that we don't sort attribute targets, so a rather trivial change to the source changed the ordering in an error message even though its meaning stayed the same. See: rust-lang#147418 (comment) I think sorting might be a good thing to do in general. I also prefer it when reading error messages. Quite a few tests changed, but not in meaning, only sorting order obviously. r? `@jieyouxu`
Rollup merge of #147445 - jdonszelmann:sort-targets, r=JonathanBrouwer sort attribute targets for more consistent error messages In this PR I noticed that we don't sort attribute targets, so a rather trivial change to the source changed the ordering in an error message even though its meaning stayed the same. See: #147418 (comment) I think sorting might be a good thing to do in general. I also prefer it when reading error messages. Quite a few tests changed, but not in meaning, only sorting order obviously. r? `@jieyouxu`
Fixes #147411 (comment)
r? @jdonszelmann
@rustbot beta-nominate T-compiler