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

Fix exposing fields marked unstable or doc hidden #90358

Merged
merged 9 commits into from
Mar 13, 2022
6 changes: 3 additions & 3 deletions compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -692,11 +692,11 @@ impl<'tcx> Constructor<'tcx> {
}

/// Checks if the `Constructor` is a `Constructor::Variant` with a `#[doc(hidden)]`
/// attribute.
/// attribute from a type not local to the current crate.
pub(super) fn is_doc_hidden_variant(&self, pcx: PatCtxt<'_, '_, 'tcx>) -> bool {
if let Constructor::Variant(idx) = self && let ty::Adt(adt, _) = pcx.ty.kind() {
let variant_def_id = adt.variant(*idx).def_id;
return pcx.cx.tcx.is_doc_hidden(variant_def_id);
let variant_def_id = adt.variants()[*idx].def_id;
return pcx.cx.tcx.is_doc_hidden(variant_def_id) && !variant_def_id.is_local();
}
false
}
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_typeck/src/check/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use rustc_hir::pat_util::EnumerateAndAdjustIterator;
use rustc_hir::{HirId, Pat, PatKind};
use rustc_infer::infer;
use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
use rustc_middle::middle::stability::EvalResult;
use rustc_middle::ty::{self, Adt, BindingMode, Ty, TypeFoldable};
use rustc_session::lint::builtin::NON_EXHAUSTIVE_OMITTED_PATTERNS;
use rustc_span::hygiene::DesugaringKind;
Expand Down Expand Up @@ -1308,6 +1309,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.copied()
.filter(|(field, _)| {
field.vis.is_accessible_from(tcx.parent_module(pat.hir_id).to_def_id(), tcx)
&& !matches!(
tcx.eval_stability(field.did, None, DUMMY_SP, None),
EvalResult::Deny { .. }
)
// We only want to report the error if it is hidden and not local
&& !(tcx.is_doc_hidden(field.did) && !field.did.is_local())
})
.collect();

