Function pointer docs may need updating #46989

vermiculus · 2017-12-24T21:52:20Z

We may need documentation updated for function pointers when used in enums. I won't pretend I understand half of what's written here, but here's the log from IRC (#rust_beginners) where we debugged this issue:

Sharlin    :: vermiculus: okay... wtf [1]

vermiculus :: Sharlin: Right!?

Sharlin    :: what makes fns with ref arguments not implement Ord or
              anything

vermiculus :: I 100% need the argument to be mutable, but I can't have it be
              mutable without being a ref, and apparently it's choking on the ref

spear2     :: maybe it has to do with the reference having an implicit lifetime?

vermiculus :: ...Ok the '100%' is not completely true, but it's the cleanest option

Sharlin    :: spear2: indeed it seems to be

spear2     :: [2]

vermiculus :: spear2: now there's a topic I still don't understand half or less

Sharlin    :: this is definitely something that should be documented in the fn docs :D

vermiculus feels useful :D

spear2     :: vermiculus: the way i understand it, every reference has an associated
              lifetime, but most of the time it can be 'inferred' and you don't see it

Sharlin    :: so this is about that higher ranked trait bound thingie

vermiculus :: spear2: that's the extent of my understanding as well; I get tripped up
              trying to 'infer' the lifetime myself (effectively seeing what the
              compiler would see)

Sharlin    :: `Bar(fn(&i32))` is actually `Bar(for<'a> fn(&'a i32))`

Sharlin    :: which means the fn is not an ordinary function pointer but a higher
              something

vermiculus :: spooky

vermiculus :: that makes me wonder if this behavior is intentional or if there's a
              more explicit means to express this idea

vermiculus :: throwing that explicit lifetime parameter is making the rest of the
              compiler go a little nutty -- wanting lifetime parameters for *everything* :(

Sharlin    :: yeah

Sharlin    :: so it goes

The text was updated successfully, but these errors were encountered:

nagisa · 2017-12-24T22:22:13Z

This probably refers to:

Function pointers implement the following traits:

Clone
PartialEq
Eq
PartialOrd
Ord
Hash
Pointer
Debug

Due to a temporary restriction in Rust's type system, these traits are only implemented on functions that take 12 arguments or less, with the "Rust" and "C" ABIs. In the future, this may change.

Which is not true as proved by the more simple example below:

fn assert_partialeq<T: PartialEq<T>>() {}

fn main() {
    assert_partialeq::<fn(&i32)>();
}

Rewording the last paragraph so it directs user to see the list of implementations on the relevant traits’ pages should suffice as a fix.

jdahlstrom · 2017-12-24T23:09:06Z

Wait... So it's just that eg. impl<A> Eq for fn(A) doesn't actually implement anything for any fn(A) where A is a reference type? You have to have a separate impl<A> Eq for fn(&A)? And another for &mut A? Which obviously would cause a combinatorial explosion in the current language...

Rewording the last paragraph so it directs user to see the list of implementations on the relevant traits’ pages should suffice as a fix.

I find the lists... less than useful. It's impossible to find anything there, really, and if you do, it doesn't help you understand why Eq is not implemented for my reference-taking function.

vermiculus · 2017-12-25T00:05:16Z

It's impossible to find anything there, really, and if you do, it doesn't help you understand why Eq is not implemented for my reference-taking function.

Agreed; as a newcomer, it's not obvious why this is the case. (read: I still don't know why this is case…)

If it's possible to write such a impl<A> Eq for fn(&A), that would also be useful information to include.

nagisa · 2017-12-25T20:58:19Z

cc @nikomatsakis

jethrogb · 2019-02-04T10:13:48Z

Triage: something changed here, but I don't know what. @nagisa's example compiles on beta.

jethrogb · 2019-02-04T10:15:45Z

Related #45048

nagisa · 2019-02-04T12:45:21Z

So this now needs a test. #55986 looks somewhat related, @eddyb pls confirm?

eddyb · 2019-02-04T13:11:26Z

I doubt #55986 is related. @rust-lang/wg-traits might know which PR it was.

nagisa · 2019-02-04T13:15:41Z

@nikomatsakis said that somebody has been flxing a problem with Clone impls for higher ranked functions. That fix would most likely also apply to the other traits. I didn’t quite catch who it was and I’m not finding the PR in question either.

jethrogb · 2019-02-04T13:36:05Z

Through bisection I found it started working between ec19464 (nightly-2019-01-03) and c0bbc39 (nightly-2019-01-04)

jethrogb · 2019-02-04T13:37:17Z

So #57282, #56507, or #55517

eddyb · 2019-02-04T14:00:02Z

Only #55517 seems plausible, IMO.

jethrogb · 2019-02-04T14:01:13Z

Confirmed with rustup-toolchain-install-master

jethrogb · 2019-02-04T14:21:42Z

I tried to test something like fn x<'a, 'b: 'a>(_a: &'a mut i32, _b: &'b mut i32) {} but I'm not sure how to write the type for a pointer to that function. I get “lifetime bounds cannot be used in this context” or “one type is more general than the other”.

jethrogb · 2019-02-06T05:59:36Z

I tried reading PR #55517 but I have no clue what it does. Can someone (@eddyb @nikomatsakis @scalexm ?) elaborate and also say whether the resulting change to the behavior observed here was intentional? We have about 3 weeks to revert the behavior from beta before it becomes stable.

jethrogb · 2019-02-12T06:13:04Z

ping @eddyb @nikomatsakis @scalexm

@aturon

Re-implement leak check in terms of universes This PR temporarily restores the leak-check, but implemented in terms of universes. This is not because the leak check behavior was necessarily **correct**, but because (a) we may want to have a transition period and because (b) we want to have more breathing room to work through the full implications of handling higher-ranked types correctly. Note that this PR builds atop #58056. Fixes #58451 Fixes #46989 Fixes #57639 r? @aturon cc @arielb1, @lqd ~~Temporary note: I've not finished running `./x.py test` locally -- I'm confident a lot of error messages in tests will need updating. I sort of expect them to revert to the older, (imo) less good error messages, which is mildly unfortunate. There might be a way to preserve the new error messages, not sure.~~

jethrogb · 2019-02-25T23:58:16Z

I don't think this issue is actually fixed? #58592 just brings this back to the state the issue was in before #55517.

pnkfelix · 2019-02-27T21:11:55Z

removing E-needstest since #58592 implies it is no longer implicitly working, at least for now...

pnkfelix · 2019-02-28T12:26:45Z

triage: P-high.

Reasoning: the fallout that came out of universes (where said fallout has been undone by the re-introduction of a leak-check) implies to me that this is P-high.

Removing nomination label since this is now prioritized and assigned (@aturon volunteered)

pnkfelix · 2019-04-04T13:19:07Z

note: blocks #59490

pnkfelix · 2019-04-04T13:20:33Z

triage: re-prioritizing as P-medium. The investigation of fallout described here still needs to happen, but its not sufficiently pressing to warrant P-high label.

jesskfullwood · 2019-07-23T08:16:21Z

I just want to compare two fn pointers to see if they are equal. As a workaround, is it safe simply to transmute them both to usize? It seems to work...

eddyb · 2019-07-23T10:13:54Z

@jesskfullwood You can just cast them safely, e.g.: f as usize.

(Or more precisely, a pair of such traits: one for `derive(PartialEq)` and one for `derive(Eq)`.) ((The addition of the second marker trait, `StructuralEq`, is largely a hack to work-around `fn (&T)` not implementing `PartialEq` and `Eq`; see also issue rust-lang#46989; otherwise I would just check if `Eq` is implemented.)) Note: this does not use trait fulfillment error-reporting machinery; it just uses the trait system to determine if the ADT was tagged or not. (Nonetheless, I have kept an `on_unimplemented` message on the new trait for structural_match check, even though it is currently not used.) Note also: this does *not* resolve the ICE from rust-lang#65466, as noted in a comment added in this commit. Further work is necessary to resolve that and other problems with the structural match checking, especially to do so without breaking stable code (adapted from test fn-ptr-is-structurally-matchable.rs): ```rust fn r_sm_to(_: &SM) {} fn main() { const CFN6: Wrap<fn(&SM)> = Wrap(r_sm_to); let input: Wrap<fn(&SM)> = Wrap(r_sm_to); match Wrap(input) { Wrap(CFN6) => {} Wrap(_) => {} }; } ``` where we would hit a problem with the strategy of unconditionally checking for `PartialEq` because the type `for <'a> fn(&'a SM)` does not currently even *implement* `PartialEq`. ---- added review feedback: * use an or-pattern * eschew `return` when tail position will do. * don't need fresh_expansion; just add `structural_match` to appropriate `allow_internal_unstable` attributes. also fixed example in doc comment so that it actually compiles.

XrXr · 2023-03-16T16:30:19Z

I believe this will be fixed by #108080. See #108080 (comment)

XAMPPRocky added C-enhancement Category: An issue proposing an enhancement or a PR with one. A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools labels Apr 10, 2018

steveklabnik added E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. and removed E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels May 29, 2018

steveklabnik added the P-medium Medium priority label Dec 27, 2018

bors closed this as completed in #58592 Feb 22, 2019

varkor reopened this Feb 25, 2019

pnkfelix removed the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Feb 27, 2019

pnkfelix added P-high High priority and removed I-nominated labels Feb 28, 2019

pnkfelix added P-medium Medium priority and removed P-high High priority labels Apr 4, 2019

aturon removed their assignment Apr 10, 2019

jesskfullwood mentioned this issue Jul 22, 2019

#[derive] Debug, PartialEq, Hash, etc. for any function pointers, regardless of type signature #54508

Closed

This was referenced Oct 8, 2019

ICE when trying to match on non-PartialEq slice. #61188

Closed

wart: non-Eq [T; 0] can be matched as if T were #[structural_match] #62336

Closed

pnkfelix mentioned this issue Nov 19, 2019

Expr walking structural match #66120

Closed

pnkfelix mentioned this issue May 19, 2020

Querify whether a type implements StructuralEq and PartialStructuralEq #72177

Closed

XrXr mentioned this issue Feb 28, 2023

YJIT: Compress BranchGenFn and BranchShape ruby/ruby#7401

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Function pointer docs may need updating #46989

Function pointer docs may need updating #46989

vermiculus commented Dec 24, 2017

nagisa commented Dec 24, 2017

jdahlstrom commented Dec 24, 2017

vermiculus commented Dec 25, 2017

nagisa commented Dec 25, 2017

jethrogb commented Feb 4, 2019 •

edited

Loading

jethrogb commented Feb 4, 2019

nagisa commented Feb 4, 2019

eddyb commented Feb 4, 2019

nagisa commented Feb 4, 2019

jethrogb commented Feb 4, 2019 •

edited

Loading

jethrogb commented Feb 4, 2019

eddyb commented Feb 4, 2019

jethrogb commented Feb 4, 2019

jethrogb commented Feb 4, 2019

jethrogb commented Feb 6, 2019

jethrogb commented Feb 12, 2019

jethrogb commented Feb 25, 2019

pnkfelix commented Feb 27, 2019

pnkfelix commented Feb 28, 2019 •

edited

Loading

pnkfelix commented Apr 4, 2019

pnkfelix commented Apr 4, 2019

jesskfullwood commented Jul 23, 2019

eddyb commented Jul 23, 2019

XrXr commented Mar 16, 2023

Function pointer docs may need updating #46989

Function pointer docs may need updating #46989

Comments

vermiculus commented Dec 24, 2017

nagisa commented Dec 24, 2017

jdahlstrom commented Dec 24, 2017

vermiculus commented Dec 25, 2017

nagisa commented Dec 25, 2017

jethrogb commented Feb 4, 2019 • edited Loading

jethrogb commented Feb 4, 2019

nagisa commented Feb 4, 2019

eddyb commented Feb 4, 2019

nagisa commented Feb 4, 2019

jethrogb commented Feb 4, 2019 • edited Loading

jethrogb commented Feb 4, 2019

eddyb commented Feb 4, 2019

jethrogb commented Feb 4, 2019

jethrogb commented Feb 4, 2019

jethrogb commented Feb 6, 2019

jethrogb commented Feb 12, 2019

jethrogb commented Feb 25, 2019

pnkfelix commented Feb 27, 2019

pnkfelix commented Feb 28, 2019 • edited Loading

pnkfelix commented Apr 4, 2019

pnkfelix commented Apr 4, 2019

jesskfullwood commented Jul 23, 2019

eddyb commented Jul 23, 2019

XrXr commented Mar 16, 2023

jethrogb commented Feb 4, 2019 •

edited

Loading

jethrogb commented Feb 4, 2019 •

edited

Loading

pnkfelix commented Feb 28, 2019 •

edited

Loading