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

Lint bare traits in AstConv. #89090

Merged
merged 6 commits into from
Dec 18, 2021
Merged
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
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 @@ -47,7 +47,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 @@ -56,11 +56,9 @@ use rustc_hir::intravisit;
use rustc_hir::{ConstArg, GenericArg, InferKind, 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 @@ -1190,11 +1188,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 @@ -1291,9 +1285,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 @@ -2395,39 +2386,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 @@ -635,16 +635,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 @@ -2289,13 +2291,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 @@ -2306,7 +2314,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 @@ -2325,6 +2333,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 All @@ -2345,7 +2354,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 @@ -2602,4 +2611,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 @@ -844,7 +841,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 @@ -890,7 +887,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 @@ -903,56 +899,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