Skip to content

Commit

Permalink
Rollup merge of #89090 - cjgillot:bare-dyn, r=jackh726
Browse files Browse the repository at this point in the history
Lint bare traits in AstConv.

Removing the lint from lowering allows to:
- make lowering querification easier;
- have the lint implementation in only one place.

r? `@estebank`
  • Loading branch information
matthiaskrgr authored Dec 18, 2021
2 parents 54e7946 + 6fcbc3a commit 7a626cf
Show file tree
Hide file tree
Showing 51 changed files with 731 additions and 320 deletions.
48 changes: 3 additions & 45 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::sorted_map::SortedMap;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_data_structures::sync::Lrc;
use rustc_errors::{struct_span_err, Applicability};
use rustc_errors::struct_span_err;
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Namespace, PartialRes, PerNS, Res};
use rustc_hir::def_id::{DefId, DefPathHash, LocalDefId, CRATE_DEF_ID};
Expand All @@ -55,11 +55,9 @@ use rustc_hir::intravisit;
use rustc_hir::{ConstArg, GenericArg, ParamName};
use rustc_index::vec::{Idx, IndexVec};
use rustc_query_system::ich::StableHashingContext;
use rustc_session::lint::builtin::BARE_TRAIT_OBJECTS;
use rustc_session::lint::{BuiltinLintDiagnostics, LintBuffer};
use rustc_session::lint::LintBuffer;
use rustc_session::utils::{FlattenNonterminals, NtToTokenstream};
use rustc_session::Session;
use rustc_span::edition::Edition;
use rustc_span::hygiene::ExpnId;
use rustc_span::source_map::{respan, DesugaringKind};
use rustc_span::symbol::{kw, sym, Ident, Symbol};
Expand Down Expand Up @@ -1184,11 +1182,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
) -> hir::Ty<'hir> {
let id = self.lower_node_id(t.id);
let qpath = self.lower_qpath(t.id, qself, path, param_mode, itctx);
let ty = self.ty_path(id, t.span, qpath);
if let hir::TyKind::TraitObject(..) = ty.kind {
self.maybe_lint_bare_trait(t.span, t.id, qself.is_none() && path.is_global());
}
ty
self.ty_path(id, t.span, qpath)
}

fn ty(&mut self, span: Span, kind: hir::TyKind<'hir>) -> hir::Ty<'hir> {
Expand Down Expand Up @@ -1285,9 +1279,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
lifetime_bound.unwrap_or_else(|| this.elided_dyn_bound(t.span));
(bounds, lifetime_bound)
});
if kind != TraitObjectSyntax::Dyn {
self.maybe_lint_bare_trait(t.span, t.id, false);
}
hir::TyKind::TraitObject(bounds, lifetime_bound, kind)
}
TyKind::ImplTrait(def_node_id, ref bounds) => {
Expand Down Expand Up @@ -2380,39 +2371,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
name: hir::LifetimeName::Implicit(missing),
}
}

fn maybe_lint_bare_trait(&mut self, span: Span, id: NodeId, is_global: bool) {
// FIXME(davidtwco): This is a hack to detect macros which produce spans of the
// call site which do not have a macro backtrace. See #61963.
let is_macro_callsite = self
.sess
.source_map()
.span_to_snippet(span)
.map(|snippet| snippet.starts_with("#["))
.unwrap_or(true);
if !is_macro_callsite {
if span.edition() < Edition::Edition2021 {
self.resolver.lint_buffer().buffer_lint_with_diagnostic(
BARE_TRAIT_OBJECTS,
id,
span,
"trait objects without an explicit `dyn` are deprecated",
BuiltinLintDiagnostics::BareTraitObject(span, is_global),
)
} else {
let msg = "trait objects must include the `dyn` keyword";
let label = "add `dyn` keyword before this trait";
let mut err = struct_span_err!(self.sess, span, E0782, "{}", msg,);
err.span_suggestion_verbose(
span.shrink_to_lo(),
label,
String::from("dyn "),
Applicability::MachineApplicable,
);
err.emit();
}
}
}
}

/// Helper struct for delayed construction of GenericArgs.
Expand Down
10 changes: 0 additions & 10 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -633,16 +633,6 @@ pub trait LintContext: Sized {
}
},
BuiltinLintDiagnostics::Normal => (),
BuiltinLintDiagnostics::BareTraitObject(span, is_global) => {
let (sugg, app) = match sess.source_map().span_to_snippet(span) {
Ok(s) if is_global => {
(format!("dyn ({})", s), Applicability::MachineApplicable)
}
Ok(s) => (format!("dyn {}", s), Applicability::MachineApplicable),
Err(_) => ("dyn <type>".to_string(), Applicability::HasPlaceholders),
};
db.span_suggestion(span, "use `dyn`", sugg, app);
}
BuiltinLintDiagnostics::AbsPathWithModule(span) => {
let (sugg, app) = match sess.source_map().span_to_snippet(span) {
Ok(ref s) => {
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_lint_defs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,6 @@ pub enum ExternDepSpec {
#[derive(PartialEq, Debug)]
pub enum BuiltinLintDiagnostics {
Normal,
BareTraitObject(Span, /* is_global */ bool),
AbsPathWithModule(Span),
ProcMacroDeriveResolutionFallback(Span),
MacroExpandedMacroExportsAccessedByAbsolutePaths(Span),
Expand Down
77 changes: 72 additions & 5 deletions compiler/rustc_typeck/src/astconv/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use crate::errors::{
};
use crate::middle::resolve_lifetime as rl;
use crate::require_c_abi_if_c_variadic;
use rustc_ast::TraitObjectSyntax;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_errors::{struct_span_err, Applicability, ErrorReported, FatalError};
use rustc_hir as hir;
Expand All @@ -24,7 +25,8 @@ use rustc_hir::{GenericArg, GenericArgs};
use rustc_middle::ty::subst::{self, GenericArgKind, InternalSubsts, Subst, SubstsRef};
use rustc_middle::ty::GenericParamDefKind;
use rustc_middle::ty::{self, Const, DefIdTree, Ty, TyCtxt, TypeFoldable};
use rustc_session::lint::builtin::AMBIGUOUS_ASSOCIATED_ITEMS;
use rustc_session::lint::builtin::{AMBIGUOUS_ASSOCIATED_ITEMS, BARE_TRAIT_OBJECTS};
use rustc_span::edition::Edition;
use rustc_span::lev_distance::find_best_match_for_name;
use rustc_span::symbol::{Ident, Symbol};
use rustc_span::{Span, DUMMY_SP};
Expand Down Expand Up @@ -2266,13 +2268,19 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
/// Parses the programmer's textual representation of a type into our
/// internal notion of a type.
pub fn ast_ty_to_ty(&self, ast_ty: &hir::Ty<'_>) -> Ty<'tcx> {
self.ast_ty_to_ty_inner(ast_ty, false)
self.ast_ty_to_ty_inner(ast_ty, false, false)
}

/// Parses the programmer's textual representation of a type into our
/// internal notion of a type. This is meant to be used within a path.
pub fn ast_ty_to_ty_in_path(&self, ast_ty: &hir::Ty<'_>) -> Ty<'tcx> {
self.ast_ty_to_ty_inner(ast_ty, false, true)
}

/// Turns a `hir::Ty` into a `Ty`. For diagnostics' purposes we keep track of whether trait
/// objects are borrowed like `&dyn Trait` to avoid emitting redundant errors.
#[tracing::instrument(level = "debug", skip(self))]
fn ast_ty_to_ty_inner(&self, ast_ty: &hir::Ty<'_>, borrowed: bool) -> Ty<'tcx> {
fn ast_ty_to_ty_inner(&self, ast_ty: &hir::Ty<'_>, borrowed: bool, in_path: bool) -> Ty<'tcx> {
let tcx = self.tcx();

let result_ty = match ast_ty.kind {
Expand All @@ -2283,7 +2291,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
hir::TyKind::Rptr(ref region, ref mt) => {
let r = self.ast_region_to_region(region, None);
debug!(?r);
let t = self.ast_ty_to_ty_inner(mt.ty, true);
let t = self.ast_ty_to_ty_inner(mt.ty, true, false);
tcx.mk_ref(r, ty::TypeAndMut { ty: t, mutbl: mt.mutbl })
}
hir::TyKind::Never => tcx.types.never,
Expand All @@ -2302,6 +2310,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
))
}
hir::TyKind::TraitObject(bounds, ref lifetime, _) => {
self.maybe_lint_bare_trait(ast_ty, in_path);
self.conv_object_ty_poly_trait_ref(ast_ty.span, bounds, lifetime, borrowed)
}
hir::TyKind::Path(hir::QPath::Resolved(ref maybe_qself, ref path)) => {
Expand Down Expand Up @@ -2329,7 +2338,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
}
hir::TyKind::Path(hir::QPath::TypeRelative(ref qself, ref segment)) => {
debug!(?qself, ?segment);
let ty = self.ast_ty_to_ty(qself);
let ty = self.ast_ty_to_ty_inner(qself, false, true);

let res = if let hir::TyKind::Path(hir::QPath::Resolved(_, path)) = qself.kind {
path.res
Expand Down Expand Up @@ -2586,4 +2595,62 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
}
Some(r)
}

fn maybe_lint_bare_trait(&self, self_ty: &hir::Ty<'_>, in_path: bool) {
let tcx = self.tcx();
if let hir::TyKind::TraitObject([poly_trait_ref, ..], _, TraitObjectSyntax::None) =
self_ty.kind
{
let needs_bracket = in_path
&& !tcx
.sess
.source_map()
.span_to_prev_source(self_ty.span)
.ok()
.map_or(false, |s| s.trim_end().ends_with('<'));

let is_global = poly_trait_ref.trait_ref.path.is_global();
let sugg = Vec::from_iter([
(
self_ty.span.shrink_to_lo(),
format!(
"{}dyn {}",
if needs_bracket { "<" } else { "" },
if is_global { "(" } else { "" },
),
),
(
self_ty.span.shrink_to_hi(),
format!(
"{}{}",
if is_global { ")" } else { "" },
if needs_bracket { ">" } else { "" },
),
),
]);
if self_ty.span.edition() >= Edition::Edition2021 {
let msg = "trait objects must include the `dyn` keyword";
let label = "add `dyn` keyword before this trait";
rustc_errors::struct_span_err!(tcx.sess, self_ty.span, E0782, "{}", msg)
.multipart_suggestion_verbose(label, sugg, Applicability::MachineApplicable)
.emit();
} else {
let msg = "trait objects without an explicit `dyn` are deprecated";
tcx.struct_span_lint_hir(
BARE_TRAIT_OBJECTS,
self_ty.hir_id,
self_ty.span,
|lint| {
lint.build(msg)
.multipart_suggestion_verbose(
"use `dyn`",
sugg,
Applicability::MachineApplicable,
)
.emit()
},
);
}
}
}
}
58 changes: 2 additions & 56 deletions compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,14 @@ use crate::check::callee::{self, DeferredCallResolution};
use crate::check::method::{self, MethodCallee, SelfSource};
use crate::check::{BreakableCtxt, Diverges, Expectation, FnCtxt, LocalTy};

use rustc_ast::TraitObjectSyntax;
use rustc_data_structures::captures::Captures;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::{Applicability, DiagnosticBuilder, ErrorReported};
use rustc_hir as hir;
use rustc_hir::def::{CtorOf, DefKind, Res};
use rustc_hir::def_id::DefId;
use rustc_hir::lang_items::LangItem;
use rustc_hir::{ExprKind, GenericArg, Node, QPath, TyKind};
use rustc_hir::{ExprKind, GenericArg, Node, QPath};
use rustc_infer::infer::canonical::{Canonical, OriginalQueryValues, QueryResponse};
use rustc_infer::infer::error_reporting::TypeAnnotationNeeded::E0282;
use rustc_infer::infer::{InferOk, InferResult};
Expand All @@ -28,8 +27,6 @@ use rustc_middle::ty::{
Ty, UserType,
};
use rustc_session::lint;
use rustc_session::lint::builtin::BARE_TRAIT_OBJECTS;
use rustc_span::edition::Edition;
use rustc_span::hygiene::DesugaringKind;
use rustc_span::source_map::{original_sp, DUMMY_SP};
use rustc_span::symbol::{kw, sym, Ident};
Expand Down Expand Up @@ -855,7 +852,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// to be object-safe.
// We manually call `register_wf_obligation` in the success path
// below.
(<dyn AstConv<'_>>::ast_ty_to_ty(self, qself), qself, segment)
(<dyn AstConv<'_>>::ast_ty_to_ty_in_path(self, qself), qself, segment)
}
QPath::LangItem(..) => {
bug!("`resolve_ty_and_res_fully_qualified_call` called on `LangItem`")
Expand Down Expand Up @@ -901,7 +898,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
});

if result.is_ok() {
self.maybe_lint_bare_trait(qpath, hir_id, span);
self.register_wf_obligation(ty.into(), qself.span, traits::WellFormed(None));
}

Expand All @@ -914,56 +910,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
)
}

fn maybe_lint_bare_trait(&self, qpath: &QPath<'_>, hir_id: hir::HirId, span: Span) {
if let QPath::TypeRelative(self_ty, _) = qpath {
if let TyKind::TraitObject([poly_trait_ref, ..], _, TraitObjectSyntax::None) =
self_ty.kind
{
let msg = "trait objects without an explicit `dyn` are deprecated";
let (sugg, app) = match self.tcx.sess.source_map().span_to_snippet(self_ty.span) {
Ok(s) if poly_trait_ref.trait_ref.path.is_global() => {
(format!("dyn ({})", s), Applicability::MachineApplicable)
}
Ok(s) => (format!("dyn {}", s), Applicability::MachineApplicable),
Err(_) => ("dyn <type>".to_string(), Applicability::HasPlaceholders),
};
// Wrap in `<..>` if it isn't already.
let sugg = match self.tcx.sess.source_map().span_to_snippet(span) {
Ok(s) if s.starts_with('<') => sugg,
_ => format!("<{}>", sugg),
};
let sugg_label = "use `dyn`";
if self.sess().edition() >= Edition::Edition2021 {
let mut err = rustc_errors::struct_span_err!(
self.sess(),
self_ty.span,
E0782,
"{}",
msg,
);
err.span_suggestion(
self_ty.span,
sugg_label,
sugg,
Applicability::MachineApplicable,
)
.emit();
} else {
self.tcx.struct_span_lint_hir(
BARE_TRAIT_OBJECTS,
hir_id,
self_ty.span,
|lint| {
let mut db = lint.build(msg);
db.span_suggestion(self_ty.span, sugg_label, sugg, app);
db.emit()
},
);
}
}
}
}

/// Given a function `Node`, return its `FnDecl` if it exists, or `None` otherwise.
pub(in super::super) fn get_node_fn_decl(
&self,
Expand Down
2 changes: 1 addition & 1 deletion src/test/rustdoc-ui/display-output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@
/// #![warn(unused)]
/// let x = 12;
///
/// fn foo(x: &std::fmt::Display) {}
/// fn foo(x: &dyn std::fmt::Display) {}
/// ```
pub fn foo() {}
16 changes: 3 additions & 13 deletions src/test/rustdoc-ui/display-output.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,6 @@ test $DIR/display-output.rs - foo (line 9) ... ok
successes:

---- $DIR/display-output.rs - foo (line 9) stdout ----
warning: trait objects without an explicit `dyn` are deprecated
--> $DIR/display-output.rs:13:12
|
LL | fn foo(x: &std::fmt::Display) {}
| ^^^^^^^^^^^^^^^^^ help: use `dyn`: `dyn std::fmt::Display`
|
= note: `#[warn(bare_trait_objects)]` on by default
= warning: this is accepted in the current edition (Rust 2018) but is a hard error in Rust 2021!
= note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2021/warnings-promoted-to-error.html>

warning: unused variable: `x`
--> $DIR/display-output.rs:11:5
|
Expand All @@ -31,13 +21,13 @@ LL | #![warn(unused)]
warning: unused variable: `x`
--> $DIR/display-output.rs:13:8
|
LL | fn foo(x: &std::fmt::Display) {}
LL | fn foo(x: &dyn std::fmt::Display) {}
| ^ help: if this is intentional, prefix it with an underscore: `_x`

warning: function is never used: `foo`
--> $DIR/display-output.rs:13:4
|
LL | fn foo(x: &std::fmt::Display) {}
LL | fn foo(x: &dyn std::fmt::Display) {}
| ^^^
|
note: the lint level is defined here
Expand All @@ -47,7 +37,7 @@ LL | #![warn(unused)]
| ^^^^^^
= note: `#[warn(dead_code)]` implied by `#[warn(unused)]`

warning: 4 warnings emitted
warning: 3 warnings emitted



Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ fn b() {
//~^ ERROR expected trait, found constant `BAR`
//~| ERROR expected trait, found constant `BAR`
//~| ERROR type provided when a constant was expected
//~| WARN trait objects without an explicit `dyn` are deprecated
//~| WARN this is accepted in the current edition
}
fn c() {
foo::<3 + 3>(); //~ ERROR expressions must be enclosed in braces
Expand Down
Loading

0 comments on commit 7a626cf

Please sign in to comment.