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

Tracking issue for RFC #495 (features `slice_patterns` and `advanced_slice_patterns`) #23121

Open
nikomatsakis opened this Issue Mar 6, 2015 · 73 comments

Comments

Projects
None yet
@nikomatsakis
Contributor

nikomatsakis commented Mar 6, 2015

Tracking issue for rust-lang/rfcs#495

Breaking Changes

This RFC is a breaking change for most users of slice patterns. The main change is that slice patterns now have the type [_] instead of &[_].

For example, in the old semantics

fn slice_pat(x: &[u8]) {
    // OLD!
    match x {
        [a, b..] => {}
    }
}

the [a, b..] would have the type &[u8], a would have the type u8 and b the type &[u8].

With the new semantics, [a, b..] would have the type [u8] and b the type [u8] - which are the wrong types. To fix that, add a & before the slice and a ref before the tail as if you were matching a struct (of course, use ref mut if you want a mutable reference):

fn slice_pat(x: &[u8]) {
    // NEW
    match x {
        &[a, ref b..] => {}
    }
}

Concerns to be resolved before stabilization

  • The syntax conflicts with exclusive range patterns.
  • #8636 Matches on &mut[] move the .. match & don't consider disjointness
  • #26736 cannot move into irrefutable slice patterns with multiple elements
  • #34708 double drop with slice patterns
  • #26619 (Only E-needstest) Yet another bug with slice_patterns
  • #23311 (Only E-needstest) LLVM Assertion: Both operands to ICmp instruction are not of the same type!

History and status

  • Implemented in #32202
  • Still needs better integration with MIR borrowck to resolve soundness concerns, I believe
@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Mar 6, 2015

triage: P-backcompat-lang (1.0 beta) -- at least for the backwards incompatible parts of rust-lang/rfcs#495. This is something of a nice to have, we can live with it if it doesn't get done.

@ghost

This comment has been minimized.

ghost commented Mar 16, 2015

I'm on it, by the way.

https://github.com/jakub-/rust/tree/array-pattern-changes. I'll open a PR soon.

bors added a commit that referenced this issue Mar 22, 2015

Auto merge of #23361 - petrochenkov:refdst, r=jakub-
After this patch code like `let ref a = *"abcdef"` doesn't cause ICE anymore.
Required for #23121

There are still places in rustc_trans where pointers are always assumed to be thin. In particular, #19064 is not resolved by this patch.
@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Mar 24, 2015

@jakub- go go go!

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Mar 24, 2015

🐎

@pnkfelix

This comment has been minimized.

Member

pnkfelix commented Mar 26, 2015

(this is puntable to 1.0 if necessary, but we hope to have it by 1.0 beta.)

@pnkfelix

This comment has been minimized.

Member

pnkfelix commented Mar 26, 2015

We are going to feature-gate array patterns, in order to ensure that we can soundly land these changes to the semantics in the future.

brson added a commit to brson/rust that referenced this issue Mar 27, 2015

Feature gate *all* slice patterns. rust-lang#23121
Until some backwards-compatibility hazards are fixed in rust-lang#23121,
these need to be unstable.

[breaking-change]

@fhartwig fhartwig referenced this issue Mar 29, 2015

Merged

fix: rustup #404

@aturon

This comment has been minimized.

Member

aturon commented Apr 1, 2015

Moving off of milestone, since these are now gated.

@aturon

This comment has been minimized.

Member

aturon commented Apr 1, 2015

triage: I-nominated ()

@aturon aturon removed this from the 1.0 beta milestone Apr 1, 2015

@pnkfelix

This comment has been minimized.

Member

pnkfelix commented Apr 2, 2015

P-high, not 1.0.

@pnkfelix pnkfelix added P-medium and removed I-nominated labels Apr 2, 2015

@droundy droundy referenced this issue Sep 4, 2015

Merged

Minor fixes #4

@aturon aturon changed the title from Array pattern adjustments (Tracking issue for RFC #495) to Tracking issue for RFC #495 (features `slice_patterns` and `advanced_slice_patterns`) Nov 5, 2015

@aturon

This comment has been minimized.

Member

aturon commented Nov 5, 2015

I've repurposed this as a general tracking issue for the slice_patterns and advanced_slice_patterns features.

@petrochenkov

This comment has been minimized.

Contributor

petrochenkov commented Nov 5, 2015

Syntax for named subslice a.. conflicts with potential future exclusive slice patterns. Either it should be changed, or exclusive slice patterns should be forever rejected, or something else.

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Nov 5, 2015

So, once MIR is around, I'll feel much better about this feature from the POV of soundness. It's certainly convenient once in a while. But I have realized a few quirks recently:

  1. I think there is still some debate about whether [] patterns should match &[T] values. I think no, you should write the &.
  2. I don't think that there is a syntax for taking a mut subslice.
  3. The syntax conflicts with exclusive slice patterns (as @petrochenkov pointed out).
  4. It's a bit unclear to me if the slice part is intended as a fresh subpattern. Seems like it should be.
@petrochenkov

This comment has been minimized.

Contributor

petrochenkov commented Nov 5, 2015

It would be also nice to have unnamed .. or even named sub.. sublists in tuple and especially tuple variant patterns, but whatever syntax choice is made for slices it probably can be reused there.

I don't think that there is a syntax for taking a mut subslice.

[a, ref mut b..] seems to compile, its just not have been updated to by-value semantics from rust-lang/rfcs#495. Subslice is an arbitrary pattern after all.

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Nov 9, 2015

On Thu, Nov 05, 2015 at 12:02:03PM -0800, Vadim Petrochenkov wrote:

I don't think that there is a syntax for taking a mut subslice.

[a, ref mut b..] seems to compile, its just not have been updated to by-value semantics from rust-lang/rfcs#495. Subslice is an arbitrary pattern after all.

I would expect that to produce a &mut &[T] value. Is that not what it does? (In particular, the ref isn't normally required to get a value of type &[T], is it?)

@joshtriplett

This comment has been minimized.

Member

joshtriplett commented Jan 13, 2018

Based on @petrochenkov's comment back in July, it seems like we could consider a subset of this for stabilization. @rust-lang/lang, thoughts?

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Jan 17, 2018

Hmm, I'd like to see the subset clearly defined (and see the relevant tests). I am somewhat nervous about stabilizing things here until we have the drop elaboration story fully worked out.

@joshtriplett

This comment has been minimized.

Member

joshtriplett commented Jan 18, 2018

Summary from the @rust-lang/lang meeting: We need a fix for #34708 first. As soon as we have that fix, let's revisit this and decide if we should stabilize a subset or wait to stabilize the whole thing.

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Jan 18, 2018

My take is that I would rather that we stabilize the whole thing at once. I'm a bit nervous to stabilize syntax involving .. if we haven't yet figured out how to give a name to the slice or otherwise define the recursive pattern.

@petrochenkov

This comment has been minimized.

Contributor

petrochenkov commented Jan 18, 2018

There's no good alternative to the plain .. (without a name), all other patterns (structs, tuple structs, tuples) use it to mean "rest of the list".

There are few viable alternatives for syntax ".. + name" in this thread, but I wouldn't want to have stabilization of the most useful subset blocked on bikeshedding them.

@sgrif

This comment has been minimized.

Contributor

sgrif commented Jan 18, 2018

I'd bet $5 that if we made it an error to use .. on a &[RangeFull] or ..name on a &[RangeTo<T>] that nobody would ever notice.

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Jan 19, 2018

@petrochenkov

I wouldn't want to have stabilization of the most useful subset blocked on bikeshedding them.

I am not sure that [a, .., b] is the most useful subset. I would have thought that the most useful subset is [a, b..] or [a.., b] -- that is, extract something from the front or back and slice the rest. Certainly that's the main thing that I ever want. (Though we have handy methods for it.)

@jethrogb

This comment has been minimized.

Contributor

jethrogb commented Jan 19, 2018

I just want matching against constant fixed-length arrays. The .. stuff you can already do with split_at.

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Jan 19, 2018

I agree with @jethrogb in that it would be amazing if we could move towards stabilising the simple, unambiguous cases for fixed-size arrays first.

I could get behind this plan (i.e., no .. at all).

@SoniEx2

This comment has been minimized.

SoniEx2 commented Feb 17, 2018

Why not x @ ..?

If you have something like [ref mut a, x @ .., ref mut b], what's wrong with having a mutable ref to a, a mutable ref to b, and a mutable slice x that doesn't include a or b? The aliasing still holds up, no?

Edit: Oh I see, I should've read the full thing.

Why not remove the .. entirely?

[a, @, b] if you don't want the thing, [a, x@, b] to capture the thing?

(Not sure if this has been suggested)

@petrochenkov

This comment has been minimized.

Contributor

petrochenkov commented Feb 23, 2018

#34708 was fixed by @mikhail-m1.
Any other codegen bugs blocking stabilization?

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Feb 23, 2018

@petrochenkov of what specific subset?

If you want to stabilize, can you please open up a new issue that describes the behavior we are stabilizing, giving examples of tests that show how it works? It would be great to also specify what is not being stabilized. Example: #48453

I don't know if there are other bugs, I know that @mikhail-m1 has been working on improving various double-drop bugs and analysis, but I think more for slice patterns.

@mikhail-m1

This comment has been minimized.

Contributor

mikhail-m1 commented Feb 24, 2018

my view of current state

item state
The syntax conflicts with exclusive range patterns. completed
#8636 Matches on &mut[] move the .. match & don't consider disjointness I will look, need to check is nested cases support needed
#26736 cannot move into irrefutable slice patterns with multiple elements bug only in AST borrowck
#34708 double drop with slice patterns fixed
#26619 (Only E-needstest) Yet another bug with slice_patterns doesn't relate
#23311 (Only E-needstest) LLVM Assertion: Both operands to ICmp instruction are not of the same type! marked done
@petrochenkov

This comment has been minimized.

Contributor

petrochenkov commented Feb 24, 2018

Excellent, looks like there are no codegen issues and no issues that may require breaking backward compatibility in the future.

In this case I'll prepare a stabilization PR for slice patterns without .., and a mini-RFC attempting to finalize syntax for slice patterns with ...

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Mar 8, 2018

I just proposed stabilizing the subset without .. patterns in this issue:

#48836

@petrochenkov

This comment has been minimized.

bors added a commit that referenced this issue Mar 20, 2018

Auto merge of #48516 - petrochenkov:stabsl, r=nikomatsakis
Stabilize slice patterns without `..`

And merge `feature(advanced_slice_patterns)` into `feature(slice_patterns)`.

The detailed description can be found in #48836.

Slice patterns were unstable for long time since before 1.0 due to many bugs in the implementation, now this stabilization is possible primarily due to work of @arielb1 who [wrote the new MIR-based implementation of slice patterns](#32202) and @mikhail-m1 who [fixed one remaining class of codegen issues](#47926).

Reference PR rust-lang-nursery/reference#259
cc #23121
fixes #48836
@abonander

This comment has been minimized.

Contributor

abonander commented Apr 5, 2018

@petrochenkov Given the mutual impls of PartialEq for String and &str, I expected this to work: https://play.rust-lang.org/?gist=8157ad5c25d456007b27e3b62b8ce866&version=beta

It seems too strict to require the pattern to have the same element type as the matched slice.

@petrochenkov

This comment has been minimized.

Contributor

petrochenkov commented Apr 5, 2018

@abonander
This looks more like some extension to #42640 using Deref rather than something specific to slice patterns.

kodabb pushed a commit to hedgewars/hw that referenced this issue Jun 27, 2018

nemo
neglected dep update from r87a6cad20c90:
that rev also forces nightly rust due to rust-lang/rust#23121

dlrobertson pushed a commit to dlrobertson/rust that referenced this issue Nov 29, 2018

Feature gate *all* slice patterns. rust-lang#23121
Until some backwards-compatibility hazards are fixed in rust-lang#23121,
these need to be unstable.

[breaking-change]

alexreg added a commit to alexreg/rust that referenced this issue Dec 7, 2018

alexreg added a commit to alexreg/rust that referenced this issue Dec 7, 2018

alexreg added a commit to alexreg/rust that referenced this issue Dec 7, 2018

alexreg added a commit to alexreg/rust that referenced this issue Dec 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment