-
Notifications
You must be signed in to change notification settings - Fork 14.1k
Only apply NoMangle lint to plain const items #149544
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: main
Are you sure you want to change the base?
Conversation
|
The job Click to see the possible cause of the failure (guessed by this bot) |
| { | ||
| // account for "pub const" (#45562) | ||
| let start = cx | ||
| .tcx |
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.
(unrelated) This cx.tcx.sess.source_map().span_to_snippet(it.span).map(|snippet| snippet.find("const").unwrap_or(0)).unwrap_or(0) as u32; let suggestion = it.span.with_hi(BytePos(it.span.lo().0 + start + 5)); can probably be made more robust (accounting for source code comments for example) & simplified by changing it to let suggestion = item.vis_span.to(ident.span); (at least I think that's what it's computing).
| } | ||
| } | ||
| hir::ItemKind::Const(..) => { | ||
| if find_attr!(attrs, AttributeKind::NoMangle(..)) { |
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.
We still want to emit the lint, we just want to suppress the suggestion. So make the suggestion in BuiltinConstNoMangle Optional and set it to None if there are params or a where-clause.
|
Reminder, once the PR becomes ready for a review, use |
| @@ -0,0 +1,16 @@ | |||
| //! Test that nonsense bounds prevent consts from being evaluated at all. | |||
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 is not what this test file tests. Also please give the test a better name and location.
resolve: #149511