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

langref: add basic documentation of RLS #18043

Closed
wants to merge 1 commit into from

Conversation

mlugg
Copy link
Member

@mlugg mlugg commented Nov 19, 2023

No description provided.

@mlugg mlugg enabled auto-merge (rebase) November 19, 2023 17:08
Copy link
Contributor

@matu3ba matu3ba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice overview. mostly some nits on "simple" and "some", which should be explained.

This result type information is useful for the aforementioned cast builtins, as well as to avoid
the construction of pre-coercion values, and to avoid the need for explicit type coercions in some
cases. The following table details how some common expressions propagate result types, where
{#syntax#}x{#endsyntax#} and {#syntax#}y{#endsyntax#} are arbitrary sub-expressions.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence reads like it is accidentally incomplete. Is there a complete table anywhere, ie as issue in the language specification repo? Would the algorithm be more intuitive?

Copy link
Member Author

@mlugg mlugg Nov 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what seems incomplete here, but I might be missing something - could you elaborate? I intentionally only included a small set of examples, as detailing this for the whole Zig syntax would be fairly broad. If you can think of other important examples which should be in the table, let me know.

In terms of referencing the (currently non-existent) language specification, RLS is a little more complicated than the docs let on. The concepts of result types and result locations are somewhat interleaved; the address-of operation is a little subtle when slices are involved since Zig doesn't have the concept of an unknown-length array type; type-inferred pointers work differently; and destructuring syntax has direct ties to RLS. We could definitely expand on some of this in the langref, but I don't think it's a good idea to bombard the user with all the technical details immediately.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but I might be missing something - could you elaborate?

Usually the phrase "some" is elsewhere clarified in what is meant, see for example here. It would be nice to reference the language specification or an issue for the formal specification / necessary context like how it is done elsewhere in the language reference.

doc/langref.html.in Show resolved Hide resolved
doc/langref.html.in Show resolved Hide resolved
doc/langref.html.in Show resolved Hide resolved
doc/langref.html.in Show resolved Hide resolved
The following table details how some common expressions propagate result locations, where
{#syntax#}x{#endsyntax#} and {#syntax#}y{#endsyntax#} are arbitrary sub-expressions. Note that
some expressions cannot provide meaningful result locations to sub-expressions, even if they
themselves have a result location.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the word "some" like above. Can you be more specific?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as before - this is detailed in the table, and I am intentionally not covering all syntax forms, because there are a lot!

Copy link
Contributor

@rootbeer rootbeer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Feel free to ignore my comments if they're way off base, this is my first introduction to RLS in Zig.

doc/langref.html.in Show resolved Hide resolved
doc/langref.html.in Show resolved Hide resolved
doc/langref.html.in Show resolved Hide resolved
const s: S = .{ .x = @intCast(val) };
// .{ .x = @intCast(val) } has result type `S` due to the type annotation
// @intCast(val) has result type `u32` due to the type of the field `S.x`
// val has no result type, as it is permitted to be any integer type
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain how the u64 annotation on val factors in here? I'm reading the existing text as saying that the @intCast() only expects an "integer", so it doesn't really constrain the val. But doesn't the u64 play a part? (Also, why isn't the "must be an integer" represented in RLS? Having "no result type" implies its an anytype?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The u64 annotation on val isn't related to the line of code in question here, it's literally just... making val a u64. Is this somehow unclear?

Also, why isn't the "must be an integer" represented in RLS?

RLS, in general, can only give concrete types as result types. "Some kind of integer" is not a specific type in Zig. Even if RLS could represent this, it's not actually a particularly helpful concept - I can't think of anywhere this result type would have an effect.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, this is why I was confused about "result types" being different from regular "Zig types". I assumed @intCast() here would constrain its argument with a fuzzy result type (something like "must be compatible with an integer"), and that result type would have to be compatible with the actual type of val. I think the source of my confusion is that a "result type" is not a different kind of "type", but is a way (via "results") of generating concrete types.

I guess its also a bit surprising to me that the result type can generally be precisely computed. There aren't cases of overloading or ambiguity where the "result type" is a set of types? (Especially with Zig's proliferation of uN and iN types.)

Also, then the phrase in the comment "... permitted to be any integer type" isn't about the "result type"? Its a different constraint?

Can you perhaps expand this example to be more specific about how @intCast() does (or doesn't) define a result type for its parameter? (Functions normally can constrain their arguments, right?)

(Please feel free to ignore my comments if they're not resonating with you. I don't mean to waste your time.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't worry, you're not wasting my time! Outside perspectives are valuable here; since I'm very intricately familiar with this system, it can be tricky for me to notice when things are unclear.

"permitted to be any integer type" is not referring to the result type - it's trying to explain why @intCast can't provide a result type to its parameter. In essence, because you could not write @intCast in Zig without an anytype parameter, we can't provide a result type to the argument here.

There aren't cases of overloading or ambiguity where the "result type" is a set of types?

There definitely are, but RLS in its current form does not have a way to describe them. In general, they actually wouldn't be useful to represent - if we have a result type of "any integer", how could we actually use that? It doesn't help you with any specific coercions (aside from I suppose C pointer to usize but C pointers are silly) or casts.

The main place where it would be handy today is vectors - for instance, you can't currently write my_vector * @splat(3), because we can't provide a specific result type for the RHS, even though we know the vector length which is kind of sufficient information. The thing is, every such case has to be explicitly codified into the specification of RLS, so it's not trivial. Before introducing something like that, I'd like to wait for integer types to settle down - specifically, I'd like to wait for a decision on #3806, since that drastically changes when and how we can provide result types when integers are involved.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, this raises another aspect of RLS: what is the impact/utility of the type inference? I assumed it was for type checking (i.e., to give users a useful error message when they mistakenly pass a string to @intCast() or somesuch), but it sounds like it used to inform the code generation?

doc/langref.html.in Show resolved Hide resolved
doc/langref.html.in Show resolved Hide resolved
doc/langref.html.in Show resolved Hide resolved
Copy link
Contributor

@nektro nektro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

doc/langref.html.in Show resolved Hide resolved
doc/langref.html.in Show resolved Hide resolved
doc/langref.html.in Show resolved Hide resolved
doc/langref.html.in Show resolved Hide resolved
doc/langref.html.in Show resolved Hide resolved
Copy link
Sponsor Contributor

@ianprime0509 ianprime0509 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two changes should fix the langref build.

doc/langref.html.in Show resolved Hide resolved
doc/langref.html.in Show resolved Hide resolved
@andrewrk
Copy link
Member

I'm closing all the PRs that have had no changes for months and are failing CI checks. Feel free to reopen when you want to continue working on it.

@andrewrk andrewrk closed this Mar 14, 2024
auto-merge was automatically disabled March 14, 2024 11:03

Pull request was closed

@mlugg
Copy link
Member Author

mlugg commented Mar 14, 2024

Huh, apparently you can't re-open a PR after a force push, that's... interesting. Opened #19303.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants