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

Pointwise addition (and multiplication) for array struct members? #342

Open
matthiasgoergens opened this issue Mar 20, 2024 · 3 comments · May be fixed by #343
Open

Pointwise addition (and multiplication) for array struct members? #342

matthiasgoergens opened this issue Mar 20, 2024 · 3 comments · May be fixed by #343
Assignees
Milestone

Comments

@matthiasgoergens
Copy link

matthiasgoergens commented Mar 20, 2024

Could we make point-wise addition (and multiplication etc) work for structs (and enums etc) that contain arrays as well?

#[derive(Add)]
struct<T> MyStruct {
    x: [T; 32],
    y: T,
}

At the moment, this fails.

We have a similar failure for tuples:

#[derive(Add)]
struct PointArray {
    a: i32,
    xy: (i32, i32),
    z: i32,
}
matthiasgoergens added a commit to matthiasgoergens/derive_more that referenced this issue Mar 20, 2024
Fixes JelteF#342

We want to support examples like these:

```rust
struct StructRecursive {
    a: i32,
    b: [i32; 2],
    c: [[i32; 2]; 3],
    d: (i32, i32),
    e: ((u8, [i32; 3]), i32),
    f: ((u8, i32), (u8, ((i32, u64, ((u8, u8), u16)), u8))),
    g: i32,
}

struct TupleRecursive((i32, u8), [(i32, u8); 10]);
```

Supporting arrays and tuples inside of enums would also be useful, but
that's not in this PR.
@matthiasgoergens matthiasgoergens linked a pull request Mar 20, 2024 that will close this issue
3 tasks
matthiasgoergens added a commit to matthiasgoergens/derive_more that referenced this issue Mar 20, 2024
Fixes JelteF#342

We want to support examples like these:

```rust
struct StructRecursive {
    a: i32,
    b: [i32; 2],
    c: [[i32; 2]; 3],
    d: (i32, i32),
    e: ((u8, [i32; 3]), i32),
    f: ((u8, i32), (u8, ((i32, u64, ((u8, u8), u16)), u8))),
    g: i32,
}

struct TupleRecursive((i32, u8), [(i32, u8); 10]);
```

Supporting arrays and tuples inside of enums would also be useful, but
that's not in this PR.
@tyranron
Copy link
Collaborator

@matthiasgoergens the tuple case (i32, i32) seems OK, but I'm unsure about the array case [T; 32]. It's not obvious why the Add semantics on array is by-element adding, and not the arrays concatenation.

@JelteF what do you think on this?

@JelteF
Copy link
Owner

JelteF commented Mar 20, 2024

tl;dr +1 on the idea

Concatenation seems impossible to do to me without returning a different type from Add. So while I also think concatenation is an equally valid Add implementation, I don't think that one could ever be achieved by using derive_more. So having the element-wise addition, seems fine.

And even if we ever figured out how to implement concatenation, we could still add a special attribute to choose between the two.

@matthiasgoergens
Copy link
Author

matthiasgoergens commented Mar 20, 2024

Yes, because of the types, point-wise addition is the only thing that makes sense for fixed length arrays.

Have a look at the linked PR for an implementation prototype (with some caveats as noted in my comment.)

Perhaps going with the initial approach, but explicitly adding a Copy constraint in the impl, when the struct contains an array, might be fine? Copy is pretty much mandatory for using Add and Mul etc anyway.

@tyranron tyranron added this to the 1.0.0 milestone Mar 26, 2024
@JelteF JelteF modified the milestones: 1.0.0, 1.1.0 Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants