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

Implement RFC 2052 (Epochs) #48014

Merged

Conversation

Manishearth
Copy link
Member

This adds -Zepochs and uses it for tyvar_behind_raw_pointer (#46906)

When we move this to --epoch=XXX, we'll need to gate the 2018 epoch on nightly, but not the 2015 one. I can make these changes here itself though it's kinda pointless given that the entire flag is nightly-only.

r? @nikomatsakis @aturon

cc #44581 (epoch tracking)
cc #46906 (tyvar_behind_raw_pointer)

@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 5, 2018
@@ -1297,6 +1325,13 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
`everybody_loops` (all function bodies replaced with `loop {}`),
`hir` (the HIR), `hir,identified`, or
`hir,typed` (HIR with types for each node)."),
epoch: Epoch = (Epoch::Epoch2015, parse_epoch, [TRACKED],
"Present the input source, unstable (and less-pretty) variants;
valid types are any of the types for `--pretty`, as well as:
Copy link
Member

Choose a reason for hiding this comment

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

This comment is wrong?

#[derive(Clone, Copy, Hash, PartialOrd, Ord, Eq, PartialEq)]
/// The epoch of the compiler (RFC 2052)
///
/// Pronounced e-poach
Copy link
Member

Choose a reason for hiding this comment

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

🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

😜

(i can remove it if you want)

Copy link
Member

Choose a reason for hiding this comment

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

😄

(it's fine)

@@ -112,6 +112,23 @@ pub enum OutputType {
DepInfo,
}

#[derive(Clone, Copy, Hash, PartialOrd, Ord, Eq, PartialEq)]
Copy link
Member

Choose a reason for hiding this comment

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

Nit-pick: We usually write the doc comments before the attributes, right? i.e.

/// The epoch of the compiler (RFC 2052)
#[derive(Stuff)]
pub enum Epoch {}


For example, you may have done something like:

```
Copy link
Member

Choose a reason for hiding this comment

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

This makes me wonder if it is possible to tell the doc comment which epoch it should use.

Copy link
Member Author

Choose a reason for hiding this comment

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

not really. there were plans for making the extended messages dynamic, but they didn't really take off.

Copy link
Member

Choose a reason for hiding this comment

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

@Manishearth I mean something like

```rust,epoch2018
let foo: Box<dyn Debug> = box 5;
```

```rust,epoch2015
let foo: Box<Debug> = Box::new(5);
```

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmmm.. Would be nice to have. Out of scope for this PR though 😄

Anyway, this specific warning only occurs on Rust 2018+, and the extended error stays the same.

@Manishearth
Copy link
Member Author

addressed comments

@Manishearth Manishearth force-pushed the stealing-chickens-on-the-internet branch from 8ef92f7 to 5f20d45 Compare February 5, 2018 14:54
Epoch2015 = 2015,
/// The 2018 epoch
Epoch2018 = 2018,

Copy link
Member

Choose a reason for hiding this comment

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

should this enum be non-exhasitve?

that's a little, uh, existential of a question I guess...

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, this makes sense.

#[derive(Clone, Copy, Hash, PartialOrd, Ord, Eq, PartialEq)]
pub enum Epoch {
/// The 2015 epoch
Epoch2015 = 2015,
Copy link
Member

Choose a reason for hiding this comment

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

it's kind of a shame that we end up with stutter here.... hm.

@Manishearth
Copy link
Member Author

cargo PR at rust-lang/cargo#5011

still need to do tests for both

epoch: Epoch = (Epoch::Epoch2015, parse_epoch, [TRACKED],
"The epoch to build Rust with. Newer epochs may include features
that require breaking changes. The default epoch is 2015 (the first
epoch). Crates compiled with different epoches can be linked together."),
Copy link
Contributor

Choose a reason for hiding this comment

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

epoches -> epochs

// - add a `rust_####()` function to the session

#[doc(hidden)]
__InexhaustiveMatch,
Copy link
Contributor

Choose a reason for hiding this comment

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

can't we use the #[non_exhaustive] attribute now?

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

Thanks @Manishearth! Left a few comments.

#[derive(Clone, Copy, Hash, PartialOrd, Ord, Eq, PartialEq)]
pub enum Epoch {
/// The 2015 epoch
Epoch2015 = 2015,
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rather we did not specify the discriminants. For one thing, we may still want epochs with names like 2015-pre.

@@ -112,6 +112,26 @@ pub enum OutputType {
DepInfo,
}

/// The epoch of the compiler (RFC 2052)
///
/// Pronounced e-poach
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rather we remove "E-poach" -- I like a funny comment as much as the next person, but I worry that people will actually believe us! (And this is part of rustc's public API)

More importantly, we should comment that the ordering of this enum is significant, and that epochs must go from "early to late" (since there is code that tests e.g., >= Epoch2018).

@@ -864,6 +864,11 @@ impl Session {
pub fn teach(&self, code: &DiagnosticId) -> bool {
self.opts.debugging_opts.teach && !self.parse_sess.span_diagnostic.code_emitted(code)
}

/// Are we allowed to use features from the Rust 2018 epoch?
pub fn rust_2018(&self) -> bool {
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 we should prepare for macro compatibility here, and make this some kind of test that takes a span. I'm not sure where the method should go -- maybe put it on the tcx for now? That seems like it would definitely have access to whatever data it needs to make the determination.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think the tcx makes sense; the idea behind epoches was that most of the changes would be to stuff like parsing and all, the "language frontend". We don't have a tcx when parsing. I think we should keep this unresolved for now and when someone knowledgeable about macro stuff comes along they can look into this.

I think the Session already has a lot of the Span info that can be used here; at least "originates from macro from X crate" can be done from the Session, though the interaction of many macros may be confusing.

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 think the tcx makes sense

In the future, we plan to move so that the tcx is the first thing that is created, so that everything can be query driven. This is why I suggested tcx. However, session is also fine.

I think we should keep this unresolved for now and when someone knowledgeable about macro stuff comes along they can look into this.

Mm, I'm not sure. I think it'll be useful to require a span so that, for any change, we are at least thinking about how to deal with it.

--

Eh. Having thought about it some more, while I'd rather lay out a "forwards compatible" API with what we will likely eventually need, I don't care enough to block on it. And there's always an argument for doing the simplest possible thing to start.

@kennytm kennytm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 5, 2018
For example, you may have done something like:

```
let bar = foo as *const _;
Copy link
Member

Choose a reason for hiding this comment

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

The examples needs to be runnable.

[01:24:14] failures:
[01:24:14] 
[01:24:14] ---- /checkout/obj/build/x86_64-unknown-linux-gnu/test/error-index.md - Rust_Compiler_Error_Index::E0908 (line 11384) stdout ----
[01:24:14] 	error[E0425]: cannot find value `foo` in this scope
[01:24:14]  --> /checkout/obj/build/x86_64-unknown-linux-gnu/test/error-index.md:11385:11
[01:24:14]   |
[01:24:14] 3 | let bar = foo as *const _;
[01:24:14]   |           ^^^ not found in this scope
[01:24:14] 
[01:24:14] thread 'rustc' panicked at 'couldn't compile the test', librustdoc/test.rs:298:13
[01:24:14] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[01:24:14] 
[01:24:14] ---- /checkout/obj/build/x86_64-unknown-linux-gnu/test/error-index.md - Rust_Compiler_Error_Index::E0908 (line 11395) stdout ----
[01:24:14] 	error[E0425]: cannot find value `foo` in this scope
[01:24:14]  --> /checkout/obj/build/x86_64-unknown-linux-gnu/test/error-index.md:11396:11
[01:24:14]   |
[01:24:14] 3 | let bar = foo as *const u8;
[01:24:14]   |           ^^^ not found in this scope
[01:24:14] 
[01:24:14] thread 'rustc' panicked at 'couldn't compile the test', librustdoc/test.rs:298:13
[01:24:14] 
[01:24:14] ---- /checkout/obj/build/x86_64-unknown-linux-gnu/test/error-index.md - Rust_Compiler_Error_Index::E0908 (line 11408) stdout ----
[01:24:14] 	error[E0412]: cannot find type `Foo` in this scope
[01:24:14]  --> /checkout/obj/build/x86_64-unknown-linux-gnu/test/error-index.md:11409:6
[01:24:14]   |
[01:24:14] 3 | impl Foo {
[01:24:14]   |      ^^^ not found in this scope
[01:24:14] 
[01:24:14] error[E0308]: mismatched types
[01:24:14]  --> /checkout/obj/build/x86_64-unknown-linux-gnu/test/error-index.md:11410:43
[01:24:14]   |
[01:24:14] 4 |       fn is_null(self: *const Self) -> bool {
[01:24:14]   |  ___________________________________________^
[01:24:14] 5 | |         // do something else
[01:24:14] 6 | |     }
[01:24:14]   | |_____^ expected bool, found ()
[01:24:14]   |
[01:24:14]   = note: expected type `bool`
[01:24:14]              found type `()`
[01:24:14] 
[01:24:14] thread 'rustc' panicked at 'couldn't compile the test', librustdoc/test.rs:298:13
[01:24:14] 
[01:24:14] 
[01:24:14] failures:
[01:24:14]     /checkout/obj/build/x86_64-unknown-linux-gnu/test/error-index.md - Rust_Compiler_Error_Index::E0908 (line 11384)
[01:24:14]     /checkout/obj/build/x86_64-unknown-linux-gnu/test/error-index.md - Rust_Compiler_Error_Index::E0908 (line 11395)
[01:24:14]     /checkout/obj/build/x86_64-unknown-linux-gnu/test/error-index.md - Rust_Compiler_Error_Index::E0908 (line 11408)
[01:24:14] 
[01:24:14] test result: FAILED. 666 passed; 3 failed; 21 ignored; 0 measured; 0 filtered out

@Manishearth Manishearth force-pushed the stealing-chickens-on-the-internet branch 3 times, most recently from 11eb29a to aff1216 Compare February 6, 2018 03:03
@Manishearth
Copy link
Member Author

Addressed

@Manishearth Manishearth force-pushed the stealing-chickens-on-the-internet branch from aff1216 to c69b599 Compare February 6, 2018 03:16
bors added a commit to rust-lang/cargo that referenced this pull request Feb 6, 2018
… r=alexcrichton

Implement RFC 2052: Epoches

Todo:

 - Make epoches affect the fingerprint
 - Tests

cc rust-lang/rust#44581

Rust PR: rust-lang/rust#48014

r? @acrichto
@nikomatsakis
Copy link
Contributor

@nikomatsakis
Copy link
Contributor

r=me when travis is happy

@Manishearth Manishearth force-pushed the stealing-chickens-on-the-internet branch 2 times, most recently from ddd80e3 to 19a32bd Compare February 6, 2018 19:46
@Manishearth Manishearth force-pushed the stealing-chickens-on-the-internet branch from 19a32bd to b8aa8ca Compare February 6, 2018 19:46
@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 6, 2018
@Manishearth
Copy link
Member Author

@bors r=nikomatsakis

@bors
Copy link
Contributor

bors commented Feb 6, 2018

📌 Commit b8aa8ca has been approved by nikomatsakis

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 6, 2018
Manishearth added a commit to Manishearth/rust that referenced this pull request Feb 7, 2018
…e-internet, r=nikomatsakis

Implement RFC 2052 (Epochs)

This adds -Zepochs and uses it for tyvar_behind_raw_pointer (rust-lang#46906)

When we move this to --epoch=XXX, we'll need to gate the 2018 epoch on nightly, but not the 2015 one. I can make these changes here itself though it's kinda pointless given that the entire flag is nightly-only.

r? @nikomatsakis @aturon

cc rust-lang#44581 (epoch tracking)
cc rust-lang#46906 (tyvar_behind_raw_pointer)
bors added a commit that referenced this pull request Feb 7, 2018
Rollup of 10 pull requests

- Successful merges: #47613, #47631, #47810, #47883, #47922, #47944, #48014, #48018, #48020, #48028
- Failed merges:
@bors bors merged commit b8aa8ca into rust-lang:master Feb 7, 2018
@aturon aturon mentioned this pull request Feb 22, 2018
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants