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

Add section documenting tuple unpacking #702

Merged
merged 2 commits into from
Jan 16, 2024

Conversation

WardBrian
Copy link
Member

Submission Checklist

  • Builds locally
  • New functions marked with `r since("VERSION")`
  • Declare copyright holder and open-source license: see below

Summary

Docs for stan-dev/stanc3#1360. Unfortunately I dropped the ball on documenting this earlier, so we will need to re-build the docs for the current release after this (and hopefully #678)

Copyright and Licensing

Please list the copyright holder for the work you are submitting (this will be you or your assignee, such as a university or company):
Simons Foundation

By submitting this pull request, the copyright holder is agreeing to license the submitted work under the following licenses:

Copy link
Contributor

@bob-carpenter bob-carpenter left a comment

Choose a reason for hiding this comment

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

Typo and some questions that should be resolved.

src/reference-manual/types.Rmd Outdated Show resolved Hide resolved
variables of types `v1`, ..., `vn`, where each `vi` has a type which is
[assignable](#assignment-typing) from type `Ti`, individual elements of
the tuple may be assigned to the corresponding variables in the sequence
by the statement
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it help to say these aren't general lvalues but must be indexes? That is, we can't have indexed expressions on the LHS can we?

Would it also help to say that the parens are required, unlike in Python?

Copy link
Member Author

Choose a reason for hiding this comment

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

You can have an indexed expression on the left hand side, but it is required to be unique. This sort of also answers your follow-up question, which is we implemented an overly conservative approach that means you cannot assign to different sub-parts of the same variable at the same time, since it would be impossible to statically check in general.

For example, the following is forbidden by stanc:

data {
  tuple(array[3] int, array[4] int) x;
}
model {
  array[7] real x_flat;
    // ILLEGAL
  (x_flat[ : 3], x_flat[3 : ]) = x;
}

(v1, /*...*/, vn) = T;
```

For example, if `T` is a tuple of type `tuple(int, real, complex)`,
Copy link
Contributor

Choose a reason for hiding this comment

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

[optional]
I would use lower case t or x for the tuple here to avoid confusion with T being read as a generic type.

Assigns the result of `T.1` to `i`, the result of `T.2` to `r`, and
the result of `T.3` to `z`.

These unpacking assignments can be nested if the tuple on the right hand
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the nesting needs an example. I'm imagining (i, (r, z)) = x or something like that?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. I've updated the existing example to do both at once following some re-ordering of paragraphs here.

These unpacking assignments can be nested if the tuple on the right hand
side contains nested tuples.

The left hand side of an unpacking assignment must contain only
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems to say it's general, but the earlier description says it's just variables. Which is it? And if it's indexed, how do we avoid aliasing or check when it's now a runtime issue?

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated the earlier description. See above for how we avoid aliasing, but the short version is the statement that the same variable may not appear more than once is correct. This is a bit overly-conservative, but does prevent the aliasing problem for indexed assignments appearing here.

Copy link
Contributor

@bob-carpenter bob-carpenter left a comment

Choose a reason for hiding this comment

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

Thanks for the updates.

@bob-carpenter bob-carpenter merged commit 090ea49 into master Jan 16, 2024
@WardBrian WardBrian deleted the feature/tuple-unpacking branch January 16, 2024 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants