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

Slice generator #1927

Closed

Conversation

foxtran
Copy link

@foxtran foxtran commented Aug 5, 2022

Here, I propose slices which are presented in QuickSort. The proposed syntax is closed to Fortran, Python, and Go slices (Obviously the list of languages could go on).

@foxtran foxtran marked this pull request as ready for review August 5, 2022 15:46
@foxtran foxtran mentioned this pull request Aug 5, 2022
@foxtran foxtran requested a review from a team August 6, 2022 17:17
@foxtran foxtran added the proposal A proposal label Aug 6, 2022
@foxtran foxtran mentioned this pull request Aug 7, 2022
Copy link
Contributor

@zygoloid zygoloid left a comment

Choose a reason for hiding this comment

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

I like the idea of supporting both array slicing and forming integer ranges using the same form of expression.

I'd be interested in more discussion of the type of a slice generator expression and its representation. Given the support for unbounded slices, I assume slice generators are lazy, and don't form an array of indexes up-front?

Can they be used in for loops: for (n: i32 in 0:5), without creating an intermediary array? If so, it'd be useful for the proposal to mention that in some way.

Comment on lines 43 to 44
is the same as `::`. If `s` is negative, lower and upper bounds are replaces
each other.
Copy link
Contributor

@zygoloid zygoloid Aug 8, 2022

Choose a reason for hiding this comment

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

Suggested change
is the same as `::`. If `s` is negative, lower and upper bounds are replaces
each other.
is the same as `::`. If `s` is negative, lower and upper bounds replace each
other.

I'd like to check I'm following this properly: does this mean that the lower bound is b and the upper bound is a in that case, but that we always start with a and end with b, even when the step is negative? Or is the intent that (1:5:-1) starts at 5 and counts downwards?

proposals/p1927.md Outdated Show resolved Hide resolved

Slice as generator of array:
```carbon
var array: auto = (1:5:2); // array = (1,3,5)
Copy link
Contributor

Choose a reason for hiding this comment

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

In this case, what happens if you omit a? What is the first element of (::-1)?

Copy link
Contributor

Choose a reason for hiding this comment

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

One possibility might be to add a symbolic constant end, so that the elements of (::-1) are end-1, end-2, end-3, .... It'd also be useful to be able to use this separately from slices, so that array[end-1] is the last element, much like Python's array[-1] but without the discontinuity problems near 0 and the extra branches.

(This is inspired by the C++ ranges-v3 library, which allows using end in this way.)

Copy link
Author

Choose a reason for hiding this comment

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

I'm agree with creating of end keyword. Probably, it would be better to create a new proposal for this keyword.

Copy link
Author

Choose a reason for hiding this comment

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

In this case, what happens if you omit a? What is the first element of (::-1)?

In the case of creating array: Round brackets and s may be omitted, a and b must be presented. I have updated this part and it should be clearer now.

var array: auto = (1:5:2); // array = (1,3,5)
var arrav: auto = 1:5:2; // arrav = (1,3,5)
```
Round brackets may be omitted.
Copy link
Contributor

Choose a reason for hiding this comment

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

It's probably going to be very important from a parsing ambiguity perspective that enclosing brackets (whether they're (...) or [...]) are required if either the first or last expression is omitted. For example:

// No obvious parsing problems here.
var a: auto = (1:)-x;
// This is ambiguous: (1:)-x, or 1:(-x)?
// There might be an overloaded `-` that takes a slice and an `X`,
// and `-x` might be valid as the end of a slice.
var b: auto = 1:-x;

Copy link
Author

Choose a reason for hiding this comment

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

Now, the proposal says that lower and upper limits should be mentioned.

Comment on lines +39 to +40
Slice syntax is `a:b:s`. Here, `a` is the beginning of slice, `b` is the end of
slice and `s` is the step in slice. In array indexing, `:s` can be omitted, then
Copy link
Contributor

Choose a reason for hiding this comment

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

Is b included or excluded? The examples below suggest it's included -- eg, (1:5:2) includes 5 -- but for example in Python and Rust it's excluded, and Swift provides two operators for closed and half-open ranges. I'd like to see the proposal discuss this question; maybe closed ranges are the right answer, but given that we presumably will use 0-based indexing by default, half-open ranges seem likely to fit better in that context.

Copy link
Author

Choose a reason for hiding this comment

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

b is included. For having something like half-open range, I think that end keyword proposed by you is fine.

var array: auto = (1:5:2); // array = (1,3,5)
var arrav: auto = 1:5:2; // arrav = (1,3,5)
```
Round brackets may be omitted.
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the precedence of the : / :: operator? For example:

// Is this `(1:N) - 1`, or `1:(N - 1)`, or an error?
var x: auto = 1:N - 1;

Copy link
Author

Choose a reason for hiding this comment

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

See line 54 of current version. Anyway, it should be the lowest of them for avoiding brackets while ranging in array like array[1:N-1].

@jonmeow jonmeow added the proposal rfc Proposal with request-for-comment sent out label Aug 12, 2022
@jonmeow
Copy link
Contributor

jonmeow commented Aug 12, 2022

This hadn't explicitly been marked as an rfc, but given it's been tagged as ready for review, I'm adding the label. See #1898 for context.

@jonmeow jonmeow mentioned this pull request Aug 12, 2022
- Here, there is a problem with interacting with std::span/std::mdspan
comes from C++.

## Alternatives considered
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 it's important to consider the alternative of having this feature be provided by the library, with no special core-language syntax for it.


`:` has the lowest priority for making possible `a[1:N-1]` without any brackets.

Slice generator may return `i32`, `i64`, `i128` and unsigned versions of them.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can user-defined types be customized to work with this syntax? If so, how does that customization work? See also #1885.

- Here, there is a problem with interacting with std::span/std::mdspan
comes from C++.

## Alternatives considered
Copy link
Contributor

Choose a reason for hiding this comment

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

Please talk about the alternative of using half-open ranges rather than closed ranges here. This seems like a significant divergence from what I think at least C++ and Python programmers would expect, so if it's the right choice then we need rationale for that.

Comment on lines +44 to +45
is the same as `::`. If `s` is negative and `a` or `b` or both are skipped,
instead of `a`, upper bound is and, instead of `b`, lower bound is. Slice has
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 understand this sentence. I think what you mean is that if s is negative, then if a is omitted the slice starts from the end of the array, and if b is omitted then the slice ends at the start of the array. Is that right?

var array2: auto = (1:5); // array1 = (1,2,3,4,5)
```

Round brackets, `a`, and `b` must be presented, `s` may be omitted,
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, I think this syntax is still problematic. It doesn't look like we can disambiguate between this and a binding in a pattern:

fn F(a: i32, b: i32) {
  match ((1:2)) {
    case  (a:b) => { ... }
  }
}
// vs
fn G(b:! Type, n: b) {
  match (n) {
    case (a:b) => { ... }
  }
}

We could maybe use a named constructor, eg Slice(a, b, s) for the unusual case of constructing a slice anywhere other than in [], and specify x[a:b:s] as desugaring to x[Slice(a,b,s)].


## Background

There is no requirements for background.
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be useful to describe what other languages do in this space here.

In operator `[]` of array, slices can be used in the following ways:

```
var array: auto = (1:5:2); // array = (1,3,5)
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 likely to be an efficiency surprise. As with python's range, I think we'll want a slice generator to lazily produce the elements of the slice. Having a conversion from slice generator to array seems like it might be reasonable, but only if the developer somehow asks for it (eg, by writing an array type).

@github-actions
Copy link

We triage inactive PRs and issues in order to make it easier to find active work. If this PR should remain active, please comment or remove the inactive label.
This PR is labeled inactive because the last activity was over 90 days ago. This PR will be closed and archived after 14 additional days without activity.

@github-actions github-actions bot added the inactive Issues and PRs which have been inactive for at least 90 days. label Dec 14, 2022
@github-actions
Copy link

We triage inactive PRs and issues in order to make it easier to find active work. If this PR should remain active or becomes active again, please reopen it.
This PR was closed and archived because there has been no new activity in the 14 days since the inactive label was added.

@github-actions github-actions bot closed this Dec 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inactive Issues and PRs which have been inactive for at least 90 days. proposal rfc Proposal with request-for-comment sent out proposal A proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants