Use slicing syntax instead of methods and add a feature gate #17620

nrc · 2014-09-29T01:38:52Z

cc @aturon

r? anyone?

rust-highfive · 2014-09-29T01:38:56Z

Warning

These commits modify unsafe code. Please review it carefully!

sfackler · 2014-09-29T02:18:33Z

Does it really make sense to deprecate methods if the replacement is feature gated?

ftxqxd · 2014-09-29T02:52:13Z

src/doc/reference.md

@@ -3407,9 +3407,9 @@ may be specified with `..`. For example:
 # let x = 2i;


Three lines above, .. needs to be changed to ... as well (i.e., ‘A range of values may be specified with ....’)

Oh whoops, that change shouldn't even be part of this branch.

Gankra · 2014-09-29T03:00:19Z

Can you do collections::btree as well?

nrc · 2014-09-29T04:09:06Z

@sfackler yes, I think so. They are feature gated because the syntax might change, not because they might go away. Slicing syntax is definitely the preferred use.

aturon · 2014-09-29T16:21:24Z

I agree with @sfackler: I would prefer not to deprecate until slice syntax is "officially" supported, rather than encourage people to opt in to a feature gate. The reasons to have this behind a feature gate are (1) working through bugs and (2) the fact that at least the [] syntax may (in fact, is likely to) change or disappear before we finalize. (And the mut syntax may change as well, I know @nikomatsakis had some further thoughts on that.)

That said, it seems fine to land the internal use of this syntax for now.

I'm willing to review this PR.

aturon · 2014-09-29T19:13:57Z

src/doc/reference.md

@@ -3813,7 +3813,7 @@ type signature of `print`, and the cast expression in `main`.
 Within the body of an item that has type parameter declarations, the names of
 its type parameters are types:

-```
+```ignore


If you undo the deprecation, please remove the ignore here. (And in general, better to add a silent #[allow(deprecated)] but otherwise keep the test.)

For some reason I couldn't add attributes to this code snippet - I tried to allow slicing syntax but there was no way I could get it to work.

aturon · 2014-09-29T19:26:18Z

Hm, so I just noticed that the new Slice traits take indices by reference, whereas the RFC takes them by value. This is an issue for the Index trait as well. As far as I know, taking them by value is strictly more general, because you can always say:

impl<'a> Slice<&'a Idx, T> for U { ... }

to allow references with arbitrary lifetimes. But for Copy data (the common case, since these are usually uints), allowing it to go by value is more convenient.

aturon · 2014-09-29T19:27:37Z

We may want to resolve the by val/by ref issue before moving forward here.

aturon · 2014-09-29T19:30:10Z

We may want to resolve the by val/by ref issue before moving forward here.

Nevermind -- this only affects usage of the methods directly, not the syntax. We should still consider what's desired, but we can move forward with this PR.

aturon · 2014-09-29T19:35:29Z

src/libcore/slice.rs

    fn split_at(&self, mid: uint) -> (&'a [T], &'a [T]) {
-        (self.slice(0, mid), self.slice(mid, self.len()))
+        ((*self)[..mid], (*self)[mid..])


It's a shame the (*self) is needed here. Is that fundamental?

I think it is a choice - when looking up the trait method we don't autoderef the receiver (self has type &&[T] here) - that is just a flag we pass in to the trait matching algorithm though, so it should be easy to change. However, I was following what index does, I assume we should be consistent with that, one way or the other.

And I don't know why index is implemented that way. Perhaps @pcwalton does?

I agree that we should be consistent, but I'm not sure that the current design of things like Index is where we want it to be. I know that @nikomatsakis has been thinking about this, especially as it related to method dispatch issues elsewhere.

aturon · 2014-09-29T19:41:06Z

Ok, I've finished reviewing -- looks great overall, this is a nice readability win!

I'm curious to hear more about the (*self) issue in particular, but that's more a question of the slice syntax mechanism than this library work. Otherwise, r=me with the deprecations lifted.

Deprecates slicing methods from ImmutableSlice/MutableSlice in favour of slicing syntax or the methods in Slice/SliceMut. Closes rust-lang#17273.

[breaking-change] If you are using slicing syntax you will need to add #![feature(slicing_syntax)] to your crate.

@aturon

cc @aturon r? anyone?

@alexcrichton

This PR reverts #17620, which caused a significant regression for slices. As discussed with @alexcrichton, all of the public-facing changes of the earlier PR need to be rolled back, and it's not clear that we should move the libraries over to this new notation yet anyway (especially given its feature-gated status). Closes #17710

@alexcrichton

This PR reverts #17620, which caused a significant regression for slices. As discussed with @alexcrichton, all of the public-facing changes of the earlier PR need to be rolled back, and it's not clear that we should move the libraries over to this new notation yet anyway (especially given its feature-gated status). Closes #17710

Edition aware parser Fixes rust-lang/rust-analyzer#16324 by allowing us to properly thread through the edition to the parser

ftxqxd reviewed Sep 29, 2014
View reviewed changes

aturon reviewed Sep 29, 2014
View reviewed changes

nrc force-pushed the slice4 branch from 22fdbf2 to 717198d Compare September 30, 2014 02:49

nrc force-pushed the slice4 branch 2 times, most recently from 88260ff to b1b27db Compare October 1, 2014 02:18

nrc added 3 commits October 2, 2014 13:19

Use slice syntax instead of slice_to, etc.

40b9f5d

Remove the _ suffix from slice methods.

df2f1fa

Deprecates slicing methods from ImmutableSlice/MutableSlice in favour of slicing syntax or the methods in Slice/SliceMut. Closes rust-lang#17273.

Put slicing syntax behind a feature gate.

95cfc35

[breaking-change] If you are using slicing syntax you will need to add #![feature(slicing_syntax)] to your crate.

nrc force-pushed the slice4 branch from b1b27db to 83a1894 Compare October 2, 2014 00:40

Review and rebasing changes

6e0611a

nrc force-pushed the slice4 branch from 83a1894 to 6e0611a Compare October 2, 2014 01:50

bors added a commit that referenced this pull request Oct 2, 2014

auto merge of #17620 : nick29581/rust/slice4, r=aturon

07b2c1b

cc @aturon r? anyone?

bors closed this Oct 2, 2014

pnkfelix mentioned this pull request Oct 2, 2014

std::ops::Slice is not in the prelude #17710

Closed

This was referenced Oct 2, 2014

Add overloaded slice operations to the prelude #17711

Closed

Revert overloaded slice notation being merged with library slicing #17715

Merged

lnicola pushed a commit to lnicola/rust that referenced this pull request Jul 28, 2024

Auto merge of rust-lang#17620 - Veykril:edition-aware-parser, r=Veykril

fd240c0

Edition aware parser Fixes rust-lang/rust-analyzer#16324 by allowing us to properly thread through the edition to the parser

RalfJung pushed a commit to RalfJung/rust that referenced this pull request Aug 1, 2024

Auto merge of rust-lang#17620 - Veykril:edition-aware-parser, r=Veykril

4667848

Edition aware parser Fixes rust-lang/rust-analyzer#16324 by allowing us to properly thread through the edition to the parser

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use slicing syntax instead of methods and add a feature gate #17620

Use slicing syntax instead of methods and add a feature gate #17620

nrc commented Sep 29, 2014

rust-highfive commented Sep 29, 2014

sfackler commented Sep 29, 2014

ftxqxd Sep 29, 2014

nrc Sep 29, 2014

Gankra commented Sep 29, 2014

nrc commented Sep 29, 2014

aturon commented Sep 29, 2014

aturon Sep 29, 2014

nrc Sep 29, 2014

aturon commented Sep 29, 2014

aturon commented Sep 29, 2014

aturon commented Sep 29, 2014

aturon Sep 29, 2014

nrc Sep 29, 2014

nrc Sep 29, 2014

aturon Sep 29, 2014

aturon commented Sep 29, 2014

		@@ -3407,9 +3407,9 @@ may be specified with `..`. For example:
		# let x = 2i;

Use slicing syntax instead of methods and add a feature gate #17620

Use slicing syntax instead of methods and add a feature gate #17620

Conversation

nrc commented Sep 29, 2014

rust-highfive commented Sep 29, 2014

sfackler commented Sep 29, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Gankra commented Sep 29, 2014

nrc commented Sep 29, 2014

aturon commented Sep 29, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aturon commented Sep 29, 2014

aturon commented Sep 29, 2014

aturon commented Sep 29, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aturon commented Sep 29, 2014