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. |
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
huonw
force-pushed
the
field-stability
branch
2 times, most recently
from
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 |
huonw
force-pushed
the
field-stability
branch
from
1494196 to
060661d
Compare
|
@bors r=alexcrichton 0606 |
|
⌛ Testing commit 060661d with merge 73d4eb5... |
|
💔 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 |
7 participants
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.