Skip to content

Remove prelude dependency for range notation; rename FullRange #21263

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

Closed
aturon opened this issue Jan 16, 2015 · 11 comments
Closed

Remove prelude dependency for range notation; rename FullRange #21263

aturon opened this issue Jan 16, 2015 · 11 comments
Assignees
Milestone

Comments

@aturon
Copy link
Member

aturon commented Jan 16, 2015

The range notation currently desugars directly to uses of e.g. RangeFrom and requires these structs to be present in the prelude.

In addition, the FullRange struct should probably be called RangeFull for conventions/consistency.

@aturon
Copy link
Member Author

aturon commented Jan 16, 2015

cc @nick29581

Nominating for 1.0 beta, P-backcompat-lang.

@nrc
Copy link
Member

nrc commented Jan 17, 2015

As far as I see it, there are two options: we either put RangeFull in the prelude and desugar to RangeFull or we don't and we desugar to ::core::ops::RangeFull, both are equally easy to implement, the former is the current implementation. Note that we can't make RangeFull a lang item because we desugar in the parser, not later in the compiler (there is an issue for making lang items available in libsyntax, but I can't find it right now and it's a fair bit of work, so unlikely to be done soon).

For most users the two approaches are identical. Not having anything in the prelude is an advantage of the second approach.

The difference is for users who aren't using the prelude. Off the bat, the former approach does not work, and the latter one does (assuming they are using libcore). If they are not using libcore, then under the second approach, they are screwed (they could I guess create their own lib called core with ops in it and a RangeFull struct in that, but that kind of sucks).

Without a prelude, users would have to provide a RangeFull struct in scope, presumably with an import in each file. This is sub-optimal, but it does work even if they are not using libcore.

Finally, there is the overriding case. The first approach allows users to override RangeFull with their own struct, whereas the second approach does not. (Because only prelude items can be shadowed). However, since we don't use the struct for anything other than a place holder, I don't really see any benefit of overriding it.

On balance, it seems that the second approach is slightly better, the only people it really sucks for are those running without libcore. Do we expect that to be a sizeable group or not?

@nrc
Copy link
Member

nrc commented Jan 17, 2015

And another thought: approach 2 is easier to future proof - I'm not exactly clear on how the stability stuff would work here, but could we mark RangeFull as unstable? We would allow stable use only via the range syntax, and one day when we have libsyntax lang items, we could make it a lang item, rather than hard coded. I guess this would not work for people implementing slicing who have to use RangeFull in their impls.

Perhaps, even if we mark it stable, we could still change to using a lang item in the future without breaking anyone's code, so it is backwards compatible? Whereas approach 1 would be backwards incompatible because users could rely on it being in the prelude.

@lambda-fairy
Copy link
Contributor

I think using ::core::ops::RangeFull would be more consistent, given that the std macros (println! and try! and friends) all use absolute paths too.

@aturon
Copy link
Member Author

aturon commented Jan 17, 2015

@nick29581 I pretty much agree with your thinking here -- it should be a lang item in the long run, but welding it to a path for now should allow us to future-proof that change. (And no-core scenarios are not considered stable anyway).

(I think running without core is very unusual, because core imposes so few constraints.)

We do need to stabilize the various structs so that people can provide custom impls of traits for them, and so on. But I agree that making it a lang item should be a fine change, especially since no-core is not considered stable.

cc @alexcrichton

@alexcrichton
Copy link
Member

I agree that an absolute path is the best way forward for now.

@pnkfelix
Copy link
Member

1.0 beta, P-backcompat-libs

@nrc
Copy link
Member

nrc commented Jan 28, 2015

So, I implemented this (expand to ::core::ops::RangeFull. One downside I had not anticipated is that now every crate has to extern crate core so that core is in scope. Are we OK with that? Alternatively, we could re-export RangeFull from std, but then expanding to ::std::ops::RangeFull would fail for anyone building no_std (including a bunch of our libs).

Any other ideas?

@alexcrichton
Copy link
Member

We should stick to expanding to ::std::ops::RangeFull and not inject extern crate core, we can work on the no_std case later (this is only one pain point in a long line). I believe @kmc's $crate for macros should work well here (to be adapted at a later date)

@nrc
Copy link
Member

nrc commented Jan 28, 2015

@alexcrichton this would break a lot of code in libcore and libcollections - nothing there will be able to use full range slicing syntax (including the proposed standalone .. and the &foo[] syntax for taking a slice).

@alexcrichton
Copy link
Member

That's typically worked around with the "curious inner module" hack via:

#![no_std]
extern crate core;

mod std {
    pub use core::ops;
}

@nrc nrc closed this as completed in a9d465f Jan 30, 2015
zsiciarz added a commit to zsiciarz/rust-itertools that referenced this issue Jan 31, 2015
As per rust-lang/rust#21263, `FullRange` got renamed to `RangeFull`.
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

No branches or pull requests

5 participants