Skip to content

introduce @FieldEnum and use it for the operand type of builtins that accept a field name #23618

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

Open
andrewrk opened this issue Apr 20, 2025 · 9 comments
Labels
breaking Implementing this issue could cause existing code to no longer compile or have different behavior. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@andrewrk
Copy link
Member

std.meta.FieldEnum already exists for this purpose.

This proposal is to promote such functionality to a builtin and use it as the field parameter type for these functions rather than []const u8:

  • @field
  • @fieldParentPtr
  • @offsetOf
  • @bitOffsetOf
  • @unionInit
  • @FieldType
@fieldParentPtr("foo", foo);

🔽

@fieldParentPtr(.foo, foo);
@andrewrk andrewrk added breaking Implementing this issue could cause existing code to no longer compile or have different behavior. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. labels Apr 20, 2025
@andrewrk andrewrk added this to the 0.15.0 milestone Apr 20, 2025
@190n
Copy link
Contributor

190n commented Apr 20, 2025

Would this apply to @typeInfo's return value for containers, too?

@InKryption
Copy link
Contributor

InKryption commented Apr 20, 2025

Question: how does this apply to @field on a namespace? Currently you'd do @field(struct { const foo = 0; }, "foo"), but under this proposal, it would need to be an enum. Would we get an equivalent @DeclEnum for @field(type, @DeclEnum())?

Also, just want to point out, with one of the main purposes of @field being to programmatically access fields/declarations, how does that fit under this proposal?

@jklw10
Copy link

jklw10 commented Apr 20, 2025

Wouldn't this disallow doing wacky things with constructing wanted field programmatically.

@rohlem
Copy link
Contributor

rohlem commented Apr 20, 2025

#3839 proposes to split decl access into its own builtin @decl.
As pointed out above, constructing field names as comptime strings would regress under this proposal.
If we add @decl(comptime namespace: type, decl_name: []const u8),
current use cases can use @field(T, @decl(@FieldEnum(T), name)) for fields.
(Less readable imo.)

The only 2 benefits I see to this proposal (assuming it only applies to builtins on struct/union instances):

  • The corresponding langref documentation would become a little more readable.
  • We get rid of a function in std.meta. (I personally don't mind std.meta).

@castholm
Copy link
Contributor

Wouldn't this disallow doing wacky things with constructing wanted field programmatically.

I assume that you would still be able to do @field(thing, std.meta.stringToEnum(@FieldEnum(T), "foo") or iterate over the type info yourself to find the enum value corresponding to your field. The only regression will be that you might need to increase your comptime backward branch quota with @setEvalBranchQuota() in some places.

@scottredig
Copy link
Contributor

scottredig commented Apr 20, 2025

Did a quick look through of code I've written checking usages of the proposed changed builtins for new problems, and noted only one concern:
This would separate the type received from std.meta.fields or @typeInfo's field names from what is passed to @field, @offsetOf, etc. This adds some minor friction to code which is iterating over and accessing the fields of a struct (or other container).
I don't see any other issues this would cause in my code, the rest of the usages are static strings which easily could be converted.

@nektro
Copy link
Contributor

nektro commented Apr 20, 2025

I think introducing @FieldEnum would be a net-positive
but for the reasons others have mentioned I think using it for the operand type of builtins that accept a field name would be net-negative

@matklad
Copy link
Contributor

matklad commented Apr 22, 2025

The only 2 benefits I see to this proposal (assuming it only applies to builtins on struct/union instances)

The biggest benefit here is grepability. Consider finding all usages of field foo. The status quo is that you grep for \.foo, which gives you all non-meta usages, but misses meta-usages. This is especially confusing if you don't know if there are meta-usage for the particular field you are looking for. I have an anecdotal evidence where I indeed was confused by a field which seemed unused to me, but which actually was used via string literal which I didn't notice during search.

This argument requires an assumption that most meta usages don't construct field names by string concatenation, but rather specify it as a specific string at the call-site, and then thread through several abstraction layers:

// Call site:
do_some_magic("foo");

// 6 functions down:
const field_value = @field(t, field_name); // field_name is "foo" here. 

But I think this is generally true, there seems to be Pareto-thing going on here --- 80% usages are non-meta, and among the meta usages, 80% does not do string concatenation.

The second, minor, benefit here is that library abstractions already use field enums. In MultiArrayList, we write xs.items(.foo), not xs.items("foo"). So library abstractions include a translation layer from enums to strings, and the proposal would eliminate that.

@rohlem
Copy link
Contributor

rohlem commented Apr 23, 2025

The only 2 benefits I see to this proposal (assuming it only applies to builtins on struct/union instances)

The biggest benefit here is grepability.

I think I understand your point, I've just never myself come into the situation you describe.
(So thanks for sharing, I'm just leaving my 2c, but your point certainly holds water regardless!)

My most common uses of @field combine with @typeInfo information (like iterating over all fields), which is a string in status-quo.
(We can't even really change this in this proposal, because @Type reification needs to allow string names, otherwise it becomes circular.
We could split std.builtin.Type(Info) into 2 separate types for @typeInfo vs @Type, which I don't like, but #10710 seems to kind of go in this direction anyway, so we'll see.)

Besides that, there are two types of uses that code wants to achieve, which the proposal doesn't fundamentally affect in any way:

  • If I know I want a certain field, I'd specify "field" in status-quo and .field with the proposal.
    When looking for a field, I'd personally search just for the field name (without the dot), because the name appearing in comments etc. would probably also be useful clues as to when/how it's used.
    Then again, if it's a common word, or the name of a concept that comes up unrelatedly, I could see the dot mattering for grep-ability.
  • If I need to construct the field name, in status-quo there is rather small friction.
    With the proposal we require an extra mapping helper function - not a big deal, but not an improvement.

library abstractions include a translation layer from enums to strings, and the proposal would eliminate that.

The status-quo translation enum -> string is @tagName and string -> enum is @field(FieldEnum(T), name).
Both quite succinct, but if most code bases net-improve with the change, of course that's a solid argument.

Actually, having a builtin @enumLiteral(state_name: []const u8) would greatly improve the string -> enum direction I think.
Maybe that would be worth considering? Both because it might improve the uses related to / under this proposal, and otherwise.
(The "obvious way" would still be .state instead of @enumLiteral("state"), and I don't think we should design around making misuses impossible anyway.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Implementing this issue could cause existing code to no longer compile or have different behavior. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

No branches or pull requests

9 participants