Skip to content
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

E0425: Don't recommend the use of intrinsics first #97618

Closed
dave-tucker opened this issue Jun 1, 2022 · 6 comments · Fixed by #97822
Closed

E0425: Don't recommend the use of intrinsics first #97618

dave-tucker opened this issue Jun 1, 2022 · 6 comments · Fixed by #97822
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@dave-tucker
Copy link

Given the following code: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=dc1180286b0a5d9e0f1dbac02ccdb7b3

fn main() {
    let sz = size_of::<usize>();
    println!("Hello, world! A usize is {}", sz);
}

The current output is:

error[[E0425]](https://doc.rust-lang.org/stable/error-index.html#E0425): cannot find function `size_of` in this scope
 --> src/main.rs:2:14
  |
2 |     let sz = size_of::<usize>();
  |              ^^^^^^^ not found in this scope
  |
help: consider importing one of these items
  |
1 | [use core::intrinsics::size_of;](https://play.rust-lang.org/#)
  |
1 | [use core::mem::size_of;](https://play.rust-lang.org/#)
  |
1 | [use memoffset::__priv::mem::size_of;](https://play.rust-lang.org/#)
  |
1 | [use pin_utils::core_reexport::intrinsics::size_of;](https://play.rust-lang.org/#)
  |
    and 3 other candidates

Ideally the output should look like:

error[[E0425]](https://doc.rust-lang.org/stable/error-index.html#E0425): cannot find function `size_of` in this scope
 --> src/main.rs:2:14
  |
2 |     let sz = size_of::<usize>();
  |              ^^^^^^^ not found in this scope
  |
help: consider importing one of these items
  |
1 | [use core::mem::size_of;](https://play.rust-lang.org/#)
  |
1 | [use core::intrinsics::size_of;](https://play.rust-lang.org/#)
 |
1 | [use memoffset::__priv::mem::size_of;](https://play.rust-lang.org/#)
  |
1 | [use pin_utils::core_reexport::intrinsics::size_of;](https://play.rust-lang.org/#)
  |
    and 3 other candidates

In cases where intrinsics have been stabilised in core|std it would be useful to suggest these to users first.
Especially as using intrinsics generates another rustc error that tells you to prefer stabilized functions in std.
This would hopefully make handling this error easier for new rust users coming from languages like C/C++ and looking for the equivalent of sizeof()

The output is the same on stable, beta and nightly

@dave-tucker dave-tucker added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 1, 2022
@dave-tucker dave-tucker changed the title E0452: Don't recommend the use of intrinsics first E0425: Don't recommend the use of intrinsics first Jun 1, 2022
@compiler-errors
Copy link
Member

I'd be satisfied if we filtered out intrinsics altogether except for when the core_intrinsics feature was enabled.

@LoganDark
Copy link

LoganDark commented Jun 2, 2022

I'd be satisfied if we filtered out intrinsics altogether except for when the core_intrinsics feature was enabled.

I also get quite annoyed when stable rustc recommends I use unstable features (maybe that should be fixed in an RFC)

@estebank
Copy link
Contributor

estebank commented Jun 2, 2022

Checking the DefId of each suggested path to see if they are annotated with unstable should be doable.

@compiler-errors
Copy link
Member

Checking the DefId of each suggested path

This seems to be harder than I initially thought... so since we emit this "consider importing" suggestion in LateResolutionVisitor, we do not have access to TyCtxt it seems. And I have no idea how to extract the attributes from each Res without TyCtxt... 🤔

@estebank
Copy link
Contributor

estebank commented Jun 6, 2022

We might want to consider adding yet another dumping ground field in the Res itself or in Resolver... But at that point it seems like too much mem consumption for little gain. The other option is to delay the emitting of these errors until later, but that also seems... like a lot of additional complexity :-/

@compiler-errors
Copy link
Member

For now, I guess it wouldn't hurt just to filter suggesting paths that are literally ["core", "intrinsic", ..] (and std::intrinsic::*) instead of going thru all of the work of adding more info to the Res. I can put up a PR that does that later.

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Jun 14, 2022
…-intrinsics, r=oli-obk

Filter out intrinsics if we have other import candidates to suggest

Fixes rust-lang#97618

Also open to just sorting these candidates to be last. Pretty easy to modify the code to do that, too.
JohnTitor added a commit to JohnTitor/rust that referenced this issue Jun 14, 2022
…-intrinsics, r=oli-obk

Filter out intrinsics if we have other import candidates to suggest

Fixes rust-lang#97618

Also open to just sorting these candidates to be last. Pretty easy to modify the code to do that, too.
@bors bors closed this as completed in 0ee1504 Jun 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants