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

Error on private items with stability attributes #83397

Closed
wants to merge 3 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Error on private items with stability attributes
It doesn't make sense for a private item to have a stability attribute,
and it is likely a mistake. Report an error when we see a case of this.
camelid committed Jul 22, 2021
commit ebee8e1a4d5e5072de8af42a88dab3ccb021c96a
29 changes: 28 additions & 1 deletion compiler/rustc_passes/src/stability.rs
Original file line number Diff line number Diff line change
@@ -219,7 +219,7 @@ impl<'a, 'tcx> Annotator<'a, 'tcx> {
if kind == AnnotationKind::Prohibited
|| (kind == AnnotationKind::Container && stab.level.is_stable() && is_deprecated)
{
self.tcx.sess.struct_span_err(span,"this stability annotation is useless")
self.tcx.sess.struct_span_err(span, "this stability annotation is useless")
.span_label(span, "useless stability annotation")
.span_label(item_sp, "the stability attribute annotates this item")
.emit();
@@ -605,6 +605,24 @@ impl<'tcx> MissingStabilityAnnotations<'tcx> {
}
}
}

fn check_private_stability(&self, hir_id: HirId, span: Span) {
let stab = self.tcx.stability().local_stability(hir_id);
let is_error = stab.is_some() && !self.access_levels.is_reachable(hir_id);
Copy link
Member

Choose a reason for hiding this comment

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

Note this is currently buggy: #64762

if is_error {
let def_id = self.tcx.hir().local_def_id(hir_id);
let descr = self.tcx.def_kind(def_id).descr(def_id.to_def_id());
self.tcx
.sess
.struct_span_err(
span,
&format!("{} is private but has a stability attribute", descr),
)
.help("if it is meant to be private, remove the stability attribute")
.help("or, if it is meant to be public, make it public")
.emit();
}
}
}

impl<'tcx> Visitor<'tcx> for MissingStabilityAnnotations<'tcx> {
@@ -635,11 +653,14 @@ impl<'tcx> Visitor<'tcx> for MissingStabilityAnnotations<'tcx> {
self.check_missing_const_stability(i.hir_id(), i.span);
}

self.check_private_stability(i.hir_id(), i.span);

intravisit::walk_item(self, i)
}

fn visit_trait_item(&mut self, ti: &'tcx hir::TraitItem<'tcx>) {
self.check_missing_stability(ti.hir_id(), ti.span);
self.check_private_stability(ti.hir_id(), ti.span);
intravisit::walk_trait_item(self, ti);
}

@@ -648,26 +669,31 @@ impl<'tcx> Visitor<'tcx> for MissingStabilityAnnotations<'tcx> {
if self.tcx.impl_trait_ref(impl_def_id).is_none() {
self.check_missing_stability(ii.hir_id(), ii.span);
}
self.check_private_stability(ii.hir_id(), ii.span);
intravisit::walk_impl_item(self, ii);
}

fn visit_variant(&mut self, var: &'tcx Variant<'tcx>, g: &'tcx Generics<'tcx>, item_id: HirId) {
self.check_missing_stability(var.id, var.span);
self.check_private_stability(var.id, var.span);
intravisit::walk_variant(self, var, g, item_id);
}

fn visit_field_def(&mut self, s: &'tcx FieldDef<'tcx>) {
self.check_missing_stability(s.hir_id, s.span);
self.check_private_stability(s.hir_id, s.span);
intravisit::walk_field_def(self, s);
}

fn visit_foreign_item(&mut self, i: &'tcx hir::ForeignItem<'tcx>) {
self.check_missing_stability(i.hir_id(), i.span);
self.check_private_stability(i.hir_id(), i.span);
intravisit::walk_foreign_item(self, i);
}

fn visit_macro_def(&mut self, md: &'tcx hir::MacroDef<'tcx>) {
self.check_missing_stability(md.hir_id(), md.span);
self.check_private_stability(md.hir_id(), md.span);
}

// Note that we don't need to `check_missing_stability` for default generic parameters,
@@ -930,6 +956,7 @@ pub fn check_unused_or_stable_features(tcx: TyCtxt<'_>) {
let krate = tcx.hir().krate();
let mut missing = MissingStabilityAnnotations { tcx, access_levels };
missing.check_missing_stability(hir::CRATE_HIR_ID, krate.item.inner);
missing.check_private_stability(hir::CRATE_HIR_ID, krate.item.inner);
intravisit::walk_crate(&mut missing, krate);
krate.visit_all_item_likes(&mut missing.as_deep_visitor());
}
29 changes: 29 additions & 0 deletions src/test/ui/stability-attribute/stability-without-publicity.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#![feature(staged_api)]
#![stable(feature = "rust1", since = "1.0.0")]
#![crate_type = "lib"]

#[stable(feature = "rust1", since = "1.0.0")]
mod mod_stable {}
//~^ ERROR module is private but has a stability attribute

#[unstable(feature = "foo", issue = "none")]
mod mod_unstable {}
//~^ ERROR module is private but has a stability attribute

#[stable(feature = "rust1", since = "1.0.0")]
fn fn_stable() {}
//~^ ERROR function is private but has a stability attribute

#[unstable(feature = "foo", issue = "none")]
fn fn_unstable() {}
//~^ ERROR function is private but has a stability attribute

#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_stable(feature = "rust1", since = "1.0.0")]
const fn const_fn_stable() {}
//~^ ERROR function is private but has a stability attribute

#[unstable(feature = "foo", issue = "none")]
#[rustc_const_unstable(feature = "foo", issue = "none")]
const fn const_fn_unstable() {}
//~^ ERROR function is private but has a stability attribute
56 changes: 56 additions & 0 deletions src/test/ui/stability-attribute/stability-without-publicity.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
error: module is private but has a stability attribute
--> $DIR/stability-without-publicity.rs:6:1
|
LL | mod mod_stable {}
| ^^^^^^^^^^^^^^^^^
|
= help: if it is meant to be private, remove the stability attribute
= help: or, if it is meant to be public, make it public

error: module is private but has a stability attribute
--> $DIR/stability-without-publicity.rs:10:1
|
LL | mod mod_unstable {}
| ^^^^^^^^^^^^^^^^^^^
|
= help: if it is meant to be private, remove the stability attribute
= help: or, if it is meant to be public, make it public

error: function is private but has a stability attribute
--> $DIR/stability-without-publicity.rs:14:1
|
LL | fn fn_stable() {}
| ^^^^^^^^^^^^^^^^^
|
= help: if it is meant to be private, remove the stability attribute
= help: or, if it is meant to be public, make it public

error: function is private but has a stability attribute
--> $DIR/stability-without-publicity.rs:18:1
|
LL | fn fn_unstable() {}
| ^^^^^^^^^^^^^^^^^^^
|
= help: if it is meant to be private, remove the stability attribute
= help: or, if it is meant to be public, make it public

error: function is private but has a stability attribute
--> $DIR/stability-without-publicity.rs:23:1
|
LL | const fn const_fn_stable() {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: if it is meant to be private, remove the stability attribute
= help: or, if it is meant to be public, make it public

error: function is private but has a stability attribute
--> $DIR/stability-without-publicity.rs:28:1
|
LL | const fn const_fn_unstable() {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: if it is meant to be private, remove the stability attribute
= help: or, if it is meant to be public, make it public

error: aborting due to 6 previous errors