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

RFC: Allow cfg-attributes on elements of tuple type declarations #3532

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

recatek
Copy link
Contributor

@recatek recatek commented Nov 23, 2023

Let's make it more elegant to conditionally compile tuple type declarations by allowing cfg-attributes directly on their element types.

Rendered

@ehuss ehuss added the T-lang Relevant to the language team, which will review and decide on the RFC. label Nov 23, 2023
@ahicks92
Copy link

I agree that this makes the language consistent and so it's probably worth doing, however the idea of conditionally including or excluding elements from tuples or tuple structs fills me with dread because of all the cases where maybe you get two different 3-tuples and the types happen to line up or something. Maybe it's worth finding a use case for the motivation that's not a generic "maybe this shouldn't be exceptional".

@recatek
Copy link
Contributor Author

recatek commented Nov 25, 2023

My particular use case is for working on gecs, a compile-time entity component system that relies on tuples to work properly. There are a number of features I can't easily add to this system while also supporting conditional compilation. Conditional compilation in this case is important because you may have components that only exist on the client, or on the server, or only exist in debug builds, and so on. While I agree you could accidentally misalign types with this method, you already could do that with the combinatorial alternative of redefining each tuple type -- something that is likely to happen in a macro due to the amount of boilerplate involved.

@clarfonthey
Copy link

clarfonthey commented Nov 25, 2023

I like this change, although my biggest concern is that because tuples are indexed by numbers, this change technically makes the exact indices of fields… weird.

One thing that might be a bit less disruptive is extending the tuple struct expression syntax to work with defining structs. Specifically, this syntax:

struct Tuple(&'static str, bool);

// you can just do this
const TUP: Tuple = Tuple { 0: "hello", 1: true };

So, the proposal would be that you could define structs like this:

struct Tuple {
    0: &'static str,
    1: bool,
}

And then, you could do weird things like:

struct Tuple {
    0: &'static str,
    #[cfg(one_thing)]
    1: bool,
    #[cfg(another_thing)]
    1: u8,
    #[cfg(third_thing)]
    2: i32,
}

And this way, you know what the indices of the fields are, and you also have the guarantee where, if this causes a gap in the indices (for example, if one_thing and another_thing don't apply when third_thing does, removing field 1), instead of just shifting the elements along, it just throws an error.

We could also add an auto-applicable suggestion to convert from the proposed syntax, where #[cfg(thing)] attributes are added to individual fields, to this syntax.

As an aside, I do think that we should still support attributes in the positions supported, for derive macros and other niche cases, since it just being a hard syntax error is far from ideal. Just, I disagree with using cfg in this position, and would prefer the index-based approach.

@recatek
Copy link
Contributor Author

recatek commented Nov 25, 2023

I'm not sure that's all that functionally different from defining structs as you can now like so:

struct Something {
    a: SomeTypeA,
    b: SomeTypeB,
    #[cfg(feature = "foo")]
    c: SomeTypeC,
}

Just using a, b, and c instead of 0, 1, and 2 for accessors. Unfortunately that doesn't cover the use case in question here. The reason I ran into this issue is that I need to use tuples specifically to construct archetype storage, in a way similar to how bevy constructs its queries (see here and here). Queries use tuples here to get variadic-like behavior, and doing so with tuples would allow me to construct ECS archetypes like so:

type SomeArchetype = EcsArchetype<(
    ComponentA,
    ComponentB,
    ComponentC,
    #[cfg(feature = "server")]
    ServerOnlyComponent,
)>;

Except #[cfg] is not supported in that position. What I could do legally now is

#[cfg(not(feature = "server"))]
type SomeArchetype = EcsArchetype<(
    ComponentA,
    ComponentB,
    ComponentC,
)>;

#[cfg(feature = "server")]
type SomeArchetype = EcsArchetype<(
    ComponentA,
    ComponentB,
    ComponentC,
    ServerOnlyComponent,
)>;

But that doesn't scale well if you have multiple features at play. In the short term I will likely create a macro to do this, but it's far from ideal -- supporting #[cfg] in this position the way it is for tuple initialization would make much more sense than these workarounds. As with #3399, using macros to obscure the combinatorial boilerplate here is generally a bad plan because it separates the intent from the actual produced code and makes things harder to read in the end.

@recatek
Copy link
Contributor Author

recatek commented Nov 25, 2023

Also, as an additional consistency note, this is already supported in tuple-based structs. The following is valid code:

pub struct SomeStruct(i32, #[cfg(all())] u32);

fn main() {
    let thing = SomeStruct(0, 1);
    println!("{}", thing.1)
}

I should probably add that to the motivation of the RFC, I guess?

@ahicks92
Copy link

I think that the interesting use case is gecs. I haven't dug into the code but "these are the special things about tuples" would go a long way here. Again though, this is probably worth doing just for consistency.

I will have to look into gecs more for personal projects; if you have somehow solved being able to jump into Lua scripts in the middle of systems that may solve many problems. But that's way off topic. Still, neat idea.

@recatek
Copy link
Contributor Author

recatek commented Nov 25, 2023

Fair warning, gecs does some truly dreadful compile-times things in order to function (which I'm trying to work out better ways to do as Rust improves its metaprogramming stories over time). For tuples specifically, this kind of soft variadism is pretty common in ECS libraries. I mentioned bevy's queries above, but other libraries like hecs and legion do similar things. You can't really do this kind of logic with a struct because it relies on the type being easy to anonymously construct on the fly.

The core conceit is that you want to be able to define structs/traits in the form of Something<T>, where T is in fact a collection of types described by a tuple, and all of the methods and dependent types are pre-defined for all tuples of arity 1 through, say, 16. You can't do this pre-definition if you require the end user to define an explicitly named struct, so you must use tuples here. I guess technically you could define named and numbered generic structs but it would not be very ergonomic, and that would start to defeat the purpose of these sorts of libraries -- you also couldn't easily conditionally compile those either.

@ahicks92
Copy link

Yes, I know and have used the technique. Warp takes it even further: if you define the right traits you can concatenate and flatten tuples as well.

I'm just saying that advocating for this, writing down such use cases would be a good idea. I don't like it, but I agree it should be in the language. I guess that's a weird stance. But my feedback is primarily about readability. I don't think most people consuming this are going to see us vaguely gesture at ECS and warp and go "yeah right type metaprogramming" and I know about such things and still required that you do so to remember they exist. The real problem of course is that Rust doesn't have a variadics story but that's neither here nor there.

@recatek
Copy link
Contributor Author

recatek commented Nov 26, 2023

I've added a motivation section walking through the interaction between cfg-attributes and walking through the construction of a hypothetical entity archetype for multiplayer asteroids. Does that illustrate the need well, do you think?

@ahicks92
Copy link

Yeah, that's pretty good I think.

@recatek
Copy link
Contributor Author

recatek commented Nov 28, 2023

For added context I started a discussion about the broader question of cfg-attribute rule consistency on IRLO here. I also put together a HackMD page with all of the comma-terminated fragments that I'm aware of in Rust (like this case, and also where-bounds as in RFC #3399) and whether you can or can't use a cfg-attribute on them. Suggested additions welcome if I missed any. Overall there's a lot of inconsistency that I'd like to try to resolve, either case-by-case or just all up if possible.

@clarfonthey
Copy link

Also, as an additional consistency note, this is already supported in tuple-based structs. The following is valid code:

pub struct SomeStruct(i32, #[cfg(all())] u32);

fn main() {
    let thing = SomeStruct(0, 1);
    println!("{}", thing.1)
}

Wow, I guess I was just tired when reading this, because I completely accidentally conflated the two. I think that closing the consistency gap is good, then.

I still think that allowing definitions of the type I mentioned would be a good additional, but it doesn't have to be part of this RFC.

@recatek
Copy link
Contributor Author

recatek commented Jan 22, 2025

After playing around with this a bit, it occurs to me that this should be bundled with allowing cfg-attributes in pattern matching on said tuples as well, to make it easier to break down these conditionally-described tuples. In the hecs or bevy-like ECS example, this would look like:

type MovementQuery = (Position, Rotation, #[cfg(server)] Authority);
for data in ecs.query::<MovementQuery>().iter_mut() {
    let (position, rotation, #[cfg(server)] authority) = data;
    let _output = update_movement(position, rotation);
    
    #[cfg(server)]
    update_authority(authority, position, rotation, _output);
}

I think I'll amend the RFC to include these.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants