replace @Type with individual type-creating builtins#23733
replace @Type with individual type-creating builtins#23733mlugg merged 3 commits intoziglang:masterfrom
Conversation
687062e to
c7bb559
Compare
|
Although it was not listed in the issue, would it be reasonable to add |
|
Seems like a good change. I like it. Though, is it necessary to remove |
|
|
Oh, yeah. That makes sense. Thanks. Then, I don't see why this change can't be accepted. Seems pretty useful and straightforward. |
|
Does Each of the fields of those are of different types thus It's not possible to pass the wrong type for any of those and if the |
|
Someone asked me about the status of this PR, so just commenting to say that I'm aware of it, and appreciate the effort that went into it -- I'd definitely like to avoid letting this work rot. I'll hopefully get to it soon, but we have some huge changes currently in the pipeline, so the team's all a little busy right now, and it's not a great time to merge big breaking PRs. I'm hoping I'll be able to have a look at this once the reader/writer changes, and codegen pipeline changes, are all in, and everything's settling down a little. No need to worry about rebasing or anything, I will probably be happy to resolve conflicts when I get around to reviewing this. One note: I am firmly of the opinion that there should be an By the way, @alichraghi: were the usages of |
|
Yeah i figured that this is gonna take long and im fine with handling conflicts myself. was mostly waiting for writergate branch to be merged.
They are updated by hand but it wasn't painful because most new builtins accept the same |
3d17739 to
9a324c7
Compare
506bcef to
c69419b
Compare
mlugg
left a comment
There was a problem hiding this comment.
Please do not push changes to address any of these comments! In a moment, I'm going to pull this branch, rebase it, resolve all of my comments. and force-push a merge-ready version (though the push probably won't be til tomorrow). However, feel free to read through the comments if you're interested to see what I'm going to change.
Thanks for this work!
|
I know that push took a bit longer than I claimed, but there's good reason for it! I rebased this work and handled my comments yesterday, but after discussing the change with Andrew, I ended up deciding to make some more changes. You can look at the last commit here to see what the new usages look like in practice. Also, the first commit does update the language reference to include correct signatures for all of the new builtins. The arguments are given in SoA form (e.g. there is a slice of field names and a separate slice of field types); this might feel awkward if you try to write an invocation using only literals, but since these builtins are only actually used when you need to programmatically create types, it turns out to work very nicely. The builtins I have implemented here are currently accepted; assuming this passes CI, it will be merged soon. |
|
okay apparently i failed to actually test building the langref |
The new builtins are:
* `@EnumLiteral`
* `@Int`
* `@Fn`
* `@Pointer`
* `@Tuple`
* `@Enum`
* `@Union`
* `@Struct`
Their usage is documented in the language reference.
There is no `@Array` because arrays can be created like this:
if (sentinel) |s| [n:s]T else [n]T
There is also no `@Float`. Instead, `std.meta.Float` can serve this use
case if necessary.
There is no `@ErrorSet` and intentionally no way to achieve this.
Likewise, there is intentionally no way to reify tuples with comptime
fields, or function types with comptime parameters. These decisions
simplify the Zig language specification, and moreover make Zig code more
readable by discouraging overly complex metaprogramming.
Co-authored-by: Ali Cheraghi <alichraghi@proton.me>
Resolves: ziglang#10710
Signed-off-by: Matthew Lugg <mlugg@mlugg.co.uk>
Co-authored-by: Matthew Lugg <mlugg@mlugg.co.uk>
|
This needs release notes; I'll try to write some up today, since they'll also be useful for people tracking master to migrate. |
|
What is the rationale for not using |
| <p>Returns an integer type with the given signedness and bit width.</p> | ||
| <p>For instance, {#syntax#}@Int(.unsigned, 18){#endsyntax#} returns the type {#syntax#}u18{#endsyntax#}.</p> | ||
| {#header_close#} | ||
|
|
There was a problem hiding this comment.
Builtin Functions should be arranged in alphabetical order.
There was a problem hiding this comment.
That's not true. While this list is broadly alphabetical, we deviate from that where it makes sense for organizational purposes; for instance, @sin, @cos, and @tan are grouped together.
There was a problem hiding this comment.
For what it's worth, I appreciated that they were all in one place :)
There was a problem hiding this comment.
That's not true. While this list is broadly alphabetical, we deviate from that where it makes sense for organizational purposes; for instance,
@sin,@cos, and@tanare grouped together.
You are right. Sorry :)
The SoA style here (i.e. separate arrays for each "property" of fields) is far more convenient to use in essentially all cases. It particularly shines in several extremely common cases:
I can't be sure, but I would anticipate |
|
Release notes below.
|
Closes #10710
I updated
zig1.wasmin first commit to ensure the CI is passing. once ready, one of the core team members can handle updating it.I also didn't add
@Arraybecause as @Hejsil pointed out, it can be implemented in a single line of user-land code.Same goes for
@Float.std.meta.Float()is no longer a wrapper of@Type.