Skip to content

Handle stability of struct fields - #22803

Merged
bors merged 3 commits into
rust-lang:masterfrom
huonw:field-stability
Feb 28, 2015
Merged

Handle stability of struct fields#22803
bors merged 3 commits into
rust-lang:masterfrom
huonw:field-stability

Conversation

@huonw

Copy link
Copy Markdown
Contributor

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.

@rust-highfive

Copy link
Copy Markdown
Contributor

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

@huonw

Copy link
Copy Markdown
ContributorAuthor

r? @brson, @aturon, @alexcrichton

NB. I've had to add 3 stability attributes.

@rust-highfiverust-highfive assigned brson and unassigned eddybFeb 25, 2015
@alexcrichton

Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wonder if this sort of lookup would be candidate for a good helper in middle::ty

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I think how we handle structs in the compiler will be changing with #22564, so I'll leave it for now.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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
huonwforce-pushed the field-stability branch 2 times, most recently from 8819038 to 1494196CompareFebruary 26, 2015 05:45
@huonw

Copy link
Copy Markdown
ContributorAuthor

@bors r=alexcrichton 1494

@bors

bors commented Feb 26, 2015

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 1494196 with merge 31c85cb...

@bors

bors commented Feb 26, 2015

Copy link
Copy Markdown
Collaborator

💔 Test failed - auto-linux-64-nopt-t

@alexcrichton

Copy link
Copy Markdown
Member

(cancelled build as we discovered some interesting tidbits on IRC)

@alexcrichton

Copy link
Copy Markdown
Member

@bors: retry

nevermind

@bors

bors commented Feb 26, 2015

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 1494196 with merge 50e209d...

@bors

bors commented Feb 26, 2015

Copy link
Copy Markdown
Collaborator

💔 Test failed - auto-mac-64-opt

@huonw

Copy link
Copy Markdown
ContributorAuthor

@bors r=alexcrichton 0606

@bors

bors commented Feb 27, 2015

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 060661d with merge 73d4eb5...

@bors

bors commented Feb 27, 2015

Copy link
Copy Markdown
Collaborator

💔 Test failed - auto-linux-64-x-android-t

@Manishearth

Copy link
Copy Markdown
Member

(Manual cancel due to rollup)

Manishearth added a commit to Manishearth/rust that referenced this pull request Feb 27, 2015
 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.
@alexcrichton

Copy link
Copy Markdown
Member

@bors: retry

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@huonw@rust-highfive@alexcrichton@bors@Manishearth@eddyb@brson