From 84dde3aa3386c2d699ffc7e8f5e05cfbde67918a Mon Sep 17 00:00:00 2001 From: The 8472 Date: Wed, 20 Nov 2024 22:50:58 +0100 Subject: [PATCH 01/23] Add a range argument to vec.extract_if --- library/alloc/src/vec/extract_if.rs | 23 +++++++++++--- library/alloc/src/vec/mod.rs | 43 ++++++++++++++++--------- library/alloc/tests/vec.rs | 49 ++++++++++++++++++++++------- 3 files changed, 83 insertions(+), 32 deletions(-) diff --git a/library/alloc/src/vec/extract_if.rs b/library/alloc/src/vec/extract_if.rs index 72d51e8904488..f722d89d4005a 100644 --- a/library/alloc/src/vec/extract_if.rs +++ b/library/alloc/src/vec/extract_if.rs @@ -1,4 +1,4 @@ -use core::{ptr, slice}; +use core::{ops::{Range, RangeBounds}, ptr, slice}; use super::Vec; use crate::alloc::{Allocator, Global}; @@ -14,7 +14,7 @@ use crate::alloc::{Allocator, Global}; /// #![feature(extract_if)] /// /// let mut v = vec![0, 1, 2]; -/// let iter: std::vec::ExtractIf<'_, _, _> = v.extract_if(|x| *x % 2 == 0); +/// let iter: std::vec::ExtractIf<'_, _, _> = v.extract_if(.., |x| *x % 2 == 0); /// ``` #[unstable(feature = "extract_if", reason = "recently added", issue = "43244")] #[derive(Debug)] @@ -30,6 +30,8 @@ pub struct ExtractIf< pub(super) vec: &'a mut Vec, /// The index of the item that will be inspected by the next call to `next`. pub(super) idx: usize, + /// Elements at and beyond this point will be retained. Must be equal or smaller than `old_len`. + pub(super) end: usize, /// The number of items that have been drained (removed) thus far. pub(super) del: usize, /// The original length of `vec` prior to draining. @@ -38,10 +40,21 @@ pub struct ExtractIf< pub(super) pred: F, } -impl ExtractIf<'_, T, F, A> +impl<'a, T, F, A: Allocator> ExtractIf<'a, T, F, A> where F: FnMut(&mut T) -> bool, { + pub(super) fn new>(vec: &'a mut Vec, pred: F, range: R) -> Self { + let old_len = vec.len(); + let Range { start, end } = slice::range(range, ..old_len); + + // Guard against the vec getting leaked (leak amplification) + unsafe { + vec.set_len(0); + } + ExtractIf { vec, idx: start, del: 0, end, old_len, pred } + } + /// Returns a reference to the underlying allocator. #[unstable(feature = "allocator_api", issue = "32838")] #[inline] @@ -59,7 +72,7 @@ where fn next(&mut self) -> Option { unsafe { - while self.idx < self.old_len { + while self.idx < self.end { let i = self.idx; let v = slice::from_raw_parts_mut(self.vec.as_mut_ptr(), self.old_len); let drained = (self.pred)(&mut v[i]); @@ -82,7 +95,7 @@ where } fn size_hint(&self) -> (usize, Option) { - (0, Some(self.old_len - self.idx)) + (0, Some(self.end - self.idx)) } } diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs index 07a1bd4932138..ae9d4e8c3acde 100644 --- a/library/alloc/src/vec/mod.rs +++ b/library/alloc/src/vec/mod.rs @@ -3610,12 +3610,15 @@ impl Vec { Splice { drain: self.drain(range), replace_with: replace_with.into_iter() } } - /// Creates an iterator which uses a closure to determine if an element should be removed. + /// Creates an iterator which uses a closure to determine if element in the range should be removed. /// /// If the closure returns true, then the element is removed and yielded. /// If the closure returns false, the element will remain in the vector and will not be yielded /// by the iterator. /// + /// Only elements that fall in the provided range are considered for extraction, but any elements + /// after the range will still have to be moved if any element has been extracted. + /// /// If the returned `ExtractIf` is not exhausted, e.g. because it is dropped without iterating /// or the iteration short-circuits, then the remaining elements will be retained. /// Use [`retain`] with a negated predicate if you do not need the returned iterator. @@ -3625,10 +3628,12 @@ impl Vec { /// Using this method is equivalent to the following code: /// /// ``` + /// # use std::cmp::min; /// # let some_predicate = |x: &mut i32| { *x == 2 || *x == 3 || *x == 6 }; /// # let mut vec = vec![1, 2, 3, 4, 5, 6]; - /// let mut i = 0; - /// while i < vec.len() { + /// # let range = 1..4; + /// let mut i = range.start; + /// while i < min(vec.len(), range.end) { /// if some_predicate(&mut vec[i]) { /// let val = vec.remove(i); /// // your code here @@ -3643,8 +3648,12 @@ impl Vec { /// But `extract_if` is easier to use. `extract_if` is also more efficient, /// because it can backshift the elements of the array in bulk. /// - /// Note that `extract_if` also lets you mutate every element in the filter closure, - /// regardless of whether you choose to keep or remove it. + /// Note that `extract_if` also lets you mutate the elements passed to the filter closure, + /// regardless of whether you choose to keep or remove them. + /// + /// # Panics + /// + /// If `range` is out of bounds. /// /// # Examples /// @@ -3654,25 +3663,29 @@ impl Vec { /// #![feature(extract_if)] /// let mut numbers = vec![1, 2, 3, 4, 5, 6, 8, 9, 11, 13, 14, 15]; /// - /// let evens = numbers.extract_if(|x| *x % 2 == 0).collect::>(); + /// let evens = numbers.extract_if(.., |x| *x % 2 == 0).collect::>(); /// let odds = numbers; /// /// assert_eq!(evens, vec![2, 4, 6, 8, 14]); /// assert_eq!(odds, vec![1, 3, 5, 9, 11, 13, 15]); /// ``` + /// + /// Using the range argument to only process a part of the vector: + /// + /// ``` + /// #![feature(extract_if)] + /// let mut items = vec![0, 0, 0, 0, 0, 0, 0, 1, 2, 1, 2, 1, 2]; + /// let ones = items.extract_if(7.., |x| *x == 1).collect::>(); + /// assert_eq!(items, vec![0, 0, 0, 0, 0, 0, 0, 2, 2, 2]); + /// assert_eq!(ones.len(), 3); + /// ``` #[unstable(feature = "extract_if", reason = "recently added", issue = "43244")] - pub fn extract_if(&mut self, filter: F) -> ExtractIf<'_, T, F, A> + pub fn extract_if(&mut self, range: R, filter: F) -> ExtractIf<'_, T, F, A> where F: FnMut(&mut T) -> bool, + R: RangeBounds, { - let old_len = self.len(); - - // Guard against us getting leaked (leak amplification) - unsafe { - self.set_len(0); - } - - ExtractIf { vec: self, idx: 0, del: 0, old_len, pred: filter } + ExtractIf::new(self, filter, range) } } diff --git a/library/alloc/tests/vec.rs b/library/alloc/tests/vec.rs index 0f27fdff3e182..75d36194c534a 100644 --- a/library/alloc/tests/vec.rs +++ b/library/alloc/tests/vec.rs @@ -1414,7 +1414,7 @@ fn extract_if_empty() { let mut vec: Vec = vec![]; { - let mut iter = vec.extract_if(|_| true); + let mut iter = vec.extract_if(.., |_| true); assert_eq!(iter.size_hint(), (0, Some(0))); assert_eq!(iter.next(), None); assert_eq!(iter.size_hint(), (0, Some(0))); @@ -1431,7 +1431,7 @@ fn extract_if_zst() { let initial_len = vec.len(); let mut count = 0; { - let mut iter = vec.extract_if(|_| true); + let mut iter = vec.extract_if(.., |_| true); assert_eq!(iter.size_hint(), (0, Some(initial_len))); while let Some(_) = iter.next() { count += 1; @@ -1454,7 +1454,7 @@ fn extract_if_false() { let initial_len = vec.len(); let mut count = 0; { - let mut iter = vec.extract_if(|_| false); + let mut iter = vec.extract_if(.., |_| false); assert_eq!(iter.size_hint(), (0, Some(initial_len))); for _ in iter.by_ref() { count += 1; @@ -1476,7 +1476,7 @@ fn extract_if_true() { let initial_len = vec.len(); let mut count = 0; { - let mut iter = vec.extract_if(|_| true); + let mut iter = vec.extract_if(.., |_| true); assert_eq!(iter.size_hint(), (0, Some(initial_len))); while let Some(_) = iter.next() { count += 1; @@ -1492,6 +1492,31 @@ fn extract_if_true() { assert_eq!(vec, vec![]); } +#[test] +fn extract_if_ranges() { + let mut vec = vec![0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10]; + + let mut count = 0; + let it = vec.extract_if(1..=3, |_| { + count += 1; + true + }); + assert_eq!(it.collect::>(), vec![1, 2, 3]); + assert_eq!(vec, vec![0, 4, 5, 6, 7, 8, 9, 10]); + assert_eq!(count, 3); + + let it = vec.extract_if(1..=3, |_| false); + assert_eq!(it.collect::>(), vec![]); + assert_eq!(vec, vec![0, 4, 5, 6, 7, 8, 9, 10]); +} + +#[test] +#[should_panic] +fn extraxt_if_out_of_bounds() { + let mut vec = vec![0, 1]; + let _ = vec.extract_if(5.., |_| true).for_each(drop); +} + #[test] fn extract_if_complex() { { @@ -1501,7 +1526,7 @@ fn extract_if_complex() { 39, ]; - let removed = vec.extract_if(|x| *x % 2 == 0).collect::>(); + let removed = vec.extract_if(.., |x| *x % 2 == 0).collect::>(); assert_eq!(removed.len(), 10); assert_eq!(removed, vec![2, 4, 6, 18, 20, 22, 24, 26, 34, 36]); @@ -1515,7 +1540,7 @@ fn extract_if_complex() { 2, 4, 6, 7, 9, 11, 13, 15, 17, 18, 20, 22, 24, 26, 27, 29, 31, 33, 34, 35, 36, 37, 39, ]; - let removed = vec.extract_if(|x| *x % 2 == 0).collect::>(); + let removed = vec.extract_if(.., |x| *x % 2 == 0).collect::>(); assert_eq!(removed.len(), 10); assert_eq!(removed, vec![2, 4, 6, 18, 20, 22, 24, 26, 34, 36]); @@ -1528,7 +1553,7 @@ fn extract_if_complex() { let mut vec = vec![2, 4, 6, 7, 9, 11, 13, 15, 17, 18, 20, 22, 24, 26, 27, 29, 31, 33, 34, 35, 36]; - let removed = vec.extract_if(|x| *x % 2 == 0).collect::>(); + let removed = vec.extract_if(.., |x| *x % 2 == 0).collect::>(); assert_eq!(removed.len(), 10); assert_eq!(removed, vec![2, 4, 6, 18, 20, 22, 24, 26, 34, 36]); @@ -1540,7 +1565,7 @@ fn extract_if_complex() { // [xxxxxxxxxx+++++++++++] let mut vec = vec![2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 1, 3, 5, 7, 9, 11, 13, 15, 17, 19]; - let removed = vec.extract_if(|x| *x % 2 == 0).collect::>(); + let removed = vec.extract_if(.., |x| *x % 2 == 0).collect::>(); assert_eq!(removed.len(), 10); assert_eq!(removed, vec![2, 4, 6, 8, 10, 12, 14, 16, 18, 20]); @@ -1552,7 +1577,7 @@ fn extract_if_complex() { // [+++++++++++xxxxxxxxxx] let mut vec = vec![1, 3, 5, 7, 9, 11, 13, 15, 17, 19, 2, 4, 6, 8, 10, 12, 14, 16, 18, 20]; - let removed = vec.extract_if(|x| *x % 2 == 0).collect::>(); + let removed = vec.extract_if(.., |x| *x % 2 == 0).collect::>(); assert_eq!(removed.len(), 10); assert_eq!(removed, vec![2, 4, 6, 8, 10, 12, 14, 16, 18, 20]); @@ -1600,7 +1625,7 @@ fn extract_if_consumed_panic() { } c.index < 6 }; - let drain = data.extract_if(filter); + let drain = data.extract_if(.., filter); // NOTE: The ExtractIf is explicitly consumed drain.for_each(drop); @@ -1653,7 +1678,7 @@ fn extract_if_unconsumed_panic() { } c.index < 6 }; - let _drain = data.extract_if(filter); + let _drain = data.extract_if(.., filter); // NOTE: The ExtractIf is dropped without being consumed }); @@ -1669,7 +1694,7 @@ fn extract_if_unconsumed_panic() { #[test] fn extract_if_unconsumed() { let mut vec = vec![1, 2, 3, 4]; - let drain = vec.extract_if(|&mut x| x % 2 != 0); + let drain = vec.extract_if(.., |&mut x| x % 2 != 0); drop(drain); assert_eq!(vec, [1, 2, 3, 4]); } From 94cb28c754048785c1a775080f5b454b494aa5ab Mon Sep 17 00:00:00 2001 From: The 8472 Date: Wed, 20 Nov 2024 23:13:45 +0100 Subject: [PATCH 02/23] remove bounds from vec and linkedlist ExtractIf since drain-on-drop behavior was removed those bounds no longer serve a purpose --- library/alloc/src/collections/linked_list.rs | 9 ++------- library/alloc/src/vec/extract_if.rs | 17 +++++------------ 2 files changed, 7 insertions(+), 19 deletions(-) diff --git a/library/alloc/src/collections/linked_list.rs b/library/alloc/src/collections/linked_list.rs index ca0ea1ec8b2ba..13168b7a39fe4 100644 --- a/library/alloc/src/collections/linked_list.rs +++ b/library/alloc/src/collections/linked_list.rs @@ -1939,9 +1939,7 @@ pub struct ExtractIf< T: 'a, F: 'a, #[unstable(feature = "allocator_api", issue = "32838")] A: Allocator = Global, -> where - F: FnMut(&mut T) -> bool, -{ +> { list: &'a mut LinkedList, it: Option>>, pred: F, @@ -1979,10 +1977,7 @@ where } #[unstable(feature = "extract_if", reason = "recently added", issue = "43244")] -impl fmt::Debug for ExtractIf<'_, T, F> -where - F: FnMut(&mut T) -> bool, -{ +impl fmt::Debug for ExtractIf<'_, T, F> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_tuple("ExtractIf").field(&self.list).finish() } diff --git a/library/alloc/src/vec/extract_if.rs b/library/alloc/src/vec/extract_if.rs index f722d89d4005a..1ea7cbdc7e8a4 100644 --- a/library/alloc/src/vec/extract_if.rs +++ b/library/alloc/src/vec/extract_if.rs @@ -1,4 +1,5 @@ -use core::{ops::{Range, RangeBounds}, ptr, slice}; +use core::ops::{Range, RangeBounds}; +use core::{ptr, slice}; use super::Vec; use crate::alloc::{Allocator, Global}; @@ -24,9 +25,7 @@ pub struct ExtractIf< T, F, #[unstable(feature = "allocator_api", issue = "32838")] A: Allocator = Global, -> where - F: FnMut(&mut T) -> bool, -{ +> { pub(super) vec: &'a mut Vec, /// The index of the item that will be inspected by the next call to `next`. pub(super) idx: usize, @@ -40,10 +39,7 @@ pub struct ExtractIf< pub(super) pred: F, } -impl<'a, T, F, A: Allocator> ExtractIf<'a, T, F, A> -where - F: FnMut(&mut T) -> bool, -{ +impl<'a, T, F, A: Allocator> ExtractIf<'a, T, F, A> { pub(super) fn new>(vec: &'a mut Vec, pred: F, range: R) -> Self { let old_len = vec.len(); let Range { start, end } = slice::range(range, ..old_len); @@ -100,10 +96,7 @@ where } #[unstable(feature = "extract_if", reason = "recently added", issue = "43244")] -impl Drop for ExtractIf<'_, T, F, A> -where - F: FnMut(&mut T) -> bool, -{ +impl Drop for ExtractIf<'_, T, F, A> { fn drop(&mut self) { unsafe { if self.idx < self.old_len && self.del > 0 { From d4560bacee7bd51f6a8e2cbdea64af8363858d34 Mon Sep 17 00:00:00 2001 From: The 8472 Date: Wed, 20 Nov 2024 23:45:53 +0100 Subject: [PATCH 03/23] update uses of extract_if in the compiler --- compiler/rustc_errors/src/lib.rs | 8 ++++---- compiler/rustc_lint/src/non_ascii_idents.rs | 4 ++-- compiler/rustc_metadata/src/native_libs.rs | 2 +- compiler/rustc_middle/src/ty/diagnostics.rs | 2 +- compiler/rustc_resolve/src/diagnostics.rs | 6 +++--- compiler/rustc_resolve/src/late/diagnostics.rs | 2 +- compiler/rustc_trait_selection/src/traits/mod.rs | 2 +- 7 files changed, 13 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 98200c367f988..ccd6b1449adce 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -1558,18 +1558,18 @@ impl DiagCtxtInner { debug!(?diagnostic); debug!(?self.emitted_diagnostics); - let already_emitted_sub = |sub: &mut Subdiag| { + let not_yet_emitted = |sub: &mut Subdiag| { debug!(?sub); if sub.level != OnceNote && sub.level != OnceHelp { - return false; + return true; } let mut hasher = StableHasher::new(); sub.hash(&mut hasher); let diagnostic_hash = hasher.finish(); debug!(?diagnostic_hash); - !self.emitted_diagnostics.insert(diagnostic_hash) + self.emitted_diagnostics.insert(diagnostic_hash) }; - diagnostic.children.extract_if(already_emitted_sub).for_each(|_| {}); + diagnostic.children.retain_mut(not_yet_emitted); if already_emitted { let msg = "duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`"; diagnostic.sub(Note, msg, MultiSpan::new()); diff --git a/compiler/rustc_lint/src/non_ascii_idents.rs b/compiler/rustc_lint/src/non_ascii_idents.rs index 9b495c19990c1..50c64a9c94757 100644 --- a/compiler/rustc_lint/src/non_ascii_idents.rs +++ b/compiler/rustc_lint/src/non_ascii_idents.rs @@ -205,7 +205,7 @@ impl EarlyLintPass for NonAsciiIdents { (IdentifierType::Not_NFKC, "Not_NFKC"), ] { let codepoints: Vec<_> = - chars.extract_if(|(_, ty)| *ty == Some(id_ty)).collect(); + chars.extract_if(.., |(_, ty)| *ty == Some(id_ty)).collect(); if codepoints.is_empty() { continue; } @@ -217,7 +217,7 @@ impl EarlyLintPass for NonAsciiIdents { } let remaining = chars - .extract_if(|(c, _)| !GeneralSecurityProfile::identifier_allowed(*c)) + .extract_if(.., |(c, _)| !GeneralSecurityProfile::identifier_allowed(*c)) .collect::>(); if !remaining.is_empty() { cx.emit_span_lint(UNCOMMON_CODEPOINTS, sp, IdentifierUncommonCodepoints { diff --git a/compiler/rustc_metadata/src/native_libs.rs b/compiler/rustc_metadata/src/native_libs.rs index ace46891f83da..0dcaefe53bfeb 100644 --- a/compiler/rustc_metadata/src/native_libs.rs +++ b/compiler/rustc_metadata/src/native_libs.rs @@ -540,7 +540,7 @@ impl<'tcx> Collector<'tcx> { // can move them to the end of the list below. let mut existing = self .libs - .extract_if(|lib| { + .extract_if(.., |lib| { if lib.name.as_str() == passed_lib.name { // FIXME: This whole logic is questionable, whether modifiers are // involved or not, library reordering and kind overriding without diff --git a/compiler/rustc_middle/src/ty/diagnostics.rs b/compiler/rustc_middle/src/ty/diagnostics.rs index 84f52bfe48f23..1c54a905dcd4f 100644 --- a/compiler/rustc_middle/src/ty/diagnostics.rs +++ b/compiler/rustc_middle/src/ty/diagnostics.rs @@ -290,7 +290,7 @@ pub fn suggest_constraining_type_params<'a>( let Some(param) = param else { return false }; { - let mut sized_constraints = constraints.extract_if(|(_, def_id)| { + let mut sized_constraints = constraints.extract_if(.., |(_, def_id)| { def_id.is_some_and(|def_id| tcx.is_lang_item(def_id, LangItem::Sized)) }); if let Some((_, def_id)) = sized_constraints.next() { diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index 5b78acd904a75..d890eae80274c 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -2805,11 +2805,11 @@ fn show_candidates( path_strings.sort_by(|a, b| a.0.cmp(&b.0)); path_strings.dedup_by(|a, b| a.0 == b.0); let core_path_strings = - path_strings.extract_if(|p| p.0.starts_with("core::")).collect::>(); + path_strings.extract_if(.., |p| p.0.starts_with("core::")).collect::>(); let std_path_strings = - path_strings.extract_if(|p| p.0.starts_with("std::")).collect::>(); + path_strings.extract_if(.., |p| p.0.starts_with("std::")).collect::>(); let foreign_crate_path_strings = - path_strings.extract_if(|p| !p.0.starts_with("crate::")).collect::>(); + path_strings.extract_if(.., |p| !p.0.starts_with("crate::")).collect::>(); // We list the `crate` local paths first. // Then we list the `std`/`core` paths. diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 09f5a8e96d3de..f2f6399a18ada 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -628,7 +628,7 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> { // Try to filter out intrinsics candidates, as long as we have // some other candidates to suggest. let intrinsic_candidates: Vec<_> = candidates - .extract_if(|sugg| { + .extract_if(.., |sugg| { let path = path_names_to_string(&sugg.path); path.starts_with("core::intrinsics::") || path.starts_with("std::intrinsics::") }) diff --git a/compiler/rustc_trait_selection/src/traits/mod.rs b/compiler/rustc_trait_selection/src/traits/mod.rs index c0603c06d4225..b4ad806512b3e 100644 --- a/compiler/rustc_trait_selection/src/traits/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/mod.rs @@ -447,7 +447,7 @@ pub fn normalize_param_env_or_error<'tcx>( // This works fairly well because trait matching does not actually care about param-env // TypeOutlives predicates - these are normally used by regionck. let outlives_predicates: Vec<_> = predicates - .extract_if(|predicate| { + .extract_if(.., |predicate| { matches!(predicate.kind().skip_binder(), ty::ClauseKind::TypeOutlives(..)) }) .collect(); From aabed334c5cb884f8c016a1e18a4ad8b2b7976d5 Mon Sep 17 00:00:00 2001 From: The 8472 Date: Wed, 20 Nov 2024 23:53:35 +0100 Subject: [PATCH 04/23] remove obsolete comment and pub(super) visibility --- library/alloc/src/vec/extract_if.rs | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/library/alloc/src/vec/extract_if.rs b/library/alloc/src/vec/extract_if.rs index 1ea7cbdc7e8a4..4db13981596bc 100644 --- a/library/alloc/src/vec/extract_if.rs +++ b/library/alloc/src/vec/extract_if.rs @@ -26,17 +26,17 @@ pub struct ExtractIf< F, #[unstable(feature = "allocator_api", issue = "32838")] A: Allocator = Global, > { - pub(super) vec: &'a mut Vec, + vec: &'a mut Vec, /// The index of the item that will be inspected by the next call to `next`. - pub(super) idx: usize, + idx: usize, /// Elements at and beyond this point will be retained. Must be equal or smaller than `old_len`. - pub(super) end: usize, + end: usize, /// The number of items that have been drained (removed) thus far. - pub(super) del: usize, + del: usize, /// The original length of `vec` prior to draining. - pub(super) old_len: usize, + old_len: usize, /// The filter test predicate. - pub(super) pred: F, + pred: F, } impl<'a, T, F, A: Allocator> ExtractIf<'a, T, F, A> { @@ -100,12 +100,6 @@ impl Drop for ExtractIf<'_, T, F, A> { fn drop(&mut self) { unsafe { if self.idx < self.old_len && self.del > 0 { - // This is a pretty messed up state, and there isn't really an - // obviously right thing to do. We don't want to keep trying - // to execute `pred`, so we just backshift all the unprocessed - // elements and tell the vec that they still exist. The backshift - // is required to prevent a double-drop of the last successfully - // drained item prior to a panic in the predicate. let ptr = self.vec.as_mut_ptr(); let src = ptr.add(self.idx); let dst = src.sub(self.del); From 5477d856ae3a994ecc45d573083c6d13b11d2e71 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Fri, 6 Dec 2024 12:05:25 +0000 Subject: [PATCH 05/23] Pass the arch rather than full target name to windows_registry::find_tool The full target name can be anything with custom target specs. Passing just the arch wasn't possible before cc 1.2, but is now. --- compiler/rustc_codegen_ssa/src/back/link.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index ad81ff272c62f..c3281c8c2e5c0 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -1021,12 +1021,8 @@ fn link_natively( && (code < 1000 || code > 9999) { let is_vs_installed = windows_registry::find_vs_version().is_ok(); - // FIXME(cc-rs#1265) pass only target arch to find_tool() - let has_linker = windows_registry::find_tool( - sess.opts.target_triple.tuple(), - "link.exe", - ) - .is_some(); + let has_linker = + windows_registry::find_tool(&sess.target.arch, "link.exe").is_some(); sess.dcx().emit_note(errors::LinkExeUnexpectedError); if is_vs_installed && has_linker { From 18f8657415da5ebc480e63abe03f87d4b4ce4157 Mon Sep 17 00:00:00 2001 From: Kai Luo Date: Thu, 8 Aug 2024 22:08:57 -0400 Subject: [PATCH 06/23] Pass -bnoipath when adding rust upstream dynamic crates --- compiler/rustc_codegen_ssa/src/back/link.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index ad81ff272c62f..b3bb32bedf4b3 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -2753,6 +2753,13 @@ fn add_upstream_rust_crates( .find(|(ty, _)| *ty == crate_type) .expect("failed to find crate type in dependency format list"); + if sess.target.is_like_aix { + // Unlike GNU's ld, AIX linker doesn't feature `-soname=...` when output + // a shared library. Instead, AIX linker offers `(no)ipath`. See + // https://www.ibm.com/docs/en/aix/7.3?topic=l-ld-command. + cmd.link_or_cc_arg("-bnoipath"); + } + for &cnum in &codegen_results.crate_info.used_crates { // We may not pass all crates through to the linker. Some crates may appear statically in // an existing dylib, meaning we'll pick up all the symbols from the dylib. From 1ae1f8ce9cb327eaa22454bbf5a8e25123091ffb Mon Sep 17 00:00:00 2001 From: David Tenty Date: Fri, 6 Dec 2024 10:53:26 -0500 Subject: [PATCH 07/23] Clarify comment --- compiler/rustc_codegen_ssa/src/back/link.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index b3bb32bedf4b3..eb8f7cf6d407c 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -2754,8 +2754,10 @@ fn add_upstream_rust_crates( .expect("failed to find crate type in dependency format list"); if sess.target.is_like_aix { - // Unlike GNU's ld, AIX linker doesn't feature `-soname=...` when output - // a shared library. Instead, AIX linker offers `(no)ipath`. See + // Unlike ELF linkers, AIX doesn't feature `DT_SONAME` to override + // the dependency name when outputing a shared library. Thus, `ld` will + // use the full path to shared libraries as the dependency if passed it + // by default unless `noipath` is passed. // https://www.ibm.com/docs/en/aix/7.3?topic=l-ld-command. cmd.link_or_cc_arg("-bnoipath"); } From 61fd92e28411c2594c9f46cac9d96b96ef894d90 Mon Sep 17 00:00:00 2001 From: rohit141914 Date: Sat, 7 Dec 2024 01:01:03 +0530 Subject: [PATCH 08/23] 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 414acdad1cc996fe90c4e866f20df4d6022fe399 Mon Sep 17 00:00:00 2001 From: the8472 Date: Fri, 6 Dec 2024 21:33:15 +0100 Subject: [PATCH 09/23] fix spelling in library/alloc/tests/vec.rs Co-authored-by: Josh Stone --- library/alloc/tests/vec.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/alloc/tests/vec.rs b/library/alloc/tests/vec.rs index 75d36194c534a..84679827ba1c0 100644 --- a/library/alloc/tests/vec.rs +++ b/library/alloc/tests/vec.rs @@ -1512,7 +1512,7 @@ fn extract_if_ranges() { #[test] #[should_panic] -fn extraxt_if_out_of_bounds() { +fn extract_if_out_of_bounds() { let mut vec = vec![0, 1]; let _ = vec.extract_if(5.., |_| true).for_each(drop); } From f021d99cc8c83ccf08d1afcd2d0b4ede5744f317 Mon Sep 17 00:00:00 2001 From: niacdoial Date: Tue, 5 Nov 2024 22:54:48 +0100 Subject: [PATCH 10/23] 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 11/23] 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 12/23] 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 13/23] 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 14/23] 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 15/23] 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 16/23] 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 17/23] 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 120d6b2808243c7c20297b92240d5164df013c80 Mon Sep 17 00:00:00 2001 From: LuanOnCode <165278456+LuanOldCode@users.noreply.github.com> Date: Fri, 6 Dec 2024 22:09:17 -0300 Subject: [PATCH 18/23] Fix: typo in E0751 error explanation Corrected a grammatical error in the explanation for E0751. Changed "exists" to "exist" to improve clarity and ensure proper grammar in the error message. --- compiler/rustc_error_codes/src/error_codes/E0751.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_error_codes/src/error_codes/E0751.md b/compiler/rustc_error_codes/src/error_codes/E0751.md index 8794f7868f302..825809b229aa4 100644 --- a/compiler/rustc_error_codes/src/error_codes/E0751.md +++ b/compiler/rustc_error_codes/src/error_codes/E0751.md @@ -9,4 +9,4 @@ impl !MyTrait for i32 { } // error! ``` Negative implementations are a promise that the trait will never be implemented -for the given types. Therefore, both cannot exists at the same time. +for the given types. Therefore, both cannot exist at the same time. From ab2ee7aa2f0eb8906b5d04104c93050e703b3936 Mon Sep 17 00:00:00 2001 From: Xing Xue Date: Fri, 6 Dec 2024 15:22:46 -0500 Subject: [PATCH 19/23] Use option "-sf" for the AIX "ln" command. --- tests/run-make/libs-through-symlinks/Makefile | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/run-make/libs-through-symlinks/Makefile b/tests/run-make/libs-through-symlinks/Makefile index 592eae663a49a..c6ff566a0e86e 100644 --- a/tests/run-make/libs-through-symlinks/Makefile +++ b/tests/run-make/libs-through-symlinks/Makefile @@ -3,10 +3,20 @@ include ../tools.mk # ignore-windows +# The option -n for the AIX ln command has a different purpose than it does +# on Linux. On Linux, the -n option is used to treat the destination path as +# normal file if it is a symbolic link to a directory, which is the default +# behavior of the AIX ln command. +ifeq ($(UNAME),AIX) +LN_FLAGS := -sf +else +LN_FLAGS := -nsf +endif + NAME := $(shell $(RUSTC) --print file-names foo.rs) all: mkdir -p $(TMPDIR)/outdir $(RUSTC) foo.rs -o $(TMPDIR)/outdir/$(NAME) - ln -nsf outdir/$(NAME) $(TMPDIR) + ln $(LN_FLAGS) outdir/$(NAME) $(TMPDIR) RUSTC_LOG=rustc_metadata::loader $(RUSTC) bar.rs From f884f18151d63ac9ce7e573cadb165ecbe8d7f9b Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sat, 7 Dec 2024 12:35:19 +1100 Subject: [PATCH 20/23] Move tests for `-l` and `#[link(..)]` into `tests/ui/link-native-libs` --- .../empty-kind-1.rs | 0 .../empty-kind-1.stderr | 0 .../empty-kind-2.rs | 0 .../empty-kind-2.stderr | 0 .../link-arg-error.rs | 0 .../link-arg-error.stderr | 0 .../link-arg-from-rs.rs | 0 .../link-arg-from-rs.stderr | 0 tests/ui/{manual => link-native-libs}/manual-link-bad-form.rs | 0 tests/ui/{manual => link-native-libs}/manual-link-bad-form.stderr | 0 tests/ui/{manual => link-native-libs}/manual-link-bad-kind.rs | 0 tests/ui/{manual => link-native-libs}/manual-link-bad-kind.stderr | 0 .../{manual => link-native-libs}/manual-link-bad-search-path.rs | 0 .../manual-link-bad-search-path.stderr | 0 tests/ui/{manual => link-native-libs}/manual-link-framework.rs | 0 .../ui/{manual => link-native-libs}/manual-link-framework.stderr | 0 .../{manual => link-native-libs}/manual-link-unsupported-kind.rs | 0 .../manual-link-unsupported-kind.stderr | 0 .../modifiers-bad.blank.stderr | 0 .../modifiers-bad.no-prefix.stderr | 0 .../modifiers-bad.prefix-only.stderr | 0 .../modifiers-bad.rs | 0 .../modifiers-bad.unknown.stderr | 0 .../modifiers-override-2.rs | 0 .../modifiers-override-2.stderr | 0 .../modifiers-override-3.rs | 0 .../modifiers-override-3.stderr | 0 .../modifiers-override.rs | 0 .../modifiers-override.stderr | 0 .../msvc-non-utf8-output.rs | 0 .../msvc-non-utf8-output.stderr | 0 .../suggest-libname-only-1.rs | 0 .../suggest-libname-only-1.stderr | 0 .../suggest-libname-only-2.rs | 0 .../suggest-libname-only-2.stderr | 0 35 files changed, 0 insertions(+), 0 deletions(-) rename tests/ui/{native-library-link-flags => link-native-libs}/empty-kind-1.rs (100%) rename tests/ui/{native-library-link-flags => link-native-libs}/empty-kind-1.stderr (100%) rename tests/ui/{native-library-link-flags => link-native-libs}/empty-kind-2.rs (100%) rename tests/ui/{native-library-link-flags => link-native-libs}/empty-kind-2.stderr (100%) rename tests/ui/{native-library-link-flags => link-native-libs}/link-arg-error.rs (100%) rename tests/ui/{native-library-link-flags => link-native-libs}/link-arg-error.stderr (100%) rename tests/ui/{native-library-link-flags => link-native-libs}/link-arg-from-rs.rs (100%) rename tests/ui/{native-library-link-flags => link-native-libs}/link-arg-from-rs.stderr (100%) rename tests/ui/{manual => link-native-libs}/manual-link-bad-form.rs (100%) rename tests/ui/{manual => link-native-libs}/manual-link-bad-form.stderr (100%) rename tests/ui/{manual => link-native-libs}/manual-link-bad-kind.rs (100%) rename tests/ui/{manual => link-native-libs}/manual-link-bad-kind.stderr (100%) rename tests/ui/{manual => link-native-libs}/manual-link-bad-search-path.rs (100%) rename tests/ui/{manual => link-native-libs}/manual-link-bad-search-path.stderr (100%) rename tests/ui/{manual => link-native-libs}/manual-link-framework.rs (100%) rename tests/ui/{manual => link-native-libs}/manual-link-framework.stderr (100%) rename tests/ui/{manual => link-native-libs}/manual-link-unsupported-kind.rs (100%) rename tests/ui/{manual => link-native-libs}/manual-link-unsupported-kind.stderr (100%) rename tests/ui/{native-library-link-flags => link-native-libs}/modifiers-bad.blank.stderr (100%) rename tests/ui/{native-library-link-flags => link-native-libs}/modifiers-bad.no-prefix.stderr (100%) rename tests/ui/{native-library-link-flags => link-native-libs}/modifiers-bad.prefix-only.stderr (100%) rename tests/ui/{native-library-link-flags => link-native-libs}/modifiers-bad.rs (100%) rename tests/ui/{native-library-link-flags => link-native-libs}/modifiers-bad.unknown.stderr (100%) rename tests/ui/{native-library-link-flags => link-native-libs}/modifiers-override-2.rs (100%) rename tests/ui/{native-library-link-flags => link-native-libs}/modifiers-override-2.stderr (100%) rename tests/ui/{native-library-link-flags => link-native-libs}/modifiers-override-3.rs (100%) rename tests/ui/{native-library-link-flags => link-native-libs}/modifiers-override-3.stderr (100%) rename tests/ui/{native-library-link-flags => link-native-libs}/modifiers-override.rs (100%) rename tests/ui/{native-library-link-flags => link-native-libs}/modifiers-override.stderr (100%) rename tests/ui/{native-library-link-flags => link-native-libs}/msvc-non-utf8-output.rs (100%) rename tests/ui/{native-library-link-flags => link-native-libs}/msvc-non-utf8-output.stderr (100%) rename tests/ui/{native-library-link-flags => link-native-libs}/suggest-libname-only-1.rs (100%) rename tests/ui/{native-library-link-flags => link-native-libs}/suggest-libname-only-1.stderr (100%) rename tests/ui/{native-library-link-flags => link-native-libs}/suggest-libname-only-2.rs (100%) rename tests/ui/{native-library-link-flags => link-native-libs}/suggest-libname-only-2.stderr (100%) diff --git a/tests/ui/native-library-link-flags/empty-kind-1.rs b/tests/ui/link-native-libs/empty-kind-1.rs similarity index 100% rename from tests/ui/native-library-link-flags/empty-kind-1.rs rename to tests/ui/link-native-libs/empty-kind-1.rs diff --git a/tests/ui/native-library-link-flags/empty-kind-1.stderr b/tests/ui/link-native-libs/empty-kind-1.stderr similarity index 100% rename from tests/ui/native-library-link-flags/empty-kind-1.stderr rename to tests/ui/link-native-libs/empty-kind-1.stderr diff --git a/tests/ui/native-library-link-flags/empty-kind-2.rs b/tests/ui/link-native-libs/empty-kind-2.rs similarity index 100% rename from tests/ui/native-library-link-flags/empty-kind-2.rs rename to tests/ui/link-native-libs/empty-kind-2.rs diff --git a/tests/ui/native-library-link-flags/empty-kind-2.stderr b/tests/ui/link-native-libs/empty-kind-2.stderr similarity index 100% rename from tests/ui/native-library-link-flags/empty-kind-2.stderr rename to tests/ui/link-native-libs/empty-kind-2.stderr diff --git a/tests/ui/native-library-link-flags/link-arg-error.rs b/tests/ui/link-native-libs/link-arg-error.rs similarity index 100% rename from tests/ui/native-library-link-flags/link-arg-error.rs rename to tests/ui/link-native-libs/link-arg-error.rs diff --git a/tests/ui/native-library-link-flags/link-arg-error.stderr b/tests/ui/link-native-libs/link-arg-error.stderr similarity index 100% rename from tests/ui/native-library-link-flags/link-arg-error.stderr rename to tests/ui/link-native-libs/link-arg-error.stderr diff --git a/tests/ui/native-library-link-flags/link-arg-from-rs.rs b/tests/ui/link-native-libs/link-arg-from-rs.rs similarity index 100% rename from tests/ui/native-library-link-flags/link-arg-from-rs.rs rename to tests/ui/link-native-libs/link-arg-from-rs.rs diff --git a/tests/ui/native-library-link-flags/link-arg-from-rs.stderr b/tests/ui/link-native-libs/link-arg-from-rs.stderr similarity index 100% rename from tests/ui/native-library-link-flags/link-arg-from-rs.stderr rename to tests/ui/link-native-libs/link-arg-from-rs.stderr diff --git a/tests/ui/manual/manual-link-bad-form.rs b/tests/ui/link-native-libs/manual-link-bad-form.rs similarity index 100% rename from tests/ui/manual/manual-link-bad-form.rs rename to tests/ui/link-native-libs/manual-link-bad-form.rs diff --git a/tests/ui/manual/manual-link-bad-form.stderr b/tests/ui/link-native-libs/manual-link-bad-form.stderr similarity index 100% rename from tests/ui/manual/manual-link-bad-form.stderr rename to tests/ui/link-native-libs/manual-link-bad-form.stderr diff --git a/tests/ui/manual/manual-link-bad-kind.rs b/tests/ui/link-native-libs/manual-link-bad-kind.rs similarity index 100% rename from tests/ui/manual/manual-link-bad-kind.rs rename to tests/ui/link-native-libs/manual-link-bad-kind.rs diff --git a/tests/ui/manual/manual-link-bad-kind.stderr b/tests/ui/link-native-libs/manual-link-bad-kind.stderr similarity index 100% rename from tests/ui/manual/manual-link-bad-kind.stderr rename to tests/ui/link-native-libs/manual-link-bad-kind.stderr diff --git a/tests/ui/manual/manual-link-bad-search-path.rs b/tests/ui/link-native-libs/manual-link-bad-search-path.rs similarity index 100% rename from tests/ui/manual/manual-link-bad-search-path.rs rename to tests/ui/link-native-libs/manual-link-bad-search-path.rs diff --git a/tests/ui/manual/manual-link-bad-search-path.stderr b/tests/ui/link-native-libs/manual-link-bad-search-path.stderr similarity index 100% rename from tests/ui/manual/manual-link-bad-search-path.stderr rename to tests/ui/link-native-libs/manual-link-bad-search-path.stderr diff --git a/tests/ui/manual/manual-link-framework.rs b/tests/ui/link-native-libs/manual-link-framework.rs similarity index 100% rename from tests/ui/manual/manual-link-framework.rs rename to tests/ui/link-native-libs/manual-link-framework.rs diff --git a/tests/ui/manual/manual-link-framework.stderr b/tests/ui/link-native-libs/manual-link-framework.stderr similarity index 100% rename from tests/ui/manual/manual-link-framework.stderr rename to tests/ui/link-native-libs/manual-link-framework.stderr diff --git a/tests/ui/manual/manual-link-unsupported-kind.rs b/tests/ui/link-native-libs/manual-link-unsupported-kind.rs similarity index 100% rename from tests/ui/manual/manual-link-unsupported-kind.rs rename to tests/ui/link-native-libs/manual-link-unsupported-kind.rs diff --git a/tests/ui/manual/manual-link-unsupported-kind.stderr b/tests/ui/link-native-libs/manual-link-unsupported-kind.stderr similarity index 100% rename from tests/ui/manual/manual-link-unsupported-kind.stderr rename to tests/ui/link-native-libs/manual-link-unsupported-kind.stderr diff --git a/tests/ui/native-library-link-flags/modifiers-bad.blank.stderr b/tests/ui/link-native-libs/modifiers-bad.blank.stderr similarity index 100% rename from tests/ui/native-library-link-flags/modifiers-bad.blank.stderr rename to tests/ui/link-native-libs/modifiers-bad.blank.stderr diff --git a/tests/ui/native-library-link-flags/modifiers-bad.no-prefix.stderr b/tests/ui/link-native-libs/modifiers-bad.no-prefix.stderr similarity index 100% rename from tests/ui/native-library-link-flags/modifiers-bad.no-prefix.stderr rename to tests/ui/link-native-libs/modifiers-bad.no-prefix.stderr diff --git a/tests/ui/native-library-link-flags/modifiers-bad.prefix-only.stderr b/tests/ui/link-native-libs/modifiers-bad.prefix-only.stderr similarity index 100% rename from tests/ui/native-library-link-flags/modifiers-bad.prefix-only.stderr rename to tests/ui/link-native-libs/modifiers-bad.prefix-only.stderr diff --git a/tests/ui/native-library-link-flags/modifiers-bad.rs b/tests/ui/link-native-libs/modifiers-bad.rs similarity index 100% rename from tests/ui/native-library-link-flags/modifiers-bad.rs rename to tests/ui/link-native-libs/modifiers-bad.rs diff --git a/tests/ui/native-library-link-flags/modifiers-bad.unknown.stderr b/tests/ui/link-native-libs/modifiers-bad.unknown.stderr similarity index 100% rename from tests/ui/native-library-link-flags/modifiers-bad.unknown.stderr rename to tests/ui/link-native-libs/modifiers-bad.unknown.stderr diff --git a/tests/ui/native-library-link-flags/modifiers-override-2.rs b/tests/ui/link-native-libs/modifiers-override-2.rs similarity index 100% rename from tests/ui/native-library-link-flags/modifiers-override-2.rs rename to tests/ui/link-native-libs/modifiers-override-2.rs diff --git a/tests/ui/native-library-link-flags/modifiers-override-2.stderr b/tests/ui/link-native-libs/modifiers-override-2.stderr similarity index 100% rename from tests/ui/native-library-link-flags/modifiers-override-2.stderr rename to tests/ui/link-native-libs/modifiers-override-2.stderr diff --git a/tests/ui/native-library-link-flags/modifiers-override-3.rs b/tests/ui/link-native-libs/modifiers-override-3.rs similarity index 100% rename from tests/ui/native-library-link-flags/modifiers-override-3.rs rename to tests/ui/link-native-libs/modifiers-override-3.rs diff --git a/tests/ui/native-library-link-flags/modifiers-override-3.stderr b/tests/ui/link-native-libs/modifiers-override-3.stderr similarity index 100% rename from tests/ui/native-library-link-flags/modifiers-override-3.stderr rename to tests/ui/link-native-libs/modifiers-override-3.stderr diff --git a/tests/ui/native-library-link-flags/modifiers-override.rs b/tests/ui/link-native-libs/modifiers-override.rs similarity index 100% rename from tests/ui/native-library-link-flags/modifiers-override.rs rename to tests/ui/link-native-libs/modifiers-override.rs diff --git a/tests/ui/native-library-link-flags/modifiers-override.stderr b/tests/ui/link-native-libs/modifiers-override.stderr similarity index 100% rename from tests/ui/native-library-link-flags/modifiers-override.stderr rename to tests/ui/link-native-libs/modifiers-override.stderr diff --git a/tests/ui/native-library-link-flags/msvc-non-utf8-output.rs b/tests/ui/link-native-libs/msvc-non-utf8-output.rs similarity index 100% rename from tests/ui/native-library-link-flags/msvc-non-utf8-output.rs rename to tests/ui/link-native-libs/msvc-non-utf8-output.rs diff --git a/tests/ui/native-library-link-flags/msvc-non-utf8-output.stderr b/tests/ui/link-native-libs/msvc-non-utf8-output.stderr similarity index 100% rename from tests/ui/native-library-link-flags/msvc-non-utf8-output.stderr rename to tests/ui/link-native-libs/msvc-non-utf8-output.stderr diff --git a/tests/ui/native-library-link-flags/suggest-libname-only-1.rs b/tests/ui/link-native-libs/suggest-libname-only-1.rs similarity index 100% rename from tests/ui/native-library-link-flags/suggest-libname-only-1.rs rename to tests/ui/link-native-libs/suggest-libname-only-1.rs diff --git a/tests/ui/native-library-link-flags/suggest-libname-only-1.stderr b/tests/ui/link-native-libs/suggest-libname-only-1.stderr similarity index 100% rename from tests/ui/native-library-link-flags/suggest-libname-only-1.stderr rename to tests/ui/link-native-libs/suggest-libname-only-1.stderr diff --git a/tests/ui/native-library-link-flags/suggest-libname-only-2.rs b/tests/ui/link-native-libs/suggest-libname-only-2.rs similarity index 100% rename from tests/ui/native-library-link-flags/suggest-libname-only-2.rs rename to tests/ui/link-native-libs/suggest-libname-only-2.rs diff --git a/tests/ui/native-library-link-flags/suggest-libname-only-2.stderr b/tests/ui/link-native-libs/suggest-libname-only-2.stderr similarity index 100% rename from tests/ui/native-library-link-flags/suggest-libname-only-2.stderr rename to tests/ui/link-native-libs/suggest-libname-only-2.stderr From db9e3681f968c326039d56de478c2f7d3dae3e7e Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 7 Dec 2024 01:11:23 +0000 Subject: [PATCH 21/23] 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 0a48b96859d3d66138082ffad6a1057b54cd55c7 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sat, 7 Dec 2024 12:35:30 +1100 Subject: [PATCH 22/23] Move more tests into `tests/ui/link-native-libs` --- src/tools/tidy/src/issues.txt | 10 +++++----- src/tools/tidy/src/ui_tests.rs | 2 +- .../auxiliary/link-cfg-works-transitive-dylib.rs | 0 .../auxiliary/link-cfg-works-transitive-rlib.rs | 0 .../{linkage-attr => link-native-libs}/issue-109144.rs | 0 .../issue-109144.stderr | 0 tests/ui/{issues => link-native-libs}/issue-43925.rs | 0 .../ui/{issues => link-native-libs}/issue-43925.stderr | 0 tests/ui/{issues => link-native-libs}/issue-43926.rs | 0 .../ui/{issues => link-native-libs}/issue-43926.stderr | 0 .../issue-70093/issue-70093-link-directives.rs | 0 .../issue-70093/issue-70093.rs | 0 .../kind-framework.rs | 0 .../kind-framework.stderr | 0 .../link-attr-validation-early.rs | 0 .../link-attr-validation-early.stderr | 0 .../link-attr-validation-late.rs | 0 .../link-attr-validation-late.stderr | 0 .../link-cfg-works.rs | 0 .../uikit-framework.rs | 0 20 files changed, 6 insertions(+), 6 deletions(-) rename tests/ui/{linkage-attr => link-native-libs}/auxiliary/link-cfg-works-transitive-dylib.rs (100%) rename tests/ui/{linkage-attr => link-native-libs}/auxiliary/link-cfg-works-transitive-rlib.rs (100%) rename tests/ui/{linkage-attr => link-native-libs}/issue-109144.rs (100%) rename tests/ui/{linkage-attr => link-native-libs}/issue-109144.stderr (100%) rename tests/ui/{issues => link-native-libs}/issue-43925.rs (100%) rename tests/ui/{issues => link-native-libs}/issue-43925.stderr (100%) rename tests/ui/{issues => link-native-libs}/issue-43926.rs (100%) rename tests/ui/{issues => link-native-libs}/issue-43926.stderr (100%) rename tests/ui/{issues => link-native-libs}/issue-70093/issue-70093-link-directives.rs (100%) rename tests/ui/{issues => link-native-libs}/issue-70093/issue-70093.rs (100%) rename tests/ui/{linkage-attr => link-native-libs}/kind-framework.rs (100%) rename tests/ui/{linkage-attr => link-native-libs}/kind-framework.stderr (100%) rename tests/ui/{linkage-attr => link-native-libs}/link-attr-validation-early.rs (100%) rename tests/ui/{linkage-attr => link-native-libs}/link-attr-validation-early.stderr (100%) rename tests/ui/{linkage-attr => link-native-libs}/link-attr-validation-late.rs (100%) rename tests/ui/{linkage-attr => link-native-libs}/link-attr-validation-late.stderr (100%) rename tests/ui/{linkage-attr => link-native-libs}/link-cfg-works.rs (100%) rename tests/ui/{linkage-attr => link-native-libs}/uikit-framework.rs (100%) diff --git a/src/tools/tidy/src/issues.txt b/src/tools/tidy/src/issues.txt index ac82a17e1459f..4088b2de2cdbd 100644 --- a/src/tools/tidy/src/issues.txt +++ b/src/tools/tidy/src/issues.txt @@ -2297,8 +2297,6 @@ ui/issues/issue-43853.rs ui/issues/issue-4387.rs ui/issues/issue-43910.rs ui/issues/issue-43923.rs -ui/issues/issue-43925.rs -ui/issues/issue-43926.rs ui/issues/issue-43988.rs ui/issues/issue-44023.rs ui/issues/issue-44056.rs @@ -2547,8 +2545,6 @@ ui/issues/issue-6936.rs ui/issues/issue-69455.rs ui/issues/issue-69602-type-err-during-codegen-ice.rs ui/issues/issue-69683.rs -ui/issues/issue-70093/issue-70093-link-directives.rs -ui/issues/issue-70093/issue-70093.rs ui/issues/issue-7012.rs ui/issues/issue-70381.rs ui/issues/issue-7044.rs @@ -2713,11 +2709,15 @@ ui/limits/issue-17913.rs ui/limits/issue-55878.rs ui/limits/issue-69485-var-size-diffs-too-large.rs ui/limits/issue-75158-64.rs +ui/link-native-libs/issue-109144.rs +ui/link-native-libs/issue-43925.rs +ui/link-native-libs/issue-43926.rs +ui/link-native-libs/issue-70093/issue-70093-link-directives.rs +ui/link-native-libs/issue-70093/issue-70093.rs ui/linkage-attr/auxiliary/issue-12133-dylib.rs ui/linkage-attr/auxiliary/issue-12133-dylib2.rs ui/linkage-attr/auxiliary/issue-12133-rlib.rs ui/linkage-attr/issue-10755.rs -ui/linkage-attr/issue-109144.rs ui/linkage-attr/issue-12133-1.rs ui/linkage-attr/issue-12133-2.rs ui/linkage-attr/issue-12133-3.rs diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index 11f9d5bb03df7..401169c838f71 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -17,7 +17,7 @@ use ignore::Walk; const ENTRY_LIMIT: u32 = 901; // FIXME: The following limits should be reduced eventually. -const ISSUES_ENTRY_LIMIT: u32 = 1672; +const ISSUES_ENTRY_LIMIT: u32 = 1667; const EXPECTED_TEST_FILE_EXTENSIONS: &[&str] = &[ "rs", // test source files diff --git a/tests/ui/linkage-attr/auxiliary/link-cfg-works-transitive-dylib.rs b/tests/ui/link-native-libs/auxiliary/link-cfg-works-transitive-dylib.rs similarity index 100% rename from tests/ui/linkage-attr/auxiliary/link-cfg-works-transitive-dylib.rs rename to tests/ui/link-native-libs/auxiliary/link-cfg-works-transitive-dylib.rs diff --git a/tests/ui/linkage-attr/auxiliary/link-cfg-works-transitive-rlib.rs b/tests/ui/link-native-libs/auxiliary/link-cfg-works-transitive-rlib.rs similarity index 100% rename from tests/ui/linkage-attr/auxiliary/link-cfg-works-transitive-rlib.rs rename to tests/ui/link-native-libs/auxiliary/link-cfg-works-transitive-rlib.rs diff --git a/tests/ui/linkage-attr/issue-109144.rs b/tests/ui/link-native-libs/issue-109144.rs similarity index 100% rename from tests/ui/linkage-attr/issue-109144.rs rename to tests/ui/link-native-libs/issue-109144.rs diff --git a/tests/ui/linkage-attr/issue-109144.stderr b/tests/ui/link-native-libs/issue-109144.stderr similarity index 100% rename from tests/ui/linkage-attr/issue-109144.stderr rename to tests/ui/link-native-libs/issue-109144.stderr diff --git a/tests/ui/issues/issue-43925.rs b/tests/ui/link-native-libs/issue-43925.rs similarity index 100% rename from tests/ui/issues/issue-43925.rs rename to tests/ui/link-native-libs/issue-43925.rs diff --git a/tests/ui/issues/issue-43925.stderr b/tests/ui/link-native-libs/issue-43925.stderr similarity index 100% rename from tests/ui/issues/issue-43925.stderr rename to tests/ui/link-native-libs/issue-43925.stderr diff --git a/tests/ui/issues/issue-43926.rs b/tests/ui/link-native-libs/issue-43926.rs similarity index 100% rename from tests/ui/issues/issue-43926.rs rename to tests/ui/link-native-libs/issue-43926.rs diff --git a/tests/ui/issues/issue-43926.stderr b/tests/ui/link-native-libs/issue-43926.stderr similarity index 100% rename from tests/ui/issues/issue-43926.stderr rename to tests/ui/link-native-libs/issue-43926.stderr diff --git a/tests/ui/issues/issue-70093/issue-70093-link-directives.rs b/tests/ui/link-native-libs/issue-70093/issue-70093-link-directives.rs similarity index 100% rename from tests/ui/issues/issue-70093/issue-70093-link-directives.rs rename to tests/ui/link-native-libs/issue-70093/issue-70093-link-directives.rs diff --git a/tests/ui/issues/issue-70093/issue-70093.rs b/tests/ui/link-native-libs/issue-70093/issue-70093.rs similarity index 100% rename from tests/ui/issues/issue-70093/issue-70093.rs rename to tests/ui/link-native-libs/issue-70093/issue-70093.rs diff --git a/tests/ui/linkage-attr/kind-framework.rs b/tests/ui/link-native-libs/kind-framework.rs similarity index 100% rename from tests/ui/linkage-attr/kind-framework.rs rename to tests/ui/link-native-libs/kind-framework.rs diff --git a/tests/ui/linkage-attr/kind-framework.stderr b/tests/ui/link-native-libs/kind-framework.stderr similarity index 100% rename from tests/ui/linkage-attr/kind-framework.stderr rename to tests/ui/link-native-libs/kind-framework.stderr diff --git a/tests/ui/linkage-attr/link-attr-validation-early.rs b/tests/ui/link-native-libs/link-attr-validation-early.rs similarity index 100% rename from tests/ui/linkage-attr/link-attr-validation-early.rs rename to tests/ui/link-native-libs/link-attr-validation-early.rs diff --git a/tests/ui/linkage-attr/link-attr-validation-early.stderr b/tests/ui/link-native-libs/link-attr-validation-early.stderr similarity index 100% rename from tests/ui/linkage-attr/link-attr-validation-early.stderr rename to tests/ui/link-native-libs/link-attr-validation-early.stderr diff --git a/tests/ui/linkage-attr/link-attr-validation-late.rs b/tests/ui/link-native-libs/link-attr-validation-late.rs similarity index 100% rename from tests/ui/linkage-attr/link-attr-validation-late.rs rename to tests/ui/link-native-libs/link-attr-validation-late.rs diff --git a/tests/ui/linkage-attr/link-attr-validation-late.stderr b/tests/ui/link-native-libs/link-attr-validation-late.stderr similarity index 100% rename from tests/ui/linkage-attr/link-attr-validation-late.stderr rename to tests/ui/link-native-libs/link-attr-validation-late.stderr diff --git a/tests/ui/linkage-attr/link-cfg-works.rs b/tests/ui/link-native-libs/link-cfg-works.rs similarity index 100% rename from tests/ui/linkage-attr/link-cfg-works.rs rename to tests/ui/link-native-libs/link-cfg-works.rs diff --git a/tests/ui/linkage-attr/uikit-framework.rs b/tests/ui/link-native-libs/uikit-framework.rs similarity index 100% rename from tests/ui/linkage-attr/uikit-framework.rs rename to tests/ui/link-native-libs/uikit-framework.rs From 8aacd1c6a86ba4211b541aaf5b6d270ebb8727fa Mon Sep 17 00:00:00 2001 From: jyn Date: Sun, 1 Dec 2024 23:57:09 -0500 Subject: [PATCH 23/23] compiletest: show the difference between the normalized output and the actual output for lines which didn't match example output: ``` failures: ---- [ui] tests/ui/layout/enum.rs stdout ---- diff of stderr: - error: align: AbiAndPrefAlign { abi: Align(2 bytes), pref: $PREF_ALIGN } + error: align: AbiAndPrefAlign { abi: Align(2 bytes), pref: $PREF_ALIN } 2 --> $DIR/enum.rs:9:1 3 | 4 LL | enum UninhabitedVariantAlign { Note: some mismatched output was normalized before being compared - error: align: AbiAndPrefAlign { abi: Align(2 bytes), pref: Align(8 bytes) } - --> /home/jyn/src/rust2/tests/ui/layout/enum.rs:9:1 + error: align: AbiAndPrefAlign { abi: Align(2 bytes), pref: $PREF_ALIN } ``` --- src/tools/compiletest/src/runtest.rs | 136 ++++++++++++++---- src/tools/compiletest/src/runtest/coverage.rs | 16 ++- src/tools/compiletest/src/runtest/ui.rs | 2 +- 3 files changed, 119 insertions(+), 35 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 84269fd44a1e0..7b11bf3b1219c 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -22,7 +22,7 @@ use crate::common::{ UI_STDERR, UI_STDOUT, UI_SVG, UI_WINDOWS_SVG, Ui, expected_output_path, incremental_dir, output_base_dir, output_base_name, output_testname_unique, }; -use crate::compute_diff::{write_diff, write_filtered_diff}; +use crate::compute_diff::{DiffLine, make_diff, write_diff, write_filtered_diff}; use crate::errors::{self, Error, ErrorKind}; use crate::header::TestProps; use crate::read2::{Truncated, read2_abbreviated}; @@ -2295,17 +2295,31 @@ impl<'test> TestCx<'test> { match output_kind { TestOutput::Compile => { if !self.props.dont_check_compiler_stdout { - errors += - self.compare_output(stdout_kind, &normalized_stdout, &expected_stdout); + errors += self.compare_output( + stdout_kind, + &normalized_stdout, + &proc_res.stdout, + &expected_stdout, + ); } if !self.props.dont_check_compiler_stderr { - errors += - self.compare_output(stderr_kind, &normalized_stderr, &expected_stderr); + errors += self.compare_output( + stderr_kind, + &normalized_stderr, + &stderr, + &expected_stderr, + ); } } TestOutput::Run => { - errors += self.compare_output(stdout_kind, &normalized_stdout, &expected_stdout); - errors += self.compare_output(stderr_kind, &normalized_stderr, &expected_stderr); + errors += self.compare_output( + stdout_kind, + &normalized_stdout, + &proc_res.stdout, + &expected_stdout, + ); + errors += + self.compare_output(stderr_kind, &normalized_stderr, &stderr, &expected_stderr); } } errors @@ -2533,7 +2547,13 @@ impl<'test> TestCx<'test> { } } - fn compare_output(&self, stream: &str, actual: &str, expected: &str) -> usize { + fn compare_output( + &self, + stream: &str, + actual: &str, + actual_unnormalized: &str, + expected: &str, + ) -> usize { let are_different = match (self.force_color_svg(), expected.find('\n'), actual.find('\n')) { // FIXME: We ignore the first line of SVG files // because the width parameter is non-deterministic. @@ -2590,28 +2610,14 @@ impl<'test> TestCx<'test> { if expected.is_empty() { println!("normalized {}:\n{}\n", stream, actual); } else { - println!("diff of {stream}:\n"); - if let Some(diff_command) = self.config.diff_command.as_deref() { - let mut args = diff_command.split_whitespace(); - let name = args.next().unwrap(); - match Command::new(name) - .args(args) - .args([&expected_path, &actual_path]) - .output() - { - Err(err) => { - self.fatal(&format!( - "failed to call custom diff command `{diff_command}`: {err}" - )); - } - Ok(output) => { - let output = String::from_utf8_lossy(&output.stdout); - print!("{output}"); - } - } - } else { - print!("{}", write_diff(expected, actual, 3)); - } + self.show_diff( + stream, + &expected_path, + &actual_path, + expected, + actual, + actual_unnormalized, + ); } } else { // Delete non-revision .stderr/.stdout file if revisions are used. @@ -2633,6 +2639,76 @@ impl<'test> TestCx<'test> { if self.config.bless { 0 } else { 1 } } + /// Returns whether to show the full stderr/stdout. + fn show_diff( + &self, + stream: &str, + expected_path: &Path, + actual_path: &Path, + expected: &str, + actual: &str, + actual_unnormalized: &str, + ) { + eprintln!("diff of {stream}:\n"); + if let Some(diff_command) = self.config.diff_command.as_deref() { + let mut args = diff_command.split_whitespace(); + let name = args.next().unwrap(); + match Command::new(name).args(args).args([expected_path, actual_path]).output() { + Err(err) => { + self.fatal(&format!( + "failed to call custom diff command `{diff_command}`: {err}" + )); + } + Ok(output) => { + let output = String::from_utf8_lossy(&output.stdout); + eprint!("{output}"); + } + } + } else { + eprint!("{}", write_diff(expected, actual, 3)); + } + + // NOTE: argument order is important, we need `actual` to be on the left so the line number match up when we compare it to `actual_unnormalized` below. + let diff_results = make_diff(actual, expected, 0); + + let (mut mismatches_normalized, mut mismatch_line_nos) = (String::new(), vec![]); + for hunk in diff_results { + let mut line_no = hunk.line_number; + for line in hunk.lines { + // NOTE: `Expected` is actually correct here, the argument order is reversed so our line numbers match up + if let DiffLine::Expected(normalized) = line { + mismatches_normalized += &normalized; + mismatches_normalized += "\n"; + mismatch_line_nos.push(line_no); + line_no += 1; + } + } + } + let mut mismatches_unnormalized = String::new(); + let diff_normalized = make_diff(actual, actual_unnormalized, 0); + for hunk in diff_normalized { + if mismatch_line_nos.contains(&hunk.line_number) { + for line in hunk.lines { + if let DiffLine::Resulting(unnormalized) = line { + mismatches_unnormalized += &unnormalized; + mismatches_unnormalized += "\n"; + } + } + } + } + + let normalized_diff = make_diff(&mismatches_normalized, &mismatches_unnormalized, 0); + // HACK: instead of checking if each hunk is empty, this only checks if the whole input is empty. we should be smarter about this so we don't treat added or removed output as normalized. + if !normalized_diff.is_empty() + && !mismatches_unnormalized.is_empty() + && !mismatches_normalized.is_empty() + { + eprintln!("Note: some mismatched output was normalized before being compared"); + // FIXME: respect diff_command + eprint!("{}", write_diff(&mismatches_unnormalized, &mismatches_normalized, 0)); + } + } + fn check_and_prune_duplicate_outputs( &self, proc_res: &ProcRes, diff --git a/src/tools/compiletest/src/runtest/coverage.rs b/src/tools/compiletest/src/runtest/coverage.rs index 961a160298631..030ca5ebb2474 100644 --- a/src/tools/compiletest/src/runtest/coverage.rs +++ b/src/tools/compiletest/src/runtest/coverage.rs @@ -39,8 +39,12 @@ impl<'test> TestCx<'test> { let expected_coverage_dump = self.load_expected_output(kind); let actual_coverage_dump = self.normalize_output(&proc_res.stdout, &[]); - let coverage_dump_errors = - self.compare_output(kind, &actual_coverage_dump, &expected_coverage_dump); + let coverage_dump_errors = self.compare_output( + kind, + &actual_coverage_dump, + &proc_res.stdout, + &expected_coverage_dump, + ); if coverage_dump_errors > 0 { self.fatal_proc_rec( @@ -135,8 +139,12 @@ impl<'test> TestCx<'test> { self.fatal_proc_rec(&err, &proc_res); }); - let coverage_errors = - self.compare_output(kind, &normalized_actual_coverage, &expected_coverage); + let coverage_errors = self.compare_output( + kind, + &normalized_actual_coverage, + &proc_res.stdout, + &expected_coverage, + ); if coverage_errors > 0 { self.fatal_proc_rec( diff --git a/src/tools/compiletest/src/runtest/ui.rs b/src/tools/compiletest/src/runtest/ui.rs index bb747c6802926..172b1e32aad3b 100644 --- a/src/tools/compiletest/src/runtest/ui.rs +++ b/src/tools/compiletest/src/runtest/ui.rs @@ -100,7 +100,7 @@ impl TestCx<'_> { ) }); - errors += self.compare_output("fixed", &fixed_code, &expected_fixed); + errors += self.compare_output("fixed", &fixed_code, &fixed_code, &expected_fixed); } else if !expected_fixed.is_empty() { panic!( "the `//@ run-rustfix` directive wasn't found but a `*.fixed` \