Expand Down
10 changes: 9 additions & 1 deletion src/test/ui/pattern/usefulness/auxiliary/hidden.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
pub enum Foo {
pub enum HiddenEnum {
A,
B,
#[doc(hidden)]
C,
}

#[derive(Default)]
pub struct HiddenStruct {
pub one: u8,
pub two: bool,
#[doc(hidden)]
pub hide: usize,
}
13 changes: 12 additions & 1 deletion src/test/ui/pattern/usefulness/auxiliary/unstable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,22 @@
#![stable(feature = "stable_test_feature", since = "1.0.0")]

#[stable(feature = "stable_test_feature", since = "1.0.0")]
pub enum Foo {
pub enum UnstableEnum {
#[stable(feature = "stable_test_feature", since = "1.0.0")]
Stable,
#[stable(feature = "stable_test_feature", since = "1.0.0")]
Stable2,
#[unstable(feature = "unstable_test_feature", issue = "none")]
Unstable,
}

#[derive(Default)]
#[stable(feature = "stable_test_feature", since = "1.0.0")]
pub struct UnstableStruct {
#[stable(feature = "stable_test_feature", since = "1.0.0")]
pub stable: bool,
#[stable(feature = "stable_test_feature", since = "1.0.0")]
pub stable2: usize,
#[unstable(feature = "unstable_test_feature", issue = "none")]
pub unstable: u8,
}
26 changes: 26 additions & 0 deletions src/test/ui/pattern/usefulness/doc-hidden-fields.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// aux-build:hidden.rs

extern crate hidden;

use hidden::HiddenStruct;

struct InCrate {
a: usize,
b: bool,
#[doc(hidden)]
im_hidden: u8
}

fn main() {
let HiddenStruct { one, two } = HiddenStruct::default();
//~^ pattern requires `..` due to inaccessible fields

let HiddenStruct { one } = HiddenStruct::default();
//~^ pattern does not mention field `two` and inaccessible fields

let HiddenStruct { one, hide } = HiddenStruct::default();
//~^ pattern does not mention field `two`

let InCrate { a, b } = InCrate { a: 0, b: false, im_hidden: 0 };
//~^ pattern does not mention field `im_hidden`
}
59 changes: 59 additions & 0 deletions src/test/ui/pattern/usefulness/doc-hidden-fields.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
error: pattern requires `..` due to inaccessible fields
--> $DIR/doc-hidden-fields.rs:15:9
|
LL | let HiddenStruct { one, two } = HiddenStruct::default();
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: ignore the inaccessible and unused fields
|
LL | let HiddenStruct { one, two, .. } = HiddenStruct::default();
| ++++

error[E0027]: pattern does not mention field `two` and inaccessible fields
--> $DIR/doc-hidden-fields.rs:18:9
|
LL | let HiddenStruct { one } = HiddenStruct::default();
| ^^^^^^^^^^^^^^^^^^^^ missing field `two` and inaccessible fields
|
help: include the missing field in the pattern and ignore the inaccessible fields
|
LL | let HiddenStruct { one, two, .. } = HiddenStruct::default();
| ~~~~~~~~~~~
help: if you don't care about this missing field, you can explicitly ignore it
|
LL | let HiddenStruct { one, .. } = HiddenStruct::default();
| ~~~~~~

error[E0027]: pattern does not mention field `two`
--> $DIR/doc-hidden-fields.rs:21:9
|
LL | let HiddenStruct { one, hide } = HiddenStruct::default();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ missing field `two`
|
help: include the missing field in the pattern
|
LL | let HiddenStruct { one, hide, two } = HiddenStruct::default();
Copy link
Member

Choose a reason for hiding this comment

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

I think this can be left as followup, but ideally we should also suggest hiding in one go.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even if the doc(hidden) field is being explicitly matched on?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm. Good point. Not sure.

| ~~~~~~~
help: if you don't care about this missing field, you can explicitly ignore it
|
LL | let HiddenStruct { one, hide, .. } = HiddenStruct::default();
| ~~~~~~

error[E0027]: pattern does not mention field `im_hidden`
--> $DIR/doc-hidden-fields.rs:24:9
|
LL | let InCrate { a, b } = InCrate { a: 0, b: false, im_hidden: 0 };
| ^^^^^^^^^^^^^^^^ missing field `im_hidden`
|
help: include the missing field in the pattern
|
LL | let InCrate { a, b, im_hidden } = InCrate { a: 0, b: false, im_hidden: 0 };
| ~~~~~~~~~~~~~
help: if you don't care about this missing field, you can explicitly ignore it
|
LL | let InCrate { a, b, .. } = InCrate { a: 0, b: false, im_hidden: 0 };
| ~~~~~~

error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0027`.
33 changes: 23 additions & 10 deletions src/test/ui/pattern/usefulness/doc-hidden-non-exhaustive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,42 @@

extern crate hidden;

use hidden::Foo;
use hidden::HiddenEnum;

enum InCrate {
A,
B,
#[doc(hidden)]
C,
}

fn main() {
match Foo::A {
Foo::A => {}
Foo::B => {}
match HiddenEnum::A {
HiddenEnum::A => {}
HiddenEnum::B => {}
}
//~^^^^ non-exhaustive patterns: `_` not covered

match Foo::A {
Foo::A => {}
Foo::C => {}
match HiddenEnum::A {
HiddenEnum::A => {}
HiddenEnum::C => {}
}
//~^^^^ non-exhaustive patterns: `B` not covered

match Foo::A {
Foo::A => {}
match HiddenEnum::A {
HiddenEnum::A => {}
}
//~^^^ non-exhaustive patterns: `B` and `_` not covered

match None {
None => {}
Some(Foo::A) => {}
Some(HiddenEnum::A) => {}
}
//~^^^^ non-exhaustive patterns: `Some(B)` and `Some(_)` not covered

match InCrate::A {
InCrate::A => {}
InCrate::B => {}
}
//~^^^^ non-exhaustive patterns: `C` not covered
}
73 changes: 47 additions & 26 deletions src/test/ui/pattern/usefulness/doc-hidden-non-exhaustive.stderr
Original file line number Diff line number Diff line change
@@ -1,81 +1,81 @@
error[E0004]: non-exhaustive patterns: `_` not covered
--> $DIR/doc-hidden-non-exhaustive.rs:8:11
--> $DIR/doc-hidden-non-exhaustive.rs:15:11
|
LL | match Foo::A {
| ^^^^^^ pattern `_` not covered
LL | match HiddenEnum::A {
| ^^^^^^^^^^^^^ pattern `_` not covered
|
note: `Foo` defined here
note: `HiddenEnum` defined here
--> $DIR/auxiliary/hidden.rs:1:1
|
LL | / pub enum Foo {
LL | / pub enum HiddenEnum {
LL | | A,
LL | | B,
LL | | #[doc(hidden)]
LL | | C,
LL | | }
| |_^
= note: the matched value is of type `Foo`
= note: the matched value is of type `HiddenEnum`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
|
LL ~ Foo::B => {}
LL ~ HiddenEnum::B => {}
LL + _ => todo!()
|

error[E0004]: non-exhaustive patterns: `B` not covered
--> $DIR/doc-hidden-non-exhaustive.rs:14:11
--> $DIR/doc-hidden-non-exhaustive.rs:21:11
|
LL | match Foo::A {
| ^^^^^^ pattern `B` not covered
LL | match HiddenEnum::A {
| ^^^^^^^^^^^^^ pattern `B` not covered
|
note: `Foo` defined here
note: `HiddenEnum` defined here
--> $DIR/auxiliary/hidden.rs:3:5
|
LL | / pub enum Foo {
LL | / pub enum HiddenEnum {
LL | | A,
LL | | B,
| | ^ not covered
LL | | #[doc(hidden)]
LL | | C,
LL | | }
| |_-
= note: the matched value is of type `Foo`
= note: the matched value is of type `HiddenEnum`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
|
LL ~ Foo::C => {}
LL ~ HiddenEnum::C => {}
LL + B => todo!()
|

error[E0004]: non-exhaustive patterns: `B` and `_` not covered
--> $DIR/doc-hidden-non-exhaustive.rs:20:11
--> $DIR/doc-hidden-non-exhaustive.rs:27:11
|
LL | match Foo::A {
| ^^^^^^ patterns `B` and `_` not covered
LL | match HiddenEnum::A {
| ^^^^^^^^^^^^^ patterns `B` and `_` not covered
|
note: `Foo` defined here
note: `HiddenEnum` defined here
--> $DIR/auxiliary/hidden.rs:3:5
|
LL | / pub enum Foo {
LL | / pub enum HiddenEnum {
LL | | A,
LL | | B,
| | ^ not covered
LL | | #[doc(hidden)]
LL | | C,
LL | | }
| |_-
= note: the matched value is of type `Foo`
= note: the matched value is of type `HiddenEnum`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern, a match arm with multiple or-patterns as shown, or multiple match arms
|
LL ~ Foo::A => {}
LL ~ HiddenEnum::A => {}
LL + B | _ => todo!()
|

error[E0004]: non-exhaustive patterns: `Some(B)` and `Some(_)` not covered
--> $DIR/doc-hidden-non-exhaustive.rs:25:11
--> $DIR/doc-hidden-non-exhaustive.rs:32:11
|
LL | match None {
| ^^^^ patterns `Some(B)` and `Some(_)` not covered
|
note: `Option<Foo>` defined here
note: `Option<HiddenEnum>` defined here
--> $SRC_DIR/core/src/option.rs:LL:COL
|
LL | / pub enum Option<T> {
Expand All @@ -87,13 +87,34 @@ LL | | Some(#[stable(feature = "rust1", since = "1.0.0")] T),
| | ^^^^ not covered
LL | | }
| |_-
= note: the matched value is of type `Option<Foo>`
= note: the matched value is of type `Option<HiddenEnum>`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern, a match arm with multiple or-patterns as shown, or multiple match arms
|
LL ~ Some(Foo::A) => {}
LL ~ Some(HiddenEnum::A) => {}
LL + Some(B) | Some(_) => todo!()
|

error: aborting due to 4 previous errors
error[E0004]: non-exhaustive patterns: `C` not covered
--> $DIR/doc-hidden-non-exhaustive.rs:38:11
|
LL | match InCrate::A {
| ^^^^^^^^^^ pattern `C` not covered
|
note: `InCrate` defined here
--> $DIR/doc-hidden-non-exhaustive.rs:11:5
|
LL | enum InCrate {
| -------
...
LL | C,
| ^ not covered
= note: the matched value is of type `InCrate`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
|
LL ~ InCrate::B => {}
LL + C => todo!()
|

error: aborting due to 5 previous errors

For more information about this error, try `rustc --explain E0004`.
16 changes: 16 additions & 0 deletions src/test/ui/pattern/usefulness/stable-gated-fields.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// aux-build:unstable.rs

extern crate unstable;

use unstable::UnstableStruct;

fn main() {
let UnstableStruct { stable } = UnstableStruct::default();
//~^ pattern does not mention field `stable2` and inaccessible fields

let UnstableStruct { stable, stable2 } = UnstableStruct::default();
//~^ pattern requires `..` due to inaccessible fields

// OK: stable field is matched
let UnstableStruct { stable, stable2, .. } = UnstableStruct::default();
}
Loading