From 61fd92e28411c2594c9f46cac9d96b96ef894d90 Mon Sep 17 00:00:00 2001 From: rohit141914 Date: Sat, 7 Dec 2024 01:01:03 +0530 Subject: [PATCH 01/13] Removed Unnecessary Spaces From RELEASES.md --- RELEASES.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/RELEASES.md b/RELEASES.md index 8702bb021184a..99733bade32c4 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -503,7 +503,7 @@ Compatibility Notes * We have renamed `std::panic::PanicInfo` to `std::panic::PanicHookInfo`. The old name will continue to work as an alias, but will result in a deprecation warning starting in Rust 1.82.0. `core::panic::PanicInfo` will remain unchanged, however, as this is now a *different type*. - + The reason is that these types have different roles: `std::panic::PanicHookInfo` is the argument to the [panic hook](https://doc.rust-lang.org/stable/std/panic/fn.set_hook.html) in std context (where panics can have an arbitrary payload), while `core::panic::PanicInfo` is the argument to the [`#[panic_handler]`](https://doc.rust-lang.org/nomicon/panic-handler.html) in no_std context (where panics always carry a formatted *message*). Separating these types allows us to add more useful methods to these types, such as `std::panic::PanicHookInfo::payload_as_str()` and `core::panic::PanicInfo::message()`. * The new sort implementations may panic if a type's implementation of [`Ord`](https://doc.rust-lang.org/std/cmp/trait.Ord.html) (or the given comparison function) does not implement a [total order](https://en.wikipedia.org/wiki/Total_order) as the trait requires. `Ord`'s supertraits (`PartialOrd`, `Eq`, and `PartialEq`) must also be consistent. The previous implementations would not "notice" any problem, but the new implementations have a good chance of detecting inconsistencies, throwing a panic rather than returning knowingly unsorted data. @@ -584,7 +584,7 @@ Stabilized APIs - [`impl Default for Arc`](https://doc.rust-lang.org/beta/alloc/sync/struct.Arc.html#impl-Default-for-Arc%3CCStr%3E) - [`impl Default for Arc<[T]>`](https://doc.rust-lang.org/beta/alloc/sync/struct.Arc.html#impl-Default-for-Arc%3C%5BT%5D%3E) - [`impl IntoIterator for Box<[T]>`](https://doc.rust-lang.org/beta/alloc/boxed/struct.Box.html#impl-IntoIterator-for-Box%3C%5BI%5D,+A%3E) -- [`impl FromIterator for Box`](https://doc.rust-lang.org/beta/alloc/boxed/struct.Box.html#impl-FromIterator%3CString%3E-for-Box%3Cstr%3E) +- [`impl FromIterator for Box`](https://doc.rust-lang.org/beta/alloc/boxed/struct.Box.html#impl-FromIterator%3CString%3E-for-Box%3Cstr%3E) - [`impl FromIterator for Box`](https://doc.rust-lang.org/beta/alloc/boxed/struct.Box.html#impl-FromIterator%3Cchar%3E-for-Box%3Cstr%3E) - [`LazyCell`](https://doc.rust-lang.org/beta/core/cell/struct.LazyCell.html) - [`LazyLock`](https://doc.rust-lang.org/beta/std/sync/struct.LazyLock.html) @@ -1816,7 +1816,7 @@ Compiler - [Detect uninhabited types early in const eval](https://github.com/rust-lang/rust/pull/109435/) - [Switch to LLD as default linker for {arm,thumb}v4t-none-eabi](https://github.com/rust-lang/rust/pull/109721/) - [Add tier 3 target `loongarch64-unknown-linux-gnu`](https://github.com/rust-lang/rust/pull/96971) -- [Add tier 3 target for `i586-pc-nto-qnx700` (QNX Neutrino RTOS, version 7.0)](https://github.com/rust-lang/rust/pull/109173/), +- [Add tier 3 target for `i586-pc-nto-qnx700` (QNX Neutrino RTOS, version 7.0)](https://github.com/rust-lang/rust/pull/109173/), - [Insert alignment checks for pointer dereferences as debug assertions](https://github.com/rust-lang/rust/pull/98112) This catches undefined behavior at runtime, and may cause existing code to fail. @@ -2023,7 +2023,7 @@ Compatibility Notes If `tools = [...]` is set in config.toml, we will respect a missing rustdoc in that list. By default rustdoc remains included. To retain the prior behavior explicitly add `"rustdoc"` to the list. - + Internal Changes From f021d99cc8c83ccf08d1afcd2d0b4ede5744f317 Mon Sep 17 00:00:00 2001 From: niacdoial Date: Tue, 5 Nov 2024 22:54:48 +0100 Subject: [PATCH 02/13] lint: revamp ImproperCTypes diagnostic architecture for nested notes and help messages --- compiler/rustc_lint/src/lints.rs | 45 ++++++++-- compiler/rustc_lint/src/types.rs | 146 +++++++++++++++++++++++-------- 2 files changed, 146 insertions(+), 45 deletions(-) diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 20822f23bf15d..9fa263799ebf1 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1851,13 +1851,44 @@ pub(crate) struct UnpredictableFunctionPointerComparisonsSuggestion<'a> { pub right: Span, } +pub(crate) struct ImproperCTypesLayer<'a> { + pub ty: Ty<'a>, + pub inner_ty: Option>, + pub note: DiagMessage, + pub span_note: Option, + pub help: Option, +} + +impl<'a> Subdiagnostic for ImproperCTypesLayer<'a> { + fn add_to_diag_with>( + self, + diag: &mut Diag<'_, G>, + f: &F, + ) { + diag.arg("ty", self.ty); + if let Some(ty) = self.inner_ty { + diag.arg("inner_ty", ty); + } + + if let Some(help) = self.help { + let msg = f(diag, help.into()); + diag.help(msg); + } + + let msg = f(diag, self.note.into()); + diag.note(msg); + if let Some(note) = self.span_note { + let msg = f(diag, fluent::lint_note.into()); + diag.span_note(note, msg); + }; + } +} + pub(crate) struct ImproperCTypes<'a> { pub ty: Ty<'a>, pub desc: &'a str, pub label: Span, - pub help: Option, - pub note: DiagMessage, - pub span_note: Option, + pub reasons: Vec>, } // Used because of the complexity of Option, DiagMessage, and Option @@ -1867,12 +1898,8 @@ impl<'a> LintDiagnostic<'a, ()> for ImproperCTypes<'_> { diag.arg("ty", self.ty); diag.arg("desc", self.desc); diag.span_label(self.label, fluent::lint_label); - if let Some(help) = self.help { - diag.help(help); - } - diag.note(self.note); - if let Some(note) = self.span_note { - diag.span_note(note, fluent::lint_note); + for reason in self.reasons.into_iter() { + diag.subdiagnostic(reason); } } } diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index 33650be056dd1..26ec9e7f21efe 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -22,10 +22,10 @@ mod improper_ctypes; use crate::lints::{ AmbiguousWidePointerComparisons, AmbiguousWidePointerComparisonsAddrMetadataSuggestion, AmbiguousWidePointerComparisonsAddrSuggestion, AtomicOrderingFence, AtomicOrderingLoad, - AtomicOrderingStore, ImproperCTypes, InvalidAtomicOrderingDiag, InvalidNanComparisons, - InvalidNanComparisonsSuggestion, UnpredictableFunctionPointerComparisons, - UnpredictableFunctionPointerComparisonsSuggestion, UnusedComparisons, - VariantSizeDifferencesDiag, + AtomicOrderingStore, ImproperCTypes, ImproperCTypesLayer, InvalidAtomicOrderingDiag, + InvalidNanComparisons, InvalidNanComparisonsSuggestion, + UnpredictableFunctionPointerComparisons, UnpredictableFunctionPointerComparisonsSuggestion, + UnusedComparisons, VariantSizeDifferencesDiag, }; use crate::{LateContext, LateLintPass, LintContext, fluent_generated as fluent}; @@ -727,7 +727,19 @@ struct CTypesVisitorState<'tcx> { enum FfiResult<'tcx> { FfiSafe, FfiPhantom(Ty<'tcx>), - FfiUnsafe { ty: Ty<'tcx>, reason: DiagMessage, help: Option }, + FfiUnsafe { + ty: Ty<'tcx>, + reason: DiagMessage, + help: Option, + }, + // NOTE: this `allow` is only here for one retroactively-added commit + #[allow(dead_code)] + FfiUnsafeWrapper { + ty: Ty<'tcx>, + reason: DiagMessage, + help: Option, + wrapped: Box>, + }, } pub(crate) fn nonnull_optimization_guaranteed<'tcx>( @@ -933,12 +945,13 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { /// Check if the type is array and emit an unsafe type lint. fn check_for_array_ty(&mut self, sp: Span, ty: Ty<'tcx>) -> bool { if let ty::Array(..) = ty.kind() { - self.emit_ffi_unsafe_type_lint( + self.emit_ffi_unsafe_type_lint(ty.clone(), sp, vec![ImproperCTypesLayer { ty, - sp, - fluent::lint_improper_ctypes_array_reason, - Some(fluent::lint_improper_ctypes_array_help), - ); + note: fluent::lint_improper_ctypes_array_reason, + help: Some(fluent::lint_improper_ctypes_array_help), + inner_ty: None, + span_note: None, + }]); true } else { false @@ -995,9 +1008,9 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { all_phantom &= match self.check_field_type_for_ffi(acc, field, args) { FfiSafe => false, // `()` fields are FFI-safe! - FfiUnsafe { ty, .. } if ty.is_unit() => false, + FfiUnsafe { ty, .. } | FfiUnsafeWrapper { ty, .. } if ty.is_unit() => false, FfiPhantom(..) => true, - r @ FfiUnsafe { .. } => return r, + r @ (FfiUnsafe { .. } | FfiUnsafeWrapper { .. }) => return r, } } @@ -1278,8 +1291,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { &mut self, ty: Ty<'tcx>, sp: Span, - note: DiagMessage, - help: Option, + mut reasons: Vec>, ) { let lint = match self.mode { CItemKind::Declaration => IMPROPER_CTYPES, @@ -1289,21 +1301,17 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { CItemKind::Declaration => "block", CItemKind::Definition => "fn", }; - let span_note = if let ty::Adt(def, _) = ty.kind() - && let Some(sp) = self.cx.tcx.hir().span_if_local(def.did()) - { - Some(sp) - } else { - None - }; - self.cx.emit_span_lint(lint, sp, ImproperCTypes { - ty, - desc, - label: sp, - help, - note, - span_note, - }); + for reason in reasons.iter_mut() { + reason.span_note = if let ty::Adt(def, _) = reason.ty.kind() + && let Some(sp) = self.cx.tcx.hir().span_if_local(def.did()) + { + Some(sp) + } else { + None + }; + } + + self.cx.emit_span_lint(lint, sp, ImproperCTypes { ty, desc, label: sp, reasons }); } fn check_for_opaque_ty(&mut self, sp: Span, ty: Ty<'tcx>) -> bool { @@ -1332,7 +1340,13 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { .visit_with(&mut ProhibitOpaqueTypes) .break_value() { - self.emit_ffi_unsafe_type_lint(ty, sp, fluent::lint_improper_ctypes_opaque, None); + self.emit_ffi_unsafe_type_lint(ty.clone(), sp, vec![ImproperCTypesLayer { + ty, + note: fluent::lint_improper_ctypes_opaque, + span_note: Some(sp), + help: None, + inner_ty: None, + }]); true } else { false @@ -1371,15 +1385,75 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { match self.check_type_for_ffi(&mut acc, ty) { FfiResult::FfiSafe => {} FfiResult::FfiPhantom(ty) => { - self.emit_ffi_unsafe_type_lint( + self.emit_ffi_unsafe_type_lint(ty.clone(), sp, vec![ImproperCTypesLayer { ty, - sp, - fluent::lint_improper_ctypes_only_phantomdata, - None, - ); + note: fluent::lint_improper_ctypes_only_phantomdata, + span_note: None, // filled later + help: None, + inner_ty: None, + }]); } FfiResult::FfiUnsafe { ty, reason, help } => { - self.emit_ffi_unsafe_type_lint(ty, sp, reason, help); + self.emit_ffi_unsafe_type_lint(ty.clone(), sp, vec![ImproperCTypesLayer { + ty, + help, + note: reason, + span_note: None, // filled later + inner_ty: None, + }]); + } + ffir @ FfiResult::FfiUnsafeWrapper { .. } => { + let mut last_ty = None; + let mut ffiresult_recursor = Some(&ffir); + let mut cimproper_layers: Vec> = vec![]; + + // this whole while block converts the arbitrarily-deep + // FfiResult stack to a ImproperCTypesLayer Vec + while let Some(ref ffir_rec) = ffiresult_recursor { + match ffir_rec { + FfiResult::FfiPhantom(ty) => { + last_ty = Some(ty.clone()); + let len = cimproper_layers.len(); + if len > 0 { + cimproper_layers[len - 1].inner_ty = last_ty.clone(); + } + cimproper_layers.push(ImproperCTypesLayer { + ty: ty.clone(), + inner_ty: None, + help: None, + note: fluent::lint_improper_ctypes_only_phantomdata, + span_note: None, // filled later + }); + ffiresult_recursor = None; + } + FfiResult::FfiUnsafe { ty, reason, help } + | FfiResult::FfiUnsafeWrapper { ty, reason, help, .. } => { + last_ty = Some(ty.clone()); + let len = cimproper_layers.len(); + if len > 0 { + cimproper_layers[len - 1].inner_ty = last_ty.clone(); + } + cimproper_layers.push(ImproperCTypesLayer { + ty: ty.clone(), + inner_ty: None, + help: help.clone(), + note: reason.clone(), + span_note: None, // filled later + }); + + if let FfiResult::FfiUnsafeWrapper { wrapped, .. } = ffir_rec { + ffiresult_recursor = Some(wrapped.as_ref()); + } else { + ffiresult_recursor = None; + } + } + FfiResult::FfiSafe => { + bug!("malformed FfiResult stack: it should be unsafe all the way down") + } + }; + } + let last_ty = last_ty.unwrap(); // populated by any run of the `while` block + self.emit_ffi_unsafe_type_lint(last_ty, sp, cimproper_layers); } } } From 1d521310439ecb0e243a5056768175a51e40a9b6 Mon Sep 17 00:00:00 2001 From: niacdoial Date: Tue, 5 Nov 2024 23:43:44 +0100 Subject: [PATCH 03/13] lint: rework some ImproperCTypes messages (especially around indirections to !Sized) --- compiler/rustc_lint/messages.ftl | 13 +- compiler/rustc_lint/src/types.rs | 136 +++++++++++++++--- ...extern-C-non-FFI-safe-arg-ice-52334.stderr | 2 + .../extern/extern-C-str-arg-ice-80125.stderr | 2 + tests/ui/lint/extern-C-fnptr-lints-slices.rs | 2 +- .../lint/extern-C-fnptr-lints-slices.stderr | 7 +- tests/ui/lint/lint-ctypes-73249-2.stderr | 1 + tests/ui/lint/lint-ctypes-94223.stderr | 26 +++- tests/ui/lint/lint-ctypes-cstr.rs | 4 +- tests/ui/lint/lint-ctypes-cstr.stderr | 11 +- tests/ui/lint/lint-ctypes-fn.rs | 6 +- tests/ui/lint/lint-ctypes-fn.stderr | 22 +-- tests/ui/lint/lint-ctypes.rs | 8 +- tests/ui/lint/lint-ctypes.stderr | 22 +-- 14 files changed, 195 insertions(+), 67 deletions(-) diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 49e6b763590b0..b1564bb11b243 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -359,7 +359,6 @@ lint_improper_ctypes_128bit = 128-bit integers don't currently have a known stab lint_improper_ctypes_array_help = consider passing a pointer to the array lint_improper_ctypes_array_reason = passing raw arrays by value is not FFI-safe -lint_improper_ctypes_box = box cannot be represented as a single pointer lint_improper_ctypes_char_help = consider using `u32` or `libc::wchar_t` instead @@ -377,7 +376,9 @@ lint_improper_ctypes_enum_repr_help = lint_improper_ctypes_enum_repr_reason = enum has no representation hint lint_improper_ctypes_fnptr_help = consider using an `extern fn(...) -> ...` function pointer instead +lint_improper_ctypes_fnptr_indirect_reason = the function pointer to `{$ty}` is FFI-unsafe due to `{$inner_ty}` lint_improper_ctypes_fnptr_reason = this function pointer has Rust-specific calling convention + lint_improper_ctypes_non_exhaustive = this enum is non-exhaustive lint_improper_ctypes_non_exhaustive_variant = this enum has non-exhaustive variants @@ -388,7 +389,11 @@ lint_improper_ctypes_opaque = opaque types have no C equivalent lint_improper_ctypes_pat_help = consider using the base type instead lint_improper_ctypes_pat_reason = pattern types have no C equivalent -lint_improper_ctypes_slice_help = consider using a raw pointer instead + +lint_improper_ctypes_sized_ptr_to_unsafe_type = + this reference (`{$ty}`) is can safely be translated into a C pointer, but `{$inner_ty}` itself is not FFI-safe + +lint_improper_ctypes_slice_help = consider using a raw pointer to the slice's first element (and a length) instead lint_improper_ctypes_slice_reason = slices have no C equivalent lint_improper_ctypes_str_help = consider using `*const u8` and a length instead @@ -414,6 +419,10 @@ lint_improper_ctypes_union_layout_help = consider adding a `#[repr(C)]` or `#[re lint_improper_ctypes_union_layout_reason = this union has unspecified layout lint_improper_ctypes_union_non_exhaustive = this union is non-exhaustive +lint_improper_ctypes_unsized_box = this box for an unsized rust type contains metadata, which makes it incompatible with a C pointer +lint_improper_ctypes_unsized_ptr = this pointer to an unsized rust type contains metadata, which makes it incompatible with a C pointer +lint_improper_ctypes_unsized_ref = this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer + lint_incomplete_include = include macro expected single expression in source diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index 26ec9e7f21efe..111be742696b9 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -732,8 +732,6 @@ enum FfiResult<'tcx> { reason: DiagMessage, help: Option, }, - // NOTE: this `allow` is only here for one retroactively-added commit - #[allow(dead_code)] FfiUnsafeWrapper { ty: Ty<'tcx>, reason: DiagMessage, @@ -1044,16 +1042,47 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { match *ty.kind() { ty::Adt(def, args) => { - if let Some(boxed) = ty.boxed_ty() - && matches!(self.mode, CItemKind::Definition) - { - if boxed.is_sized(tcx, self.cx.typing_env()) { - return FfiSafe; + if let Some(inner_ty) = ty.boxed_ty() { + if inner_ty.is_sized(tcx, self.cx.typing_env()) + || matches!(inner_ty.kind(), ty::Foreign(..)) + { + // discussion on declaration vs definition: + // see the `ty::RawPtr(inner_ty, _) | ty::Ref(_, inner_ty, _)` arm + // of this `match *ty.kind()` block + if matches!(self.mode, CItemKind::Definition) { + return FfiSafe; + } else { + let inner_res = self.check_type_for_ffi(acc, inner_ty); + return match inner_res { + FfiUnsafe { .. } | FfiUnsafeWrapper { .. } => FfiUnsafeWrapper { + ty, + reason: fluent::lint_improper_ctypes_sized_ptr_to_unsafe_type, + wrapped: Box::new(inner_res), + help: None, + }, + _ => inner_res, + }; + } } else { + let help = match inner_ty.kind() { + ty::Str => Some(fluent::lint_improper_ctypes_str_help), + ty::Slice(_) => Some(fluent::lint_improper_ctypes_slice_help), + ty::Adt(def, _) + if matches!(def.adt_kind(), AdtKind::Struct | AdtKind::Union) + && matches!( + tcx.get_diagnostic_name(def.did()), + Some(sym::cstring_type | sym::cstr_type) + ) + && !acc.base_ty.is_mutable_ptr() => + { + Some(fluent::lint_improper_ctypes_cstr_help) + } + _ => None, + }; return FfiUnsafe { ty, - reason: fluent::lint_improper_ctypes_box, - help: None, + reason: fluent::lint_improper_ctypes_unsized_box, + help, }; } } @@ -1209,15 +1238,6 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { help: Some(fluent::lint_improper_ctypes_tuple_help), }, - ty::RawPtr(ty, _) | ty::Ref(_, ty, _) - if { - matches!(self.mode, CItemKind::Definition) - && ty.is_sized(self.cx.tcx, self.cx.typing_env()) - } => - { - FfiSafe - } - ty::RawPtr(ty, _) if match ty.kind() { ty::Tuple(tuple) => tuple.is_empty(), @@ -1227,7 +1247,66 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { FfiSafe } - ty::RawPtr(ty, _) | ty::Ref(_, ty, _) => self.check_type_for_ffi(acc, ty), + ty::RawPtr(inner_ty, _) | ty::Ref(_, inner_ty, _) => { + if inner_ty.is_sized(tcx, self.cx.typing_env()) + || matches!(inner_ty.kind(), ty::Foreign(..)) + { + // there's a nuance on what this lint should do for function definitions + // (touched upon in https://github.com/rust-lang/rust/issues/66220 and https://github.com/rust-lang/rust/pull/72700) + // + // (`extern "C" fn fn_name(...) {...}`) versus declarations (`extern "C" {fn fn_name(...);}`). + // The big question is: what does "ABI safety" mean? if you have something translated to a C pointer + // (which has a stable layout) but points to FFI-unsafe type, is it safe? + // on one hand, the function's ABI will match that of a similar C-declared function API, + // on the other, dereferencing the pointer in not-rust will be painful. + // In this code, the opinion is split between function declarations and function definitions. + // For declarations, we see this as unsafe, but for definitions, we see this as safe. + // This is mostly because, for extern function declarations, the actual definition of the function is written somewhere else, + // so the fact that a pointer's pointee should be treated as opaque to one side or the other can be explicitely written out. + // For extern function definitions, however, both callee and some callers can be written in rust, + // so developers need to keep as much typing information as possible. + if matches!(self.mode, CItemKind::Definition) { + return FfiSafe; + } else if matches!(ty.kind(), ty::RawPtr(..)) + && matches!(inner_ty.kind(), ty::Tuple(tuple) if tuple.is_empty()) + { + FfiSafe + } else { + let inner_res = self.check_type_for_ffi(acc, inner_ty); + return match inner_res { + FfiSafe => inner_res, + _ => FfiUnsafeWrapper { + ty, + reason: fluent::lint_improper_ctypes_sized_ptr_to_unsafe_type, + wrapped: Box::new(inner_res), + help: None, + }, + }; + } + } else { + let help = match inner_ty.kind() { + ty::Str => Some(fluent::lint_improper_ctypes_str_help), + ty::Slice(_) => Some(fluent::lint_improper_ctypes_slice_help), + ty::Adt(def, _) + if matches!(def.adt_kind(), AdtKind::Struct | AdtKind::Union) + && matches!( + tcx.get_diagnostic_name(def.did()), + Some(sym::cstring_type | sym::cstr_type) + ) + && !acc.base_ty.is_mutable_ptr() => + { + Some(fluent::lint_improper_ctypes_cstr_help) + } + _ => None, + }; + let reason = match ty.kind() { + ty::RawPtr(..) => fluent::lint_improper_ctypes_unsized_ptr, + ty::Ref(..) => fluent::lint_improper_ctypes_unsized_ref, + _ => unreachable!(), + }; + FfiUnsafe { ty, reason, help } + } + } ty::Array(inner_ty, _) => self.check_type_for_ffi(acc, inner_ty), @@ -1245,7 +1324,14 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { for arg in sig.inputs() { match self.check_type_for_ffi(acc, *arg) { FfiSafe => {} - r => return r, + r => { + return FfiUnsafeWrapper { + ty, + reason: fluent::lint_improper_ctypes_fnptr_indirect_reason, + help: None, + wrapped: Box::new(r), + }; + } } } @@ -1254,7 +1340,15 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { return FfiSafe; } - self.check_type_for_ffi(acc, ret_ty) + match self.check_type_for_ffi(acc, ret_ty) { + r @ (FfiSafe | FfiPhantom(_)) => r, + r => FfiUnsafeWrapper { + ty: ty.clone(), + reason: fluent::lint_improper_ctypes_fnptr_indirect_reason, + help: None, + wrapped: Box::new(r), + }, + } } ty::Foreign(..) => FfiSafe, diff --git a/tests/ui/extern/extern-C-non-FFI-safe-arg-ice-52334.stderr b/tests/ui/extern/extern-C-non-FFI-safe-arg-ice-52334.stderr index 044c1ae2dd42f..b5c718ec38147 100644 --- a/tests/ui/extern/extern-C-non-FFI-safe-arg-ice-52334.stderr +++ b/tests/ui/extern/extern-C-non-FFI-safe-arg-ice-52334.stderr @@ -4,6 +4,7 @@ warning: `extern` fn uses type `CStr`, which is not FFI-safe LL | type Foo = extern "C" fn(::std::ffi::CStr); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe | + = note: the function pointer to `extern "C" fn(CStr)` is FFI-unsafe due to `CStr` = help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()` = note: `CStr`/`CString` do not have a guaranteed layout = note: `#[warn(improper_ctypes_definitions)]` on by default @@ -14,6 +15,7 @@ warning: `extern` block uses type `CStr`, which is not FFI-safe LL | fn meh(blah: Foo); | ^^^ not FFI-safe | + = note: the function pointer to `extern "C" fn(CStr)` is FFI-unsafe due to `CStr` = help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()` = note: `CStr`/`CString` do not have a guaranteed layout = note: `#[warn(improper_ctypes)]` on by default diff --git a/tests/ui/extern/extern-C-str-arg-ice-80125.stderr b/tests/ui/extern/extern-C-str-arg-ice-80125.stderr index ebd6cec6ecd3f..f2ee21c316658 100644 --- a/tests/ui/extern/extern-C-str-arg-ice-80125.stderr +++ b/tests/ui/extern/extern-C-str-arg-ice-80125.stderr @@ -4,6 +4,7 @@ warning: `extern` fn uses type `str`, which is not FFI-safe LL | type ExternCallback = extern "C" fn(*const u8, u32, str); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe | + = note: the function pointer to `extern "C" fn(*const u8, u32, str)` is FFI-unsafe due to `str` = help: consider using `*const u8` and a length instead = note: string slices have no C equivalent = note: `#[warn(improper_ctypes_definitions)]` on by default @@ -14,6 +15,7 @@ warning: `extern` fn uses type `str`, which is not FFI-safe LL | pub extern "C" fn register_something(bind: ExternCallback) -> Struct { | ^^^^^^^^^^^^^^ not FFI-safe | + = note: the function pointer to `extern "C" fn(*const u8, u32, str)` is FFI-unsafe due to `str` = help: consider using `*const u8` and a length instead = note: string slices have no C equivalent diff --git a/tests/ui/lint/extern-C-fnptr-lints-slices.rs b/tests/ui/lint/extern-C-fnptr-lints-slices.rs index 0c35eb37a4890..4e3832ab1b672 100644 --- a/tests/ui/lint/extern-C-fnptr-lints-slices.rs +++ b/tests/ui/lint/extern-C-fnptr-lints-slices.rs @@ -3,7 +3,7 @@ // It's an improper ctype (a slice) arg in an extern "C" fnptr. pub type F = extern "C" fn(&[u8]); -//~^ ERROR: `extern` fn uses type `[u8]`, which is not FFI-safe +//~^ ERROR: `extern` fn uses type `&[u8]`, which is not FFI-safe fn main() {} diff --git a/tests/ui/lint/extern-C-fnptr-lints-slices.stderr b/tests/ui/lint/extern-C-fnptr-lints-slices.stderr index d13f93ca96f22..bbd16ce8ce16e 100644 --- a/tests/ui/lint/extern-C-fnptr-lints-slices.stderr +++ b/tests/ui/lint/extern-C-fnptr-lints-slices.stderr @@ -1,11 +1,12 @@ -error: `extern` fn uses type `[u8]`, which is not FFI-safe +error: `extern` fn uses type `&[u8]`, which is not FFI-safe --> $DIR/extern-C-fnptr-lints-slices.rs:5:14 | LL | pub type F = extern "C" fn(&[u8]); | ^^^^^^^^^^^^^^^^^^^^ not FFI-safe | - = help: consider using a raw pointer instead - = note: slices have no C equivalent + = note: the function pointer to `for<'a> extern "C" fn(&'a [u8])` is FFI-unsafe due to `&[u8]` + = help: consider using a raw pointer to the slice's first element (and a length) instead + = note: this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer note: the lint level is defined here --> $DIR/extern-C-fnptr-lints-slices.rs:1:8 | diff --git a/tests/ui/lint/lint-ctypes-73249-2.stderr b/tests/ui/lint/lint-ctypes-73249-2.stderr index ef30a406969d3..9143e57174e40 100644 --- a/tests/ui/lint/lint-ctypes-73249-2.stderr +++ b/tests/ui/lint/lint-ctypes-73249-2.stderr @@ -4,6 +4,7 @@ error: `extern` block uses type `Qux`, which is not FFI-safe LL | fn lint_me() -> A<()>; | ^^^^^ not FFI-safe | + = note: this reference (`&Qux`) is can safely be translated into a C pointer, but `Qux` itself is not FFI-safe = note: opaque types have no C equivalent note: the lint level is defined here --> $DIR/lint-ctypes-73249-2.rs:2:9 diff --git a/tests/ui/lint/lint-ctypes-94223.stderr b/tests/ui/lint/lint-ctypes-94223.stderr index bd127cf60044c..4bebca69b7f3b 100644 --- a/tests/ui/lint/lint-ctypes-94223.stderr +++ b/tests/ui/lint/lint-ctypes-94223.stderr @@ -4,7 +4,8 @@ error: `extern` fn uses type `[u8]`, which is not FFI-safe LL | pub fn bad(f: extern "C" fn([u8])) {} | ^^^^^^^^^^^^^^^^^^^ not FFI-safe | - = help: consider using a raw pointer instead + = note: the function pointer to `extern "C" fn([u8])` is FFI-unsafe due to `[u8]` + = help: consider using a raw pointer to the slice's first element (and a length) instead = note: slices have no C equivalent note: the lint level is defined here --> $DIR/lint-ctypes-94223.rs:2:9 @@ -18,7 +19,8 @@ error: `extern` fn uses type `[u8]`, which is not FFI-safe LL | pub fn bad_twice(f: Result) {} | ^^^^^^^^^^^^^^^^^^^ not FFI-safe | - = help: consider using a raw pointer instead + = note: the function pointer to `extern "C" fn([u8])` is FFI-unsafe due to `[u8]` + = help: consider using a raw pointer to the slice's first element (and a length) instead = note: slices have no C equivalent error: `extern` fn uses type `[u8]`, which is not FFI-safe @@ -27,7 +29,8 @@ error: `extern` fn uses type `[u8]`, which is not FFI-safe LL | pub fn bad_twice(f: Result) {} | ^^^^^^^^^^^^^^^^^^^ not FFI-safe | - = help: consider using a raw pointer instead + = note: the function pointer to `extern "C" fn([u8])` is FFI-unsafe due to `[u8]` + = help: consider using a raw pointer to the slice's first element (and a length) instead = note: slices have no C equivalent error: `extern` fn uses type `[u8]`, which is not FFI-safe @@ -36,7 +39,8 @@ error: `extern` fn uses type `[u8]`, which is not FFI-safe LL | struct BadStruct(extern "C" fn([u8])); | ^^^^^^^^^^^^^^^^^^^ not FFI-safe | - = help: consider using a raw pointer instead + = note: the function pointer to `extern "C" fn([u8])` is FFI-unsafe due to `[u8]` + = help: consider using a raw pointer to the slice's first element (and a length) instead = note: slices have no C equivalent error: `extern` fn uses type `[u8]`, which is not FFI-safe @@ -45,7 +49,8 @@ error: `extern` fn uses type `[u8]`, which is not FFI-safe LL | A(extern "C" fn([u8])), | ^^^^^^^^^^^^^^^^^^^ not FFI-safe | - = help: consider using a raw pointer instead + = note: the function pointer to `extern "C" fn([u8])` is FFI-unsafe due to `[u8]` + = help: consider using a raw pointer to the slice's first element (and a length) instead = note: slices have no C equivalent error: `extern` fn uses type `[u8]`, which is not FFI-safe @@ -54,7 +59,8 @@ error: `extern` fn uses type `[u8]`, which is not FFI-safe LL | A(extern "C" fn([u8])), | ^^^^^^^^^^^^^^^^^^^ not FFI-safe | - = help: consider using a raw pointer instead + = note: the function pointer to `extern "C" fn([u8])` is FFI-unsafe due to `[u8]` + = help: consider using a raw pointer to the slice's first element (and a length) instead = note: slices have no C equivalent error: `extern` fn uses type `[u8]`, which is not FFI-safe @@ -63,7 +69,8 @@ error: `extern` fn uses type `[u8]`, which is not FFI-safe LL | type Foo = extern "C" fn([u8]); | ^^^^^^^^^^^^^^^^^^^ not FFI-safe | - = help: consider using a raw pointer instead + = note: the function pointer to `extern "C" fn([u8])` is FFI-unsafe due to `[u8]` + = help: consider using a raw pointer to the slice's first element (and a length) instead = note: slices have no C equivalent error: `extern` fn uses type `Option<&::FooType>`, which is not FFI-safe @@ -72,6 +79,7 @@ error: `extern` fn uses type `Option<&::FooType>`, which is not F LL | pub type Foo2 = extern "C" fn(Option<&::FooType>); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe | + = note: the function pointer to `for<'a> extern "C" fn(Option<&'a ::FooType>)` is FFI-unsafe due to `Option<&::FooType>` = help: consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum = note: enum has no representation hint @@ -81,6 +89,7 @@ error: `extern` fn uses type `FfiUnsafe`, which is not FFI-safe LL | pub static BAD: extern "C" fn(FfiUnsafe) = f; | ^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe | + = note: the function pointer to `extern "C" fn(FfiUnsafe)` is FFI-unsafe due to `FfiUnsafe` = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct = note: this struct has unspecified layout note: the type is defined here @@ -95,6 +104,7 @@ error: `extern` fn uses type `FfiUnsafe`, which is not FFI-safe LL | pub static BAD_TWICE: Result = Ok(f); | ^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe | + = note: the function pointer to `extern "C" fn(FfiUnsafe)` is FFI-unsafe due to `FfiUnsafe` = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct = note: this struct has unspecified layout note: the type is defined here @@ -109,6 +119,7 @@ error: `extern` fn uses type `FfiUnsafe`, which is not FFI-safe LL | pub static BAD_TWICE: Result = Ok(f); | ^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe | + = note: the function pointer to `extern "C" fn(FfiUnsafe)` is FFI-unsafe due to `FfiUnsafe` = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct = note: this struct has unspecified layout note: the type is defined here @@ -123,6 +134,7 @@ error: `extern` fn uses type `FfiUnsafe`, which is not FFI-safe LL | pub const BAD_CONST: extern "C" fn(FfiUnsafe) = f; | ^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe | + = note: the function pointer to `extern "C" fn(FfiUnsafe)` is FFI-unsafe due to `FfiUnsafe` = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct = note: this struct has unspecified layout note: the type is defined here diff --git a/tests/ui/lint/lint-ctypes-cstr.rs b/tests/ui/lint/lint-ctypes-cstr.rs index b04decd0bcacc..c4de5a44a9623 100644 --- a/tests/ui/lint/lint-ctypes-cstr.rs +++ b/tests/ui/lint/lint-ctypes-cstr.rs @@ -8,7 +8,7 @@ extern "C" { //~^ ERROR `extern` block uses type `CStr`, which is not FFI-safe //~| HELP consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()` fn take_cstr_ref(s: &CStr); - //~^ ERROR `extern` block uses type `CStr`, which is not FFI-safe + //~^ ERROR `extern` block uses type `&CStr`, which is not FFI-safe //~| HELP consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()` fn take_cstring(s: CString); //~^ ERROR `extern` block uses type `CString`, which is not FFI-safe @@ -27,7 +27,7 @@ extern "C" { } extern "C" fn rust_take_cstr_ref(s: &CStr) {} -//~^ ERROR `extern` fn uses type `CStr`, which is not FFI-safe +//~^ ERROR `extern` fn uses type `&CStr`, which is not FFI-safe //~| HELP consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()` extern "C" fn rust_take_cstring(s: CString) {} //~^ ERROR `extern` fn uses type `CString`, which is not FFI-safe diff --git a/tests/ui/lint/lint-ctypes-cstr.stderr b/tests/ui/lint/lint-ctypes-cstr.stderr index 8957758d57732..7bf4d1068027f 100644 --- a/tests/ui/lint/lint-ctypes-cstr.stderr +++ b/tests/ui/lint/lint-ctypes-cstr.stderr @@ -12,14 +12,14 @@ note: the lint level is defined here LL | #![deny(improper_ctypes, improper_ctypes_definitions)] | ^^^^^^^^^^^^^^^ -error: `extern` block uses type `CStr`, which is not FFI-safe +error: `extern` block uses type `&CStr`, which is not FFI-safe --> $DIR/lint-ctypes-cstr.rs:10:25 | LL | fn take_cstr_ref(s: &CStr); | ^^^^^ not FFI-safe | = help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()` - = note: `CStr`/`CString` do not have a guaranteed layout + = note: this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer error: `extern` block uses type `CString`, which is not FFI-safe --> $DIR/lint-ctypes-cstr.rs:13:24 @@ -36,6 +36,7 @@ error: `extern` block uses type `CString`, which is not FFI-safe LL | fn take_cstring_ref(s: &CString); | ^^^^^^^^ not FFI-safe | + = note: this reference (`&CString`) is can safely be translated into a C pointer, but `CString` itself is not FFI-safe = help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()` = note: `CStr`/`CString` do not have a guaranteed layout @@ -45,6 +46,7 @@ error: `extern` block uses type `CString`, which is not FFI-safe LL | fn no_special_help_for_mut_cstring(s: *mut CString); | ^^^^^^^^^^^^ not FFI-safe | + = note: this reference (`*mut CString`) is can safely be translated into a C pointer, but `CString` itself is not FFI-safe = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct = note: this struct has unspecified layout @@ -54,17 +56,18 @@ error: `extern` block uses type `CString`, which is not FFI-safe LL | fn no_special_help_for_mut_cstring_ref(s: &mut CString); | ^^^^^^^^^^^^ not FFI-safe | + = note: this reference (`&mut CString`) is can safely be translated into a C pointer, but `CString` itself is not FFI-safe = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct = note: this struct has unspecified layout -error: `extern` fn uses type `CStr`, which is not FFI-safe +error: `extern` fn uses type `&CStr`, which is not FFI-safe --> $DIR/lint-ctypes-cstr.rs:29:37 | LL | extern "C" fn rust_take_cstr_ref(s: &CStr) {} | ^^^^^ not FFI-safe | = help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()` - = note: `CStr`/`CString` do not have a guaranteed layout + = note: this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer note: the lint level is defined here --> $DIR/lint-ctypes-cstr.rs:2:26 | diff --git a/tests/ui/lint/lint-ctypes-fn.rs b/tests/ui/lint/lint-ctypes-fn.rs index 73820c86d1a02..e16ff9573fd18 100644 --- a/tests/ui/lint/lint-ctypes-fn.rs +++ b/tests/ui/lint/lint-ctypes-fn.rs @@ -68,10 +68,10 @@ pub extern "C" fn ptr_unit(p: *const ()) { } pub extern "C" fn ptr_tuple(p: *const ((),)) { } pub extern "C" fn slice_type(p: &[u32]) { } -//~^ ERROR: uses type `[u32]` +//~^ ERROR: uses type `&[u32]` pub extern "C" fn str_type(p: &str) { } -//~^ ERROR: uses type `str` +//~^ ERROR: uses type `&str` pub extern "C" fn box_type(p: Box) { } @@ -124,7 +124,7 @@ pub extern "C" fn transparent_i128(p: TransparentI128) { } //~^ ERROR: uses type `i128` pub extern "C" fn transparent_str(p: TransparentStr) { } -//~^ ERROR: uses type `str` +//~^ ERROR: uses type `&str` pub extern "C" fn transparent_fn(p: TransparentBadFn) { } diff --git a/tests/ui/lint/lint-ctypes-fn.stderr b/tests/ui/lint/lint-ctypes-fn.stderr index a62533a4be17b..0af4ded135da6 100644 --- a/tests/ui/lint/lint-ctypes-fn.stderr +++ b/tests/ui/lint/lint-ctypes-fn.stderr @@ -1,25 +1,25 @@ -error: `extern` fn uses type `[u32]`, which is not FFI-safe +error: `extern` fn uses type `&[u32]`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:70:33 | LL | pub extern "C" fn slice_type(p: &[u32]) { } | ^^^^^^ not FFI-safe | - = help: consider using a raw pointer instead - = note: slices have no C equivalent + = help: consider using a raw pointer to the slice's first element (and a length) instead + = note: this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer note: the lint level is defined here --> $DIR/lint-ctypes-fn.rs:2:9 | LL | #![deny(improper_ctypes_definitions)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: `extern` fn uses type `str`, which is not FFI-safe +error: `extern` fn uses type `&str`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:73:31 | LL | pub extern "C" fn str_type(p: &str) { } | ^^^^ not FFI-safe | = help: consider using `*const u8` and a length instead - = note: string slices have no C equivalent + = note: this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer error: `extern` fn uses type `Box<[u8]>`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:80:34 @@ -27,7 +27,8 @@ error: `extern` fn uses type `Box<[u8]>`, which is not FFI-safe LL | pub extern "C" fn boxed_slice(p: Box<[u8]>) { } | ^^^^^^^^^ not FFI-safe | - = note: box cannot be represented as a single pointer + = help: consider using a raw pointer to the slice's first element (and a length) instead + = note: this box for an unsized rust type contains metadata, which makes it incompatible with a C pointer error: `extern` fn uses type `Box`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:83:35 @@ -35,7 +36,8 @@ error: `extern` fn uses type `Box`, which is not FFI-safe LL | pub extern "C" fn boxed_string(p: Box) { } | ^^^^^^^^ not FFI-safe | - = note: box cannot be represented as a single pointer + = help: consider using `*const u8` and a length instead + = note: this box for an unsized rust type contains metadata, which makes it incompatible with a C pointer error: `extern` fn uses type `Box`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:86:34 @@ -43,7 +45,7 @@ error: `extern` fn uses type `Box`, which is not FFI-safe LL | pub extern "C" fn boxed_trait(p: Box) { } | ^^^^^^^^^^^^^^ not FFI-safe | - = note: box cannot be represented as a single pointer + = note: this box for an unsized rust type contains metadata, which makes it incompatible with a C pointer error: `extern` fn uses type `char`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:89:32 @@ -149,14 +151,14 @@ LL | pub extern "C" fn transparent_i128(p: TransparentI128) { } | = note: 128-bit integers don't currently have a known stable ABI -error: `extern` fn uses type `str`, which is not FFI-safe +error: `extern` fn uses type `&str`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:126:38 | LL | pub extern "C" fn transparent_str(p: TransparentStr) { } | ^^^^^^^^^^^^^^ not FFI-safe | = help: consider using `*const u8` and a length instead - = note: string slices have no C equivalent + = note: this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer error: `extern` fn uses type `PhantomData`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:172:43 diff --git a/tests/ui/lint/lint-ctypes.rs b/tests/ui/lint/lint-ctypes.rs index dae07930aba60..615cdfd738ac3 100644 --- a/tests/ui/lint/lint-ctypes.rs +++ b/tests/ui/lint/lint-ctypes.rs @@ -48,15 +48,15 @@ extern "C" { pub fn ptr_type2(size: *const Foo); //~ ERROR: uses type `Foo` pub fn ptr_unit(p: *const ()); pub fn ptr_tuple(p: *const ((),)); //~ ERROR: uses type `((),)` - pub fn slice_type(p: &[u32]); //~ ERROR: uses type `[u32]` - pub fn str_type(p: &str); //~ ERROR: uses type `str` + pub fn slice_type(p: &[u32]); //~ ERROR: uses type `&[u32]` + pub fn str_type(p: &str); //~ ERROR: uses type `&str` pub fn box_type(p: Box); //~ ERROR uses type `Box` pub fn opt_box_type(p: Option>); //~^ ERROR uses type `Option>` pub fn char_type(p: char); //~ ERROR uses type `char` pub fn i128_type(p: i128); //~ ERROR uses type `i128` pub fn u128_type(p: u128); //~ ERROR uses type `u128` - pub fn trait_type(p: &dyn Bar); //~ ERROR uses type `dyn Bar` + pub fn trait_type(p: &dyn Bar); //~ ERROR uses type `&dyn Bar` pub fn tuple_type(p: (i32, i32)); //~ ERROR uses type `(i32, i32)` pub fn tuple_type2(p: I32Pair); //~ ERROR uses type `(i32, i32)` pub fn zero_size(p: ZeroSize); //~ ERROR uses type `ZeroSize` @@ -68,7 +68,7 @@ extern "C" { pub fn fn_type2(p: fn()); //~ ERROR uses type `fn()` pub fn fn_contained(p: RustBadRet); //~ ERROR: uses type `Box` pub fn transparent_i128(p: TransparentI128); //~ ERROR: uses type `i128` - pub fn transparent_str(p: TransparentStr); //~ ERROR: uses type `str` + pub fn transparent_str(p: TransparentStr); //~ ERROR: uses type `&str` pub fn transparent_fn(p: TransparentBadFn); //~ ERROR: uses type `Box` pub fn raw_array(arr: [u8; 8]); //~ ERROR: uses type `[u8; 8]` diff --git a/tests/ui/lint/lint-ctypes.stderr b/tests/ui/lint/lint-ctypes.stderr index 2c81c7b8e4b67..fdb7ef1261085 100644 --- a/tests/ui/lint/lint-ctypes.stderr +++ b/tests/ui/lint/lint-ctypes.stderr @@ -4,6 +4,7 @@ error: `extern` block uses type `Foo`, which is not FFI-safe LL | pub fn ptr_type1(size: *const Foo); | ^^^^^^^^^^ not FFI-safe | + = note: this reference (`*const Foo`) is can safely be translated into a C pointer, but `Foo` itself is not FFI-safe = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct = note: this struct has unspecified layout note: the type is defined here @@ -23,6 +24,7 @@ error: `extern` block uses type `Foo`, which is not FFI-safe LL | pub fn ptr_type2(size: *const Foo); | ^^^^^^^^^^ not FFI-safe | + = note: this reference (`*const Foo`) is can safely be translated into a C pointer, but `Foo` itself is not FFI-safe = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct = note: this struct has unspecified layout note: the type is defined here @@ -37,26 +39,27 @@ error: `extern` block uses type `((),)`, which is not FFI-safe LL | pub fn ptr_tuple(p: *const ((),)); | ^^^^^^^^^^^^ not FFI-safe | + = note: this reference (`*const ((),)`) is can safely be translated into a C pointer, but `((),)` itself is not FFI-safe = help: consider using a struct instead = note: tuples have unspecified layout -error: `extern` block uses type `[u32]`, which is not FFI-safe +error: `extern` block uses type `&[u32]`, which is not FFI-safe --> $DIR/lint-ctypes.rs:51:26 | LL | pub fn slice_type(p: &[u32]); | ^^^^^^ not FFI-safe | - = help: consider using a raw pointer instead - = note: slices have no C equivalent + = help: consider using a raw pointer to the slice's first element (and a length) instead + = note: this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer -error: `extern` block uses type `str`, which is not FFI-safe +error: `extern` block uses type `&str`, which is not FFI-safe --> $DIR/lint-ctypes.rs:52:24 | LL | pub fn str_type(p: &str); | ^^^^ not FFI-safe | = help: consider using `*const u8` and a length instead - = note: string slices have no C equivalent + = note: this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer error: `extern` block uses type `Box`, which is not FFI-safe --> $DIR/lint-ctypes.rs:53:24 @@ -101,13 +104,13 @@ LL | pub fn u128_type(p: u128); | = note: 128-bit integers don't currently have a known stable ABI -error: `extern` block uses type `dyn Bar`, which is not FFI-safe +error: `extern` block uses type `&dyn Bar`, which is not FFI-safe --> $DIR/lint-ctypes.rs:59:26 | LL | pub fn trait_type(p: &dyn Bar); | ^^^^^^^^ not FFI-safe | - = note: trait objects have no C equivalent + = note: this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer error: `extern` block uses type `(i32, i32)`, which is not FFI-safe --> $DIR/lint-ctypes.rs:60:26 @@ -197,14 +200,14 @@ LL | pub fn transparent_i128(p: TransparentI128); | = note: 128-bit integers don't currently have a known stable ABI -error: `extern` block uses type `str`, which is not FFI-safe +error: `extern` block uses type `&str`, which is not FFI-safe --> $DIR/lint-ctypes.rs:71:31 | LL | pub fn transparent_str(p: TransparentStr); | ^^^^^^^^^^^^^^ not FFI-safe | = help: consider using `*const u8` and a length instead - = note: string slices have no C equivalent + = note: this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer error: `extern` block uses type `Box`, which is not FFI-safe --> $DIR/lint-ctypes.rs:72:30 @@ -259,4 +262,3 @@ LL | pub static static_u128_array_type: [u128; 16]; = note: 128-bit integers don't currently have a known stable ABI error: aborting due to 27 previous errors - From 7962a2de3a37c97cc08789a6db2cdebe5d465a95 Mon Sep 17 00:00:00 2001 From: niacdoial Date: Fri, 8 Nov 2024 01:28:16 +0100 Subject: [PATCH 04/13] lint: fix ImproperCTypes edge case for unsized structs due to foreign types --- compiler/rustc_lint/src/types.rs | 67 ++++++++++++++++++++++++++++++- tests/ui/lint/lint-ctypes.rs | 16 ++++++++ tests/ui/lint/lint-ctypes.stderr | 68 ++++++++++++++++++-------------- 3 files changed, 119 insertions(+), 32 deletions(-) diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index 111be742696b9..842dc2baae458 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -740,6 +740,69 @@ enum FfiResult<'tcx> { }, } +pub(crate) fn is_unsized_because_foreign<'tcx, 'a>( + cx: &'a LateContext<'tcx>, + ty: Ty<'tcx>, +) -> Result { + let tcx = cx.tcx; + + if ty.is_sized(tcx, cx.typing_env()) { + Err(()) + } else { + Ok(match ty.kind() { + ty::Slice(_) => false, + ty::Str => false, + ty::Dynamic(..) => false, + ty::Foreign(..) => true, + ty::Alias(ty::Opaque, ..) => todo!("why"), + ty::Adt(def, args) => { + // for now assume: boxes and phantoms don't mess with this + match def.adt_kind() { + AdtKind::Union | AdtKind::Enum => true, + AdtKind::Struct => { + if let Some(sym::cstring_type | sym::cstr_type) = + tcx.get_diagnostic_name(def.did()) + { + return Ok(false); + } + // FIXME: how do we deal with non-exhaustive unsized structs/unions? + + if def.non_enum_variant().fields.is_empty() { + bug!("empty unsized struct/union. what?"); + } + + let variant = def.non_enum_variant(); + + // only the last field may be unsized + let n_fields = variant.fields.len(); + let last_field = &variant.fields[(n_fields - 1).into()]; + let field_ty = last_field.ty(cx.tcx, args); + let field_ty = cx + .tcx + .try_normalize_erasing_regions(cx.typing_env(), field_ty) + .unwrap_or(field_ty); + return Ok(is_unsized_because_foreign(cx, field_ty).unwrap()); + } + } + } + ty::Tuple(tuple) => { + // only the last field may be unsized + let n_fields = tuple.len(); + let field_ty: Ty<'tcx> = tuple[n_fields - 1]; + //let field_ty = last_field.ty(cx.tcx, args); + let field_ty = cx + .tcx + .try_normalize_erasing_regions(cx.typing_env(), field_ty) + .unwrap_or(field_ty); + is_unsized_because_foreign(cx, field_ty).unwrap() + } + t => { + bug!("we shouldn't be looking if this is unsized for a reason or another: {:?}", t) + } + }) + } +} + pub(crate) fn nonnull_optimization_guaranteed<'tcx>( tcx: TyCtxt<'tcx>, def: ty::AdtDef<'tcx>, @@ -1044,7 +1107,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { ty::Adt(def, args) => { if let Some(inner_ty) = ty.boxed_ty() { if inner_ty.is_sized(tcx, self.cx.typing_env()) - || matches!(inner_ty.kind(), ty::Foreign(..)) + || is_unsized_because_foreign(self.cx, inner_ty).unwrap() { // discussion on declaration vs definition: // see the `ty::RawPtr(inner_ty, _) | ty::Ref(_, inner_ty, _)` arm @@ -1249,7 +1312,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { ty::RawPtr(inner_ty, _) | ty::Ref(_, inner_ty, _) => { if inner_ty.is_sized(tcx, self.cx.typing_env()) - || matches!(inner_ty.kind(), ty::Foreign(..)) + || is_unsized_because_foreign(self.cx, inner_ty).unwrap() { // there's a nuance on what this lint should do for function definitions // (touched upon in https://github.com/rust-lang/rust/issues/66220 and https://github.com/rust-lang/rust/pull/72700) diff --git a/tests/ui/lint/lint-ctypes.rs b/tests/ui/lint/lint-ctypes.rs index 615cdfd738ac3..28ff038134935 100644 --- a/tests/ui/lint/lint-ctypes.rs +++ b/tests/ui/lint/lint-ctypes.rs @@ -1,4 +1,5 @@ #![feature(rustc_private)] +#![feature(extern_types)] #![allow(private_interfaces)] #![deny(improper_ctypes)] @@ -6,7 +7,9 @@ use std::cell::UnsafeCell; use std::marker::PhantomData; use std::ffi::{c_int, c_uint}; +use std::fmt::Debug; +unsafe extern "C" {type UnsizedOpaque;} trait Bar { } trait Mirror { type It: ?Sized; } impl Mirror for T { type It = Self; } @@ -39,6 +42,16 @@ pub struct TransparentLifetime<'a>(*const u8, PhantomData<&'a ()>); pub struct TransparentUnit(f32, PhantomData); #[repr(transparent)] pub struct TransparentCustomZst(i32, ZeroSize); +#[repr(C)] +pub struct UnsizedStructBecauseForeign { + sized: u32, + unszd: UnsizedOpaque, +} +#[repr(C)] +pub struct UnsizedStructBecauseDyn { + sized: u32, + unszd: dyn Debug, +} #[repr(C)] pub struct ZeroSizeWithPhantomData(::std::marker::PhantomData); @@ -72,6 +85,9 @@ extern "C" { pub fn transparent_fn(p: TransparentBadFn); //~ ERROR: uses type `Box` pub fn raw_array(arr: [u8; 8]); //~ ERROR: uses type `[u8; 8]` + pub fn struct_unsized_ptr_no_metadata(p: &UnsizedStructBecauseForeign); + pub fn struct_unsized_ptr_has_metadata(p: &UnsizedStructBecauseDyn); //~ ERROR uses type `&UnsizedStructBecauseDyn` + pub fn no_niche_a(a: Option>); //~^ ERROR: uses type `Option>` pub fn no_niche_b(b: Option>); diff --git a/tests/ui/lint/lint-ctypes.stderr b/tests/ui/lint/lint-ctypes.stderr index fdb7ef1261085..59f6243e01fbb 100644 --- a/tests/ui/lint/lint-ctypes.stderr +++ b/tests/ui/lint/lint-ctypes.stderr @@ -1,5 +1,5 @@ error: `extern` block uses type `Foo`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:47:28 + --> $DIR/lint-ctypes.rs:60:28 | LL | pub fn ptr_type1(size: *const Foo); | ^^^^^^^^^^ not FFI-safe @@ -8,18 +8,18 @@ LL | pub fn ptr_type1(size: *const Foo); = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct = note: this struct has unspecified layout note: the type is defined here - --> $DIR/lint-ctypes.rs:25:1 + --> $DIR/lint-ctypes.rs:28:1 | LL | pub struct Foo; | ^^^^^^^^^^^^^^ note: the lint level is defined here - --> $DIR/lint-ctypes.rs:4:9 + --> $DIR/lint-ctypes.rs:5:9 | LL | #![deny(improper_ctypes)] | ^^^^^^^^^^^^^^^ error: `extern` block uses type `Foo`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:48:28 + --> $DIR/lint-ctypes.rs:61:28 | LL | pub fn ptr_type2(size: *const Foo); | ^^^^^^^^^^ not FFI-safe @@ -28,13 +28,13 @@ LL | pub fn ptr_type2(size: *const Foo); = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct = note: this struct has unspecified layout note: the type is defined here - --> $DIR/lint-ctypes.rs:25:1 + --> $DIR/lint-ctypes.rs:28:1 | LL | pub struct Foo; | ^^^^^^^^^^^^^^ error: `extern` block uses type `((),)`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:50:25 + --> $DIR/lint-ctypes.rs:63:25 | LL | pub fn ptr_tuple(p: *const ((),)); | ^^^^^^^^^^^^ not FFI-safe @@ -44,7 +44,7 @@ LL | pub fn ptr_tuple(p: *const ((),)); = note: tuples have unspecified layout error: `extern` block uses type `&[u32]`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:51:26 + --> $DIR/lint-ctypes.rs:64:26 | LL | pub fn slice_type(p: &[u32]); | ^^^^^^ not FFI-safe @@ -53,7 +53,7 @@ LL | pub fn slice_type(p: &[u32]); = note: this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer error: `extern` block uses type `&str`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:52:24 + --> $DIR/lint-ctypes.rs:65:24 | LL | pub fn str_type(p: &str); | ^^^^ not FFI-safe @@ -71,7 +71,7 @@ LL | pub fn box_type(p: Box); = note: this struct has unspecified layout error: `extern` block uses type `Option>`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:54:28 + --> $DIR/lint-ctypes.rs:67:28 | LL | pub fn opt_box_type(p: Option>); | ^^^^^^^^^^^^^^^^ not FFI-safe @@ -80,7 +80,7 @@ LL | pub fn opt_box_type(p: Option>); = note: enum has no representation hint error: `extern` block uses type `char`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:56:25 + --> $DIR/lint-ctypes.rs:69:25 | LL | pub fn char_type(p: char); | ^^^^ not FFI-safe @@ -89,7 +89,7 @@ LL | pub fn char_type(p: char); = note: the `char` type has no C equivalent error: `extern` block uses type `i128`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:57:25 + --> $DIR/lint-ctypes.rs:70:25 | LL | pub fn i128_type(p: i128); | ^^^^ not FFI-safe @@ -97,7 +97,7 @@ LL | pub fn i128_type(p: i128); = note: 128-bit integers don't currently have a known stable ABI error: `extern` block uses type `u128`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:58:25 + --> $DIR/lint-ctypes.rs:71:25 | LL | pub fn u128_type(p: u128); | ^^^^ not FFI-safe @@ -105,7 +105,7 @@ LL | pub fn u128_type(p: u128); = note: 128-bit integers don't currently have a known stable ABI error: `extern` block uses type `&dyn Bar`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:59:26 + --> $DIR/lint-ctypes.rs:72:26 | LL | pub fn trait_type(p: &dyn Bar); | ^^^^^^^^ not FFI-safe @@ -113,7 +113,7 @@ LL | pub fn trait_type(p: &dyn Bar); = note: this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer error: `extern` block uses type `(i32, i32)`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:60:26 + --> $DIR/lint-ctypes.rs:73:26 | LL | pub fn tuple_type(p: (i32, i32)); | ^^^^^^^^^^ not FFI-safe @@ -122,7 +122,7 @@ LL | pub fn tuple_type(p: (i32, i32)); = note: tuples have unspecified layout error: `extern` block uses type `(i32, i32)`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:61:27 + --> $DIR/lint-ctypes.rs:74:27 | LL | pub fn tuple_type2(p: I32Pair); | ^^^^^^^ not FFI-safe @@ -131,7 +131,7 @@ LL | pub fn tuple_type2(p: I32Pair); = note: tuples have unspecified layout error: `extern` block uses type `ZeroSize`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:62:25 + --> $DIR/lint-ctypes.rs:75:25 | LL | pub fn zero_size(p: ZeroSize); | ^^^^^^^^ not FFI-safe @@ -139,26 +139,26 @@ LL | pub fn zero_size(p: ZeroSize); = help: consider adding a member to this struct = note: this struct has no fields note: the type is defined here - --> $DIR/lint-ctypes.rs:21:1 + --> $DIR/lint-ctypes.rs:24:1 | LL | pub struct ZeroSize; | ^^^^^^^^^^^^^^^^^^^ error: `extern` block uses type `ZeroSizeWithPhantomData`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:63:33 + --> $DIR/lint-ctypes.rs:76:33 | LL | pub fn zero_size_phantom(p: ZeroSizeWithPhantomData); | ^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe | = note: composed only of `PhantomData` note: the type is defined here - --> $DIR/lint-ctypes.rs:44:1 + --> $DIR/lint-ctypes.rs:57:1 | LL | pub struct ZeroSizeWithPhantomData(::std::marker::PhantomData); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: `extern` block uses type `PhantomData`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:66:12 + --> $DIR/lint-ctypes.rs:79:12 | LL | -> ::std::marker::PhantomData; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -166,7 +166,7 @@ LL | -> ::std::marker::PhantomData; = note: composed only of `PhantomData` error: `extern` block uses type `fn()`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:67:23 + --> $DIR/lint-ctypes.rs:80:23 | LL | pub fn fn_type(p: RustFn); | ^^^^^^ not FFI-safe @@ -175,7 +175,7 @@ LL | pub fn fn_type(p: RustFn); = note: this function pointer has Rust-specific calling convention error: `extern` block uses type `fn()`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:68:24 + --> $DIR/lint-ctypes.rs:81:24 | LL | pub fn fn_type2(p: fn()); | ^^^^ not FFI-safe @@ -193,7 +193,7 @@ LL | pub fn fn_contained(p: RustBadRet); = note: this struct has unspecified layout error: `extern` block uses type `i128`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:70:32 + --> $DIR/lint-ctypes.rs:83:32 | LL | pub fn transparent_i128(p: TransparentI128); | ^^^^^^^^^^^^^^^ not FFI-safe @@ -201,7 +201,7 @@ LL | pub fn transparent_i128(p: TransparentI128); = note: 128-bit integers don't currently have a known stable ABI error: `extern` block uses type `&str`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:71:31 + --> $DIR/lint-ctypes.rs:84:31 | LL | pub fn transparent_str(p: TransparentStr); | ^^^^^^^^^^^^^^ not FFI-safe @@ -219,7 +219,7 @@ LL | pub fn transparent_fn(p: TransparentBadFn); = note: this struct has unspecified layout error: `extern` block uses type `[u8; 8]`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:73:27 + --> $DIR/lint-ctypes.rs:86:27 | LL | pub fn raw_array(arr: [u8; 8]); | ^^^^^^^ not FFI-safe @@ -227,8 +227,16 @@ LL | pub fn raw_array(arr: [u8; 8]); = help: consider passing a pointer to the array = note: passing raw arrays by value is not FFI-safe +error: `extern` block uses type `&UnsizedStructBecauseDyn`, which is not FFI-safe + --> $DIR/lint-ctypes.rs:89:47 + | +LL | pub fn struct_unsized_ptr_has_metadata(p: &UnsizedStructBecauseDyn); + | ^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe + | + = note: this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer + error: `extern` block uses type `Option>`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:75:26 + --> $DIR/lint-ctypes.rs:91:26 | LL | pub fn no_niche_a(a: Option>); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -237,7 +245,7 @@ LL | pub fn no_niche_a(a: Option>); = note: enum has no representation hint error: `extern` block uses type `Option>`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:77:26 + --> $DIR/lint-ctypes.rs:93:26 | LL | pub fn no_niche_b(b: Option>); | ^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -246,7 +254,7 @@ LL | pub fn no_niche_b(b: Option>); = note: enum has no representation hint error: `extern` block uses type `u128`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:80:34 + --> $DIR/lint-ctypes.rs:96:34 | LL | pub static static_u128_type: u128; | ^^^^ not FFI-safe @@ -254,11 +262,11 @@ LL | pub static static_u128_type: u128; = note: 128-bit integers don't currently have a known stable ABI error: `extern` block uses type `u128`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:81:40 + --> $DIR/lint-ctypes.rs:97:40 | LL | pub static static_u128_array_type: [u128; 16]; | ^^^^^^^^^^ not FFI-safe | = note: 128-bit integers don't currently have a known stable ABI -error: aborting due to 27 previous errors +error: aborting due to 29 previous errors From d857bc8fbb7959291055ec7aceb68b8d091b7641 Mon Sep 17 00:00:00 2001 From: niacdoial Date: Fri, 8 Nov 2024 23:35:35 +0100 Subject: [PATCH 05/13] lint: polish code from the last few commits --- compiler/rustc_lint/src/types.rs | 85 ++++++++++++++++++++++---------- 1 file changed, 60 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index 842dc2baae458..49fa312fe13d3 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -740,35 +740,49 @@ enum FfiResult<'tcx> { }, } -pub(crate) fn is_unsized_because_foreign<'tcx, 'a>( - cx: &'a LateContext<'tcx>, - ty: Ty<'tcx>, -) -> Result { +/// Determine if a type is sized or not, and wether it affects references/pointers/boxes to it +#[derive(Clone, Copy)] +enum TypeSizedness { + /// sized type (pointers are C-compatible) + Sized, + /// unsized type because it includes an opaque/foreign type (pointers are C-compatible) + UnsizedBecauseForeign, + /// unsized type for other reasons (slice, string, dyn Trait, closure, ...) (pointers are not C-compatible) + UnsizedWithMetadata, +} + +/// Is this type unsized because it contains (or is) a foreign type? +/// (Returns Err if the type happens to be sized after all) +fn get_type_sizedness<'tcx, 'a>(cx: &'a LateContext<'tcx>, ty: Ty<'tcx>) -> TypeSizedness { let tcx = cx.tcx; if ty.is_sized(tcx, cx.typing_env()) { - Err(()) + TypeSizedness::Sized } else { - Ok(match ty.kind() { - ty::Slice(_) => false, - ty::Str => false, - ty::Dynamic(..) => false, - ty::Foreign(..) => true, - ty::Alias(ty::Opaque, ..) => todo!("why"), + match ty.kind() { + ty::Slice(_) => TypeSizedness::UnsizedWithMetadata, + ty::Str => TypeSizedness::UnsizedWithMetadata, + ty::Dynamic(..) => TypeSizedness::UnsizedWithMetadata, + ty::Foreign(..) => TypeSizedness::UnsizedBecauseForeign, + // While opaque types are checked for earlier, if a projection in a struct field + // normalizes to an opaque type, then it will reach this branch. + ty::Alias(ty::Opaque, ..) => todo!("We... don't know enough about this type yet?"), ty::Adt(def, args) => { // for now assume: boxes and phantoms don't mess with this match def.adt_kind() { - AdtKind::Union | AdtKind::Enum => true, + AdtKind::Union | AdtKind::Enum => { + bug!("unions and enums are necessarily sized") + } AdtKind::Struct => { if let Some(sym::cstring_type | sym::cstr_type) = tcx.get_diagnostic_name(def.did()) { - return Ok(false); + return TypeSizedness::UnsizedWithMetadata; } // FIXME: how do we deal with non-exhaustive unsized structs/unions? if def.non_enum_variant().fields.is_empty() { - bug!("empty unsized struct/union. what?"); + bug!("an empty struct is necessarily sized"); } let variant = def.non_enum_variant(); @@ -781,7 +795,13 @@ pub(crate) fn is_unsized_because_foreign<'tcx, 'a>( .tcx .try_normalize_erasing_regions(cx.typing_env(), field_ty) .unwrap_or(field_ty); - return Ok(is_unsized_because_foreign(cx, field_ty).unwrap()); + match get_type_sizedness(cx, field_ty) { + s @ (TypeSizedness::UnsizedWithMetadata + | TypeSizedness::UnsizedBecauseForeign) => s, + TypeSizedness::Sized => { + bug!("failed to find the reason why struct `{:?}` is unsized", ty) + } + } } } } @@ -794,12 +814,21 @@ pub(crate) fn is_unsized_because_foreign<'tcx, 'a>( .tcx .try_normalize_erasing_regions(cx.typing_env(), field_ty) .unwrap_or(field_ty); - is_unsized_because_foreign(cx, field_ty).unwrap() + match get_type_sizedness(cx, field_ty) { + s @ (TypeSizedness::UnsizedWithMetadata + | TypeSizedness::UnsizedBecauseForeign) => s, + TypeSizedness::Sized => { + bug!("failed to find the reason why tuple `{:?}` is unsized", ty) + } + } } t => { - bug!("we shouldn't be looking if this is unsized for a reason or another: {:?}", t) + bug!( + "we shouldn't be trying to determine if this is unsized for a reason or another: `{:?}`", + t + ) } - }) + } } } @@ -1106,8 +1135,8 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { match *ty.kind() { ty::Adt(def, args) => { if let Some(inner_ty) = ty.boxed_ty() { - if inner_ty.is_sized(tcx, self.cx.typing_env()) - || is_unsized_because_foreign(self.cx, inner_ty).unwrap() + if let TypeSizedness::UnsizedBecauseForeign | TypeSizedness::Sized = + get_type_sizedness(self.cx, inner_ty) { // discussion on declaration vs definition: // see the `ty::RawPtr(inner_ty, _) | ty::Ref(_, inner_ty, _)` arm @@ -1311,13 +1340,14 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } ty::RawPtr(inner_ty, _) | ty::Ref(_, inner_ty, _) => { - if inner_ty.is_sized(tcx, self.cx.typing_env()) - || is_unsized_because_foreign(self.cx, inner_ty).unwrap() + if let TypeSizedness::UnsizedBecauseForeign | TypeSizedness::Sized = + get_type_sizedness(self.cx, inner_ty) { // there's a nuance on what this lint should do for function definitions - // (touched upon in https://github.com/rust-lang/rust/issues/66220 and https://github.com/rust-lang/rust/pull/72700) - // // (`extern "C" fn fn_name(...) {...}`) versus declarations (`extern "C" {fn fn_name(...);}`). + // (this is touched upon in https://github.com/rust-lang/rust/issues/66220 + // and https://github.com/rust-lang/rust/pull/72700) + // // The big question is: what does "ABI safety" mean? if you have something translated to a C pointer // (which has a stable layout) but points to FFI-unsafe type, is it safe? // on one hand, the function's ABI will match that of a similar C-declared function API, @@ -1609,7 +1639,12 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } }; } - let last_ty = last_ty.unwrap(); // populated by any run of the `while` block + let last_ty = match last_ty { + Some(last_ty) => last_ty, + None => bug!( + "This option should definitely have been filled by the loop that just finished" + ), + }; self.emit_ffi_unsafe_type_lint(last_ty, sp, cimproper_layers); } } From 9b59dd817886a2b90432a5c078cfe501fd48e1fc Mon Sep 17 00:00:00 2001 From: niacdoial Date: Sun, 10 Nov 2024 01:38:54 +0100 Subject: [PATCH 06/13] lint ImproperCTypes: confirm that Box and Option> are FFI-safe in function declarations too --- compiler/rustc_lint/src/types.rs | 2 +- tests/ui/lint/lint-ctypes.rs | 11 +++-- tests/ui/lint/lint-ctypes.stderr | 77 +++++++++----------------------- 3 files changed, 27 insertions(+), 63 deletions(-) diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index 49fa312fe13d3..f9185b3f529a9 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -866,7 +866,7 @@ fn ty_is_known_nonnull<'tcx>( match ty.kind() { ty::FnPtr(..) => true, ty::Ref(..) => true, - ty::Adt(def, _) if def.is_box() && matches!(mode, CItemKind::Definition) => true, + ty::Adt(def, _) if def.is_box() => true, ty::Adt(def, args) if def.repr().transparent() && !def.is_union() => { let marked_non_null = nonnull_optimization_guaranteed(tcx, *def); diff --git a/tests/ui/lint/lint-ctypes.rs b/tests/ui/lint/lint-ctypes.rs index 28ff038134935..8c516ab8428b4 100644 --- a/tests/ui/lint/lint-ctypes.rs +++ b/tests/ui/lint/lint-ctypes.rs @@ -23,7 +23,7 @@ pub type I32Pair = (i32, i32); #[repr(C)] pub struct ZeroSize; pub type RustFn = fn(); -pub type RustBadRet = extern "C" fn() -> Box; +pub type RustBoxRet = extern "C" fn() -> Box; pub type CVoidRet = (); pub struct Foo; #[repr(transparent)] @@ -31,7 +31,7 @@ pub struct TransparentI128(i128); #[repr(transparent)] pub struct TransparentStr(&'static str); #[repr(transparent)] -pub struct TransparentBadFn(RustBadRet); +pub struct TransparentBoxFn(RustBoxRet); #[repr(transparent)] pub struct TransparentInt(u32); #[repr(transparent)] @@ -63,9 +63,8 @@ extern "C" { pub fn ptr_tuple(p: *const ((),)); //~ ERROR: uses type `((),)` pub fn slice_type(p: &[u32]); //~ ERROR: uses type `&[u32]` pub fn str_type(p: &str); //~ ERROR: uses type `&str` - pub fn box_type(p: Box); //~ ERROR uses type `Box` + pub fn box_type(p: Box); pub fn opt_box_type(p: Option>); - //~^ ERROR uses type `Option>` pub fn char_type(p: char); //~ ERROR uses type `char` pub fn i128_type(p: i128); //~ ERROR uses type `i128` pub fn u128_type(p: u128); //~ ERROR uses type `u128` @@ -79,10 +78,10 @@ extern "C" { -> ::std::marker::PhantomData; //~ ERROR uses type `PhantomData` pub fn fn_type(p: RustFn); //~ ERROR uses type `fn()` pub fn fn_type2(p: fn()); //~ ERROR uses type `fn()` - pub fn fn_contained(p: RustBadRet); //~ ERROR: uses type `Box` + pub fn fn_contained(p: RustBoxRet); pub fn transparent_i128(p: TransparentI128); //~ ERROR: uses type `i128` pub fn transparent_str(p: TransparentStr); //~ ERROR: uses type `&str` - pub fn transparent_fn(p: TransparentBadFn); //~ ERROR: uses type `Box` + pub fn transparent_fn(p: TransparentBoxFn); pub fn raw_array(arr: [u8; 8]); //~ ERROR: uses type `[u8; 8]` pub fn struct_unsized_ptr_no_metadata(p: &UnsizedStructBecauseForeign); diff --git a/tests/ui/lint/lint-ctypes.stderr b/tests/ui/lint/lint-ctypes.stderr index 59f6243e01fbb..042ca240818b1 100644 --- a/tests/ui/lint/lint-ctypes.stderr +++ b/tests/ui/lint/lint-ctypes.stderr @@ -61,26 +61,8 @@ LL | pub fn str_type(p: &str); = help: consider using `*const u8` and a length instead = note: this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer -error: `extern` block uses type `Box`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:53:24 - | -LL | pub fn box_type(p: Box); - | ^^^^^^^^ not FFI-safe - | - = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct - = note: this struct has unspecified layout - -error: `extern` block uses type `Option>`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:67:28 - | -LL | pub fn opt_box_type(p: Option>); - | ^^^^^^^^^^^^^^^^ not FFI-safe - | - = help: consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum - = note: enum has no representation hint - error: `extern` block uses type `char`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:69:25 + --> $DIR/lint-ctypes.rs:68:25 | LL | pub fn char_type(p: char); | ^^^^ not FFI-safe @@ -89,7 +71,7 @@ LL | pub fn char_type(p: char); = note: the `char` type has no C equivalent error: `extern` block uses type `i128`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:70:25 + --> $DIR/lint-ctypes.rs:69:25 | LL | pub fn i128_type(p: i128); | ^^^^ not FFI-safe @@ -97,7 +79,7 @@ LL | pub fn i128_type(p: i128); = note: 128-bit integers don't currently have a known stable ABI error: `extern` block uses type `u128`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:71:25 + --> $DIR/lint-ctypes.rs:70:25 | LL | pub fn u128_type(p: u128); | ^^^^ not FFI-safe @@ -105,7 +87,7 @@ LL | pub fn u128_type(p: u128); = note: 128-bit integers don't currently have a known stable ABI error: `extern` block uses type `&dyn Bar`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:72:26 + --> $DIR/lint-ctypes.rs:71:26 | LL | pub fn trait_type(p: &dyn Bar); | ^^^^^^^^ not FFI-safe @@ -113,7 +95,7 @@ LL | pub fn trait_type(p: &dyn Bar); = note: this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer error: `extern` block uses type `(i32, i32)`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:73:26 + --> $DIR/lint-ctypes.rs:72:26 | LL | pub fn tuple_type(p: (i32, i32)); | ^^^^^^^^^^ not FFI-safe @@ -122,7 +104,7 @@ LL | pub fn tuple_type(p: (i32, i32)); = note: tuples have unspecified layout error: `extern` block uses type `(i32, i32)`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:74:27 + --> $DIR/lint-ctypes.rs:73:27 | LL | pub fn tuple_type2(p: I32Pair); | ^^^^^^^ not FFI-safe @@ -131,7 +113,7 @@ LL | pub fn tuple_type2(p: I32Pair); = note: tuples have unspecified layout error: `extern` block uses type `ZeroSize`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:75:25 + --> $DIR/lint-ctypes.rs:74:25 | LL | pub fn zero_size(p: ZeroSize); | ^^^^^^^^ not FFI-safe @@ -145,7 +127,7 @@ LL | pub struct ZeroSize; | ^^^^^^^^^^^^^^^^^^^ error: `extern` block uses type `ZeroSizeWithPhantomData`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:76:33 + --> $DIR/lint-ctypes.rs:75:33 | LL | pub fn zero_size_phantom(p: ZeroSizeWithPhantomData); | ^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -158,7 +140,7 @@ LL | pub struct ZeroSizeWithPhantomData(::std::marker::PhantomData); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: `extern` block uses type `PhantomData`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:79:12 + --> $DIR/lint-ctypes.rs:78:12 | LL | -> ::std::marker::PhantomData; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -166,7 +148,7 @@ LL | -> ::std::marker::PhantomData; = note: composed only of `PhantomData` error: `extern` block uses type `fn()`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:80:23 + --> $DIR/lint-ctypes.rs:79:23 | LL | pub fn fn_type(p: RustFn); | ^^^^^^ not FFI-safe @@ -175,7 +157,7 @@ LL | pub fn fn_type(p: RustFn); = note: this function pointer has Rust-specific calling convention error: `extern` block uses type `fn()`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:81:24 + --> $DIR/lint-ctypes.rs:80:24 | LL | pub fn fn_type2(p: fn()); | ^^^^ not FFI-safe @@ -183,17 +165,8 @@ LL | pub fn fn_type2(p: fn()); = help: consider using an `extern fn(...) -> ...` function pointer instead = note: this function pointer has Rust-specific calling convention -error: `extern` block uses type `Box`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:69:28 - | -LL | pub fn fn_contained(p: RustBadRet); - | ^^^^^^^^^^ not FFI-safe - | - = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct - = note: this struct has unspecified layout - error: `extern` block uses type `i128`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:83:32 + --> $DIR/lint-ctypes.rs:82:32 | LL | pub fn transparent_i128(p: TransparentI128); | ^^^^^^^^^^^^^^^ not FFI-safe @@ -201,7 +174,7 @@ LL | pub fn transparent_i128(p: TransparentI128); = note: 128-bit integers don't currently have a known stable ABI error: `extern` block uses type `&str`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:84:31 + --> $DIR/lint-ctypes.rs:83:31 | LL | pub fn transparent_str(p: TransparentStr); | ^^^^^^^^^^^^^^ not FFI-safe @@ -209,17 +182,8 @@ LL | pub fn transparent_str(p: TransparentStr); = help: consider using `*const u8` and a length instead = note: this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer -error: `extern` block uses type `Box`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:72:30 - | -LL | pub fn transparent_fn(p: TransparentBadFn); - | ^^^^^^^^^^^^^^^^ not FFI-safe - | - = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct - = note: this struct has unspecified layout - error: `extern` block uses type `[u8; 8]`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:86:27 + --> $DIR/lint-ctypes.rs:85:27 | LL | pub fn raw_array(arr: [u8; 8]); | ^^^^^^^ not FFI-safe @@ -228,7 +192,7 @@ LL | pub fn raw_array(arr: [u8; 8]); = note: passing raw arrays by value is not FFI-safe error: `extern` block uses type `&UnsizedStructBecauseDyn`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:89:47 + --> $DIR/lint-ctypes.rs:88:47 | LL | pub fn struct_unsized_ptr_has_metadata(p: &UnsizedStructBecauseDyn); | ^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -236,7 +200,7 @@ LL | pub fn struct_unsized_ptr_has_metadata(p: &UnsizedStructBecauseDyn); = note: this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer error: `extern` block uses type `Option>`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:91:26 + --> $DIR/lint-ctypes.rs:90:26 | LL | pub fn no_niche_a(a: Option>); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -245,7 +209,7 @@ LL | pub fn no_niche_a(a: Option>); = note: enum has no representation hint error: `extern` block uses type `Option>`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:93:26 + --> $DIR/lint-ctypes.rs:92:26 | LL | pub fn no_niche_b(b: Option>); | ^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe @@ -254,7 +218,7 @@ LL | pub fn no_niche_b(b: Option>); = note: enum has no representation hint error: `extern` block uses type `u128`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:96:34 + --> $DIR/lint-ctypes.rs:95:34 | LL | pub static static_u128_type: u128; | ^^^^ not FFI-safe @@ -262,11 +226,12 @@ LL | pub static static_u128_type: u128; = note: 128-bit integers don't currently have a known stable ABI error: `extern` block uses type `u128`, which is not FFI-safe - --> $DIR/lint-ctypes.rs:97:40 + --> $DIR/lint-ctypes.rs:96:40 | LL | pub static static_u128_array_type: [u128; 16]; | ^^^^^^^^^^ not FFI-safe | = note: 128-bit integers don't currently have a known stable ABI -error: aborting due to 29 previous errors +error: aborting due to 24 previous errors + From 8b6289f6ae9639fa1c3dc67a5386e79938a94735 Mon Sep 17 00:00:00 2001 From: niacdoial Date: Tue, 3 Dec 2024 23:57:56 +0100 Subject: [PATCH 07/13] lint ImproperCTypes: message tweaks and refactoring from code review --- compiler/rustc_lint/messages.ftl | 8 +-- compiler/rustc_lint/src/types.rs | 51 ++++++++++--------- .../lint/extern-C-fnptr-lints-slices.stderr | 2 +- tests/ui/lint/lint-ctypes-73249-2.stderr | 2 +- tests/ui/lint/lint-ctypes-cstr.stderr | 10 ++-- tests/ui/lint/lint-ctypes-fn.stderr | 12 ++--- tests/ui/lint/lint-ctypes.stderr | 16 +++--- 7 files changed, 52 insertions(+), 49 deletions(-) diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index b1564bb11b243..422629cd11d08 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -391,7 +391,7 @@ lint_improper_ctypes_pat_help = consider using the base type instead lint_improper_ctypes_pat_reason = pattern types have no C equivalent lint_improper_ctypes_sized_ptr_to_unsafe_type = - this reference (`{$ty}`) is can safely be translated into a C pointer, but `{$inner_ty}` itself is not FFI-safe + this reference (`{$ty}`) is ABI-compatible with a C pointer, but `{$inner_ty}` itself does not have a C layout lint_improper_ctypes_slice_help = consider using a raw pointer to the slice's first element (and a length) instead @@ -419,9 +419,9 @@ lint_improper_ctypes_union_layout_help = consider adding a `#[repr(C)]` or `#[re lint_improper_ctypes_union_layout_reason = this union has unspecified layout lint_improper_ctypes_union_non_exhaustive = this union is non-exhaustive -lint_improper_ctypes_unsized_box = this box for an unsized rust type contains metadata, which makes it incompatible with a C pointer -lint_improper_ctypes_unsized_ptr = this pointer to an unsized rust type contains metadata, which makes it incompatible with a C pointer -lint_improper_ctypes_unsized_ref = this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer +lint_improper_ctypes_unsized_box = this box for an unsized type contains metadata, which makes it incompatible with a C pointer +lint_improper_ctypes_unsized_ptr = this pointer to an unsized type contains metadata, which makes it incompatible with a C pointer +lint_improper_ctypes_unsized_ref = this reference to an unsized type contains metadata, which makes it incompatible with a C pointer lint_incomplete_include = include macro expected single expression in source diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index f9185b3f529a9..df8c556f25ec1 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -743,10 +743,10 @@ enum FfiResult<'tcx> { /// Determine if a type is sized or not, and wether it affects references/pointers/boxes to it #[derive(Clone, Copy)] enum TypeSizedness { - /// sized type (pointers are C-compatible) - Sized, + /// type of definite size (pointers are C-compatible) + Definite, /// unsized type because it includes an opaque/foreign type (pointers are C-compatible) - UnsizedBecauseForeign, + UnsizedWithExternType, /// unsized type for other reasons (slice, string, dyn Trait, closure, ...) (pointers are not C-compatible) UnsizedWithMetadata, } @@ -757,13 +757,13 @@ fn get_type_sizedness<'tcx, 'a>(cx: &'a LateContext<'tcx>, ty: Ty<'tcx>) -> Type let tcx = cx.tcx; if ty.is_sized(tcx, cx.typing_env()) { - TypeSizedness::Sized + TypeSizedness::Definite } else { match ty.kind() { ty::Slice(_) => TypeSizedness::UnsizedWithMetadata, ty::Str => TypeSizedness::UnsizedWithMetadata, ty::Dynamic(..) => TypeSizedness::UnsizedWithMetadata, - ty::Foreign(..) => TypeSizedness::UnsizedBecauseForeign, + ty::Foreign(..) => TypeSizedness::UnsizedWithExternType, // While opaque types are checked for earlier, if a projection in a struct field // normalizes to an opaque type, then it will reach this branch. ty::Alias(ty::Opaque, ..) => todo!("We... don't know enough about this type yet?"), @@ -797,8 +797,8 @@ fn get_type_sizedness<'tcx, 'a>(cx: &'a LateContext<'tcx>, ty: Ty<'tcx>) -> Type .unwrap_or(field_ty); match get_type_sizedness(cx, field_ty) { s @ (TypeSizedness::UnsizedWithMetadata - | TypeSizedness::UnsizedBecauseForeign) => s, - TypeSizedness::Sized => { + | TypeSizedness::UnsizedWithExternType) => s, + TypeSizedness::Definite => { bug!("failed to find the reason why struct `{:?}` is unsized", ty) } } @@ -816,16 +816,16 @@ fn get_type_sizedness<'tcx, 'a>(cx: &'a LateContext<'tcx>, ty: Ty<'tcx>) -> Type .unwrap_or(field_ty); match get_type_sizedness(cx, field_ty) { s @ (TypeSizedness::UnsizedWithMetadata - | TypeSizedness::UnsizedBecauseForeign) => s, - TypeSizedness::Sized => { + | TypeSizedness::UnsizedWithExternType) => s, + TypeSizedness::Definite => { bug!("failed to find the reason why tuple `{:?}` is unsized", ty) } } } - t => { + ty => { bug!( "we shouldn't be trying to determine if this is unsized for a reason or another: `{:?}`", - t + ty ) } } @@ -1135,7 +1135,7 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { match *ty.kind() { ty::Adt(def, args) => { if let Some(inner_ty) = ty.boxed_ty() { - if let TypeSizedness::UnsizedBecauseForeign | TypeSizedness::Sized = + if let TypeSizedness::UnsizedWithExternType | TypeSizedness::Definite = get_type_sizedness(self.cx, inner_ty) { // discussion on declaration vs definition: @@ -1340,24 +1340,27 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } ty::RawPtr(inner_ty, _) | ty::Ref(_, inner_ty, _) => { - if let TypeSizedness::UnsizedBecauseForeign | TypeSizedness::Sized = + if let TypeSizedness::UnsizedWithExternType | TypeSizedness::Definite = get_type_sizedness(self.cx, inner_ty) { - // there's a nuance on what this lint should do for function definitions - // (`extern "C" fn fn_name(...) {...}`) versus declarations (`extern "C" {fn fn_name(...);}`). - // (this is touched upon in https://github.com/rust-lang/rust/issues/66220 - // and https://github.com/rust-lang/rust/pull/72700) + // there's a nuance on what this lint should do for + // function definitions (`extern "C" fn fn_name(...) {...}`) + // versus declarations (`unsafe extern "C" {fn fn_name(...);}`). + // This is touched upon in https://github.com/rust-lang/rust/issues/66220 + // and https://github.com/rust-lang/rust/pull/72700 // // The big question is: what does "ABI safety" mean? if you have something translated to a C pointer // (which has a stable layout) but points to FFI-unsafe type, is it safe? - // on one hand, the function's ABI will match that of a similar C-declared function API, - // on the other, dereferencing the pointer in not-rust will be painful. - // In this code, the opinion is split between function declarations and function definitions. + // On one hand, the function's ABI will match that of a similar C-declared function API, + // on the other, dereferencing the pointer on the other side of the FFI boundary will be painful. + // In this code, the opinion on is split between function declarations and function definitions, + // with the idea that at least one side of the FFI boundary needs to treat the pointee as an opaque type. // For declarations, we see this as unsafe, but for definitions, we see this as safe. - // This is mostly because, for extern function declarations, the actual definition of the function is written somewhere else, - // so the fact that a pointer's pointee should be treated as opaque to one side or the other can be explicitely written out. - // For extern function definitions, however, both callee and some callers can be written in rust, - // so developers need to keep as much typing information as possible. + // + // For extern function declarations, the actual definition of the function is written somewhere else, + // meaning the declaration is free to express this opaqueness with an extern type (opaque caller-side) or a std::ffi::c_void (opaque callee-side) + // For extern function definitions, however, in the case where the type is opaque caller-side, it is not opaque callee-side, + // and having the full type information is necessary to compile the function. if matches!(self.mode, CItemKind::Definition) { return FfiSafe; } else if matches!(ty.kind(), ty::RawPtr(..)) diff --git a/tests/ui/lint/extern-C-fnptr-lints-slices.stderr b/tests/ui/lint/extern-C-fnptr-lints-slices.stderr index bbd16ce8ce16e..c0923dd96c8c3 100644 --- a/tests/ui/lint/extern-C-fnptr-lints-slices.stderr +++ b/tests/ui/lint/extern-C-fnptr-lints-slices.stderr @@ -6,7 +6,7 @@ LL | pub type F = extern "C" fn(&[u8]); | = note: the function pointer to `for<'a> extern "C" fn(&'a [u8])` is FFI-unsafe due to `&[u8]` = help: consider using a raw pointer to the slice's first element (and a length) instead - = note: this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer + = note: this reference to an unsized type contains metadata, which makes it incompatible with a C pointer note: the lint level is defined here --> $DIR/extern-C-fnptr-lints-slices.rs:1:8 | diff --git a/tests/ui/lint/lint-ctypes-73249-2.stderr b/tests/ui/lint/lint-ctypes-73249-2.stderr index 9143e57174e40..f035cdb213efe 100644 --- a/tests/ui/lint/lint-ctypes-73249-2.stderr +++ b/tests/ui/lint/lint-ctypes-73249-2.stderr @@ -4,7 +4,7 @@ error: `extern` block uses type `Qux`, which is not FFI-safe LL | fn lint_me() -> A<()>; | ^^^^^ not FFI-safe | - = note: this reference (`&Qux`) is can safely be translated into a C pointer, but `Qux` itself is not FFI-safe + = note: this reference (`&Qux`) is ABI-compatible with a C pointer, but `Qux` itself does not have a C layout = note: opaque types have no C equivalent note: the lint level is defined here --> $DIR/lint-ctypes-73249-2.rs:2:9 diff --git a/tests/ui/lint/lint-ctypes-cstr.stderr b/tests/ui/lint/lint-ctypes-cstr.stderr index 7bf4d1068027f..da15b748f2110 100644 --- a/tests/ui/lint/lint-ctypes-cstr.stderr +++ b/tests/ui/lint/lint-ctypes-cstr.stderr @@ -19,7 +19,7 @@ LL | fn take_cstr_ref(s: &CStr); | ^^^^^ not FFI-safe | = help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()` - = note: this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer + = note: this reference to an unsized type contains metadata, which makes it incompatible with a C pointer error: `extern` block uses type `CString`, which is not FFI-safe --> $DIR/lint-ctypes-cstr.rs:13:24 @@ -36,7 +36,7 @@ error: `extern` block uses type `CString`, which is not FFI-safe LL | fn take_cstring_ref(s: &CString); | ^^^^^^^^ not FFI-safe | - = note: this reference (`&CString`) is can safely be translated into a C pointer, but `CString` itself is not FFI-safe + = note: this reference (`&CString`) is ABI-compatible with a C pointer, but `CString` itself does not have a C layout = help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()` = note: `CStr`/`CString` do not have a guaranteed layout @@ -46,7 +46,7 @@ error: `extern` block uses type `CString`, which is not FFI-safe LL | fn no_special_help_for_mut_cstring(s: *mut CString); | ^^^^^^^^^^^^ not FFI-safe | - = note: this reference (`*mut CString`) is can safely be translated into a C pointer, but `CString` itself is not FFI-safe + = note: this reference (`*mut CString`) is ABI-compatible with a C pointer, but `CString` itself does not have a C layout = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct = note: this struct has unspecified layout @@ -56,7 +56,7 @@ error: `extern` block uses type `CString`, which is not FFI-safe LL | fn no_special_help_for_mut_cstring_ref(s: &mut CString); | ^^^^^^^^^^^^ not FFI-safe | - = note: this reference (`&mut CString`) is can safely be translated into a C pointer, but `CString` itself is not FFI-safe + = note: this reference (`&mut CString`) is ABI-compatible with a C pointer, but `CString` itself does not have a C layout = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct = note: this struct has unspecified layout @@ -67,7 +67,7 @@ LL | extern "C" fn rust_take_cstr_ref(s: &CStr) {} | ^^^^^ not FFI-safe | = help: consider passing a `*const std::ffi::c_char` instead, and use `CStr::as_ptr()` - = note: this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer + = note: this reference to an unsized type contains metadata, which makes it incompatible with a C pointer note: the lint level is defined here --> $DIR/lint-ctypes-cstr.rs:2:26 | diff --git a/tests/ui/lint/lint-ctypes-fn.stderr b/tests/ui/lint/lint-ctypes-fn.stderr index 0af4ded135da6..c86c02c80064c 100644 --- a/tests/ui/lint/lint-ctypes-fn.stderr +++ b/tests/ui/lint/lint-ctypes-fn.stderr @@ -5,7 +5,7 @@ LL | pub extern "C" fn slice_type(p: &[u32]) { } | ^^^^^^ not FFI-safe | = help: consider using a raw pointer to the slice's first element (and a length) instead - = note: this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer + = note: this reference to an unsized type contains metadata, which makes it incompatible with a C pointer note: the lint level is defined here --> $DIR/lint-ctypes-fn.rs:2:9 | @@ -19,7 +19,7 @@ LL | pub extern "C" fn str_type(p: &str) { } | ^^^^ not FFI-safe | = help: consider using `*const u8` and a length instead - = note: this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer + = note: this reference to an unsized type contains metadata, which makes it incompatible with a C pointer error: `extern` fn uses type `Box<[u8]>`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:80:34 @@ -28,7 +28,7 @@ LL | pub extern "C" fn boxed_slice(p: Box<[u8]>) { } | ^^^^^^^^^ not FFI-safe | = help: consider using a raw pointer to the slice's first element (and a length) instead - = note: this box for an unsized rust type contains metadata, which makes it incompatible with a C pointer + = note: this box for an unsized type contains metadata, which makes it incompatible with a C pointer error: `extern` fn uses type `Box`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:83:35 @@ -37,7 +37,7 @@ LL | pub extern "C" fn boxed_string(p: Box) { } | ^^^^^^^^ not FFI-safe | = help: consider using `*const u8` and a length instead - = note: this box for an unsized rust type contains metadata, which makes it incompatible with a C pointer + = note: this box for an unsized type contains metadata, which makes it incompatible with a C pointer error: `extern` fn uses type `Box`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:86:34 @@ -45,7 +45,7 @@ error: `extern` fn uses type `Box`, which is not FFI-safe LL | pub extern "C" fn boxed_trait(p: Box) { } | ^^^^^^^^^^^^^^ not FFI-safe | - = note: this box for an unsized rust type contains metadata, which makes it incompatible with a C pointer + = note: this box for an unsized type contains metadata, which makes it incompatible with a C pointer error: `extern` fn uses type `char`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:89:32 @@ -158,7 +158,7 @@ LL | pub extern "C" fn transparent_str(p: TransparentStr) { } | ^^^^^^^^^^^^^^ not FFI-safe | = help: consider using `*const u8` and a length instead - = note: this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer + = note: this reference to an unsized type contains metadata, which makes it incompatible with a C pointer error: `extern` fn uses type `PhantomData`, which is not FFI-safe --> $DIR/lint-ctypes-fn.rs:172:43 diff --git a/tests/ui/lint/lint-ctypes.stderr b/tests/ui/lint/lint-ctypes.stderr index 042ca240818b1..8580a10b21538 100644 --- a/tests/ui/lint/lint-ctypes.stderr +++ b/tests/ui/lint/lint-ctypes.stderr @@ -4,7 +4,7 @@ error: `extern` block uses type `Foo`, which is not FFI-safe LL | pub fn ptr_type1(size: *const Foo); | ^^^^^^^^^^ not FFI-safe | - = note: this reference (`*const Foo`) is can safely be translated into a C pointer, but `Foo` itself is not FFI-safe + = note: this reference (`*const Foo`) is ABI-compatible with a C pointer, but `Foo` itself does not have a C layout = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct = note: this struct has unspecified layout note: the type is defined here @@ -24,7 +24,7 @@ error: `extern` block uses type `Foo`, which is not FFI-safe LL | pub fn ptr_type2(size: *const Foo); | ^^^^^^^^^^ not FFI-safe | - = note: this reference (`*const Foo`) is can safely be translated into a C pointer, but `Foo` itself is not FFI-safe + = note: this reference (`*const Foo`) is ABI-compatible with a C pointer, but `Foo` itself does not have a C layout = help: consider adding a `#[repr(C)]` or `#[repr(transparent)]` attribute to this struct = note: this struct has unspecified layout note: the type is defined here @@ -39,7 +39,7 @@ error: `extern` block uses type `((),)`, which is not FFI-safe LL | pub fn ptr_tuple(p: *const ((),)); | ^^^^^^^^^^^^ not FFI-safe | - = note: this reference (`*const ((),)`) is can safely be translated into a C pointer, but `((),)` itself is not FFI-safe + = note: this reference (`*const ((),)`) is ABI-compatible with a C pointer, but `((),)` itself does not have a C layout = help: consider using a struct instead = note: tuples have unspecified layout @@ -50,7 +50,7 @@ LL | pub fn slice_type(p: &[u32]); | ^^^^^^ not FFI-safe | = help: consider using a raw pointer to the slice's first element (and a length) instead - = note: this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer + = note: this reference to an unsized type contains metadata, which makes it incompatible with a C pointer error: `extern` block uses type `&str`, which is not FFI-safe --> $DIR/lint-ctypes.rs:65:24 @@ -59,7 +59,7 @@ LL | pub fn str_type(p: &str); | ^^^^ not FFI-safe | = help: consider using `*const u8` and a length instead - = note: this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer + = note: this reference to an unsized type contains metadata, which makes it incompatible with a C pointer error: `extern` block uses type `char`, which is not FFI-safe --> $DIR/lint-ctypes.rs:68:25 @@ -92,7 +92,7 @@ error: `extern` block uses type `&dyn Bar`, which is not FFI-safe LL | pub fn trait_type(p: &dyn Bar); | ^^^^^^^^ not FFI-safe | - = note: this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer + = note: this reference to an unsized type contains metadata, which makes it incompatible with a C pointer error: `extern` block uses type `(i32, i32)`, which is not FFI-safe --> $DIR/lint-ctypes.rs:72:26 @@ -180,7 +180,7 @@ LL | pub fn transparent_str(p: TransparentStr); | ^^^^^^^^^^^^^^ not FFI-safe | = help: consider using `*const u8` and a length instead - = note: this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer + = note: this reference to an unsized type contains metadata, which makes it incompatible with a C pointer error: `extern` block uses type `[u8; 8]`, which is not FFI-safe --> $DIR/lint-ctypes.rs:85:27 @@ -197,7 +197,7 @@ error: `extern` block uses type `&UnsizedStructBecauseDyn`, which is not FFI-saf LL | pub fn struct_unsized_ptr_has_metadata(p: &UnsizedStructBecauseDyn); | ^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe | - = note: this reference to an unsized rust type contains metadata, which makes it incompatible with a C pointer + = note: this reference to an unsized type contains metadata, which makes it incompatible with a C pointer error: `extern` block uses type `Option>`, which is not FFI-safe --> $DIR/lint-ctypes.rs:90:26 From 02072fd83ada1ad690057b28bf01c33fb494378d Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Wed, 4 Dec 2024 17:28:30 -0800 Subject: [PATCH 08/13] compiler: Tighten up ImproperCTypesLayer recursion --- compiler/rustc_lint/src/types.rs | 33 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index df8c556f25ec1..90d44371ab5ac 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -1593,19 +1593,16 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { }]); } ffir @ FfiResult::FfiUnsafeWrapper { .. } => { - let mut last_ty = None; - let mut ffiresult_recursor = Some(&ffir); + let mut ffiresult_recursor = ControlFlow::Continue(&ffir); let mut cimproper_layers: Vec> = vec![]; // this whole while block converts the arbitrarily-deep - // FfiResult stack to a ImproperCTypesLayer Vec - while let Some(ref ffir_rec) = ffiresult_recursor { + // FfiResult stack to an ImproperCTypesLayer Vec + while let ControlFlow::Continue(ref ffir_rec) = ffiresult_recursor { match ffir_rec { FfiResult::FfiPhantom(ty) => { - last_ty = Some(ty.clone()); - let len = cimproper_layers.len(); - if len > 0 { - cimproper_layers[len - 1].inner_ty = last_ty.clone(); + if let Some(layer) = cimproper_layers.last_mut() { + layer.inner_ty = Some(ty.clone()); } cimproper_layers.push(ImproperCTypesLayer { ty: ty.clone(), @@ -1614,14 +1611,12 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { note: fluent::lint_improper_ctypes_only_phantomdata, span_note: None, // filled later }); - ffiresult_recursor = None; + ffiresult_recursor = ControlFlow::Break(()); } FfiResult::FfiUnsafe { ty, reason, help } | FfiResult::FfiUnsafeWrapper { ty, reason, help, .. } => { - last_ty = Some(ty.clone()); - let len = cimproper_layers.len(); - if len > 0 { - cimproper_layers[len - 1].inner_ty = last_ty.clone(); + if let Some(layer) = cimproper_layers.last_mut() { + layer.inner_ty = Some(ty.clone()); } cimproper_layers.push(ImproperCTypesLayer { ty: ty.clone(), @@ -1632,9 +1627,9 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { }); if let FfiResult::FfiUnsafeWrapper { wrapped, .. } = ffir_rec { - ffiresult_recursor = Some(wrapped.as_ref()); + ffiresult_recursor = ControlFlow::Continue(wrapped.as_ref()); } else { - ffiresult_recursor = None; + ffiresult_recursor = ControlFlow::Break(()); } } FfiResult::FfiSafe => { @@ -1642,12 +1637,8 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { } }; } - let last_ty = match last_ty { - Some(last_ty) => last_ty, - None => bug!( - "This option should definitely have been filled by the loop that just finished" - ), - }; + // should always have at least one type + let last_ty = cimproper_layers.last().unwrap().ty.clone(); self.emit_ffi_unsafe_type_lint(last_ty, sp, cimproper_layers); } } From 5d8233edbfdafa59e6efae77a74d3ce87ee5347d Mon Sep 17 00:00:00 2001 From: Will-Low <26700668+Will-Low@users.noreply.github.com> Date: Fri, 6 Dec 2024 15:33:05 -0800 Subject: [PATCH 09/13] Define acronym for thread local storage There are multiple references in this module's documentation to the acronym "TLS", without defining it. This is confusing for the reader. I propose that this acronym be defined during the first use of the term. --- library/std/src/thread/local.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/thread/local.rs b/library/std/src/thread/local.rs index 56cf438bd9bbe..2313f4b5beb11 100644 --- a/library/std/src/thread/local.rs +++ b/library/std/src/thread/local.rs @@ -12,7 +12,7 @@ use crate::cell::{Cell, RefCell}; use crate::error::Error; use crate::fmt; -/// A thread local storage key which owns its contents. +/// A thread local storage (TLS) key which owns its contents. /// /// This key uses the fastest possible implementation available to it for the /// target platform. It is instantiated with the [`thread_local!`] macro and the From db9e3681f968c326039d56de478c2f7d3dae3e7e Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 7 Dec 2024 01:11:23 +0000 Subject: [PATCH 10/13] Actually walk into lifetimes and attrs in EarlyContextAndPass --- compiler/rustc_lint/src/early.rs | 2 + .../src/hidden_unicode_codepoints.rs | 1 + compiler/rustc_lint_defs/src/builtin.rs | 1 + tests/ui/rust-2024/gen-kw.e2015.stderr | 40 ++++++++++++++++++- tests/ui/rust-2024/gen-kw.e2018.stderr | 40 ++++++++++++++++++- tests/ui/rust-2024/gen-kw.rs | 16 +++++++- 6 files changed, 95 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_lint/src/early.rs b/compiler/rustc_lint/src/early.rs index 4f3184f1d7cff..a68a2a7f98380 100644 --- a/compiler/rustc_lint/src/early.rs +++ b/compiler/rustc_lint/src/early.rs @@ -245,6 +245,7 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T> fn visit_lifetime(&mut self, lt: &'a ast::Lifetime, _: ast_visit::LifetimeCtxt) { self.check_id(lt.id); + ast_visit::walk_lifetime(self, lt); } fn visit_path(&mut self, p: &'a ast::Path, id: ast::NodeId) { @@ -259,6 +260,7 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T> fn visit_attribute(&mut self, attr: &'a ast::Attribute) { lint_callback!(self, check_attribute, attr); + ast_visit::walk_attribute(self, attr); } fn visit_mac_def(&mut self, mac: &'a ast::MacroDef, id: ast::NodeId) { diff --git a/compiler/rustc_lint/src/hidden_unicode_codepoints.rs b/compiler/rustc_lint/src/hidden_unicode_codepoints.rs index 025fd45204064..28368e1ab462b 100644 --- a/compiler/rustc_lint/src/hidden_unicode_codepoints.rs +++ b/compiler/rustc_lint/src/hidden_unicode_codepoints.rs @@ -9,6 +9,7 @@ use crate::lints::{ use crate::{EarlyContext, EarlyLintPass, LintContext}; declare_lint! { + #[allow(text_direction_codepoint_in_literal)] /// The `text_direction_codepoint_in_literal` lint detects Unicode codepoints that change the /// visual representation of text on screen in a way that does not correspond to their on /// memory representation. diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index d2b7ae620e2b4..54e927df3c42b 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -3929,6 +3929,7 @@ declare_lint! { } declare_lint! { + #[allow(text_direction_codepoint_in_literal)] /// The `text_direction_codepoint_in_comment` lint detects Unicode codepoints in comments that /// change the visual representation of text on screen in a way that does not correspond to /// their on memory representation. diff --git a/tests/ui/rust-2024/gen-kw.e2015.stderr b/tests/ui/rust-2024/gen-kw.e2015.stderr index ff552f663c741..5c42d65abf031 100644 --- a/tests/ui/rust-2024/gen-kw.e2015.stderr +++ b/tests/ui/rust-2024/gen-kw.e2015.stderr @@ -34,11 +34,47 @@ LL | () => { mod test { fn gen() {} } } error: `gen` is a keyword in the 2024 edition --> $DIR/gen-kw.rs:25:9 | -LL | fn test<'gen>() {} +LL | fn test<'gen>(_: &'gen i32) {} | ^^^^ help: you can use a raw identifier to stay compatible: `'r#gen` | = warning: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2024! = note: for more information, see issue #49716 -error: aborting due to 4 previous errors +error: `gen` is a keyword in the 2024 edition + --> $DIR/gen-kw.rs:25:19 + | +LL | fn test<'gen>(_: &'gen i32) {} + | ^^^^ help: you can use a raw identifier to stay compatible: `'r#gen` + | + = warning: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2024! + = note: for more information, see issue #49716 + +error: `gen` is a keyword in the 2024 edition + --> $DIR/gen-kw.rs:33:13 + | +LL | struct Test<'gen>(Box>, &'gen ()); + | ^^^^ help: you can use a raw identifier to stay compatible: `'r#gen` + | + = warning: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2024! + = note: for more information, see issue #49716 + +error: `gen` is a keyword in the 2024 edition + --> $DIR/gen-kw.rs:33:28 + | +LL | struct Test<'gen>(Box>, &'gen ()); + | ^^^^ help: you can use a raw identifier to stay compatible: `'r#gen` + | + = warning: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2024! + = note: for more information, see issue #49716 + +error: `gen` is a keyword in the 2024 edition + --> $DIR/gen-kw.rs:33:37 + | +LL | struct Test<'gen>(Box>, &'gen ()); + | ^^^^ help: you can use a raw identifier to stay compatible: `'r#gen` + | + = warning: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2024! + = note: for more information, see issue #49716 + +error: aborting due to 8 previous errors diff --git a/tests/ui/rust-2024/gen-kw.e2018.stderr b/tests/ui/rust-2024/gen-kw.e2018.stderr index efa812069c386..050e58c119bfa 100644 --- a/tests/ui/rust-2024/gen-kw.e2018.stderr +++ b/tests/ui/rust-2024/gen-kw.e2018.stderr @@ -34,11 +34,47 @@ LL | () => { mod test { fn gen() {} } } error: `gen` is a keyword in the 2024 edition --> $DIR/gen-kw.rs:25:9 | -LL | fn test<'gen>() {} +LL | fn test<'gen>(_: &'gen i32) {} | ^^^^ help: you can use a raw identifier to stay compatible: `'r#gen` | = warning: this is accepted in the current edition (Rust 2018) but is a hard error in Rust 2024! = note: for more information, see issue #49716 -error: aborting due to 4 previous errors +error: `gen` is a keyword in the 2024 edition + --> $DIR/gen-kw.rs:25:19 + | +LL | fn test<'gen>(_: &'gen i32) {} + | ^^^^ help: you can use a raw identifier to stay compatible: `'r#gen` + | + = warning: this is accepted in the current edition (Rust 2018) but is a hard error in Rust 2024! + = note: for more information, see issue #49716 + +error: `gen` is a keyword in the 2024 edition + --> $DIR/gen-kw.rs:33:13 + | +LL | struct Test<'gen>(Box>, &'gen ()); + | ^^^^ help: you can use a raw identifier to stay compatible: `'r#gen` + | + = warning: this is accepted in the current edition (Rust 2018) but is a hard error in Rust 2024! + = note: for more information, see issue #49716 + +error: `gen` is a keyword in the 2024 edition + --> $DIR/gen-kw.rs:33:28 + | +LL | struct Test<'gen>(Box>, &'gen ()); + | ^^^^ help: you can use a raw identifier to stay compatible: `'r#gen` + | + = warning: this is accepted in the current edition (Rust 2018) but is a hard error in Rust 2024! + = note: for more information, see issue #49716 + +error: `gen` is a keyword in the 2024 edition + --> $DIR/gen-kw.rs:33:37 + | +LL | struct Test<'gen>(Box>, &'gen ()); + | ^^^^ help: you can use a raw identifier to stay compatible: `'r#gen` + | + = warning: this is accepted in the current edition (Rust 2018) but is a hard error in Rust 2024! + = note: for more information, see issue #49716 + +error: aborting due to 8 previous errors diff --git a/tests/ui/rust-2024/gen-kw.rs b/tests/ui/rust-2024/gen-kw.rs index 5a658470c0a8e..3c075a4c022a2 100644 --- a/tests/ui/rust-2024/gen-kw.rs +++ b/tests/ui/rust-2024/gen-kw.rs @@ -22,9 +22,23 @@ macro_rules! t { //[e2018]~| WARNING this is accepted in the current edition (Rust 2018) but is a hard error in Rust 2024! } -fn test<'gen>() {} +fn test<'gen>(_: &'gen i32) {} //~^ ERROR `gen` is a keyword in the 2024 edition +//~| ERROR `gen` is a keyword in the 2024 edition //[e2015]~| WARNING this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2024! +//[e2015]~| WARNING this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2024! +//[e2018]~| WARNING this is accepted in the current edition (Rust 2018) but is a hard error in Rust 2024! +//[e2018]~| WARNING this is accepted in the current edition (Rust 2018) but is a hard error in Rust 2024! + +struct Test<'gen>(Box>, &'gen ()); +//~^ ERROR `gen` is a keyword in the 2024 edition +//~| ERROR `gen` is a keyword in the 2024 edition +//~| ERROR `gen` is a keyword in the 2024 edition +//[e2015]~| WARNING this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2024! +//[e2015]~| WARNING this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2024! +//[e2015]~| WARNING this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2024! +//[e2018]~| WARNING this is accepted in the current edition (Rust 2018) but is a hard error in Rust 2024! +//[e2018]~| WARNING this is accepted in the current edition (Rust 2018) but is a hard error in Rust 2024! //[e2018]~| WARNING this is accepted in the current edition (Rust 2018) but is a hard error in Rust 2024! t!(); From 9b07e750553fa9ad9b87e8e567bf05edf1001424 Mon Sep 17 00:00:00 2001 From: shamb0 Date: Wed, 4 Dec 2024 21:09:50 +0530 Subject: [PATCH 11/13] Add allocate_bytes and refactor allocate_str in InterpCx for raw byte allocation Signed-off-by: shamb0 --- .../rustc_const_eval/src/interpret/place.rs | 45 +++++++++++++------ 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/place.rs b/compiler/rustc_const_eval/src/interpret/place.rs index 2beec544fad97..f54a932e1b6f5 100644 --- a/compiler/rustc_const_eval/src/interpret/place.rs +++ b/compiler/rustc_const_eval/src/interpret/place.rs @@ -1018,29 +1018,48 @@ where self.allocate_dyn(layout, kind, MemPlaceMeta::None) } - /// Returns a wide MPlace of type `str` to a new 1-aligned allocation. - /// Immutable strings are deduplicated and stored in global memory. - pub fn allocate_str( + /// Allocates a sequence of bytes in the interpreter's memory. + /// For immutable allocations, uses deduplication to reuse existing memory. + /// For mutable allocations, creates a new unique allocation. + pub fn allocate_bytes( &mut self, - str: &str, + bytes: &[u8], + align: Align, kind: MemoryKind, mutbl: Mutability, - ) -> InterpResult<'tcx, MPlaceTy<'tcx, M::Provenance>> { - let tcx = self.tcx.tcx; - + ) -> InterpResult<'tcx, Pointer> { // Use cache for immutable strings. - let ptr = if mutbl.is_not() { + if mutbl.is_not() { // Use dedup'd allocation function. let salt = M::get_global_alloc_salt(self, None); - let id = tcx.allocate_bytes_dedup(str.as_bytes(), salt); + let id = self.tcx.allocate_bytes_dedup(bytes, salt); // Turn untagged "global" pointers (obtained via `tcx`) into the machine pointer to the allocation. - M::adjust_alloc_root_pointer(&self, Pointer::from(id), Some(kind))? + M::adjust_alloc_root_pointer(&self, Pointer::from(id), Some(kind)) } else { - self.allocate_bytes_ptr(str.as_bytes(), Align::ONE, kind, mutbl)? - }; - let meta = Scalar::from_target_usize(u64::try_from(str.len()).unwrap(), self); + // Allocate new memory for mutable data. + self.allocate_bytes_ptr(bytes, align, kind, mutbl) + } + } + + /// Allocates a string in the interpreter's memory with metadata for length. + /// Uses `allocate_bytes` internally but adds string-specific metadata handling. + pub fn allocate_str( + &mut self, + str: &str, + kind: MemoryKind, + mutbl: Mutability, + ) -> InterpResult<'tcx, MPlaceTy<'tcx, M::Provenance>> { + let bytes = str.as_bytes(); + let ptr = self.allocate_bytes(bytes, Align::ONE, kind, mutbl)?; + + // Create length metadata for the string. + let meta = Scalar::from_target_usize(u64::try_from(bytes.len()).unwrap(), self); + + // Get layout for Rust's str type. let layout = self.layout_of(self.tcx.types.str_).unwrap(); + + // Combine pointer and metadata into a wide pointer. interp_ok(self.ptr_with_meta_to_mplace( ptr.into(), MemPlaceMeta::Meta(meta), From 16bf7223eabd5cde19fab869ea964c14d59a4734 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 2 Dec 2024 20:21:09 +0000 Subject: [PATCH 12/13] Add more info on type/trait mismatches for different crate versions When encountering a type or trait mismatch for two types coming from two different crates with the same name, detect if it is either mixing two types/traits from the same crate on different versions: ``` error[E0308]: mismatched types --> replaced | LL | do_something_type(Type); | ----------------- ^^^^ expected `dependency::Type`, found `dep_2_reexport::Type` | | | arguments to this function are incorrect | note: two different versions of crate `dependency` are being used; two types coming from two different versions of the same crate are different types even if they look the same --> replaced | LL | pub struct Type(pub i32); | ^^^^^^^^^^^^^^^ this is the expected type `dependency::Type` | ::: replaced | LL | pub struct Type; | ^^^^^^^^^^^^^^^ this is the found type `dep_2_reexport::Type` | ::: replaced | LL | extern crate dep_2_reexport; | ---------------------------- one version of crate `dependency` is used here, as a dependency of crate `foo` LL | extern crate dependency; | ------------------------ one version of crate `dependency` is used here, as a direct dependency of the current crate = help: you can use `cargo tree` to explore your dependency tree note: function defined here --> replaced | LL | pub fn do_something_type(_: Type) {} | ^^^^^^^^^^^^^^^^^ error[E0308]: mismatched types --> replaced | LL | do_something_trait(Box::new(Type) as Box); | ------------------ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected trait `dependency::Trait2`, found trait `dep_2_reexport::Trait2` | | | arguments to this function are incorrect | note: two different versions of crate `dependency` are being used; two types coming from two different versions of the same crate are different types even if they look the same --> replaced | LL | pub trait Trait2 {} | ^^^^^^^^^^^^^^^^ this is the expected trait `dependency::Trait2` | ::: replaced | LL | pub trait Trait2 {} | ^^^^^^^^^^^^^^^^ this is the found trait `dep_2_reexport::Trait2` | ::: replaced | LL | extern crate dep_2_reexport; | ---------------------------- one version of crate `dependency` is used here, as a dependency of crate `foo` LL | extern crate dependency; | ------------------------ one version of crate `dependency` is used here, as a direct dependency of the current crate = help: you can use `cargo tree` to explore your dependency tree note: function defined here --> replaced | LL | pub fn do_something_trait(_: Box) {} | ^^^^^^^^^^^^^^^^^^ ``` or if it is different crates that were renamed to the same name: ``` error[E0308]: mismatched types --> $DIR/type-mismatch-same-crate-name.rs:21:20 | LL | a::try_foo(foo2); | ---------- ^^^^ expected `main::a::Foo`, found a different `main::a::Foo` | | | arguments to this function are incorrect | note: two types coming from two different crates are different types even if they look the same --> $DIR/auxiliary/crate_a2.rs:1:1 | LL | pub struct Foo; | ^^^^^^^^^^^^^^ this is the found type `crate_a2::Foo` | ::: $DIR/auxiliary/crate_a1.rs:1:1 | LL | pub struct Foo; | ^^^^^^^^^^^^^^ this is the expected type `crate_a1::Foo` | ::: $DIR/type-mismatch-same-crate-name.rs:13:17 | LL | let foo2 = {extern crate crate_a2 as a; a::Foo}; | --------------------------- one type comes from crate `crate_a2` is used here, which is renamed locally to `a` ... LL | extern crate crate_a1 as a; | --------------------------- one type comes from crate `crate_a1` is used here, which is renamed locally to `a` note: function defined here --> $DIR/auxiliary/crate_a1.rs:10:8 | LL | pub fn try_foo(x: Foo){} | ^^^^^^^ error[E0308]: mismatched types --> $DIR/type-mismatch-same-crate-name.rs:27:20 | LL | a::try_bar(bar2); | ---------- ^^^^ expected trait `main::a::Bar`, found a different trait `main::a::Bar` | | | arguments to this function are incorrect | note: two types coming from two different crates are different types even if they look the same --> $DIR/auxiliary/crate_a2.rs:3:1 | LL | pub trait Bar {} | ^^^^^^^^^^^^^ this is the found trait `crate_a2::Bar` | ::: $DIR/auxiliary/crate_a1.rs:3:1 | LL | pub trait Bar {} | ^^^^^^^^^^^^^ this is the expected trait `crate_a1::Bar` | ::: $DIR/type-mismatch-same-crate-name.rs:13:17 | LL | let foo2 = {extern crate crate_a2 as a; a::Foo}; | --------------------------- one trait comes from crate `crate_a2` is used here, which is renamed locally to `a` ... LL | extern crate crate_a1 as a; | --------------------------- one trait comes from crate `crate_a1` is used here, which is renamed locally to `a` note: function defined here --> $DIR/auxiliary/crate_a1.rs:11:8 | LL | pub fn try_bar(x: Box){} | ^^^^^^^ ``` This new output unifies the E0308 errors detail with the pre-existing E0277 errors, and better differentiates the "`extern crate` renamed" and "same crate, different versions" cases. --- .../src/error_reporting/infer/mod.rs | 134 +++++++++++++++--- .../traits/fulfillment_errors.rs | 4 +- tests/incremental/circular-dependencies.rs | 16 +-- .../foo.stderr | 2 +- .../crate-loading/multiple-dep-versions-1.rs | 3 + .../crate-loading/multiple-dep-versions-2.rs | 4 + .../crate-loading/multiple-dep-versions-3.rs | 2 +- .../crate-loading/multiple-dep-versions.rs | 6 +- .../multiple-dep-versions.stderr | 80 +++++++++-- .../similar_paths_primitive.rs | 16 ++- .../similar_paths_primitive.stderr | 14 +- tests/ui/type/auxiliary/crate_a1.rs | 2 +- tests/ui/type/auxiliary/crate_a2.rs | 2 +- .../ui/type/type-mismatch-same-crate-name.rs | 27 ++-- .../type/type-mismatch-same-crate-name.stderr | 45 ++++-- 15 files changed, 281 insertions(+), 76 deletions(-) diff --git a/compiler/rustc_trait_selection/src/error_reporting/infer/mod.rs b/compiler/rustc_trait_selection/src/error_reporting/infer/mod.rs index 8ba7969207e8a..f856a8d7abbe7 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/infer/mod.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/infer/mod.rs @@ -52,7 +52,9 @@ use std::{cmp, fmt, iter}; use rustc_abi::ExternAbi; use rustc_data_structures::fx::{FxIndexMap, FxIndexSet}; -use rustc_errors::{Applicability, Diag, DiagStyledString, IntoDiagArg, StringPart, pluralize}; +use rustc_errors::{ + Applicability, Diag, DiagStyledString, IntoDiagArg, MultiSpan, StringPart, pluralize, +}; use rustc_hir::def::DefKind; use rustc_hir::def_id::DefId; use rustc_hir::intravisit::Visitor; @@ -67,6 +69,7 @@ use rustc_middle::ty::{ self, List, Region, Ty, TyCtxt, TypeFoldable, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, }; +use rustc_span::def_id::LOCAL_CRATE; use rustc_span::{BytePos, DesugaringKind, Pos, Span, sym}; use tracing::{debug, instrument}; @@ -211,7 +214,9 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { } /// Adds a note if the types come from similarly named crates - fn check_and_note_conflicting_crates(&self, err: &mut Diag<'_>, terr: TypeError<'tcx>) { + fn check_and_note_conflicting_crates(&self, err: &mut Diag<'_>, terr: TypeError<'tcx>) -> bool { + // FIXME(estebank): unify with `report_similar_impl_candidates`. The message is similar, + // even if the logic needed to detect the case is very different. use hir::def_id::CrateNum; use rustc_hir::definitions::DisambiguatedDefPathData; use ty::GenericArg; @@ -285,7 +290,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { } } - let report_path_match = |err: &mut Diag<'_>, did1: DefId, did2: DefId| { + let report_path_match = |err: &mut Diag<'_>, did1: DefId, did2: DefId, ty: &str| -> bool { // Only report definitions from different crates. If both definitions // are from a local module we could have false positives, e.g. // let _ = [{struct Foo; Foo}, {struct Foo; Foo}]; @@ -297,24 +302,112 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { // We compare strings because DefPath can be different // for imported and non-imported crates + let expected_str = self.tcx.def_path_str(did1); + let found_str = self.tcx.def_path_str(did2); + let Ok(expected_abs) = abs_path(did1) else { return false }; + let Ok(found_abs) = abs_path(did2) else { return false }; let same_path = || -> Result<_, PrintError> { - Ok(self.tcx.def_path_str(did1) == self.tcx.def_path_str(did2) - || abs_path(did1)? == abs_path(did2)?) + Ok(expected_str == found_str || expected_abs == found_abs) + }; + // We want to use as unique a type path as possible. If both types are "locally + // known" by the same name, we use the "absolute path" which uses the original + // crate name instead. + let (expected, found) = if expected_str == found_str { + (expected_abs.join("::"), found_abs.join("::")) + } else { + (expected_str.clone(), found_str.clone()) }; if same_path().unwrap_or(false) { - let crate_name = self.tcx.crate_name(did1.krate); - let msg = if did1.is_local() || did2.is_local() { + // We've displayed "expected `a::b`, found `a::b`". We add context to + // differentiate the different cases where that might happen. + let expected_crate_name = self.tcx.crate_name(did1.krate); + let found_crate_name = self.tcx.crate_name(did2.krate); + let same_crate = expected_crate_name == found_crate_name; + let expected_sp = self.tcx.def_span(did1); + let found_sp = self.tcx.def_span(did2); + + let both_direct_dependencies = if !did1.is_local() + && !did2.is_local() + && let Some(data1) = self.tcx.extern_crate(did1.krate) + && let Some(data2) = self.tcx.extern_crate(did2.krate) + && data1.dependency_of == LOCAL_CRATE + && data2.dependency_of == LOCAL_CRATE + { + // If both crates are directly depended on, we don't want to mention that + // in the final message, as it is redundant wording. + // We skip the case of semver trick, where one version of the local crate + // depends on another version of itself by checking that both crates at play + // are not the current one. + true + } else { + false + }; + + let mut span: MultiSpan = vec![expected_sp, found_sp].into(); + span.push_span_label( + self.tcx.def_span(did1), + format!("this is the expected {ty} `{expected}`"), + ); + span.push_span_label( + self.tcx.def_span(did2), + format!("this is the found {ty} `{found}`"), + ); + for def_id in [did1, did2] { + let crate_name = self.tcx.crate_name(def_id.krate); + if !def_id.is_local() + && let Some(data) = self.tcx.extern_crate(def_id.krate) + { + let descr = if same_crate { + "one version of".to_string() + } else { + format!("one {ty} comes from") + }; + let dependency = if both_direct_dependencies { + if let rustc_session::cstore::ExternCrateSource::Extern(def_id) = + data.src + && let Some(name) = self.tcx.opt_item_name(def_id) + { + format!(", which is renamed locally to `{name}`") + } else { + String::new() + } + } else if data.dependency_of == LOCAL_CRATE { + ", as a direct dependency of the current crate".to_string() + } else { + let dep = self.tcx.crate_name(data.dependency_of); + format!(", as a dependency of crate `{dep}`") + }; + span.push_span_label( + data.span, + format!("{descr} crate `{crate_name}` used here{dependency}"), + ); + } + } + let msg = if (did1.is_local() || did2.is_local()) && same_crate { + format!( + "the crate `{expected_crate_name}` is compiled multiple times, \ + possibly with different configurations", + ) + } else if same_crate { format!( - "the crate `{crate_name}` is compiled multiple times, possibly with different configurations" + "two different versions of crate `{expected_crate_name}` are being \ + used; two types coming from two different versions of the same crate \ + are different types even if they look the same", ) } else { format!( - "perhaps two different versions of crate `{crate_name}` are being used?" + "two types coming from two different crates are different types even \ + if they look the same", ) }; - err.note(msg); + err.span_note(span, msg); + if same_crate { + err.help("you can use `cargo tree` to explore your dependency tree"); + } + return true; } } + false }; match terr { TypeError::Sorts(ref exp_found) => { @@ -323,14 +416,15 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { if let (&ty::Adt(exp_adt, _), &ty::Adt(found_adt, _)) = (exp_found.expected.kind(), exp_found.found.kind()) { - report_path_match(err, exp_adt.did(), found_adt.did()); + return report_path_match(err, exp_adt.did(), found_adt.did(), "type"); } } TypeError::Traits(ref exp_found) => { - report_path_match(err, exp_found.expected, exp_found.found); + return report_path_match(err, exp_found.expected, exp_found.found, "trait"); } _ => (), // FIXME(#22750) handle traits and stuff } + false } fn note_error_origin( @@ -1409,6 +1503,10 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { label_or_note(span, terr.to_string(self.tcx)); } + if self.check_and_note_conflicting_crates(diag, terr) { + return; + } + if let Some((expected, found, path)) = expected_found { let (expected_label, found_label, exp_found) = match exp_found { Mismatch::Variable(ef) => ( @@ -1470,15 +1568,17 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { |prim: Ty<'tcx>, shadow: Ty<'tcx>, defid: DefId, diag: &mut Diag<'_>| { let name = shadow.sort_string(self.tcx); diag.note(format!( - "{prim} and {name} have similar names, but are actually distinct types" + "`{prim}` and {name} have similar names, but are actually distinct types" + )); + diag.note(format!( + "one `{prim}` is a primitive defined by the language", )); - diag.note(format!("{prim} is a primitive defined by the language")); let def_span = self.tcx.def_span(defid); let msg = if defid.is_local() { - format!("{name} is defined in the current crate") + format!("the other {name} is defined in the current crate") } else { let crate_name = self.tcx.crate_name(defid.krate); - format!("{name} is defined in crate `{crate_name}`") + format!("the other {name} is defined in crate `{crate_name}`") }; diag.span_note(def_span, msg); }; @@ -1666,8 +1766,6 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { } } - self.check_and_note_conflicting_crates(diag, terr); - self.note_and_explain_type_err(diag, terr, cause, span, cause.body_id.to_def_id()); if let Some(exp_found) = exp_found && let exp_found = TypeError::Sorts(exp_found) diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs index 90b1825362908..4fb02f609438e 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs @@ -1745,9 +1745,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { }; ( data.span, - format!( - "one version of crate `{crate_name}` is used here, as a {dependency}" - ), + format!("one version of crate `{crate_name}` used here, as a {dependency}"), ) }) { diff --git a/tests/incremental/circular-dependencies.rs b/tests/incremental/circular-dependencies.rs index 320edad3fde9f..c7b5b931fbbec 100644 --- a/tests/incremental/circular-dependencies.rs +++ b/tests/incremental/circular-dependencies.rs @@ -6,10 +6,12 @@ //@ [cfail2] compile-flags: --test --extern aux={{build-base}}/circular-dependencies/auxiliary/libcircular_dependencies_aux.rmeta -L dependency={{build-base}}/circular-dependencies pub struct Foo; -//[cfail2]~^ NOTE `Foo` is defined in the current crate -//[cfail2]~| NOTE `Foo` is defined in the current crate -//[cfail2]~| NOTE `circular_dependencies::Foo` is defined in crate `circular_dependencies` -//[cfail2]~| NOTE `circular_dependencies::Foo` is defined in crate `circular_dependencies` +//[cfail2]~^ NOTE the crate `circular_dependencies` is compiled multiple times, possibly with different configurations +//[cfail2]~| NOTE the crate `circular_dependencies` is compiled multiple times, possibly with different configurations +//[cfail2]~| NOTE this is the expected type `Foo` +//[cfail2]~| NOTE this is the expected type `circular_dependencies::Foo` +//[cfail2]~| NOTE this is the found type `Foo` +//[cfail2]~| NOTE this is the found type `circular_dependencies::Foo` pub fn consume_foo(_: Foo) {} //[cfail2]~^ NOTE function defined here @@ -24,14 +26,12 @@ fn test() { //[cfail2]~^ ERROR mismatched types [E0308] //[cfail2]~| NOTE expected `circular_dependencies::Foo`, found `Foo` //[cfail2]~| NOTE arguments to this function are incorrect - //[cfail2]~| NOTE `Foo` and `circular_dependencies::Foo` have similar names, but are actually distinct types - //[cfail2]~| NOTE the crate `circular_dependencies` is compiled multiple times, possibly with different configurations //[cfail2]~| NOTE function defined here + //[cfail2]~| NOTE one version of crate `circular_dependencies` used here, as a dependency of crate `circular_dependencies_aux` + //[cfail2]~| NOTE one version of crate `circular_dependencies` used here, as a dependency of crate `circular_dependencies_aux` consume_foo(aux::produce_foo()); //[cfail2]~^ ERROR mismatched types [E0308] //[cfail2]~| NOTE expected `Foo`, found `circular_dependencies::Foo` //[cfail2]~| NOTE arguments to this function are incorrect - //[cfail2]~| NOTE `circular_dependencies::Foo` and `Foo` have similar names, but are actually distinct types - //[cfail2]~| NOTE the crate `circular_dependencies` is compiled multiple times, possibly with different configurations } diff --git a/tests/run-make/crate-loading-crate-depends-on-itself/foo.stderr b/tests/run-make/crate-loading-crate-depends-on-itself/foo.stderr index 36379429530bc..7f131153540bb 100644 --- a/tests/run-make/crate-loading-crate-depends-on-itself/foo.stderr +++ b/tests/run-make/crate-loading-crate-depends-on-itself/foo.stderr @@ -8,7 +8,7 @@ note: there are multiple different versions of crate `foo` in the dependency gra --> foo-current.rs:7:1 | 4 | extern crate foo; - | ----------------- one version of crate `foo` is used here, as a direct dependency of the current crate + | ----------------- one version of crate `foo` used here, as a direct dependency of the current crate 5 | 6 | pub struct Struct; | ----------------- this type implements the required trait diff --git a/tests/run-make/crate-loading/multiple-dep-versions-1.rs b/tests/run-make/crate-loading/multiple-dep-versions-1.rs index d81462504dd57..bfeabccf5c146 100644 --- a/tests/run-make/crate-loading/multiple-dep-versions-1.rs +++ b/tests/run-make/crate-loading/multiple-dep-versions-1.rs @@ -5,8 +5,11 @@ pub trait Trait { fn foo(&self); fn bar(); } +pub trait Trait2 {} impl Trait for Type { fn foo(&self) {} fn bar() {} } pub fn do_something(_: X) {} +pub fn do_something_type(_: Type) {} +pub fn do_something_trait(_: Box) {} diff --git a/tests/run-make/crate-loading/multiple-dep-versions-2.rs b/tests/run-make/crate-loading/multiple-dep-versions-2.rs index 0a566fe2c6051..682d1ff64b822 100644 --- a/tests/run-make/crate-loading/multiple-dep-versions-2.rs +++ b/tests/run-make/crate-loading/multiple-dep-versions-2.rs @@ -5,8 +5,12 @@ pub trait Trait { fn foo(&self); fn bar(); } +pub trait Trait2 {} +impl Trait2 for Type {} impl Trait for Type { fn foo(&self) {} fn bar() {} } pub fn do_something(_: X) {} +pub fn do_something_type(_: Type) {} +pub fn do_something_trait(_: Box) {} diff --git a/tests/run-make/crate-loading/multiple-dep-versions-3.rs b/tests/run-make/crate-loading/multiple-dep-versions-3.rs index f5c4d1baa811a..07444511472fa 100644 --- a/tests/run-make/crate-loading/multiple-dep-versions-3.rs +++ b/tests/run-make/crate-loading/multiple-dep-versions-3.rs @@ -2,7 +2,7 @@ #![crate_type = "rlib"] extern crate dependency; -pub use dependency::Type; +pub use dependency::{Trait2, Type, do_something_trait, do_something_type}; pub struct OtherType; impl dependency::Trait for OtherType { fn foo(&self) {} diff --git a/tests/run-make/crate-loading/multiple-dep-versions.rs b/tests/run-make/crate-loading/multiple-dep-versions.rs index c68a9e6489f54..3a4a20d38fc87 100644 --- a/tests/run-make/crate-loading/multiple-dep-versions.rs +++ b/tests/run-make/crate-loading/multiple-dep-versions.rs @@ -1,11 +1,13 @@ extern crate dep_2_reexport; extern crate dependency; -use dep_2_reexport::{OtherType, Type}; -use dependency::{Trait, do_something}; +use dep_2_reexport::{OtherType, Trait2, Type}; +use dependency::{Trait, do_something, do_something_trait, do_something_type}; fn main() { do_something(Type); Type.foo(); Type::bar(); do_something(OtherType); + do_something_type(Type); + do_something_trait(Box::new(Type) as Box); } diff --git a/tests/run-make/crate-loading/multiple-dep-versions.stderr b/tests/run-make/crate-loading/multiple-dep-versions.stderr index 5888aad8f37bc..6e1d6111b5810 100644 --- a/tests/run-make/crate-loading/multiple-dep-versions.stderr +++ b/tests/run-make/crate-loading/multiple-dep-versions.stderr @@ -17,9 +17,9 @@ LL | pub trait Trait { ::: replaced | LL | extern crate dep_2_reexport; - | ---------------------------- one version of crate `dependency` is used here, as a dependency of crate `foo` + | ---------------------------- one version of crate `dependency` used here, as a dependency of crate `foo` LL | extern crate dependency; - | ------------------------ one version of crate `dependency` is used here, as a direct dependency of the current crate + | ------------------------ one version of crate `dependency` used here, as a direct dependency of the current crate | ::: replaced | @@ -51,7 +51,7 @@ LL | fn foo(&self); | ::: replaced | -LL | use dependency::{Trait, do_something}; +LL | use dependency::{Trait, do_something, do_something_trait, do_something_type}; | ----- `Trait` imported here doesn't correspond to the right version of crate `dependency` | ::: replaced @@ -76,7 +76,7 @@ LL | fn bar(); | ::: replaced | -LL | use dependency::{Trait, do_something}; +LL | use dependency::{Trait, do_something, do_something_trait, do_something_type}; | ----- `Trait` imported here doesn't correspond to the right version of crate `dependency` | ::: replaced @@ -101,9 +101,9 @@ LL | pub trait Trait { ::: replaced | LL | extern crate dep_2_reexport; - | ---------------------------- one version of crate `dependency` is used here, as a dependency of crate `foo` + | ---------------------------- one version of crate `dependency` used here, as a dependency of crate `foo` LL | extern crate dependency; - | ------------------------ one version of crate `dependency` is used here, as a direct dependency of the current crate + | ------------------------ one version of crate `dependency` used here, as a direct dependency of the current crate | ::: replaced | @@ -121,7 +121,71 @@ note: required by a bound in `do_something` LL | pub fn do_something(_: X) {} | ^^^^^ required by this bound in `do_something` -error: aborting due to 4 previous errors +error[E0308]: mismatched types + --> replaced + | +LL | do_something_type(Type); + | ----------------- ^^^^ expected `dependency::Type`, found `dep_2_reexport::Type` + | | + | arguments to this function are incorrect + | +note: two different versions of crate `dependency` are being used; two types coming from two different versions of the same crate are different types even if they look the same + --> replaced + | +LL | pub struct Type(pub i32); + | ^^^^^^^^^^^^^^^ this is the expected type `dependency::Type` + | + ::: replaced + | +LL | pub struct Type; + | ^^^^^^^^^^^^^^^ this is the found type `dep_2_reexport::Type` + | + ::: replaced + | +LL | extern crate dep_2_reexport; + | ---------------------------- one version of crate `dependency` used here, as a dependency of crate `foo` +LL | extern crate dependency; + | ------------------------ one version of crate `dependency` used here, as a direct dependency of the current crate + = help: you can use `cargo tree` to explore your dependency tree +note: function defined here + --> replaced + | +LL | pub fn do_something_type(_: Type) {} + | ^^^^^^^^^^^^^^^^^ + +error[E0308]: mismatched types + --> replaced + | +LL | do_something_trait(Box::new(Type) as Box); + | ------------------ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected trait `dependency::Trait2`, found trait `dep_2_reexport::Trait2` + | | + | arguments to this function are incorrect + | +note: two different versions of crate `dependency` are being used; two types coming from two different versions of the same crate are different types even if they look the same + --> replaced + | +LL | pub trait Trait2 {} + | ^^^^^^^^^^^^^^^^ this is the expected trait `dependency::Trait2` + | + ::: replaced + | +LL | pub trait Trait2 {} + | ^^^^^^^^^^^^^^^^ this is the found trait `dep_2_reexport::Trait2` + | + ::: replaced + | +LL | extern crate dep_2_reexport; + | ---------------------------- one version of crate `dependency` used here, as a dependency of crate `foo` +LL | extern crate dependency; + | ------------------------ one version of crate `dependency` used here, as a direct dependency of the current crate + = help: you can use `cargo tree` to explore your dependency tree +note: function defined here + --> replaced + | +LL | pub fn do_something_trait(_: Box) {} + | ^^^^^^^^^^^^^^^^^^ + +error: aborting due to 6 previous errors -Some errors have detailed explanations: E0277, E0599. +Some errors have detailed explanations: E0277, E0308, E0599. For more information about an error, try `rustc --explain E0277`. \ No newline at end of file diff --git a/tests/ui/mismatched_types/similar_paths_primitive.rs b/tests/ui/mismatched_types/similar_paths_primitive.rs index 98890a15d98b1..a58fe68b86381 100644 --- a/tests/ui/mismatched_types/similar_paths_primitive.rs +++ b/tests/ui/mismatched_types/similar_paths_primitive.rs @@ -1,14 +1,22 @@ #![allow(non_camel_case_types)] -struct bool; -struct str; +struct bool; //~ NOTE the other `bool` is defined in the current crate +struct str; //~ NOTE the other `str` is defined in the current crate -fn foo(_: bool) {} -fn bar(_: &str) {} +fn foo(_: bool) {} //~ NOTE function defined here +fn bar(_: &str) {} //~ NOTE function defined here fn main() { foo(true); //~^ ERROR mismatched types [E0308] + //~| NOTE expected `bool`, found a different `bool` + //~| NOTE arguments to this function are incorrect + //~| NOTE `bool` and `bool` have similar names, but are actually distinct types + //~| NOTE one `bool` is a primitive defined by the language bar("hello"); //~^ ERROR mismatched types [E0308] + //~| NOTE expected `str`, found a different `str` + //~| NOTE arguments to this function are incorrect + //~| NOTE `str` and `str` have similar names, but are actually distinct types + //~| NOTE one `str` is a primitive defined by the language } diff --git a/tests/ui/mismatched_types/similar_paths_primitive.stderr b/tests/ui/mismatched_types/similar_paths_primitive.stderr index 0530bf5863e69..cf26234dba857 100644 --- a/tests/ui/mismatched_types/similar_paths_primitive.stderr +++ b/tests/ui/mismatched_types/similar_paths_primitive.stderr @@ -6,9 +6,9 @@ LL | foo(true); | | | arguments to this function are incorrect | - = note: bool and `bool` have similar names, but are actually distinct types - = note: bool is a primitive defined by the language -note: `bool` is defined in the current crate + = note: `bool` and `bool` have similar names, but are actually distinct types + = note: one `bool` is a primitive defined by the language +note: the other `bool` is defined in the current crate --> $DIR/similar_paths_primitive.rs:3:1 | LL | struct bool; @@ -20,16 +20,16 @@ LL | fn foo(_: bool) {} | ^^^ ------- error[E0308]: mismatched types - --> $DIR/similar_paths_primitive.rs:12:9 + --> $DIR/similar_paths_primitive.rs:16:9 | LL | bar("hello"); | --- ^^^^^^^ expected `str`, found a different `str` | | | arguments to this function are incorrect | - = note: str and `str` have similar names, but are actually distinct types - = note: str is a primitive defined by the language -note: `str` is defined in the current crate + = note: `str` and `str` have similar names, but are actually distinct types + = note: one `str` is a primitive defined by the language +note: the other `str` is defined in the current crate --> $DIR/similar_paths_primitive.rs:4:1 | LL | struct str; diff --git a/tests/ui/type/auxiliary/crate_a1.rs b/tests/ui/type/auxiliary/crate_a1.rs index e2e18500541ef..616493193fd6c 100644 --- a/tests/ui/type/auxiliary/crate_a1.rs +++ b/tests/ui/type/auxiliary/crate_a1.rs @@ -1,6 +1,6 @@ pub struct Foo; -pub trait Bar{} +pub trait Bar {} pub fn bar() -> Box { unimplemented!() diff --git a/tests/ui/type/auxiliary/crate_a2.rs b/tests/ui/type/auxiliary/crate_a2.rs index d16a4ac10e0c4..57a7685b77c4f 100644 --- a/tests/ui/type/auxiliary/crate_a2.rs +++ b/tests/ui/type/auxiliary/crate_a2.rs @@ -1,6 +1,6 @@ pub struct Foo; -pub trait Bar{} +pub trait Bar {} pub fn bar() -> Box { unimplemented!() diff --git a/tests/ui/type/type-mismatch-same-crate-name.rs b/tests/ui/type/type-mismatch-same-crate-name.rs index da76616523826..e88960364a251 100644 --- a/tests/ui/type/type-mismatch-same-crate-name.rs +++ b/tests/ui/type/type-mismatch-same-crate-name.rs @@ -3,25 +3,32 @@ // This tests the extra note reported when a type error deals with // seemingly identical types. -// The main use case of this error is when there are two crates -// (generally different versions of the same crate) with the same name -// causing a type mismatch. Here, we simulate that error using block-scoped -// aliased `extern crate` declarations. +// The main use case of this error is when there are two crates imported +// with the same name, causing a type mismatch. Here, we simulate that error +// using block-scoped aliased `extern crate` declarations. +// This is *not* the same case as two different crate versions in the +// dependency tree. That is tested in `tests/run-make/crate-loading/`. fn main() { let foo2 = {extern crate crate_a2 as a; a::Foo}; + //~^ NOTE one type comes from crate `crate_a2` used here, which is renamed locally to `a` + //~| NOTE one trait comes from crate `crate_a2` used here, which is renamed locally to `a` let bar2 = {extern crate crate_a2 as a; a::bar()}; { extern crate crate_a1 as a; + //~^ NOTE one type comes from crate `crate_a1` used here, which is renamed locally to `a` + //~| NOTE one trait comes from crate `crate_a1` used here, which is renamed locally to `a` a::try_foo(foo2); //~^ ERROR mismatched types - //~| perhaps two different versions of crate `crate_a1` - //~| expected `main::a::Foo`, found a different `main::a::Foo` + //~| NOTE expected `main::a::Foo`, found a different `main::a::Foo` + //~| NOTE arguments to this function are incorrect + //~| NOTE two types coming from two different crates are different types even if they look the same + //~| NOTE function defined here a::try_bar(bar2); //~^ ERROR mismatched types - //~| perhaps two different versions of crate `crate_a1` - //~| expected trait `main::a::Bar` - //~| expected struct `Box<(dyn main::a::Bar + 'static)>` - //~| found struct `Box` + //~| NOTE expected trait `main::a::Bar`, found a different trait `main::a::Bar` + //~| NOTE arguments to this function are incorrect + //~| NOTE two types coming from two different crates are different types even if they look the same + //~| NOTE function defined here } } diff --git a/tests/ui/type/type-mismatch-same-crate-name.stderr b/tests/ui/type/type-mismatch-same-crate-name.stderr index 504812f5867a6..7b791549f5632 100644 --- a/tests/ui/type/type-mismatch-same-crate-name.stderr +++ b/tests/ui/type/type-mismatch-same-crate-name.stderr @@ -1,23 +1,29 @@ error[E0308]: mismatched types - --> $DIR/type-mismatch-same-crate-name.rs:16:20 + --> $DIR/type-mismatch-same-crate-name.rs:21:20 | LL | a::try_foo(foo2); | ---------- ^^^^ expected `main::a::Foo`, found a different `main::a::Foo` | | | arguments to this function are incorrect | - = note: `main::a::Foo` and `main::a::Foo` have similar names, but are actually distinct types -note: `main::a::Foo` is defined in crate `crate_a2` +note: two types coming from two different crates are different types even if they look the same --> $DIR/auxiliary/crate_a2.rs:1:1 | LL | pub struct Foo; - | ^^^^^^^^^^^^^^ -note: `main::a::Foo` is defined in crate `crate_a1` - --> $DIR/auxiliary/crate_a1.rs:1:1 + | ^^^^^^^^^^^^^^ this is the found type `crate_a2::Foo` + | + ::: $DIR/auxiliary/crate_a1.rs:1:1 | LL | pub struct Foo; - | ^^^^^^^^^^^^^^ - = note: perhaps two different versions of crate `crate_a1` are being used? + | ^^^^^^^^^^^^^^ this is the expected type `crate_a1::Foo` + | + ::: $DIR/type-mismatch-same-crate-name.rs:13:17 + | +LL | let foo2 = {extern crate crate_a2 as a; a::Foo}; + | --------------------------- one type comes from crate `crate_a2` used here, which is renamed locally to `a` +... +LL | extern crate crate_a1 as a; + | --------------------------- one type comes from crate `crate_a1` used here, which is renamed locally to `a` note: function defined here --> $DIR/auxiliary/crate_a1.rs:10:8 | @@ -25,16 +31,31 @@ LL | pub fn try_foo(x: Foo){} | ^^^^^^^ error[E0308]: mismatched types - --> $DIR/type-mismatch-same-crate-name.rs:20:20 + --> $DIR/type-mismatch-same-crate-name.rs:27:20 | LL | a::try_bar(bar2); | ---------- ^^^^ expected trait `main::a::Bar`, found a different trait `main::a::Bar` | | | arguments to this function are incorrect | - = note: expected struct `Box<(dyn main::a::Bar + 'static)>` - found struct `Box` - = note: perhaps two different versions of crate `crate_a1` are being used? +note: two types coming from two different crates are different types even if they look the same + --> $DIR/auxiliary/crate_a2.rs:3:1 + | +LL | pub trait Bar {} + | ^^^^^^^^^^^^^ this is the found trait `crate_a2::Bar` + | + ::: $DIR/auxiliary/crate_a1.rs:3:1 + | +LL | pub trait Bar {} + | ^^^^^^^^^^^^^ this is the expected trait `crate_a1::Bar` + | + ::: $DIR/type-mismatch-same-crate-name.rs:13:17 + | +LL | let foo2 = {extern crate crate_a2 as a; a::Foo}; + | --------------------------- one trait comes from crate `crate_a2` used here, which is renamed locally to `a` +... +LL | extern crate crate_a1 as a; + | --------------------------- one trait comes from crate `crate_a1` used here, which is renamed locally to `a` note: function defined here --> $DIR/auxiliary/crate_a1.rs:11:8 | From 159ed85d242fab974e5032177b6647987a6ca6f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Kr=C3=BCger?= Date: Sat, 16 Nov 2024 13:32:13 +0100 Subject: [PATCH 13/13] crashes: add test for #131451 --- tests/crashes/131451.rs | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 tests/crashes/131451.rs diff --git a/tests/crashes/131451.rs b/tests/crashes/131451.rs new file mode 100644 index 0000000000000..cd5b44bad8a5b --- /dev/null +++ b/tests/crashes/131451.rs @@ -0,0 +1,9 @@ +//@ known-bug: #131451 +//@ needs-rustc-debug-assertions +//@ compile-flags: -Zmir-enable-passes=+GVN -Zmir-enable-passes=+JumpThreading --crate-type=lib + +pub fn fun(terminate: bool) { + while true {} + + while !terminate {} +}