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

Printed function definition types can be confusing #69232

Closed
glandium opened this issue Feb 17, 2020 · 12 comments · Fixed by #133538
Closed

Printed function definition types can be confusing #69232

glandium opened this issue Feb 17, 2020 · 12 comments · Fixed by #133538
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-FFI Area: Foreign function interface (FFI) C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@glandium
Copy link
Contributor

glandium commented Feb 17, 2020

I tried this code:

use curl_sys::{curl_easy_setopt, CURLOPT_READFUNCTION};
use libc;

// (...)
curl_easy_setopt(curl, CURLOPT_READFUNCTION, libc::fread);

I expected to see this happen: maybe not a failure, or a failure that is not completely confusing.

Instead, this happened:

error[E0617]: can't pass `unsafe extern "C" fn(*mut core::ffi::c_void, usize, usize, *mut libc::unix::FILE) -> usize {libc::unix::fread}` to variadic function
   --> hg_connect.rs:844:62
    |
844 |                 curl_easy_setopt(curl, CURLOPT_READFUNCTION, libc::fread);
    |                                                              ^^^^^^^^^^^
    |
help: cast the value to `unsafe extern "C" fn(*mut core::ffi::c_void, usize, usize, *mut libc::unix::FILE) -> usize`
    |
844 |                 curl_easy_setopt(curl, CURLOPT_READFUNCTION, libc::fread as unsafe extern "C" fn(*mut core::ffi::c_void, usize, usize, *mut libc::unix::FILE) -> usize);
    |                                                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Note how it suggests to cast to the exact same type it started with.

rustc --version --verbose:

rustc 1.43.0-nightly (5e7af4669 2020-02-16)
binary: rustc
commit-hash: 5e7af4669f80e5f682141f050193ab679afdb4b1
commit-date: 2020-02-16
host: aarch64-unknown-linux-gnu
release: 1.43.0-nightly
LLVM version: 9.0
@glandium glandium added the C-bug Category: This is a bug. label Feb 17, 2020
@jonas-schievink jonas-schievink added A-diagnostics Area: Messages for errors, warnings, and lints A-FFI Area: Foreign function interface (FFI) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-enhancement Category: An issue proposing an enhancement or a PR with one. and removed C-bug Category: This is a bug. labels Feb 17, 2020
@jonas-schievink
Copy link
Contributor

That's not the same type, it just prints "fn item" in a very similar way to "fn pointer". The former is zero-sized, while the latter is pointer-sized.

@glandium
Copy link
Contributor Author

Are you saying that the {libc::unix::fread} part is supposed to indicate that?

@jonas-schievink
Copy link
Contributor

Yeah, it indicates that it's the type of that specific fn item, and not an fn pointer.

@glandium
Copy link
Contributor Author

That... is really not obvious.

@Centril Centril added the D-confusing Diagnostics: Confusing error or lint that should be reworked. label Feb 17, 2020
@Centril
Copy link
Contributor

Centril commented Feb 17, 2020

I agree it isn't; but on the other hand I'm not sure what would be clear without also taking up a lot of space... ideas?

cc @estebank

@estebank
Copy link
Contributor

An option would be to move the function name after the fn instead of after the signature, but doubt that would be enough. Best bet would be to add extra wording to the message about this being a specific function and expecting a function pointer, but the wording of those two is too close and lends itself to the same confusion we have now. Because of that, I think we should add a note explaining the nuance for these cases.

@jumbatm
Copy link
Contributor

jumbatm commented Feb 17, 2020

How about something like this?

error[E0617]: can't pass `{individual function type for libc::unix::fread}` to variadic function
   --> hg_connect.rs:844:62
    |
844 |                 curl_easy_setopt(curl, CURLOPT_READFUNCTION, libc::fread);
    |                                                              ^^^^^^^^^^^
    |
help: cast the value to a function pointer:
    |
844 |                 curl_easy_setopt(curl, CURLOPT_READFUNCTION, libc::fread as unsafe extern "C" fn(*mut core::ffi::c_void, usize, usize, *mut libc::unix::FILE) -> usize);
    |                                                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I'd argue that confusion comes from seeing the "same" function signature so many times, so it's vital to distinguish between the individual function type vs the function pointer type as much (and as obviously) as possible.

@HTGAzureX1212
Copy link
Contributor

@rustbot release-assignment

@HTGAzureX1212
Copy link
Contributor

@rustbot assign

@rustbot
Copy link
Collaborator

rustbot commented Mar 5, 2022

Error: Parsing assign command in comment failed: ...'bot assign' | error: specify user to assign to at >| ''...

Please let @rust-lang/release know if you're having trouble with this bot.

@HTGAzureX1212
Copy link
Contributor

@rustbot claim

@Noratrieb Noratrieb changed the title Error message for E0617 is confusing when functions are involved Printing function definition types can be confusing Jan 21, 2024
@Noratrieb Noratrieb changed the title Printing function definition types can be confusing Printed function definition types can be confusing Jan 21, 2024
@dev-ardi
Copy link
Contributor

@rustbot claim

Zalathar added a commit to Zalathar/rust that referenced this issue Nov 29, 2024
…iler-errors

Better diagnostic for fn items in variadic functions

closes rust-lang#69232
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 29, 2024
…iler-errors

Better diagnostic for fn items in variadic functions

closes rust-lang#69232
@bors bors closed this as completed in 72cf40d Nov 29, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Nov 29, 2024
Rollup merge of rust-lang#133538 - dev-ardi:69232-better-diag, r=compiler-errors

Better diagnostic for fn items in variadic functions

closes rust-lang#69232
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 A-FFI Area: Foreign function interface (FFI) C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
8 participants