Skip to content

incorrect suggestion span for no-mangle-const-items lint #45562

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

Closed
zackmdavis opened this issue Oct 26, 2017 · 2 comments
Closed

incorrect suggestion span for no-mangle-const-items lint #45562

zackmdavis opened this issue Oct 26, 2017 · 2 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. WG-diagnostics Working group: Diagnostics

Comments

@zackmdavis
Copy link
Member

zackmdavis commented Oct 26, 2017

error: const items should never be #[no_mangle]
 --> const.rs:1:14
  |
1 | #[no_mangle] pub const RAH: usize = 5;
  |              -----^^^^^^^^^^^^^^^^^^^^
  |              |
  |              help: try a static value: `pub static`
  |
  = note: #[deny(no_mangle_const_items)] on by default

should look more like

error: const items should never be #[no_mangle]
 --> const.rs:1:14
  |
1 | #[no_mangle] pub const RAH: usize = 5;
  |                  -----^^^^^^^^^^^^^^^^
  |                  |
  |                  help: try a static value: `static`
  |
  = note: #[deny(no_mangle_const_items)] on by default

due to faulty assumptions introduced in #45232

@TimNN TimNN added A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Oct 31, 2017
@estebank estebank added E-needs-mentor WG-diagnostics Working group: Diagnostics labels Dec 7, 2017
@zackmdavis
Copy link
Member Author

Illustration of implications for RLS users:

faulty_pub_correction

@zackmdavis
Copy link
Member Author

This sort of thing (correct suggestions at the front of item-definitions, where there could be a pub, pub(crate) or pub(in path) item visibility modifier, or not) would be easier if hir::Visibility included the span of the visibility modifier. Would it be OK to plumb that through? If not, we're reduced to text-manipulation shenanigans on the item-definition span snippet, which would be messy to get correct across all cases.

frewsxcv added a commit to frewsxcv/rust that referenced this issue Jan 10, 2018
Account for `pub` in `const` -> `static` suggestion

Fix rust-lang#45562.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. WG-diagnostics Working group: Diagnostics
Projects
None yet
Development

No branches or pull requests

3 participants