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

Stability annotations on generic parameters (take 2) #72314

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
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
4 changes: 4 additions & 0 deletions src/librustc_metadata/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1612,13 +1612,17 @@ impl EncodeContext<'tcx> {
EntryKind::TypeParam,
default.is_some(),
);
if default.is_some() {
self.encode_stability(def_id.to_def_id());
}
}
GenericParamKind::Const { .. } => {
self.encode_info_for_generic_param(
def_id.to_def_id(),
EntryKind::ConstParam,
true,
);
// FIXME(const_generics:defaults)
varkor marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Expand Down
28 changes: 21 additions & 7 deletions src/librustc_middle/middle/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,9 +374,27 @@ impl<'tcx> TyCtxt<'tcx> {
/// If the item defined by `def_id` is unstable and the corresponding `#![feature]` does not
/// exist, emits an error.
///
/// Additionally, this function will also check if the item is deprecated. If so, and `id` is
/// not `None`, a deprecated lint attached to `id` will be emitted.
/// This function will also check if the item is deprecated.
/// If so, and `id` is not `None`, a deprecated lint attached to `id` will be emitted.
pub fn check_stability(self, def_id: DefId, id: Option<HirId>, span: Span) {
self.check_optional_stability(def_id, id, span, |span, def_id| {
// The API could be uncallable for other reasons, for example when a private module
// was referenced.
self.sess.delay_span_bug(span, &format!("encountered unmarked API: {:?}", def_id));
})
}

/// Like `check_stability`, except that we permit items to have custom behaviour for
/// missing stability attributes (not necessarily just emit a `bug!`). This is necessary
/// for default generic parameters, which only have stability attributes if they were
/// added after the type on which they're defined.
pub fn check_optional_stability(
self,
def_id: DefId,
id: Option<HirId>,
span: Span,
unmarked: impl FnOnce(Span, DefId) -> (),
) {
let soft_handler = |lint, span, msg: &_| {
self.struct_span_lint_hir(lint, id.unwrap_or(hir::CRATE_HIR_ID), span, |lint| {
lint.build(msg).emit()
Expand All @@ -387,11 +405,7 @@ impl<'tcx> TyCtxt<'tcx> {
EvalResult::Deny { feature, reason, issue, is_soft } => {
report_unstable(self.sess, feature, reason, issue, is_soft, span, soft_handler)
}
EvalResult::Unmarked => {
// The API could be uncallable for other reasons, for example when a private module
// was referenced.
self.sess.delay_span_bug(span, &format!("encountered unmarked API: {:?}", def_id));
}
EvalResult::Unmarked => unmarked(span, def_id),
}
}

Expand Down
147 changes: 123 additions & 24 deletions src/librustc_passes/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,23 @@ enum AnnotationKind {
Container,
}

/// Whether to inherit deprecation flags for nested items. In most cases, we do want to inherit
/// deprecation, because nested items rarely have individual deprecation attributes, and so
/// should be treated as deprecated if their parent is. However, default generic parameters
/// have separate deprecation attributes from their parents, so we do not wish to inherit
/// deprecation in this case. For example, inheriting deprecation for `T` in `Foo<T>`
/// would cause a duplicate warning arising from both `Foo` and `T` being deprecated.
enum InheritDeprecation {
Yes,
No,
}

impl InheritDeprecation {
fn yes(&self) -> bool {
matches!(self, InheritDeprecation::Yes)
}
}

// A private tree-walker for producing an Index.
struct Annotator<'a, 'tcx> {
tcx: TyCtxt<'tcx>,
Expand All @@ -55,12 +72,20 @@ impl<'a, 'tcx> Annotator<'a, 'tcx> {
attrs: &[Attribute],
item_sp: Span,
kind: AnnotationKind,
inherit_deprecation: InheritDeprecation,
visit_children: F,
) where
F: FnOnce(&mut Self),
{
if !self.tcx.features().staged_api {
self.forbid_staged_api_attrs(hir_id, attrs, item_sp, kind, visit_children);
self.forbid_staged_api_attrs(
hir_id,
attrs,
item_sp,
kind,
inherit_deprecation,
visit_children,
);
return;
}

Expand Down Expand Up @@ -106,8 +131,11 @@ impl<'a, 'tcx> Annotator<'a, 'tcx> {
// If parent is deprecated and we're not, inherit this by merging
// deprecated_since and its reason.
if let Some(parent_stab) = self.parent_stab {
if parent_stab.rustc_depr.is_some() && stab.rustc_depr.is_none() {
stab.rustc_depr = parent_stab.rustc_depr
if inherit_deprecation.yes()
&& parent_stab.rustc_depr.is_some()
&& stab.rustc_depr.is_none()
{
stab.rustc_depr = parent_stab.rustc_depr.clone()
}
}

Expand Down Expand Up @@ -157,7 +185,7 @@ impl<'a, 'tcx> Annotator<'a, 'tcx> {
if stab.is_none() {
debug!("annotate: stab not found, parent = {:?}", self.parent_stab);
if let Some(stab) = self.parent_stab {
if stab.level.is_unstable() {
if inherit_deprecation.yes() && stab.level.is_unstable() {
self.index.stab_map.insert(hir_id, stab);
}
}
Expand Down Expand Up @@ -200,6 +228,7 @@ impl<'a, 'tcx> Annotator<'a, 'tcx> {
attrs: &[Attribute],
item_sp: Span,
kind: AnnotationKind,
inherit_deprecation: InheritDeprecation,
visit_children: impl FnOnce(&mut Self),
) {
// Emit errors for non-staged-api crates.
Expand Down Expand Up @@ -227,7 +256,7 @@ impl<'a, 'tcx> Annotator<'a, 'tcx> {
// Propagate unstability. This can happen even for non-staged-api crates in case
// -Zforce-unstable-if-unmarked is set.
if let Some(stab) = self.parent_stab {
if stab.level.is_unstable() {
if inherit_deprecation.yes() && stab.level.is_unstable() {
self.index.stab_map.insert(hir_id, stab);
}
}
Expand Down Expand Up @@ -280,54 +309,119 @@ impl<'a, 'tcx> Visitor<'tcx> for Annotator<'a, 'tcx> {
}
hir::ItemKind::Struct(ref sd, _) => {
if let Some(ctor_hir_id) = sd.ctor_hir_id() {
self.annotate(ctor_hir_id, &i.attrs, i.span, AnnotationKind::Required, |_| {})
self.annotate(
ctor_hir_id,
&i.attrs,
i.span,
AnnotationKind::Required,
InheritDeprecation::Yes,
|_| {},
)
}
}
_ => {}
}

self.annotate(i.hir_id, &i.attrs, i.span, kind, |v| intravisit::walk_item(v, i));
self.annotate(i.hir_id, &i.attrs, i.span, kind, InheritDeprecation::Yes, |v| {
intravisit::walk_item(v, i)
});
self.in_trait_impl = orig_in_trait_impl;
}

fn visit_trait_item(&mut self, ti: &'tcx hir::TraitItem<'tcx>) {
self.annotate(ti.hir_id, &ti.attrs, ti.span, AnnotationKind::Required, |v| {
intravisit::walk_trait_item(v, ti);
});
self.annotate(
ti.hir_id,
&ti.attrs,
ti.span,
AnnotationKind::Required,
InheritDeprecation::Yes,
|v| {
intravisit::walk_trait_item(v, ti);
},
);
}

fn visit_impl_item(&mut self, ii: &'tcx hir::ImplItem<'tcx>) {
let kind =
if self.in_trait_impl { AnnotationKind::Prohibited } else { AnnotationKind::Required };
self.annotate(ii.hir_id, &ii.attrs, ii.span, kind, |v| {
self.annotate(ii.hir_id, &ii.attrs, ii.span, kind, InheritDeprecation::Yes, |v| {
intravisit::walk_impl_item(v, ii);
});
}

fn visit_variant(&mut self, var: &'tcx Variant<'tcx>, g: &'tcx Generics<'tcx>, item_id: HirId) {
self.annotate(var.id, &var.attrs, var.span, AnnotationKind::Required, |v| {
if let Some(ctor_hir_id) = var.data.ctor_hir_id() {
v.annotate(ctor_hir_id, &var.attrs, var.span, AnnotationKind::Required, |_| {});
}
self.annotate(
var.id,
&var.attrs,
var.span,
AnnotationKind::Required,
InheritDeprecation::Yes,
|v| {
if let Some(ctor_hir_id) = var.data.ctor_hir_id() {
v.annotate(
ctor_hir_id,
&var.attrs,
var.span,
AnnotationKind::Required,
InheritDeprecation::Yes,
|_| {},
);
}

intravisit::walk_variant(v, var, g, item_id)
})
intravisit::walk_variant(v, var, g, item_id)
},
)
}

fn visit_struct_field(&mut self, s: &'tcx StructField<'tcx>) {
self.annotate(s.hir_id, &s.attrs, s.span, AnnotationKind::Required, |v| {
intravisit::walk_struct_field(v, s);
});
self.annotate(
s.hir_id,
&s.attrs,
s.span,
AnnotationKind::Required,
InheritDeprecation::Yes,
|v| {
intravisit::walk_struct_field(v, s);
},
);
}

fn visit_foreign_item(&mut self, i: &'tcx hir::ForeignItem<'tcx>) {
self.annotate(i.hir_id, &i.attrs, i.span, AnnotationKind::Required, |v| {
intravisit::walk_foreign_item(v, i);
});
self.annotate(
i.hir_id,
&i.attrs,
i.span,
AnnotationKind::Required,
InheritDeprecation::Yes,
|v| {
intravisit::walk_foreign_item(v, i);
},
);
}

fn visit_macro_def(&mut self, md: &'tcx hir::MacroDef<'tcx>) {
self.annotate(md.hir_id, &md.attrs, md.span, AnnotationKind::Required, |_| {});
self.annotate(
md.hir_id,
&md.attrs,
md.span,
AnnotationKind::Required,
InheritDeprecation::Yes,
|_| {},
);
}

fn visit_generic_param(&mut self, p: &'tcx hir::GenericParam<'tcx>) {
let kind = match &p.kind {
// FIXME(const_generics:defaults)
varkor marked this conversation as resolved.
Show resolved Hide resolved
hir::GenericParamKind::Type { default, .. } if default.is_some() => {
AnnotationKind::Container
}
_ => AnnotationKind::Prohibited,
};

self.annotate(p.hir_id, &p.attrs, p.span, kind, InheritDeprecation::No, |v| {
intravisit::walk_generic_param(v, p);
});
}
}

Expand Down Expand Up @@ -401,6 +495,10 @@ impl<'a, 'tcx> Visitor<'tcx> for MissingStabilityAnnotations<'a, 'tcx> {
fn visit_macro_def(&mut self, md: &'tcx hir::MacroDef<'tcx>) {
self.check_missing_stability(md.hir_id, md.span);
}

// Note that we don't need to `check_missing_stability` for default generic parameters,
// as we assume that any default generic parameters without attributes are automatically
// stable (assuming they have not inherited instability from their parent).
}

fn new_index(tcx: TyCtxt<'tcx>) -> Index<'tcx> {
Expand Down Expand Up @@ -464,6 +562,7 @@ fn new_index(tcx: TyCtxt<'tcx>) -> Index<'tcx> {
&krate.item.attrs,
krate.item.span,
AnnotationKind::Required,
InheritDeprecation::Yes,
|v| intravisit::walk_crate(v, krate),
);
}
Expand Down
16 changes: 15 additions & 1 deletion src/librustc_typeck/astconv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -776,7 +776,21 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
(GenericParamDefKind::Lifetime, GenericArg::Lifetime(lt)) => {
self.ast_region_to_region(&lt, Some(param)).into()
}
(GenericParamDefKind::Type { .. }, GenericArg::Type(ty)) => {
(GenericParamDefKind::Type { has_default, .. }, GenericArg::Type(ty)) => {
if *has_default {
tcx.check_optional_stability(
param.def_id,
Some(arg.id()),
arg.span(),
|_, _| {
// Default generic parameters may not be marked
// with stability attributes, i.e. when the
// default parameter was defined at the same time
// as the rest of the type. As such, we ignore missing
// stability attributes.
},
)
}
if let (hir::TyKind::Infer, false) = (&ty.kind, self.allow_ty_infer()) {
inferred_params.push(ty.span);
tcx.types.err.into()
Expand Down
Loading