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

Reuse machinery from tail_expr_drop_order for if_let_rescope #137455

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3911,6 +3911,7 @@ dependencies = [
"rustc_target",
"rustc_trait_selection",
"rustc_type_ir",
"smallvec",
"tracing",
"unicode-security",
]
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_lint/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ rustc_span = { path = "../rustc_span" }
rustc_target = { path = "../rustc_target" }
rustc_trait_selection = { path = "../rustc_trait_selection" }
rustc_type_ir = { path = "../rustc_type_ir" }
smallvec = { version = "1.8.1", features = ["union", "may_dangle"] }
tracing = "0.1"
unicode-security = "0.1.0"
# tidy-alphabetical-end
5 changes: 5 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,11 @@ lint_identifier_uncommon_codepoints = identifier contains {$codepoints_len ->
*[other] {" "}{$identifier_type}
} Unicode general security profile

lint_if_let_dtor = {$dtor_kind ->
[dyn] value may invoke a custom destructor because it contains a trait object
*[concrete] value invokes this custom destructor
}

lint_if_let_rescope = `if let` assigns a shorter lifetime since Edition 2024
.label = this value has a significant drop implementation which may observe a major change in drop order and requires your discretion
.help = the value is now dropped here in Edition 2024
Expand Down
57 changes: 44 additions & 13 deletions compiler/rustc_lint/src/if_let_rescope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,16 @@ use rustc_errors::{
Applicability, Diag, EmissionGuarantee, SubdiagMessageOp, Subdiagnostic, SuggestionStyle,
};
use rustc_hir::{self as hir, HirIdSet};
use rustc_macros::LintDiagnostic;
use rustc_middle::ty::TyCtxt;
use rustc_macros::{LintDiagnostic, Subdiagnostic};
use rustc_middle::ty::significant_drop_order::{
extract_component_with_significant_dtor, ty_dtor_span,
};
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_session::lint::{FutureIncompatibilityReason, LintId};
use rustc_session::{declare_lint, impl_lint_pass};
use rustc_span::Span;
use rustc_span::edition::Edition;
use rustc_span::{DUMMY_SP, Span};
use smallvec::SmallVec;

use crate::{LateContext, LateLintPass};

Expand Down Expand Up @@ -129,13 +133,15 @@ impl IfLetRescope {
hir::ExprKind::If(_cond, _conseq, Some(alt)) => alt.span.shrink_to_hi(),
_ => return,
};
let mut seen_dyn = false;
let mut add_bracket_to_match_head = match_head_needs_bracket(tcx, expr);
let mut significant_droppers = vec![];
let mut lifetime_ends = vec![];
let mut closing_brackets = 0;
let mut alt_heads = vec![];
let mut match_heads = vec![];
let mut consequent_heads = vec![];
let mut destructors = vec![];
let mut first_if_to_lint = None;
let mut first_if_to_rewrite = false;
let mut empty_alt = false;
Expand All @@ -159,11 +165,25 @@ impl IfLetRescope {
let before_conseq = conseq.span.shrink_to_lo();
let lifetime_end = source_map.end_point(conseq.span);

if let ControlFlow::Break(significant_dropper) =
if let ControlFlow::Break((drop_span, drop_tys)) =
(FindSignificantDropper { cx }).visit_expr(init)
{
destructors.extend(drop_tys.into_iter().filter_map(|ty| {
if let Some(span) = ty_dtor_span(tcx, ty) {
Some(DestructorLabel { span, dtor_kind: "concrete" })
} else if matches!(ty.kind(), ty::Dynamic(..)) {
if seen_dyn {
None
} else {
seen_dyn = true;
Some(DestructorLabel { span: DUMMY_SP, dtor_kind: "dyn" })
}
} else {
None
}
}));
first_if_to_lint = first_if_to_lint.or_else(|| Some((span, expr.hir_id)));
significant_droppers.push(significant_dropper);
significant_droppers.push(drop_span);
lifetime_ends.push(lifetime_end);
if ty_ascription.is_some()
|| !expr.span.can_be_used_for_suggestions()
Expand Down Expand Up @@ -226,6 +246,7 @@ impl IfLetRescope {
hir_id,
span,
IfLetRescopeLint {
destructors,
significant_droppers,
lifetime_ends,
rewrite: first_if_to_rewrite.then_some(IfLetRescopeRewrite {
Expand Down Expand Up @@ -287,6 +308,8 @@ impl<'tcx> LateLintPass<'tcx> for IfLetRescope {
#[derive(LintDiagnostic)]
#[diag(lint_if_let_rescope)]
struct IfLetRescopeLint {
#[subdiagnostic]
destructors: Vec<DestructorLabel>,
#[label]
significant_droppers: Vec<Span>,
#[help]
Expand Down Expand Up @@ -346,6 +369,14 @@ impl Subdiagnostic for IfLetRescopeRewrite {
}
}

#[derive(Subdiagnostic)]
#[note(lint_if_let_dtor)]
struct DestructorLabel {
#[primary_span]
span: Span,
dtor_kind: &'static str,
}

struct AltHead(Span);

struct ConsequentRewrite {
Expand All @@ -368,16 +399,16 @@ struct FindSignificantDropper<'tcx, 'a> {
}

impl<'tcx, 'a> Visitor<'tcx> for FindSignificantDropper<'tcx, 'a> {
type Result = ControlFlow<Span>;
type Result = ControlFlow<(Span, SmallVec<[Ty<'tcx>; 4]>)>;

fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) -> Self::Result {
if self
.cx
.typeck_results()
.expr_ty(expr)
.has_significant_drop(self.cx.tcx, self.cx.typing_env())
{
return ControlFlow::Break(expr.span);
let drop_tys = extract_component_with_significant_dtor(
self.cx.tcx,
self.cx.typing_env(),
self.cx.typeck_results().expr_ty(expr),
);
if !drop_tys.is_empty() {
return ControlFlow::Break((expr.span, drop_tys));
}
match expr.kind {
hir::ExprKind::ConstBlock(_)
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ pub mod normalize_erasing_regions;
pub mod pattern;
pub mod print;
pub mod relate;
pub mod significant_drop_order;
pub mod trait_def;
pub mod util;
pub mod visit;
Expand Down
172 changes: 172 additions & 0 deletions compiler/rustc_middle/src/ty/significant_drop_order.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::unord::UnordSet;
use rustc_hir::def_id::DefId;
use rustc_span::Span;
use smallvec::{SmallVec, smallvec};
use tracing::{debug, instrument};

use crate::ty::{self, Ty, TyCtxt};

/// An additional filter to exclude well-known types from the ecosystem
/// because their drops are trivial.
/// This returns additional types to check if the drops are delegated to those.
/// A typical example is `hashbrown::HashMap<K, V>`, whose drop is delegated to `K` and `V`.
fn true_significant_drop_ty<'tcx>(
tcx: TyCtxt<'tcx>,
ty: Ty<'tcx>,
) -> Option<SmallVec<[Ty<'tcx>; 2]>> {
if let ty::Adt(def, args) = ty.kind() {
let mut did = def.did();
let mut name_rev = vec![];
loop {
let key = tcx.def_key(did);

match key.disambiguated_data.data {
rustc_hir::definitions::DefPathData::CrateRoot => {
name_rev.push(tcx.crate_name(did.krate))
}
rustc_hir::definitions::DefPathData::TypeNs(symbol) => name_rev.push(symbol),
_ => return None,
}
if let Some(parent) = key.parent {
did = DefId { krate: did.krate, index: parent };
} else {
break;
}
}
let name_str: Vec<_> = name_rev.iter().rev().map(|x| x.as_str()).collect();
debug!(?name_str);
match name_str[..] {
// These are the types from Rust core ecosystem
["syn" | "proc_macro2", ..]
| ["core" | "std", "task", "LocalWaker" | "Waker"]
| ["core" | "std", "task", "wake", "LocalWaker" | "Waker"] => Some(smallvec![]),
// These are important types from Rust ecosystem
["tracing", "instrument", "Instrumented"] | ["bytes", "Bytes"] => Some(smallvec![]),
["hashbrown", "raw", "RawTable" | "RawIntoIter"] => {
if let [ty, ..] = &***args
&& let Some(ty) = ty.as_type()
{
Some(smallvec![ty])
} else {
None
}
}
["hashbrown", "raw", "RawDrain"] => {
if let [_, ty, ..] = &***args
&& let Some(ty) = ty.as_type()
{
Some(smallvec![ty])
} else {
None
}
}
_ => None,
}
} else {
None
}
}

/// Returns the list of types with a "potentially sigificant" that may be dropped
/// by dropping a value of type `ty`.
#[instrument(level = "trace", skip(tcx, typing_env))]
pub fn extract_component_raw<'tcx>(
tcx: TyCtxt<'tcx>,
typing_env: ty::TypingEnv<'tcx>,
ty: Ty<'tcx>,
ty_seen: &mut UnordSet<Ty<'tcx>>,
) -> SmallVec<[Ty<'tcx>; 4]> {
// Droppiness does not depend on regions, so let us erase them.
let ty = tcx.try_normalize_erasing_regions(typing_env, ty).unwrap_or(ty);

let tys = tcx.list_significant_drop_tys(typing_env.as_query_input(ty));
debug!(?ty, "components");
let mut out_tys = smallvec![];
for ty in tys {
if let Some(tys) = true_significant_drop_ty(tcx, ty) {
// Some types can be further opened up because the drop is simply delegated
for ty in tys {
if ty_seen.insert(ty) {
out_tys.extend(extract_component_raw(tcx, typing_env, ty, ty_seen));
}
}
} else {
if ty_seen.insert(ty) {
out_tys.push(ty);
}
}
}
out_tys
}

#[instrument(level = "trace", skip(tcx, typing_env))]
pub fn extract_component_with_significant_dtor<'tcx>(
tcx: TyCtxt<'tcx>,
typing_env: ty::TypingEnv<'tcx>,
ty: Ty<'tcx>,
) -> SmallVec<[Ty<'tcx>; 4]> {
let mut tys = extract_component_raw(tcx, typing_env, ty, &mut Default::default());
let mut deduplicate = FxHashSet::default();
tys.retain(|oty| deduplicate.insert(*oty));
tys.into_iter().collect()
}

/// Extract the span of the custom destructor of a type
/// especially the span of the `impl Drop` header or its entire block
/// when we are working with current local crate.
#[instrument(level = "trace", skip(tcx))]
pub fn ty_dtor_span<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Option<Span> {
match ty.kind() {
ty::Bool
| ty::Char
| ty::Int(_)
| ty::Uint(_)
| ty::Float(_)
| ty::Error(_)
| ty::Str
| ty::Never
| ty::RawPtr(_, _)
| ty::Ref(_, _, _)
| ty::FnPtr(_, _)
| ty::Tuple(_)
| ty::Dynamic(_, _, _)
| ty::Alias(_, _)
| ty::Bound(_, _)
| ty::Pat(_, _)
| ty::Placeholder(_)
| ty::Infer(_)
| ty::Slice(_)
| ty::Array(_, _)
| ty::UnsafeBinder(_) => None,

ty::Adt(adt_def, _) => {
let did = adt_def.did();
let try_local_did_span = |did: DefId| {
if let Some(local) = did.as_local() {
tcx.source_span(local)
} else {
tcx.def_span(did)
}
};
let dtor = if let Some(dtor) = tcx.adt_destructor(did) {
dtor.did
} else if let Some(dtor) = tcx.adt_async_destructor(did) {
dtor.future
} else {
return Some(try_local_did_span(did));
};
let def_key = tcx.def_key(dtor);
let Some(parent_index) = def_key.parent else { return Some(try_local_did_span(dtor)) };
let parent_did = DefId { index: parent_index, krate: dtor.krate };
Some(try_local_did_span(parent_did))
}
ty::Coroutine(did, _)
| ty::CoroutineWitness(did, _)
| ty::CoroutineClosure(did, _)
| ty::Closure(did, _)
| ty::FnDef(did, _)
| ty::Foreign(did) => Some(tcx.def_span(did)),
ty::Param(_) => None,
}
}
Loading
Loading