Handle stability of struct fields #22803
Conversation
|
r? @eddyb (rust_highfive has picked a reviewer for you, use r? to override) |
|
r? @brson, @aturon, @alexcrichton NB. I've had to add 3 stability attributes. |
|
This looks good to me, nice work! Could this also be expanded to cover fields mentioned in patterns as well for structs? Other than that I think it covers everything. |
| .unwrap_or_else(|| { | ||
| tcx.sess.span_bug(field.span, | ||
| "stability::check_expr: unknown named field access") | ||
| }) |
There was a problem hiding this comment.
I wonder if this sort of lookup would be candidate for a good helper in middle::ty
There was a problem hiding this comment.
I think how we handle structs in the compiler will be changing with #22564, so I'll leave it for now.
There was a problem hiding this comment.
Ah ok, this can wait then. r=me with patterns handled as well
We were recording stability attributes applied to fields in the compiler, and even annotating it in the libs, but the compiler didn't actually do the checks to give errors/warnings in user crates.
The stability check checks the `PublicItems` map when giving errors if
there is a #[stable] item with a public contents that doesn't not have
its own stability. Without recording this, struct fields and enum
variants will not get errors for e.g. stable modules with unmarked
functions internally.
This is just improving the compiler's precision to give the standard
library developers more information earlier.
E.g.
#![staged_api]
#![feature(staged_api)]
#![crate_type = "lib"]
#[stable(feature = "rust1", since = "1.0.0")]
pub struct Foo {
pub x: i32
}
#[stable(feature = "rust1", since = "1.0.0")]
pub mod bar {
pub fn baz() {}
}
Without the patch it gives:
test.rs:12:5: 12:20 error: This node does not have a stability attribute
test.rs:12 pub fn baz() {}
^~~~~~~~~~~~~~~
error: aborting due to previous error
With the patch it gives:
test.rs:7:9: 7:15 error: This node does not have a stability attribute
test.rs:7 pub x: i32
^~~~~~
test.rs:12:5: 12:20 error: This node does not have a stability attribute
test.rs:12 pub fn baz() {}
^~~~~~~~~~~~~~~
error: aborting due to 2 previous errors
8819038
to
1494196
Compare
|
@bors r=alexcrichton 1494 |
|
⌛ Testing commit 1494196 with merge 31c85cb... |
|
💔 Test failed - auto-linux-64-nopt-t |
|
(cancelled build as we discovered some interesting tidbits on IRC) |
|
@bors: retry nevermind |
|
⌛ Testing commit 1494196 with merge 50e209d... |
|
💔 Test failed - auto-mac-64-opt |
1494196
to
060661d
Compare
|
@bors r=alexcrichton 0606 |
|
💔 Test failed - auto-linux-64-x-android-t |
|
(Manual cancel due to rollup) |
We were recording stability attributes applied to fields in the compiler, and even annotating it in the libs, but the compiler didn't actually do the checks to give errors/warnings in user crates. Details in the commit messages.
|
@bors: retry |
We were recording stability attributes applied to fields in the
compiler, and even annotating it in the libs, but the compiler didn't
actually do the checks to give errors/warnings in user crates.
Details in the commit messages.