Skip to content

CHANGE: Allow Fill to be implemented for third-party types #1650

Description

@andresovela

Today I learned that it is not possible to implement Fill for third-party types, in the form of [T].

I ran into this when I tried implementing Fill for [half::f16] (See f16)

error[E0117]: only traits defined in the current crate can be implemented for arbitrary types
  --> /home/andy/dev/projects/half-rs/src/rand_distr.rs:96:1
   |
96 | impl rand::Fill for [f16] {
   | ^^^^^^^^^^^^^^^^^^^^-----
   |                     |
   |                     this is not defined in the current crate because slices are always foreign
   |
   = note: impl doesn't have any local type before any uncovered type parameters
   = note: for more information see https://doc.rust-lang.org/reference/items/implementations.html#orphan-rules
   = note: define and implement a trait or new type instead

Note that I was trying to open a PR on the half crate to implement Fill for f16 and bf16.

Details

I opened a thread in URLO and someone suggested that this would be feasible if the function signature of fill was:

trait Fill {
    fn fill(_: &mut [Self], _: &mut impl Rng);
}

Motivation

It is currently not possible to implement this trait for third party types. If I wanted to have Fill implemented for half::f16, I'd have to add half as a dependency on rand, and gate it with a half feature. This doesn't scale.

Alternatives

Fill my [f16] using a different API? However, in my case, I'm working with APIs whose function signature looks like this

fn do_something<T: rand::Fill>(value: &mut T)

which still has the same issue, even I filled the [f16] with a different API.

Activity

  1. dhardy commented on Jul 25, 2025

    @dhardy
  2. andresovela commented on Jul 25, 2025

    @andresovela
    Author

    I did try to PR impl Fill for f16 to half. The error message on the issue description is what the compiler says when building a local copy of the half crate with the impl. As you said, the only way right now is to add a ArrayForeignType([ForeignType]) if you want to mirror the Fill implementations on rand, which is unfortunate.

  3. dhardy commented on Jul 25, 2025

    @dhardy
    Member

    Sorry, I should have read the URLO thread.

    I think in general you may be right, but implementing for [f16] seems wrong (we don't even implement for [f32], because you can't just bit-cast a random u32 to a f32 and expect a useful result).

  4. reopened this on Jul 25, 2025
  5. andresovela commented on Jul 25, 2025

    @andresovela
    Author

    In my case it happens to be useful, as I'm intentionally generating garbage data just for performance testing purposes.

  6. andresovela commented on Jul 25, 2025

    @andresovela
    Author

    I just realized I didn't mention this here or in the URLO thread, but the reason why I wanted specifically the impl Fill for [f16] and not a impl Fill for WrapperType is that I have in my performance testing harness I have a bunch of functions whose signatures look like this:

        pub fn test_params(config: &LinearConfig) -> LinearParams<'static, T>
        where
            [T]: rand::Fill {
        }
    
  7. dhardy commented on Jul 25, 2025

    @dhardy
    Member

    I was trying to ascertain when the &mut [Self] receiver type was supported; I can't find this documented though 1.41 has some mention of receiver types and 1.43 stabilised support for some others. In any case, 1.63 supports this so there is no reason we couldn't switch the Fill trait in the next breaking release.

    In my case it happens to be useful, as I'm intentionally generating garbage data just for performance testing purposes.

    In this case I would expect the half::f16 maintainers to reject such a feature addition.

  8. andresovela commented on Jul 25, 2025

    @andresovela
    Author

    I think what I'm going to do is change the where [T]: rand::Fill bound to where StandardUniform: Distribution<T> and I'll fill the data manually.

  9. conqp commented on Jul 26, 2025

    @conqp

    (we don't even implement for [f32], because you can't just bit-cast a random u32 to a f32 and expect a useful result).

    https://docs.rs/rand/latest/src/rand/rng.rs.html#388

  10. dhardy commented on Jul 27, 2025

    @dhardy
    Member

    Fair call-out — and it contradicts the docs on fn fill. Those impls should be removed in my opinion (they function differently to other impls and are easy to write an ad-hoc impl for).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions