-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Improve the error explanations for check_const #30932
Conversation
const Y: usize = &X as *const u32 as usize; | ||
println!("{}", Y); | ||
static MY_STATIC_ADDR: &'static u32 = &MY_STATIC; | ||
// ..and also |
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.
"// ..and also"? Isn't "// ... and also" better?
Looks like an overall improvement. |
improved |
To fix this error, please do not assign this value to a constant expression. | ||
Example: | ||
To fix this, you should dereference your pointer at some point in | ||
run-time. |
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 wrong. This document already contains various cases of wrong advice.
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.
Wrong?
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.
If the problem is that you need the value behind a pointer in a static variable, then not putting the value in a static variable does not fix the problem.
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.
It's not like you can solve the problem.
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 am not sure how important is the "how to fix this" advice, because each issue will have its own fix.
ba50fa0
to
5e16148
Compare
Then MY_STATIC_ADDR would contain the address of MY_STATIC. However, | ||
the address can change when the program is linked, as well as change | ||
between different executions due to ASLR, and many linkers would | ||
not be able to calculate the value of WHAT. |
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.
Nit: This paragraph should use inline code
for the constant names.
r=me once the two nits are fixed. |
5e16148
to
cad3882
Compare
fixed ^ |
@bors r+ cad3882 |
@bors rollup |
☔ The latest upstream changes (presumably #31024) made this pull request unmergeable. Please resolve the merge conflicts. |
🔒 Merge conflict |
cad3882
to
47593da
Compare
@bors r=nagisa rollup |
📌 Commit 47593da has been approved by |
Fixes #30705
r? @nagisa