Skip to content

Commit

Permalink
Auto merge of #127982 - matthiaskrgr:rollup-nzyvphj, r=matthiaskrgr
Browse files Browse the repository at this point in the history
Rollup of 6 pull requests

Successful merges:

 - #127295 (CFI: Support provided methods on traits)
 - #127814 (`C-cmse-nonsecure-call`: improved error messages)
 - #127949 (fix: explain E0120 better cover cases when its raised)
 - #127966 (Use structured suggestions for unconstrained generic parameters on impl blocks)
 - #127976 (Lazy type aliases: Diagostics: Detect bivariant ty params that are only used recursively)
 - #127978 (Avoid ref when using format! for perf)

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Jul 19, 2024
2 parents 0cd01aa + e28be0d commit ff4b398
Show file tree
Hide file tree
Showing 28 changed files with 744 additions and 212 deletions.
24 changes: 16 additions & 8 deletions compiler/rustc_error_codes/src/error_codes/E0120.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Drop was implemented on a trait, which is not allowed: only structs and
enums can implement Drop.
`Drop` was implemented on a trait object or reference, which is not allowed;
only structs, enums, and unions can implement Drop.

Erroneous code example:
Erroneous code examples:

```compile_fail,E0120
trait MyTrait {}
Expand All @@ -11,8 +11,16 @@ impl Drop for MyTrait {
}
```

A workaround for this problem is to wrap the trait up in a struct, and implement
Drop on that:
```compile_fail,E0120
struct Concrete {}
impl Drop for &'_ mut Concrete {
fn drop(&mut self) {}
}
```

A workaround for traits is to create a wrapper struct with a generic type,
add a trait bound to the type, and implement `Drop` on the wrapper:

```
trait MyTrait {}
Expand All @@ -24,13 +32,13 @@ impl <T: MyTrait> Drop for MyWrapper<T> {
```

Alternatively, wrapping trait objects requires something:
Alternatively, the `Drop` wrapper can contain the trait object:

```
trait MyTrait {}
//or Box<MyTrait>, if you wanted an owned trait object
struct MyWrapper<'a> { foo: &'a MyTrait }
// or Box<dyn MyTrait>, if you wanted an owned trait object
struct MyWrapper<'a> { foo: &'a dyn MyTrait }
impl <'a> Drop for MyWrapper<'a> {
fn drop(&mut self) {}
Expand Down
39 changes: 39 additions & 0 deletions compiler/rustc_error_codes/src/error_codes/E0798.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
Functions marked as `C-cmse-nonsecure-call` place restrictions on their
inputs and outputs.

- inputs must fit in the 4 available 32-bit argument registers. Alignment
is relevant.
- outputs must either fit in 4 bytes, or be a foundational type of
size 8 (`i64`, `u64`, `f64`).
- no generics can be used in the signature

For more information,
see [arm's aapcs32](https://github.com/ARM-software/abi-aa/releases).

Erroneous code example:

```ignore (only fails on supported targets)
#![feature(abi_c_cmse_nonsecure_call)]
#[no_mangle]
pub fn test(
f: extern "C-cmse-nonsecure-call" fn(u32, u32, u32, u32, u32) -> u32,
) -> u32 {
f(1, 2, 3, 4, 5)
}
```

Arguments' alignment is respected. In the example below, padding is inserted
so that the `u64` argument is passed in registers r2 and r3. There is then no
room left for the final `f32` argument

```ignore (only fails on supported targets)
#![feature(abi_c_cmse_nonsecure_call)]
#[no_mangle]
pub fn test(
f: extern "C-cmse-nonsecure-call" fn(u32, u64, f32) -> u32,
) -> u32 {
f(1, 2, 3.0)
}
```
1 change: 1 addition & 0 deletions compiler/rustc_error_codes/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,7 @@ E0794: 0794,
E0795: 0795,
E0796: 0796,
E0797: 0797,
E0798: 0798,
);
)
}
Expand Down
22 changes: 22 additions & 0 deletions compiler/rustc_hir_analysis/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,23 @@ hir_analysis_cannot_capture_late_bound_ty =
hir_analysis_closure_implicit_hrtb = implicit types in closure signatures are forbidden when `for<...>` is present
.label = `for<...>` is here
hir_analysis_cmse_call_generic =
function pointers with the `"C-cmse-nonsecure-call"` ABI cannot contain generics in their type
hir_analysis_cmse_call_inputs_stack_spill =
arguments for `"C-cmse-nonsecure-call"` function too large to pass via registers
.label = {$plural ->
[false] this argument doesn't
*[true] these arguments don't
} fit in the available registers
.note = functions with the `"C-cmse-nonsecure-call"` ABI must pass all their arguments via the 4 32-bit available argument registers
hir_analysis_cmse_call_output_stack_spill =
return value of `"C-cmse-nonsecure-call"` function too large to pass via registers
.label = this type doesn't fit in the available registers
.note1 = functions with the `"C-cmse-nonsecure-call"` ABI must pass their result via the available return registers
.note2 = the result must either be a (transparently wrapped) i64, u64 or f64, or be at most 4 bytes in size
hir_analysis_coerce_unsized_may = the trait `{$trait_name}` may only be implemented for a coercion between structures
hir_analysis_coerce_unsized_multi = implementing the trait `CoerceUnsized` requires multiple coercions
Expand Down Expand Up @@ -519,6 +536,11 @@ hir_analysis_typeof_reserved_keyword_used =
.suggestion = consider replacing `typeof(...)` with an actual type
.label = reserved keyword
hir_analysis_unconstrained_generic_parameter = the {$param_def_kind} `{$param_name}` is not constrained by the impl trait, self type, or predicates
.label = unconstrained {$param_def_kind}
.const_param_note = expressions using a const parameter must map each value to a distinct output value
.const_param_note2 = proving the result of expressions other than the parameter are unique is not supported
hir_analysis_unconstrained_opaque_type = unconstrained opaque type
.note = `{$name}` must be used in combination with a concrete type within the same {$what}
Expand Down
73 changes: 37 additions & 36 deletions compiler/rustc_hir_analysis/src/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1922,39 +1922,31 @@ fn report_bivariance<'tcx>(
);

if !usage_spans.is_empty() {
// First, check if the ADT is (probably) cyclical. We say probably here, since
// we're not actually looking into substitutions, just walking through fields.
// And we only recurse into the fields of ADTs, and not the hidden types of
// opaques or anything else fancy.
// First, check if the ADT/LTA is (probably) cyclical. We say probably here, since we're
// not actually looking into substitutions, just walking through fields / the "RHS".
// We don't recurse into the hidden types of opaques or anything else fancy.
let item_def_id = item.owner_id.to_def_id();
let is_probably_cyclical = if matches!(
tcx.def_kind(item_def_id),
DefKind::Struct | DefKind::Union | DefKind::Enum
) {
IsProbablyCyclical { tcx, adt_def_id: item_def_id, seen: Default::default() }
.visit_all_fields(tcx.adt_def(item_def_id))
.is_break()
} else {
false
};
// If the ADT is cyclical, then if at least one usage of the type parameter or
// the `Self` alias is present in the, then it's probably a cyclical struct, and
// we should call those parameter usages recursive rather than just saying they're
// unused...
let is_probably_cyclical =
IsProbablyCyclical { tcx, item_def_id, seen: Default::default() }
.visit_def(item_def_id)
.is_break();
// If the ADT/LTA is cyclical, then if at least one usage of the type parameter or
// the `Self` alias is present in the, then it's probably a cyclical struct/ type
// alias, and we should call those parameter usages recursive rather than just saying
// they're unused...
//
// We currently report *all* of the parameter usages, since computing the exact
// subset is very involved, and the fact we're mentioning recursion at all is
// likely to guide the user in the right direction.
if is_probably_cyclical {
let diag = tcx.dcx().create_err(errors::RecursiveGenericParameter {
return tcx.dcx().emit_err(errors::RecursiveGenericParameter {
spans: usage_spans,
param_span: param.span,
param_name,
param_def_kind: tcx.def_descr(param.def_id.to_def_id()),
help,
note: (),
});
return diag.emit();
}
}

Expand All @@ -1974,42 +1966,51 @@ fn report_bivariance<'tcx>(
diag.emit()
}

/// Detects cases where an ADT is trivially cyclical -- we want to detect this so
/// /we only mention that its parameters are used cyclically if the ADT is truly
/// Detects cases where an ADT/LTA is trivially cyclical -- we want to detect this so
/// we only mention that its parameters are used cyclically if the ADT/LTA is truly
/// cyclical.
///
/// Notably, we don't consider substitutions here, so this may have false positives.
struct IsProbablyCyclical<'tcx> {
tcx: TyCtxt<'tcx>,
adt_def_id: DefId,
item_def_id: DefId,
seen: FxHashSet<DefId>,
}

impl<'tcx> IsProbablyCyclical<'tcx> {
fn visit_all_fields(&mut self, adt_def: ty::AdtDef<'tcx>) -> ControlFlow<(), ()> {
for field in adt_def.all_fields() {
self.tcx.type_of(field.did).instantiate_identity().visit_with(self)?;
fn visit_def(&mut self, def_id: DefId) -> ControlFlow<(), ()> {
match self.tcx.def_kind(def_id) {
DefKind::Struct | DefKind::Enum | DefKind::Union => {
self.tcx.adt_def(def_id).all_fields().try_for_each(|field| {
self.tcx.type_of(field.did).instantiate_identity().visit_with(self)
})
}
DefKind::TyAlias if self.tcx.type_alias_is_lazy(def_id) => {
self.tcx.type_of(def_id).instantiate_identity().visit_with(self)
}
_ => ControlFlow::Continue(()),
}

ControlFlow::Continue(())
}
}

impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for IsProbablyCyclical<'tcx> {
type Result = ControlFlow<(), ()>;

fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<(), ()> {
if let Some(adt_def) = t.ty_adt_def() {
if adt_def.did() == self.adt_def_id {
fn visit_ty(&mut self, ty: Ty<'tcx>) -> ControlFlow<(), ()> {
let def_id = match ty.kind() {
ty::Adt(adt_def, _) => Some(adt_def.did()),
ty::Alias(ty::Weak, alias_ty) => Some(alias_ty.def_id),
_ => None,
};
if let Some(def_id) = def_id {
if def_id == self.item_def_id {
return ControlFlow::Break(());
}

if self.seen.insert(adt_def.did()) {
self.visit_all_fields(adt_def)?;
if self.seen.insert(def_id) {
self.visit_def(def_id)?;
}
}

t.super_visit_with(self)
ty.super_visit_with(self)
}
}

Expand Down
41 changes: 41 additions & 0 deletions compiler/rustc_hir_analysis/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1604,6 +1604,20 @@ pub(crate) enum UnusedGenericParameterHelp {
TyAlias { param_name: Ident },
}

#[derive(Diagnostic)]
#[diag(hir_analysis_unconstrained_generic_parameter)]
pub(crate) struct UnconstrainedGenericParameter {
#[primary_span]
#[label]
pub span: Span,
pub param_name: Symbol,
pub param_def_kind: &'static str,
#[note(hir_analysis_const_param_note)]
pub const_param_note: Option<()>,
#[note(hir_analysis_const_param_note2)]
pub const_param_note2: Option<()>,
}

#[derive(Diagnostic)]
pub enum UnnamedFieldsRepr<'a> {
#[diag(hir_analysis_unnamed_fields_repr_missing_repr_c)]
Expand Down Expand Up @@ -1673,3 +1687,30 @@ pub struct InvalidReceiverTy<'tcx> {
#[note]
#[help]
pub struct EffectsWithoutNextSolver;

#[derive(Diagnostic)]
#[diag(hir_analysis_cmse_call_inputs_stack_spill, code = E0798)]
#[note]
pub struct CmseCallInputsStackSpill {
#[primary_span]
#[label]
pub span: Span,
pub plural: bool,
}

#[derive(Diagnostic)]
#[diag(hir_analysis_cmse_call_output_stack_spill, code = E0798)]
#[note(hir_analysis_note1)]
#[note(hir_analysis_note2)]
pub struct CmseCallOutputStackSpill {
#[primary_span]
#[label]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(hir_analysis_cmse_call_generic, code = E0798)]
pub struct CmseCallGeneric {
#[primary_span]
pub span: Span,
}
Loading

0 comments on commit ff4b398

Please sign in to comment.