Skip to content

Commit

Permalink
Auto merge of rust-lang#129275 - matthiaskrgr:rollup-qv64hg6, r=matth…
Browse files Browse the repository at this point in the history
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#129194 (Fix bootstrap test `detect_src_and_out` on Windows)
 - rust-lang#129217 (safe transmute: check lifetimes)
 - rust-lang#129223 ( Fix wrong argument for `get_fn_decl`)
 - rust-lang#129235 (Check that `#[may_dangle]` is properly applied)
 - rust-lang#129245 (Fix a typo in `rustc_hir` doc comment)
 - rust-lang#129271 (Prevent double panic in query system, improve diagnostics)

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Aug 19, 2024
2 parents 5601d14 + 7730356 commit 636d7ff
Show file tree
Hide file tree
Showing 25 changed files with 746 additions and 150 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1395,7 +1395,7 @@ pub struct LetExpr<'hir> {
pub pat: &'hir Pat<'hir>,
pub ty: Option<&'hir Ty<'hir>>,
pub init: &'hir Expr<'hir>,
/// `Recovered::Yes` when this let expressions is not in a syntanctically valid location.
/// `Recovered::Yes` when this let expressions is not in a syntactically valid location.
/// Used to prevent building MIR in such situations.
pub recovered: ast::Recovered,
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1859,7 +1859,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
};

// If this is due to an explicit `return`, suggest adding a return type.
if let Some((fn_id, fn_decl, can_suggest)) = fcx.get_fn_decl(parent_id)
if let Some((fn_id, fn_decl, can_suggest)) = fcx.get_fn_decl(block_or_return_id)
&& !due_to_block
{
fcx.suggest_missing_return_type(&mut err, fn_decl, expected, found, can_suggest, fn_id);
Expand Down
7 changes: 5 additions & 2 deletions compiler/rustc_passes/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,9 @@ passes_macro_export_on_decl_macro =
passes_macro_use =
`#[{$name}]` only has an effect on `extern crate` and modules
passes_may_dangle =
`#[may_dangle]` must be applied to a lifetime or type generic parameter in `Drop` impl
passes_maybe_string_interpolation = you might have meant to use string interpolation in this string literal
passes_missing_const_err =
attributes `#[rustc_const_unstable]` and `#[rustc_const_stable]` require the function or method to be `const`
Expand Down Expand Up @@ -475,8 +478,8 @@ passes_multiple_start_functions =
.previous = previous `#[start]` function here
passes_must_not_suspend =
`must_not_suspend` attribute should be applied to a struct, enum, or trait
.label = is not a struct, enum, or trait
`must_not_suspend` attribute should be applied to a struct, enum, union, or trait
.label = is not a struct, enum, union, or trait
passes_must_use_async =
`must_use` attribute on `async` functions applies to the anonymous `Future` returned by the function, not the value within
Expand Down
25 changes: 23 additions & 2 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
[sym::collapse_debuginfo, ..] => self.check_collapse_debuginfo(attr, span, target),
[sym::must_not_suspend, ..] => self.check_must_not_suspend(attr, span, target),
[sym::must_use, ..] => self.check_must_use(hir_id, attr, target),
[sym::may_dangle, ..] => self.check_may_dangle(hir_id, attr),
[sym::rustc_pass_by_value, ..] => self.check_pass_by_value(attr, span, target),
[sym::rustc_allow_incoherent_impl, ..] => {
self.check_allow_incoherent_impl(attr, span, target)
Expand Down Expand Up @@ -255,7 +256,6 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
| sym::cfg_attr
// need to be fixed
| sym::cfi_encoding // FIXME(cfi_encoding)
| sym::may_dangle // FIXME(dropck_eyepatch)
| sym::pointee // FIXME(derive_smart_pointer)
| sym::omit_gdb_pretty_printer_section // FIXME(omit_gdb_pretty_printer_section)
| sym::used // handled elsewhere to restrict to static items
Expand Down Expand Up @@ -1363,7 +1363,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
}
}

/// Checks if `#[must_not_suspend]` is applied to a function.
/// Checks if `#[must_not_suspend]` is applied to a struct, enum, union, or trait.
fn check_must_not_suspend(&self, attr: &Attribute, span: Span, target: Target) {
match target {
Target::Struct | Target::Enum | Target::Union | Target::Trait => {}
Expand All @@ -1373,6 +1373,27 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
}
}

/// Checks if `#[may_dangle]` is applied to a lifetime or type generic parameter in `Drop` impl.
fn check_may_dangle(&self, hir_id: HirId, attr: &Attribute) {
if let hir::Node::GenericParam(param) = self.tcx.hir_node(hir_id)
&& matches!(
param.kind,
hir::GenericParamKind::Lifetime { .. } | hir::GenericParamKind::Type { .. }
)
&& matches!(param.source, hir::GenericParamSource::Generics)
&& let parent_hir_id = self.tcx.parent_hir_id(hir_id)
&& let hir::Node::Item(item) = self.tcx.hir_node(parent_hir_id)
&& let hir::ItemKind::Impl(impl_) = item.kind
&& let Some(trait_) = impl_.of_trait
&& let Some(def_id) = trait_.trait_def_id()
&& self.tcx.is_lang_item(def_id, hir::LangItem::Drop)
{
return;
}

self.dcx().emit_err(errors::InvalidMayDangle { attr_span: attr.span });
}

/// Checks if `#[cold]` is applied to a non-function.
fn check_cold(&self, hir_id: HirId, attr: &Attribute, span: Span, target: Target) {
match target {
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_passes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -737,6 +737,13 @@ pub struct NonExportedMacroInvalidAttrs {
pub attr_span: Span,
}

#[derive(Diagnostic)]
#[diag(passes_may_dangle)]
pub struct InvalidMayDangle {
#[primary_span]
pub attr_span: Span,
}

#[derive(LintDiagnostic)]
#[diag(passes_unused_duplicate)]
pub struct UnusedDuplicate {
Expand Down
10 changes: 8 additions & 2 deletions compiler/rustc_query_impl/src/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -702,11 +702,17 @@ macro_rules! define_queries {
let name = stringify!($name);
$crate::plumbing::create_query_frame(tcx, rustc_middle::query::descs::$name, key, kind, name)
};
tcx.query_system.states.$name.try_collect_active_jobs(
let res = tcx.query_system.states.$name.try_collect_active_jobs(
tcx,
make_query,
qmap,
).unwrap();
);
// this can be called during unwinding, and the function has a `try_`-prefix, so
// don't `unwrap()` here, just manually check for `None` and do best-effort error
// reporting.
if res.is_none() {
tracing::warn!("Failed to collect active jobs for query with name `{}`!", stringify!($name));
}
}

pub fn alloc_self_profile_query_strings<'tcx>(tcx: TyCtxt<'tcx>, string_cache: &mut QueryKeyStringCache) {
Expand Down
11 changes: 9 additions & 2 deletions compiler/rustc_query_system/src/query/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,15 @@ where
cache.complete(key, result, dep_node_index);

let job = {
let mut lock = state.active.lock_shard_by_value(&key);
lock.remove(&key).unwrap().expect_job()
let val = {
// don't keep the lock during the `unwrap()` of the retrieved value, or we taint the
// underlying shard.
// since unwinding also wants to look at this map, this can also prevent a double
// panic.
let mut lock = state.active.lock_shard_by_value(&key);
lock.remove(&key)
};
val.unwrap().expect_job()
};

job.signal_complete();
Expand Down
176 changes: 103 additions & 73 deletions compiler/rustc_trait_selection/src/traits/select/confirmation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ use rustc_infer::infer::{DefineOpaqueTypes, HigherRankedType, InferOk};
use rustc_infer::traits::ObligationCauseCode;
use rustc_middle::traits::{BuiltinImplSource, SignatureMismatchData};
use rustc_middle::ty::{
self, GenericArgs, GenericArgsRef, GenericParamDefKind, ToPolyTraitRef, TraitPredicate, Ty,
TyCtxt, Upcast,
self, GenericArgs, GenericArgsRef, GenericParamDefKind, ToPolyTraitRef, Ty, TyCtxt, Upcast,
};
use rustc_middle::{bug, span_bug};
use rustc_span::def_id::DefId;
Expand Down Expand Up @@ -292,90 +291,120 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
&mut self,
obligation: &PolyTraitObligation<'tcx>,
) -> Result<Vec<PredicateObligation<'tcx>>, SelectionError<'tcx>> {
use rustc_transmute::{Answer, Condition};
#[instrument(level = "debug", skip(tcx, obligation, predicate))]
use rustc_transmute::{Answer, Assume, Condition};

/// Generate sub-obligations for reference-to-reference transmutations.
fn reference_obligations<'tcx>(
tcx: TyCtxt<'tcx>,
obligation: &PolyTraitObligation<'tcx>,
(src_lifetime, src_ty, src_mut): (ty::Region<'tcx>, Ty<'tcx>, Mutability),
(dst_lifetime, dst_ty, dst_mut): (ty::Region<'tcx>, Ty<'tcx>, Mutability),
assume: Assume,
) -> Vec<PredicateObligation<'tcx>> {
let make_transmute_obl = |src, dst| {
let transmute_trait = obligation.predicate.def_id();
let assume = obligation.predicate.skip_binder().trait_ref.args.const_at(2);
let trait_ref = ty::TraitRef::new(
tcx,
transmute_trait,
[
ty::GenericArg::from(dst),
ty::GenericArg::from(src),
ty::GenericArg::from(assume),
],
);
Obligation::with_depth(
tcx,
obligation.cause.clone(),
obligation.recursion_depth + 1,
obligation.param_env,
obligation.predicate.rebind(trait_ref),
)
};

let make_freeze_obl = |ty| {
let trait_ref = ty::TraitRef::new(
tcx,
tcx.require_lang_item(LangItem::Freeze, None),
[ty::GenericArg::from(ty)],
);
Obligation::with_depth(
tcx,
obligation.cause.clone(),
obligation.recursion_depth + 1,
obligation.param_env,
trait_ref,
)
};

let make_outlives_obl = |target, region| {
let outlives = ty::OutlivesPredicate(target, region);
Obligation::with_depth(
tcx,
obligation.cause.clone(),
obligation.recursion_depth + 1,
obligation.param_env,
obligation.predicate.rebind(outlives),
)
};

// Given a transmutation from `&'a (mut) Src` and `&'dst (mut) Dst`,
// it is always the case that `Src` must be transmutable into `Dst`,
// and that that `'src` must outlive `'dst`.
let mut obls = vec![make_transmute_obl(src_ty, dst_ty)];
if !assume.lifetimes {
obls.push(make_outlives_obl(src_lifetime, dst_lifetime));
}

// Given a transmutation from `&Src`, both `Src` and `Dst` must be
// `Freeze`, otherwise, using the transmuted value could lead to
// data races.
if src_mut == Mutability::Not {
obls.extend([make_freeze_obl(src_ty), make_freeze_obl(dst_ty)])
}

// Given a transmutation into `&'dst mut Dst`, it also must be the
// case that `Dst` is transmutable into `Src`. For example,
// transmuting bool -> u8 is OK as long as you can't update that u8
// to be > 1, because you could later transmute the u8 back to a
// bool and get undefined behavior. It also must be the case that
// `'dst` lives exactly as long as `'src`.
if dst_mut == Mutability::Mut {
obls.push(make_transmute_obl(dst_ty, src_ty));
if !assume.lifetimes {
obls.push(make_outlives_obl(dst_lifetime, src_lifetime));
}
}

obls
}

/// Flatten the `Condition` tree into a conjunction of obligations.
#[instrument(level = "debug", skip(tcx, obligation))]
fn flatten_answer_tree<'tcx>(
tcx: TyCtxt<'tcx>,
obligation: &PolyTraitObligation<'tcx>,
predicate: TraitPredicate<'tcx>,
cond: Condition<rustc_transmute::layout::rustc::Ref<'tcx>>,
assume: Assume,
) -> Vec<PredicateObligation<'tcx>> {
match cond {
// FIXME(bryangarza): Add separate `IfAny` case, instead of treating as `IfAll`
// Not possible until the trait solver supports disjunctions of obligations
Condition::IfAll(conds) | Condition::IfAny(conds) => conds
.into_iter()
.flat_map(|cond| flatten_answer_tree(tcx, obligation, predicate, cond))
.flat_map(|cond| flatten_answer_tree(tcx, obligation, cond, assume))
.collect(),
Condition::IfTransmutable { src, dst } => {
let transmute_trait = obligation.predicate.def_id();
let assume_const = predicate.trait_ref.args.const_at(2);
let make_transmute_obl = |from_ty, to_ty| {
let trait_ref = ty::TraitRef::new(
tcx,
transmute_trait,
[
ty::GenericArg::from(to_ty),
ty::GenericArg::from(from_ty),
ty::GenericArg::from(assume_const),
],
);
Obligation::with_depth(
tcx,
obligation.cause.clone(),
obligation.recursion_depth + 1,
obligation.param_env,
trait_ref,
)
};

let make_freeze_obl = |ty| {
let trait_ref = ty::TraitRef::new(
tcx,
tcx.require_lang_item(LangItem::Freeze, None),
[ty::GenericArg::from(ty)],
);
Obligation::with_depth(
tcx,
obligation.cause.clone(),
obligation.recursion_depth + 1,
obligation.param_env,
trait_ref,
)
};

let mut obls = vec![];

// If the source is a shared reference, it must be `Freeze`;
// otherwise, transmuting could lead to data races.
if src.mutability == Mutability::Not {
obls.extend([make_freeze_obl(src.ty), make_freeze_obl(dst.ty)])
}

// If Dst is mutable, check bidirectionally.
// For example, transmuting bool -> u8 is OK as long as you can't update that u8
// to be > 1, because you could later transmute the u8 back to a bool and get UB.
match dst.mutability {
Mutability::Not => obls.push(make_transmute_obl(src.ty, dst.ty)),
Mutability::Mut => obls.extend([
make_transmute_obl(src.ty, dst.ty),
make_transmute_obl(dst.ty, src.ty),
]),
}

obls
}
Condition::IfTransmutable { src, dst } => reference_obligations(
tcx,
obligation,
(src.lifetime, src.ty, src.mutability),
(dst.lifetime, dst.ty, dst.mutability),
assume,
),
}
}

