From b72f55580cf78fb93f05a04ec10e9ae34ea58202 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Fri, 4 Feb 2022 11:48:18 -0800 Subject: [PATCH 1/9] Document -C symbol-mangling-version --- src/doc/rustc/src/codegen-options/index.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/doc/rustc/src/codegen-options/index.md b/src/doc/rustc/src/codegen-options/index.md index 0201b88417a8b..3e3d68da4c9fd 100644 --- a/src/doc/rustc/src/codegen-options/index.md +++ b/src/doc/rustc/src/codegen-options/index.md @@ -541,6 +541,21 @@ Supported values for this option are: - `symbols` - same as `debuginfo`, but the rest of the symbol table section is stripped as well if the linker supports it. +## symbol-mangling-version + +This option controls the [name mangling] format for encoding Rust item names +for the purpose of generating object code and linking. + +Supported values for this option are: + +* `v0` — The "v0" mangling scheme. The specific format is not specified at + this time. + +The default if not specified will use a compiler-chosen default which may +change in the future. + +[name mangling]: https://en.wikipedia.org/wiki/Name_mangling + ## target-cpu This instructs `rustc` to generate code specifically for a particular processor. From 800e0e2cfa845be5a01c611c8f370172b3a7cea5 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Fri, 4 Feb 2022 12:10:05 -0800 Subject: [PATCH 2/9] Document --json=future-incompat --- src/doc/rustc/src/command-line-arguments.md | 3 +++ src/doc/rustc/src/json.md | 26 +++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/src/doc/rustc/src/command-line-arguments.md b/src/doc/rustc/src/command-line-arguments.md index 11925ab978588..6ed28f1a84521 100644 --- a/src/doc/rustc/src/command-line-arguments.md +++ b/src/doc/rustc/src/command-line-arguments.md @@ -384,6 +384,9 @@ to customize the output: argument](#option-emit), and as soon as the artifact is available on the filesystem a notification will be emitted. +- `future-incompat` - includes a JSON message that contains a report if the + crate contains any code that may fail to compile in the future. + Note that it is invalid to combine the `--json` argument with the [`--color`](#option-color) argument, and it is required to combine `--json` with `--error-format=json`. diff --git a/src/doc/rustc/src/json.md b/src/doc/rustc/src/json.md index 5dee603142dcd..efbf861eaa68c 100644 --- a/src/doc/rustc/src/json.md +++ b/src/doc/rustc/src/json.md @@ -229,6 +229,32 @@ flag][option-emit] documentation. } ``` +## Future-incompatible reports + +If the [`--json=future-incompat`][option-json] flag is used, then a separate +JSON structure will be emitted if the crate may stop compiling in the future. +This contains diagnostic information about the particular warnings that may be +turned into a hard error in the future. This will include the diagnostic +information, even if the diagnostics have been suppressed (such as with an +`#[allow]` attribute or the `--cap-lints` option). + +```javascript +{ + /* An array of objects describing a warning that will become a hard error + in the future. + */ + "future_incompat_report": + [ + { + /* A diagnostic structure as defined in + https://doc.rust-lang.org/rustc/json.html#diagnostics + */ + "diagnostic": {...}, + } + ] +} +``` + [option-emit]: command-line-arguments.md#option-emit [option-error-format]: command-line-arguments.md#option-error-format [option-json]: command-line-arguments.md#option-json From 6096cfbfffdaebd5eca5e0816086e31f10de7eb8 Mon Sep 17 00:00:00 2001 From: bstrie <865233+bstrie@users.noreply.github.com> Date: Tue, 3 May 2022 11:57:24 -0400 Subject: [PATCH 3/9] docs: add link explaining variance to NonNull docs --- library/core/src/ptr/non_null.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/core/src/ptr/non_null.rs b/library/core/src/ptr/non_null.rs index 5ebe61509063f..9946db67db918 100644 --- a/library/core/src/ptr/non_null.rs +++ b/library/core/src/ptr/non_null.rs @@ -9,7 +9,7 @@ use crate::ops::{CoerceUnsized, DispatchFromDyn}; use crate::ptr::Unique; use crate::slice::{self, SliceIndex}; -/// `*mut T` but non-zero and covariant. +/// `*mut T` but non-zero and [covariant]. /// /// This is often the correct thing to use when building data structures using /// raw pointers, but is ultimately more dangerous to use because of its additional @@ -42,6 +42,7 @@ use crate::slice::{self, SliceIndex}; /// it is your responsibility to ensure that `as_mut` is never called, and `as_ptr` /// is never used for mutation. /// +/// [covariant]: https://doc.rust-lang.org/reference/subtyping.html /// [`PhantomData`]: crate::marker::PhantomData /// [`UnsafeCell`]: crate::cell::UnsafeCell #[stable(feature = "nonnull", since = "1.25.0")] From 96321ed7564be5264c324a7f815766ec829fd27e Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 7 May 2022 12:41:15 +0200 Subject: [PATCH 4/9] Do not lint on explicit outlives requirements from external macros. --- compiler/rustc_lint/src/builtin.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index 3564f15e210ad..eae7df0301b79 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -38,7 +38,7 @@ use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::{DefId, LocalDefId, LocalDefIdSet, CRATE_DEF_ID}; use rustc_hir::{ForeignItemKind, GenericParamKind, HirId, PatKind}; use rustc_index::vec::Idx; -use rustc_middle::lint::LintDiagnosticBuilder; +use rustc_middle::lint::{in_external_macro, LintDiagnosticBuilder}; use rustc_middle::ty::layout::{LayoutError, LayoutOf}; use rustc_middle::ty::print::with_no_trimmed_paths; use rustc_middle::ty::subst::{GenericArgKind, Subst}; @@ -2115,6 +2115,7 @@ impl ExplicitOutlivesRequirements { None } }) + .filter(|(_, span)| !in_external_macro(tcx.sess, *span)) .collect() } From 89f15bfd907b0b44a851c5810ab463fe91a206e5 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 7 May 2022 13:08:04 +0200 Subject: [PATCH 5/9] Add test. --- .../edition-lint-infer-outlives-macro.rs | 6 ++++ .../edition-lint-infer-outlives-macro.rs | 28 +++++++++++++++++++ .../edition-lint-infer-outlives-macro.stderr | 14 ++++++++++ 3 files changed, 48 insertions(+) create mode 100644 src/test/ui/rust-2018/auxiliary/edition-lint-infer-outlives-macro.rs create mode 100644 src/test/ui/rust-2018/edition-lint-infer-outlives-macro.rs create mode 100644 src/test/ui/rust-2018/edition-lint-infer-outlives-macro.stderr diff --git a/src/test/ui/rust-2018/auxiliary/edition-lint-infer-outlives-macro.rs b/src/test/ui/rust-2018/auxiliary/edition-lint-infer-outlives-macro.rs new file mode 100644 index 0000000000000..d45fa10f022aa --- /dev/null +++ b/src/test/ui/rust-2018/auxiliary/edition-lint-infer-outlives-macro.rs @@ -0,0 +1,6 @@ +pub fn foo() {} + +#[macro_export] +macro_rules! gimme_a { + ($($mac:tt)*) => { $($mac)* { 'a } } +} diff --git a/src/test/ui/rust-2018/edition-lint-infer-outlives-macro.rs b/src/test/ui/rust-2018/edition-lint-infer-outlives-macro.rs new file mode 100644 index 0000000000000..d7a832831c1d9 --- /dev/null +++ b/src/test/ui/rust-2018/edition-lint-infer-outlives-macro.rs @@ -0,0 +1,28 @@ +// edition:2018 +// aux-build:edition-lint-infer-outlives-macro.rs + +// Test that the lint does not fire if the where predicate +// is from the local crate, but all the bounds are from an +// external macro. + +#![deny(explicit_outlives_requirements)] + +#[macro_use] +extern crate edition_lint_infer_outlives_macro; + +macro_rules! make_foo { + ($a:tt) => { + struct Foo<$a, 'b> where 'b: $a { + foo: &$a &'b (), + } + } +} + +gimme_a! {make_foo!} + +struct Bar<'a, 'b: 'a> { + //~^ ERROR: outlives requirements can be inferred + bar: &'a &'b (), +} + +fn main() {} diff --git a/src/test/ui/rust-2018/edition-lint-infer-outlives-macro.stderr b/src/test/ui/rust-2018/edition-lint-infer-outlives-macro.stderr new file mode 100644 index 0000000000000..553b1cd976ad6 --- /dev/null +++ b/src/test/ui/rust-2018/edition-lint-infer-outlives-macro.stderr @@ -0,0 +1,14 @@ +error: outlives requirements can be inferred + --> $DIR/edition-lint-infer-outlives-macro.rs:23:18 + | +LL | struct Bar<'a, 'b: 'a> { + | ^^^^ help: remove this bound + | +note: the lint level is defined here + --> $DIR/edition-lint-infer-outlives-macro.rs:8:9 + | +LL | #![deny(explicit_outlives_requirements)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error + From 2c11c3d86cd067d929634852749904a9674c8e49 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 9 May 2022 17:29:59 +0200 Subject: [PATCH 6/9] make sure ScalarPair enums have ScalarPair variants; add some layout sanity checks --- compiler/rustc_middle/src/ty/layout.rs | 123 ++++++++++++++++-- src/test/ui/layout/debug.stderr | 38 +++++- ...-scalarpair-payload-might-be-uninit.stderr | 92 ++++++++++--- 3 files changed, 221 insertions(+), 32 deletions(-) diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index cd4b23fca3932..1a1b795b0a4ce 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -220,6 +220,91 @@ impl<'tcx> fmt::Display for LayoutError<'tcx> { } } +/// Enforce some basic invariants on layouts. +fn sanity_check_layout<'tcx>( + tcx: TyCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, + layout: &TyAndLayout<'tcx>, +) { + // Type-level uninhabitedness should always imply ABI uninhabitedness. + if tcx.conservative_is_privately_uninhabited(param_env.and(layout.ty)) { + assert!(layout.abi.is_uninhabited()); + } + + if cfg!(debug_assertions) { + fn check_layout_abi<'tcx>(tcx: TyCtxt<'tcx>, layout: Layout<'tcx>) { + match layout.abi() { + Abi::Scalar(_scalar) => { + // No padding in scalars. + /* FIXME(#96185): + assert_eq!( + layout.align().abi, + scalar.align(&tcx).abi, + "alignment mismatch between ABI and layout in {layout:#?}" + ); + assert_eq!( + layout.size(), + scalar.size(&tcx), + "size mismatch between ABI and layout in {layout:#?}" + );*/ + } + Abi::ScalarPair(scalar1, scalar2) => { + // Sanity-check scalar pair size. + let field2_offset = scalar1.size(&tcx).align_to(scalar2.align(&tcx).abi); + let total = field2_offset + scalar2.size(&tcx); + assert!( + layout.size() >= total, + "size mismatch between ABI and layout in {layout:#?}" + ); + } + _ => {} + } + } + + check_layout_abi(tcx, layout.layout); + + if let Variants::Multiple { variants, .. } = &layout.variants { + for variant in variants { + check_layout_abi(tcx, *variant); + // No nested "multiple". + assert!(matches!(variant.variants(), Variants::Single { .. })); + // Skip empty variants. + if variant.size() == Size::ZERO + || variant.fields().count() == 0 + || variant.abi().is_uninhabited() + { + // These are never actually accessed anyway, so we can skip them. (Note that + // sometimes, variants with fields have size 0, and sometimes, variants without + // fields have non-0 size.) + continue; + } + // Variants should have the same or a smaller size as the full thing. + if variant.size() > layout.size { + bug!( + "Type with size {} bytes has variant with size {} bytes: {layout:#?}", + layout.size.bytes(), + variant.size().bytes(), + ) + } + // The top-level ABI and the ABI of the variants should be coherent. + let abi_coherent = match (layout.abi, variant.abi()) { + (Abi::Scalar(..), Abi::Scalar(..)) => true, + (Abi::ScalarPair(..), Abi::ScalarPair(..)) => true, + (Abi::Uninhabited, _) => true, + (Abi::Aggregate { .. }, _) => true, + _ => false, + }; + if !abi_coherent { + bug!( + "Variant ABI is incompatible with top-level ABI:\nvariant={:#?}\nTop-level: {layout:#?}", + variant + ); + } + } + } + } +} + #[instrument(skip(tcx, query), level = "debug")] fn layout_of<'tcx>( tcx: TyCtxt<'tcx>, @@ -263,10 +348,7 @@ fn layout_of<'tcx>( cx.record_layout_for_printing(layout); - // Type-level uninhabitedness should always imply ABI uninhabitedness. - if tcx.conservative_is_privately_uninhabited(param_env.and(ty)) { - assert!(layout.abi.is_uninhabited()); - } + sanity_check_layout(tcx, param_env, &layout); Ok(layout) }) @@ -1313,10 +1395,22 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { }; let mut abi = Abi::Aggregate { sized: true }; - // Without latter check aligned enums with custom discriminant values - // Would result in ICE see the issue #92464 for more info - if tag.size(dl) == size || variants.iter().all(|layout| layout.is_empty()) { + if layout_variants.iter().all(|v| v.abi.is_uninhabited()) { + abi = Abi::Uninhabited; + } else if tag.size(dl) == size || variants.iter().all(|layout| layout.is_empty()) { + // Without latter check aligned enums with custom discriminant values + // Would result in ICE see the issue #92464 for more info abi = Abi::Scalar(tag); + // Make sure the variants with fields have the same ABI as the enum itself + // (since downcasting to them is a NOP). + for variant in &mut layout_variants { + if variant.fields.count() > 0 + && matches!(variant.abi, Abi::Aggregate { .. }) + { + assert_eq!(variant.size, size); + variant.abi = abi; + } + } } else { // Try to use a ScalarPair for all tagged enums. let mut common_prim = None; @@ -1385,14 +1479,21 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { // We can use `ScalarPair` only when it matches our // already computed layout (including `#[repr(C)]`). abi = pair.abi; + // Make sure the variants with fields have the same ABI as the enum itself + // (since downcasting to them is a NOP). + for variant in &mut layout_variants { + if variant.fields.count() > 0 + && matches!(variant.abi, Abi::Aggregate { .. }) + { + variant.abi = abi; + // Also need to bump up the size, so that the pair fits inside. + variant.size = size; + } + } } } } - if layout_variants.iter().all(|v| v.abi.is_uninhabited()) { - abi = Abi::Uninhabited; - } - let largest_niche = Niche::from_scalar(dl, Size::ZERO, tag); let layout_variants = diff --git a/src/test/ui/layout/debug.stderr b/src/test/ui/layout/debug.stderr index 56a1337e6a5ea..7dbcc15185501 100644 --- a/src/test/ui/layout/debug.stderr +++ b/src/test/ui/layout/debug.stderr @@ -184,9 +184,22 @@ error: layout_of(std::result::Result) = Layout { variants: Single { index: 0, }, - abi: Aggregate { - sized: true, - }, + abi: ScalarPair( + Initialized { + value: Int( + I32, + false, + ), + valid_range: 0..=1, + }, + Initialized { + value: Int( + I32, + true, + ), + valid_range: 0..=4294967295, + }, + ), largest_niche: None, align: AbiAndPrefAlign { abi: Align(4 bytes), @@ -206,9 +219,22 @@ error: layout_of(std::result::Result) = Layout { variants: Single { index: 1, }, - abi: Aggregate { - sized: true, - }, + abi: ScalarPair( + Initialized { + value: Int( + I32, + false, + ), + valid_range: 0..=1, + }, + Initialized { + value: Int( + I32, + true, + ), + valid_range: 0..=4294967295, + }, + ), largest_niche: None, align: AbiAndPrefAlign { abi: Align(4 bytes), diff --git a/src/test/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.stderr b/src/test/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.stderr index 1a724e6f59be1..33dfa307c1d27 100644 --- a/src/test/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.stderr +++ b/src/test/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.stderr @@ -30,9 +30,21 @@ error: layout_of(MissingPayloadField) = Layout { variants: Single { index: 0, }, - abi: Aggregate { - sized: true, - }, + abi: ScalarPair( + Initialized { + value: Int( + I8, + false, + ), + valid_range: 0..=1, + }, + Union { + value: Int( + I8, + false, + ), + }, + ), largest_niche: None, align: AbiAndPrefAlign { abi: Align(1 bytes), @@ -131,9 +143,22 @@ error: layout_of(CommonPayloadField) = Layout { variants: Single { index: 0, }, - abi: Aggregate { - sized: true, - }, + abi: ScalarPair( + Initialized { + value: Int( + I8, + false, + ), + valid_range: 0..=1, + }, + Initialized { + value: Int( + I8, + false, + ), + valid_range: 0..=255, + }, + ), largest_niche: None, align: AbiAndPrefAlign { abi: Align(1 bytes), @@ -153,9 +178,22 @@ error: layout_of(CommonPayloadField) = Layout { variants: Single { index: 1, }, - abi: Aggregate { - sized: true, - }, + abi: ScalarPair( + Initialized { + value: Int( + I8, + false, + ), + valid_range: 0..=1, + }, + Initialized { + value: Int( + I8, + false, + ), + valid_range: 0..=255, + }, + ), largest_niche: None, align: AbiAndPrefAlign { abi: Align(1 bytes), @@ -237,9 +275,21 @@ error: layout_of(CommonPayloadFieldIsMaybeUninit) = Layout { variants: Single { index: 0, }, - abi: Aggregate { - sized: true, - }, + abi: ScalarPair( + Initialized { + value: Int( + I8, + false, + ), + valid_range: 0..=1, + }, + Union { + value: Int( + I8, + false, + ), + }, + ), largest_niche: None, align: AbiAndPrefAlign { abi: Align(1 bytes), @@ -259,9 +309,21 @@ error: layout_of(CommonPayloadFieldIsMaybeUninit) = Layout { variants: Single { index: 1, }, - abi: Aggregate { - sized: true, - }, + abi: ScalarPair( + Initialized { + value: Int( + I8, + false, + ), + valid_range: 0..=1, + }, + Union { + value: Int( + I8, + false, + ), + }, + ), largest_niche: None, align: AbiAndPrefAlign { abi: Align(1 bytes), From 04fb9222f861607b749a04e91e832475a5c8dc0f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 9 May 2022 18:10:21 +0200 Subject: [PATCH 7/9] fix codegen test failure --- src/test/codegen/align-struct.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/codegen/align-struct.rs b/src/test/codegen/align-struct.rs index acc5a2d5499ff..f129f073e98de 100644 --- a/src/test/codegen/align-struct.rs +++ b/src/test/codegen/align-struct.rs @@ -19,7 +19,7 @@ pub enum Enum4 { A(i32), B(i32), } -// CHECK: %"Enum4::A" = type { [1 x i32], i32 } +// No Aggregate type, and hence nothing in LLVM IR. pub enum Enum64 { A(Align64), From 1d68e6d67452c0de61e133553a28c8871e96837a Mon Sep 17 00:00:00 2001 From: Jack Huey <31162821+jackh726@users.noreply.github.com> Date: Sat, 7 May 2022 14:12:01 -0400 Subject: [PATCH 8/9] Properly fix issue 96638 --- .../src/check/fn_ctxt/arg_matrix.rs | 94 ++++++++++--------- .../rustc_typeck/src/check/fn_ctxt/checks.rs | 81 ++++++++-------- 2 files changed, 89 insertions(+), 86 deletions(-) diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/arg_matrix.rs b/compiler/rustc_typeck/src/check/fn_ctxt/arg_matrix.rs index 48a66e8026bee..b7ba9d9787846 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/arg_matrix.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/arg_matrix.rs @@ -3,6 +3,7 @@ use std::cmp; use rustc_middle::ty::error::TypeError; // An issue that might be found in the compatibility matrix +#[derive(Debug)] enum Issue { /// The given argument is the invalid type for the input Invalid(usize), @@ -23,9 +24,10 @@ pub(crate) enum Compatibility<'tcx> { } /// Similar to `Issue`, but contains some extra information +#[derive(Debug)] pub(crate) enum Error<'tcx> { - /// The given argument is the invalid type for the input - Invalid(usize, Compatibility<'tcx>), + /// The provided argument is the invalid type for the expected input + Invalid(usize, usize, Compatibility<'tcx>), // provided, expected /// There is a missing input Missing(usize), /// There's a superfluous argument @@ -37,8 +39,15 @@ pub(crate) enum Error<'tcx> { } pub(crate) struct ArgMatrix<'tcx> { + /// Maps the indices in the `compatibility_matrix` rows to the indices of + /// the *user provided* inputs input_indexes: Vec, + /// Maps the indices in the `compatibility_matrix` columns to the indices + /// of the *expected* args arg_indexes: Vec, + /// The first dimension (rows) are the remaining user provided inputs to + /// match and the second dimension (cols) are the remaining expected args + /// to match compatibility_matrix: Vec>>, } @@ -52,8 +61,8 @@ impl<'tcx> ArgMatrix<'tcx> { .map(|i| (0..minimum_input_count).map(|j| is_compatible(i, j)).collect()) .collect(); ArgMatrix { - input_indexes: (0..minimum_input_count).collect(), - arg_indexes: (0..provided_arg_count).collect(), + input_indexes: (0..provided_arg_count).collect(), + arg_indexes: (0..minimum_input_count).collect(), compatibility_matrix, } } @@ -61,15 +70,15 @@ impl<'tcx> ArgMatrix<'tcx> { /// Remove a given input from consideration fn eliminate_input(&mut self, idx: usize) { self.input_indexes.remove(idx); - for row in &mut self.compatibility_matrix { - row.remove(idx); - } + self.compatibility_matrix.remove(idx); } /// Remove a given argument from consideration fn eliminate_arg(&mut self, idx: usize) { self.arg_indexes.remove(idx); - self.compatibility_matrix.remove(idx); + for row in &mut self.compatibility_matrix { + row.remove(idx); + } } /// "satisfy" an input with a given arg, removing both from consideration @@ -78,13 +87,15 @@ impl<'tcx> ArgMatrix<'tcx> { self.eliminate_arg(arg_idx); } + // Returns a `Vec` of (user input, expected arg) of matched arguments. These + // are inputs on the remaining diagonal that match. fn eliminate_satisfied(&mut self) -> Vec<(usize, usize)> { let mut i = cmp::min(self.input_indexes.len(), self.arg_indexes.len()); let mut eliminated = vec![]; while i > 0 { let idx = i - 1; if matches!(self.compatibility_matrix[idx][idx], Compatibility::Compatible) { - eliminated.push((self.arg_indexes[idx], self.input_indexes[idx])); + eliminated.push((self.input_indexes[idx], self.arg_indexes[idx])); self.satisfy_input(idx, idx); } i -= 1; @@ -92,7 +103,7 @@ impl<'tcx> ArgMatrix<'tcx> { return eliminated; } - // Check for the above mismatch cases + // Find some issue in the compatibility matrix fn find_issue(&self) -> Option { let mat = &self.compatibility_matrix; let ai = &self.arg_indexes; @@ -121,9 +132,9 @@ impl<'tcx> ArgMatrix<'tcx> { if is_arg { for j in 0..ii.len() { // If we find at least one input this argument could satisfy - // this argument isn't completely useless - if matches!(mat[i][j], Compatibility::Compatible) { - useless = false; + // this argument isn't unsatisfiable + if matches!(mat[j][i], Compatibility::Compatible) { + unsatisfiable = false; break; } } @@ -131,16 +142,16 @@ impl<'tcx> ArgMatrix<'tcx> { if is_input { for j in 0..ai.len() { // If we find at least one argument that could satisfy this input - // this argument isn't unsatisfiable - if matches!(mat[j][i], Compatibility::Compatible) { - unsatisfiable = false; + // this argument isn't useless + if matches!(mat[i][j], Compatibility::Compatible) { + useless = false; break; } } } - match (is_arg, is_input, useless, unsatisfiable) { - // If an input is unsatisfied, and the argument in its position is useless + match (is_input, is_arg, useless, unsatisfiable) { + // If an argument is unsatisfied, and the input in its position is useless // then the most likely explanation is that we just got the types wrong (true, true, true, true) => return Some(Issue::Invalid(i)), // Otherwise, if an input is useless, then indicate that this is an extra argument @@ -167,7 +178,7 @@ impl<'tcx> ArgMatrix<'tcx> { _ => { continue; } - }; + } } // We didn't find any of the individual issues above, but @@ -254,11 +265,11 @@ impl<'tcx> ArgMatrix<'tcx> { // We'll want to know which arguments and inputs these rows and columns correspond to // even after we delete them. pub(crate) fn find_errors(mut self) -> (Vec>, Vec>) { - let provided_arg_count = self.arg_indexes.len(); + let provided_arg_count = self.input_indexes.len(); let mut errors: Vec> = vec![]; // For each expected argument, the matched *actual* input - let mut matched_inputs: Vec> = vec![None; self.input_indexes.len()]; + let mut matched_inputs: Vec> = vec![None; self.arg_indexes.len()]; // Before we start looking for issues, eliminate any arguments that are already satisfied, // so that an argument which is already spoken for by the input it's in doesn't @@ -269,28 +280,28 @@ impl<'tcx> ArgMatrix<'tcx> { // Without this elimination, the first argument causes the second argument // to show up as both a missing input and extra argument, rather than // just an invalid type. - for (arg, inp) in self.eliminate_satisfied() { - matched_inputs[inp] = Some(arg); + for (inp, arg) in self.eliminate_satisfied() { + matched_inputs[arg] = Some(inp); } while self.input_indexes.len() > 0 || self.arg_indexes.len() > 0 { - // Check for the first relevant issue match self.find_issue() { Some(Issue::Invalid(idx)) => { let compatibility = self.compatibility_matrix[idx][idx].clone(); let input_idx = self.input_indexes[idx]; + let arg_idx = self.arg_indexes[idx]; self.satisfy_input(idx, idx); - errors.push(Error::Invalid(input_idx, compatibility)); + errors.push(Error::Invalid(input_idx, arg_idx, compatibility)); } Some(Issue::Extra(idx)) => { - let arg_idx = self.arg_indexes[idx]; - self.eliminate_arg(idx); - errors.push(Error::Extra(arg_idx)); - } - Some(Issue::Missing(idx)) => { let input_idx = self.input_indexes[idx]; self.eliminate_input(idx); - errors.push(Error::Missing(input_idx)); + errors.push(Error::Extra(input_idx)); + } + Some(Issue::Missing(idx)) => { + let arg_idx = self.arg_indexes[idx]; + self.eliminate_arg(idx); + errors.push(Error::Missing(arg_idx)); } Some(Issue::Swap(idx, other)) => { let input_idx = self.input_indexes[idx]; @@ -302,24 +313,21 @@ impl<'tcx> ArgMatrix<'tcx> { // Subtract 1 because we already removed the "min" row self.satisfy_input(max - 1, min); errors.push(Error::Swap(input_idx, other_input_idx, arg_idx, other_arg_idx)); - matched_inputs[input_idx] = Some(other_arg_idx); - matched_inputs[other_input_idx] = Some(arg_idx); + matched_inputs[other_arg_idx] = Some(input_idx); + matched_inputs[arg_idx] = Some(other_input_idx); } Some(Issue::Permutation(args)) => { - // FIXME: If satisfy_input ever did anything non-trivial (emit obligations to help type checking, for example) - // we'd want to call this function with the correct arg/input pairs, but for now, we just throw them in a bucket. - // This works because they force a cycle, so each row is guaranteed to also be a column let mut idxs: Vec = args.iter().filter_map(|&a| a).collect(); let mut real_idxs = vec![None; provided_arg_count]; for (src, dst) in args.iter().enumerate().filter_map(|(src, dst)| dst.map(|dst| (src, dst))) { - let src_arg = self.arg_indexes[src]; - let dst_arg = self.arg_indexes[dst]; - let dest_input = self.input_indexes[dst]; - real_idxs[src_arg] = Some((dst_arg, dest_input)); - matched_inputs[dest_input] = Some(src_arg); + let src_input_idx = self.input_indexes[src]; + let dst_input_idx = self.input_indexes[dst]; + let dest_arg_idx = self.arg_indexes[dst]; + real_idxs[src_input_idx] = Some((dest_arg_idx, dst_input_idx)); + matched_inputs[dest_arg_idx] = Some(src_input_idx); } idxs.sort(); idxs.reverse(); @@ -331,8 +339,8 @@ impl<'tcx> ArgMatrix<'tcx> { None => { // We didn't find any issues, so we need to push the algorithm forward // First, eliminate any arguments that currently satisfy their inputs - for (arg, inp) in self.eliminate_satisfied() { - matched_inputs[inp] = Some(arg); + for (inp, arg) in self.eliminate_satisfied() { + matched_inputs[arg] = Some(inp); } } }; diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs index 277743e4a46c1..847c2c32dba29 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs @@ -274,9 +274,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // A "softer" version of the helper above, which checks types without persisting them, // and treats error types differently // This will allow us to "probe" for other argument orders that would likely have been correct - let check_compatible = |arg_idx, input_idx| { - let formal_input_ty: Ty<'tcx> = formal_input_tys[input_idx]; - let expected_input_ty: Ty<'tcx> = expected_input_tys[input_idx]; + let check_compatible = |input_idx, arg_idx| { + let formal_input_ty: Ty<'tcx> = formal_input_tys[arg_idx]; + let expected_input_ty: Ty<'tcx> = expected_input_tys[arg_idx]; // If either is an error type, we defy the usual convention and consider them to *not* be // coercible. This prevents our error message heuristic from trying to pass errors into @@ -285,7 +285,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { return Compatibility::Incompatible(None); } - let provided_arg: &hir::Expr<'tcx> = &provided_args[arg_idx]; + let provided_arg: &hir::Expr<'tcx> = &provided_args[input_idx]; let expectation = Expectation::rvalue_hint(self, expected_input_ty); // FIXME: check that this is safe; I don't believe this commits any of the obligations, but I can't be sure. // @@ -429,11 +429,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let found_errors = !errors.is_empty(); errors.drain_filter(|error| { - let Error::Invalid(input_idx, Compatibility::Incompatible(error)) = error else { return false }; - let expected_ty = expected_input_tys[*input_idx]; - let Some(Some((provided_ty, _))) = final_arg_types.get(*input_idx) else { return false }; + let Error::Invalid(input_idx, arg_idx, Compatibility::Incompatible(error)) = error else { return false }; + let expected_ty = expected_input_tys[*arg_idx]; + let provided_ty = final_arg_types[*input_idx].map(|ty| ty.0).unwrap(); let cause = &self.misc(provided_args[*input_idx].span); - let trace = TypeTrace::types(cause, true, expected_ty, *provided_ty); + let trace = TypeTrace::types(cause, true, expected_ty, provided_ty); if let Some(e) = error { if !matches!(trace.cause.as_failure_code(e), FailureCode::Error0308(_)) { self.report_and_explain_type_error(trace, e).emit(); @@ -562,11 +562,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // Next special case: if there is only one "Incompatible" error, just emit that if errors.len() == 1 { - if let Some(Error::Invalid(input_idx, Compatibility::Incompatible(Some(error)))) = - errors.iter().next() + if let Some(Error::Invalid( + input_idx, + arg_idx, + Compatibility::Incompatible(Some(error)), + )) = errors.iter().next() { - let expected_ty = expected_input_tys[*input_idx]; - let provided_ty = final_arg_types[*input_idx].map(|ty| ty.0).unwrap(); + let expected_ty = expected_input_tys[*arg_idx]; + let provided_ty = final_arg_types[*arg_idx].map(|ty| ty.0).unwrap(); let expected_ty = self.resolve_vars_if_possible(expected_ty); let provided_ty = self.resolve_vars_if_possible(provided_ty); let cause = &self.misc(provided_args[*input_idx].span); @@ -631,19 +634,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let mut errors = errors.into_iter().peekable(); while let Some(error) = errors.next() { match error { - Error::Invalid(input_idx, compatibility) => { - let expected_ty = expected_input_tys[input_idx]; - let provided_ty = final_arg_types - .get(input_idx) - .and_then(|x| x.as_ref()) - .map(|ty| ty.0) - .unwrap_or(tcx.ty_error()); + Error::Invalid(input_idx, arg_idx, compatibility) => { + let expected_ty = expected_input_tys[arg_idx]; + let provided_ty = final_arg_types[input_idx].map(|ty| ty.0).unwrap(); let expected_ty = self.resolve_vars_if_possible(expected_ty); let provided_ty = self.resolve_vars_if_possible(provided_ty); if let Compatibility::Incompatible(error) = &compatibility { - let cause = &self.misc( - provided_args.get(input_idx).map(|i| i.span).unwrap_or(call_span), - ); + let cause = &self.misc(provided_args[input_idx].span); let trace = TypeTrace::types(cause, true, expected_ty, provided_ty); if let Some(e) = error { self.note_type_err( @@ -658,16 +655,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } - if let Some(expr) = provided_args.get(input_idx) { - self.emit_coerce_suggestions( - &mut err, - &expr, - final_arg_types[input_idx].map(|ty| ty.0).unwrap(), - final_arg_types[input_idx].map(|ty| ty.1).unwrap(), - None, - None, - ); - } + self.emit_coerce_suggestions( + &mut err, + &provided_args[input_idx], + final_arg_types[input_idx].map(|ty| ty.0).unwrap(), + final_arg_types[input_idx].map(|ty| ty.1).unwrap(), + None, + None, + ); } Error::Extra(arg_idx) => { let arg_type = if let Some((_, ty)) = final_arg_types[arg_idx] { @@ -843,12 +838,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } Error::Swap(input_idx, other_input_idx, arg_idx, other_arg_idx) => { - let first_span = provided_args[arg_idx].span; - let second_span = provided_args[other_arg_idx].span; + let first_span = provided_args[input_idx].span; + let second_span = provided_args[other_input_idx].span; let first_expected_ty = - self.resolve_vars_if_possible(expected_input_tys[input_idx]); - let first_provided_ty = if let Some((ty, _)) = final_arg_types[arg_idx] { + self.resolve_vars_if_possible(expected_input_tys[arg_idx]); + let first_provided_ty = if let Some((ty, _)) = final_arg_types[input_idx] { format!(",found `{}`", ty) } else { "".into() @@ -858,9 +853,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { format!("expected `{}`{}", first_expected_ty, first_provided_ty), )); let other_expected_ty = - self.resolve_vars_if_possible(expected_input_tys[other_input_idx]); + self.resolve_vars_if_possible(expected_input_tys[other_arg_idx]); let other_provided_ty = - if let Some((ty, _)) = final_arg_types[other_arg_idx] { + if let Some((ty, _)) = final_arg_types[other_input_idx] { format!(",found `{}`", ty) } else { "".into() @@ -926,14 +921,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { "{}(", source_map.span_to_snippet(full_call_span).unwrap_or_else(|_| String::new()) ); - for (idx, arg) in matched_inputs.iter().enumerate() { - let suggestion_text = if let Some(arg) = arg { - let arg_span = provided_args[*arg].span.source_callsite(); + for (arg_index, input_idx) in matched_inputs.iter().enumerate() { + let suggestion_text = if let Some(input_idx) = input_idx { + let arg_span = provided_args[*input_idx].span.source_callsite(); let arg_text = source_map.span_to_snippet(arg_span).unwrap(); arg_text } else { // Propose a placeholder of the correct type - let expected_ty = expected_input_tys[idx]; + let expected_ty = expected_input_tys[arg_index]; let input_ty = self.resolve_vars_if_possible(expected_ty); if input_ty.is_unit() { "()".to_string() @@ -942,7 +937,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } }; suggestion += &suggestion_text; - if idx < minimum_input_count - 1 { + if arg_index < minimum_input_count - 1 { suggestion += ", "; } } From 02eca34534c2f69cda539aade9fff230aae70af3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 9 May 2022 20:13:25 +0200 Subject: [PATCH 9/9] also sanity-check Abi::Vector, and slight refactoring --- compiler/rustc_middle/src/ty/layout.rs | 69 ++++++++++++++++---------- 1 file changed, 43 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 1a1b795b0a4ce..4879c622016e1 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -248,16 +248,36 @@ fn sanity_check_layout<'tcx>( "size mismatch between ABI and layout in {layout:#?}" );*/ } + Abi::Vector { count, element } => { + // No padding in vectors. Alignment can be strengthened, though. + assert!( + layout.align().abi >= element.align(&tcx).abi, + "alignment mismatch between ABI and layout in {layout:#?}" + ); + let size = element.size(&tcx) * count; + assert_eq!( + layout.size(), + size.align_to(tcx.data_layout().vector_align(size).abi), + "size mismatch between ABI and layout in {layout:#?}" + ); + } Abi::ScalarPair(scalar1, scalar2) => { - // Sanity-check scalar pair size. - let field2_offset = scalar1.size(&tcx).align_to(scalar2.align(&tcx).abi); - let total = field2_offset + scalar2.size(&tcx); + // Sanity-check scalar pairs. These are a bit more flexible and support + // padding, but we can at least ensure both fields actually fit into the layout + // and the alignment requirement has not been weakened. + let align1 = scalar1.align(&tcx).abi; + let align2 = scalar2.align(&tcx).abi; assert!( - layout.size() >= total, + layout.align().abi >= cmp::max(align1, align2), + "alignment mismatch between ABI and layout in {layout:#?}", + ); + let field2_offset = scalar1.size(&tcx).align_to(align2); + assert!( + layout.size() >= field2_offset + scalar2.size(&tcx), "size mismatch between ABI and layout in {layout:#?}" ); } - _ => {} + Abi::Uninhabited | Abi::Aggregate { .. } => {} // Nothing to check. } } @@ -1401,16 +1421,6 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { // Without latter check aligned enums with custom discriminant values // Would result in ICE see the issue #92464 for more info abi = Abi::Scalar(tag); - // Make sure the variants with fields have the same ABI as the enum itself - // (since downcasting to them is a NOP). - for variant in &mut layout_variants { - if variant.fields.count() > 0 - && matches!(variant.abi, Abi::Aggregate { .. }) - { - assert_eq!(variant.size, size); - variant.abi = abi; - } - } } else { // Try to use a ScalarPair for all tagged enums. let mut common_prim = None; @@ -1479,17 +1489,24 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { // We can use `ScalarPair` only when it matches our // already computed layout (including `#[repr(C)]`). abi = pair.abi; - // Make sure the variants with fields have the same ABI as the enum itself - // (since downcasting to them is a NOP). - for variant in &mut layout_variants { - if variant.fields.count() > 0 - && matches!(variant.abi, Abi::Aggregate { .. }) - { - variant.abi = abi; - // Also need to bump up the size, so that the pair fits inside. - variant.size = size; - } - } + } + } + } + + // If we pick a "clever" (by-value) ABI, we might have to adjust the ABI of the + // variants to ensure they are consistent. This is because a downcast is + // semantically a NOP, and thus should not affect layout. + if matches!(abi, Abi::Scalar(..) | Abi::ScalarPair(..)) { + for variant in &mut layout_variants { + // We only do this for variants with fields; the others are not accessed anyway. + // Also do not overwrite any already existing "clever" ABIs. + if variant.fields.count() > 0 + && matches!(variant.abi, Abi::Aggregate { .. }) + { + variant.abi = abi; + // Also need to bump up the size and alignment, so that the entire value fits in here. + variant.size = cmp::max(variant.size, size); + variant.align.abi = cmp::max(variant.align.abi, align.abi); } } }