// We erase regions here because transmutability calls layout queries,
// which does not handle inference regions and doesn't particularly
// care about other regions. Erasing late-bound regions is equivalent
// to instantiating the binder with placeholders then erasing those
// placeholder regions.
let predicate = self
.tcx()
.erase_regions(self.tcx().instantiate_bound_regions_with_erased(obligation.predicate));
let predicate = obligation.predicate.skip_binder();

let Some(assume) = rustc_transmute::Assume::from_const(
self.infcx.tcx,
Expand All @@ -387,6 +416,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {

let dst = predicate.trait_ref.args.type_at(0);
let src = predicate.trait_ref.args.type_at(1);

debug!(?src, ?dst);
let mut transmute_env = rustc_transmute::TransmuteTypeEnv::new(self.infcx);
let maybe_transmutable = transmute_env.is_transmutable(
Expand All @@ -397,7 +427,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {

let fully_flattened = match maybe_transmutable {
Answer::No(_) => Err(Unimplemented)?,
Answer::If(cond) => flatten_answer_tree(self.tcx(), obligation, predicate, cond),
Answer::If(cond) => flatten_answer_tree(self.tcx(), obligation, cond, assume),
Answer::Yes => vec![],
};

Expand Down
13 changes: 12 additions & 1 deletion compiler/rustc_transmute/src/layout/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@ pub mod rustc {
use std::fmt::{self, Write};

use rustc_middle::mir::Mutability;
use rustc_middle::ty::{self, Ty};
use rustc_middle::ty::layout::{LayoutCx, LayoutError};
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_target::abi::Layout;

/// A reference in the layout.
#[derive(Debug, Hash, Eq, PartialEq, Clone, Copy)]
Expand Down Expand Up @@ -120,4 +122,13 @@ pub mod rustc {
self != &Self::Primitive
}
}

pub(crate) fn layout_of<'tcx>(
cx: LayoutCx<'tcx, TyCtxt<'tcx>>,
ty: Ty<'tcx>,
) -> Result<Layout<'tcx>, &'tcx LayoutError<'tcx>> {
use rustc_middle::ty::layout::LayoutOf;
let ty = cx.tcx.erase_regions(ty);
cx.layout_of(ty).map(|tl| tl.layout)
}
}
Loading

0 comments on commit 636d7ff

Please sign in to comment.