From 7ff71e5fb4c025b261f6c7e7eb996b36382c178e Mon Sep 17 00:00:00 2001 From: yukang Date: Sat, 13 Jul 2024 12:43:35 +0800 Subject: [PATCH 01/13] Remove invalid help diagnostics for const pointer --- .../src/diagnostics/mutability_errors.rs | 33 ++++++++++++------- ...suggest_raw_pointer_syntax-issue-127562.rs | 7 ++++ ...est_raw_pointer_syntax-issue-127562.stderr | 9 +++++ 3 files changed, 37 insertions(+), 12 deletions(-) create mode 100644 tests/ui/suggestions/dont_suggest_raw_pointer_syntax-issue-127562.rs create mode 100644 tests/ui/suggestions/dont_suggest_raw_pointer_syntax-issue-127562.stderr diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index 26b0d23b16642..0b6c527b8c082 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -1146,6 +1146,12 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> { } // don't create labels for compiler-generated spans Some(_) => None, + // don't create labels for the span not from user's code + None if opt_assignment_rhs_span + .is_some_and(|span| self.infcx.tcx.sess.source_map().is_imported(span)) => + { + None + } None => { let (has_sugg, decl_span, sugg) = if name != kw::SelfLower { suggest_ampmut( @@ -1198,18 +1204,21 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> { sugg.push(s); } - err.multipart_suggestion_verbose( - format!( - "consider changing this to be a mutable {pointer_desc}{}", - if is_trait_sig { - " in the `impl` method and the `trait` definition" - } else { - "" - } - ), - sugg, - Applicability::MachineApplicable, - ); + if sugg.iter().all(|(span, _)| !self.infcx.tcx.sess.source_map().is_imported(*span)) + { + err.multipart_suggestion_verbose( + format!( + "consider changing this to be a mutable {pointer_desc}{}", + if is_trait_sig { + " in the `impl` method and the `trait` definition" + } else { + "" + } + ), + sugg, + Applicability::MachineApplicable, + ); + } } Some((false, err_label_span, message, _)) => { let def_id = self.body.source.def_id(); diff --git a/tests/ui/suggestions/dont_suggest_raw_pointer_syntax-issue-127562.rs b/tests/ui/suggestions/dont_suggest_raw_pointer_syntax-issue-127562.rs new file mode 100644 index 0000000000000..03b736e60b63a --- /dev/null +++ b/tests/ui/suggestions/dont_suggest_raw_pointer_syntax-issue-127562.rs @@ -0,0 +1,7 @@ +fn main() { + let val = 2; + let ptr = std::ptr::addr_of!(val); + unsafe { + *ptr = 3; //~ ERROR cannot assign to `*ptr`, which is behind a `*const` pointer + } +} diff --git a/tests/ui/suggestions/dont_suggest_raw_pointer_syntax-issue-127562.stderr b/tests/ui/suggestions/dont_suggest_raw_pointer_syntax-issue-127562.stderr new file mode 100644 index 0000000000000..5396db7940f72 --- /dev/null +++ b/tests/ui/suggestions/dont_suggest_raw_pointer_syntax-issue-127562.stderr @@ -0,0 +1,9 @@ +error[E0594]: cannot assign to `*ptr`, which is behind a `*const` pointer + --> $DIR/dont_suggest_raw_pointer_syntax-issue-127562.rs:5:9 + | +LL | *ptr = 3; + | ^^^^^^^^ `ptr` is a `*const` pointer, so the data it refers to cannot be written + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0594`. From e9cf280ef2ea4550d2305484873a63dfd133abb7 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Mon, 16 Oct 2023 23:56:22 -0700 Subject: [PATCH 02/13] warn less about non-exhaustive in ffi Bindgen allows generating `#[non_exhaustive] #[repr(u32)]` enums. This results in nonintuitive nonlocal `improper_ctypes` warnings, even when the types are otherwise perfectly valid in C. Adjust for actual tooling expectations by avoiding warning on simple enums with only unit variants. --- compiler/rustc_lint/src/types.rs | 27 ++++++++++++------- .../improper_ctypes/auxiliary/types.rs | 11 ++++++++ .../improper_ctypes/extern_crate_improper.rs | 10 ++++++- .../extern_crate_improper.stderr | 10 +++---- 4 files changed, 43 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index 900c496e0330d..eef1f0d133ae4 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -3,6 +3,7 @@ use std::ops::ControlFlow; use rustc_data_structures::fx::FxHashSet; use rustc_errors::DiagMessage; +use rustc_hir::def::CtorKind; use rustc_hir::{is_range_literal, Expr, ExprKind, Node}; use rustc_middle::bug; use rustc_middle::ty::layout::{IntegerExt, LayoutOf, SizeSkeleton}; @@ -1386,15 +1387,6 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { // Empty enums are okay... although sort of useless. return FfiSafe; } - - if def.is_variant_list_non_exhaustive() && !def.did().is_local() { - return FfiUnsafe { - ty, - reason: fluent::lint_improper_ctypes_non_exhaustive, - help: None, - }; - } - // Check for a repr() attribute to specify the size of the // discriminant. if !def.repr().c() && !def.repr().transparent() && def.repr().int.is_none() @@ -1413,8 +1405,25 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { }; } + // non_exhaustive suggests it is possible that someone might break ABI + // see: https://github.com/rust-lang/rust/issues/44109#issuecomment-537583344 + // so warn on complex enums being used outside their crate + let nonexhaustive_nonlocal_ffi = + def.is_variant_list_non_exhaustive() && !def.did().is_local(); + // Check the contained variants. for variant in def.variants() { + // but only warn about really_tagged_union reprs, + // exempt enums with unit ctors like C's (like rust-bindgen) + if nonexhaustive_nonlocal_ffi + && !matches!(variant.ctor_kind(), Some(CtorKind::Const)) + { + return FfiUnsafe { + ty, + reason: fluent::lint_improper_ctypes_non_exhaustive, + help: None, + }; + }; let is_non_exhaustive = variant.is_field_list_non_exhaustive(); if is_non_exhaustive && !variant.def_id.is_local() { return FfiUnsafe { diff --git a/tests/ui/rfcs/rfc-2008-non-exhaustive/improper_ctypes/auxiliary/types.rs b/tests/ui/rfcs/rfc-2008-non-exhaustive/improper_ctypes/auxiliary/types.rs index d6251fcb768f4..4dc5932feab40 100644 --- a/tests/ui/rfcs/rfc-2008-non-exhaustive/improper_ctypes/auxiliary/types.rs +++ b/tests/ui/rfcs/rfc-2008-non-exhaustive/improper_ctypes/auxiliary/types.rs @@ -27,3 +27,14 @@ pub enum NonExhaustiveVariants { #[non_exhaustive] Tuple(u32), #[non_exhaustive] Struct { field: u32 } } + +// Note the absence of repr(C): it's not necessary, and recent C code can now use repr hints too. +#[repr(u32)] +#[non_exhaustive] +pub enum NonExhaustiveCLikeEnum { + One = 1, + Two = 2, + Three = 3, + Four = 4, + Five = 5, +} diff --git a/tests/ui/rfcs/rfc-2008-non-exhaustive/improper_ctypes/extern_crate_improper.rs b/tests/ui/rfcs/rfc-2008-non-exhaustive/improper_ctypes/extern_crate_improper.rs index 7a9b465bb56f6..c7f470fb787a7 100644 --- a/tests/ui/rfcs/rfc-2008-non-exhaustive/improper_ctypes/extern_crate_improper.rs +++ b/tests/ui/rfcs/rfc-2008-non-exhaustive/improper_ctypes/extern_crate_improper.rs @@ -6,7 +6,10 @@ extern crate types; // This test checks that non-exhaustive types with `#[repr(C)]` from an extern crate are considered // improper. -use types::{NonExhaustiveEnum, NonExhaustiveVariants, NormalStruct, TupleStruct, UnitStruct}; +use types::{ + NonExhaustiveCLikeEnum, NonExhaustiveEnum, NonExhaustiveVariants, + NormalStruct, TupleStruct, UnitStruct, +}; extern "C" { pub fn non_exhaustive_enum(_: NonExhaustiveEnum); @@ -21,4 +24,9 @@ extern "C" { //~^ ERROR `extern` block uses type `NonExhaustiveVariants`, which is not FFI-safe } +// These should pass without remark, as they're C-compatible, despite being "non-exhaustive". +extern "C" { + pub fn non_exhaustive_c_compat_enum(_: NonExhaustiveCLikeEnum); +} + fn main() {} diff --git a/tests/ui/rfcs/rfc-2008-non-exhaustive/improper_ctypes/extern_crate_improper.stderr b/tests/ui/rfcs/rfc-2008-non-exhaustive/improper_ctypes/extern_crate_improper.stderr index 43c8e1015e674..afc3d3838ad38 100644 --- a/tests/ui/rfcs/rfc-2008-non-exhaustive/improper_ctypes/extern_crate_improper.stderr +++ b/tests/ui/rfcs/rfc-2008-non-exhaustive/improper_ctypes/extern_crate_improper.stderr @@ -1,5 +1,5 @@ error: `extern` block uses type `NonExhaustiveEnum`, which is not FFI-safe - --> $DIR/extern_crate_improper.rs:12:35 + --> $DIR/extern_crate_improper.rs:15:35 | LL | pub fn non_exhaustive_enum(_: NonExhaustiveEnum); | ^^^^^^^^^^^^^^^^^ not FFI-safe @@ -12,7 +12,7 @@ LL | #![deny(improper_ctypes)] | ^^^^^^^^^^^^^^^ error: `extern` block uses type `NormalStruct`, which is not FFI-safe - --> $DIR/extern_crate_improper.rs:14:44 + --> $DIR/extern_crate_improper.rs:17:44 | LL | pub fn non_exhaustive_normal_struct(_: NormalStruct); | ^^^^^^^^^^^^ not FFI-safe @@ -20,7 +20,7 @@ LL | pub fn non_exhaustive_normal_struct(_: NormalStruct); = note: this struct is non-exhaustive error: `extern` block uses type `UnitStruct`, which is not FFI-safe - --> $DIR/extern_crate_improper.rs:16:42 + --> $DIR/extern_crate_improper.rs:19:42 | LL | pub fn non_exhaustive_unit_struct(_: UnitStruct); | ^^^^^^^^^^ not FFI-safe @@ -28,7 +28,7 @@ LL | pub fn non_exhaustive_unit_struct(_: UnitStruct); = note: this struct is non-exhaustive error: `extern` block uses type `TupleStruct`, which is not FFI-safe - --> $DIR/extern_crate_improper.rs:18:43 + --> $DIR/extern_crate_improper.rs:21:43 | LL | pub fn non_exhaustive_tuple_struct(_: TupleStruct); | ^^^^^^^^^^^ not FFI-safe @@ -36,7 +36,7 @@ LL | pub fn non_exhaustive_tuple_struct(_: TupleStruct); = note: this struct is non-exhaustive error: `extern` block uses type `NonExhaustiveVariants`, which is not FFI-safe - --> $DIR/extern_crate_improper.rs:20:38 + --> $DIR/extern_crate_improper.rs:23:38 | LL | pub fn non_exhaustive_variant(_: NonExhaustiveVariants); | ^^^^^^^^^^^^^^^^^^^^^ not FFI-safe From 99d5f3b2803daa32909aa841e247ba3ed9efd1b7 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 16 Oct 2024 13:44:56 -0400 Subject: [PATCH 03/13] Stop inverting expectation in normalization errors --- .../traits/fulfillment_errors.rs | 25 +++---------------- ...mpl-trait-return-missing-constraint.stderr | 10 +++----- .../type-mismatch-signature-deduction.stderr | 6 ++--- .../as_expression.next.stderr | 5 +--- .../bound-normalization-fail.stderr | 12 ++++----- .../in-trait/default-body-type-err.stderr | 2 +- .../ui/impl-trait/issues/issue-78722-2.stderr | 2 +- tests/ui/impl-trait/issues/issue-78722.stderr | 2 +- ...ction-mismatch-in-impl-where-clause.stderr | 2 +- tests/ui/issues/issue-33941.stderr | 6 ++--- .../issue-67039-unsound-pin-partialeq.stderr | 6 ++--- tests/ui/lint/issue-106991.stderr | 2 +- .../ui/suggestions/trait-hidden-method.stderr | 8 +++--- tests/ui/traits/next-solver/async.fail.stderr | 4 +-- .../next-solver/more-object-bound.stderr | 15 ++++------- tests/ui/try-block/try-block-bad-type.stderr | 4 +-- .../ui/try-block/try-block-type-error.stderr | 9 ++----- .../hidden_type_mismatch.stderr | 8 +++--- .../type-alias-impl-trait/issue-94429.stderr | 2 +- 19 files changed, 48 insertions(+), 82 deletions(-) diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs index 26b0faca2585f..8559ea1a0588c 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs @@ -1278,19 +1278,6 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { let normalized_term = ocx.normalize(&obligation.cause, obligation.param_env, unnormalized_term); - let is_normalized_term_expected = !matches!( - obligation.cause.code().peel_derives(), - ObligationCauseCode::WhereClause(..) - | ObligationCauseCode::WhereClauseInExpr(..) - | ObligationCauseCode::Coercion { .. } - ); - - let (expected, actual) = if is_normalized_term_expected { - (normalized_term, data.term) - } else { - (data.term, normalized_term) - }; - // constrain inference variables a bit more to nested obligations from normalize so // we can have more helpful errors. // @@ -1299,12 +1286,12 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { let _ = ocx.select_where_possible(); if let Err(new_err) = - ocx.eq(&obligation.cause, obligation.param_env, expected, actual) + ocx.eq(&obligation.cause, obligation.param_env, data.term, normalized_term) { ( Some(( data.projection_term, - is_normalized_term_expected, + false, self.resolve_vars_if_possible(normalized_term), data.term, )), @@ -1444,12 +1431,8 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { &mut diag, &obligation.cause, secondary_span, - values.map(|(_, is_normalized_ty_expected, normalized_ty, expected_ty)| { - infer::ValuePairs::Terms(ExpectedFound::new( - is_normalized_ty_expected, - normalized_ty, - expected_ty, - )) + values.map(|(_, _, normalized_ty, expected_ty)| { + infer::ValuePairs::Terms(ExpectedFound::new(true, expected_ty, normalized_ty)) }), err, true, diff --git a/tests/ui/associated-types/impl-trait-return-missing-constraint.stderr b/tests/ui/associated-types/impl-trait-return-missing-constraint.stderr index 24409b32ad3eb..2c6c8ee5d190c 100644 --- a/tests/ui/associated-types/impl-trait-return-missing-constraint.stderr +++ b/tests/ui/associated-types/impl-trait-return-missing-constraint.stderr @@ -2,18 +2,16 @@ error[E0271]: type mismatch resolving `::Item == i32` --> $DIR/impl-trait-return-missing-constraint.rs:25:13 | LL | fn bar() -> impl Bar { - | -------- the expected opaque type + | -------- the found opaque type ... LL | fn baz() -> impl Bar { - | ^^^^^^^^^^^^^^^^^^^^ expected associated type, found `i32` + | ^^^^^^^^^^^^^^^^^^^^ expected `i32`, found associated type LL | LL | bar() | ----- return type was inferred to be `impl Bar` here | - = note: expected associated type `::Item` - found type `i32` - = help: consider constraining the associated type `::Item` to `i32` or calling a method that returns `::Item` - = note: for more information, visit https://doc.rust-lang.org/book/ch19-03-advanced-traits.html + = note: expected type `i32` + found associated type `::Item` help: consider constraining the associated type `::Item` to `i32` | LL | fn bar() -> impl Bar { diff --git a/tests/ui/coroutine/type-mismatch-signature-deduction.stderr b/tests/ui/coroutine/type-mismatch-signature-deduction.stderr index 0892719603752..64dc3a8b1f80c 100644 --- a/tests/ui/coroutine/type-mismatch-signature-deduction.stderr +++ b/tests/ui/coroutine/type-mismatch-signature-deduction.stderr @@ -22,10 +22,10 @@ error[E0271]: type mismatch resolving `<{coroutine@$DIR/type-mismatch-signature- --> $DIR/type-mismatch-signature-deduction.rs:5:13 | LL | fn foo() -> impl Coroutine { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `Result<{integer}, _>`, found `i32` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `i32`, found `Result<{integer}, _>` | - = note: expected enum `Result<{integer}, _>` - found type `i32` + = note: expected type `i32` + found enum `Result<{integer}, _>` error: aborting due to 2 previous errors diff --git a/tests/ui/diagnostic_namespace/do_not_recommend/as_expression.next.stderr b/tests/ui/diagnostic_namespace/do_not_recommend/as_expression.next.stderr index a686b913c55ef..8c01b61191e60 100644 --- a/tests/ui/diagnostic_namespace/do_not_recommend/as_expression.next.stderr +++ b/tests/ui/diagnostic_namespace/do_not_recommend/as_expression.next.stderr @@ -29,10 +29,7 @@ error[E0271]: type mismatch resolving `::SqlType == Tex --> $DIR/as_expression.rs:57:5 | LL | SelectInt.check("bar"); - | ^^^^^^^^^^^^^^^^^^^^^^ expected `Integer`, found `Text` - | - = note: expected struct `Integer` - found struct `Text` + | ^^^^^^^^^^^^^^^^^^^^^^ expected `Text`, found `Integer` error: aborting due to 3 previous errors diff --git a/tests/ui/impl-trait/bound-normalization-fail.stderr b/tests/ui/impl-trait/bound-normalization-fail.stderr index fcac9ac34dbc3..fc124bd117199 100644 --- a/tests/ui/impl-trait/bound-normalization-fail.stderr +++ b/tests/ui/impl-trait/bound-normalization-fail.stderr @@ -7,13 +7,13 @@ LL | LL | Foo(()) | ------- return type was inferred to be `Foo<()>` here | -note: expected this to be `()` +note: expected this to be `::Assoc` --> $DIR/bound-normalization-fail.rs:14:19 | LL | type Output = T; | ^ - = note: expected unit type `()` - found associated type `::Assoc` + = note: expected associated type `::Assoc` + found unit type `()` help: consider constraining the associated type `::Assoc` to `()` | LL | fn foo_fail>() -> impl FooLike { @@ -28,13 +28,13 @@ LL | LL | Foo(()) | ------- return type was inferred to be `Foo<()>` here | -note: expected this to be `()` +note: expected this to be `>::Assoc` --> $DIR/bound-normalization-fail.rs:14:19 | LL | type Output = T; | ^ - = note: expected unit type `()` - found associated type `>::Assoc` + = note: expected associated type `>::Assoc` + found unit type `()` help: consider constraining the associated type `>::Assoc` to `()` | LL | fn foo2_fail<'a, T: Trait<'a, Assoc = ()>>() -> impl FooLike { diff --git a/tests/ui/impl-trait/in-trait/default-body-type-err.stderr b/tests/ui/impl-trait/in-trait/default-body-type-err.stderr index 6f1ac4bce43b7..3c737f095ce1e 100644 --- a/tests/ui/impl-trait/in-trait/default-body-type-err.stderr +++ b/tests/ui/impl-trait/in-trait/default-body-type-err.stderr @@ -2,7 +2,7 @@ error[E0271]: type mismatch resolving `<&i32 as Deref>::Target == String` --> $DIR/default-body-type-err.rs:4:22 | LL | fn lol(&self) -> impl Deref { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `i32`, found `String` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `String`, found `i32` LL | LL | &1i32 | ----- return type was inferred to be `&i32` here diff --git a/tests/ui/impl-trait/issues/issue-78722-2.stderr b/tests/ui/impl-trait/issues/issue-78722-2.stderr index 2cf6b94dd9d9b..27b4b71283023 100644 --- a/tests/ui/impl-trait/issues/issue-78722-2.stderr +++ b/tests/ui/impl-trait/issues/issue-78722-2.stderr @@ -16,7 +16,7 @@ error[E0271]: expected `{async block@$DIR/issue-78722-2.rs:13:13: 13:18}` to be --> $DIR/issue-78722-2.rs:11:30 | LL | fn concrete_use() -> F { - | ^ expected `()`, found `u8` + | ^ expected `u8`, found `()` error: aborting due to 2 previous errors diff --git a/tests/ui/impl-trait/issues/issue-78722.stderr b/tests/ui/impl-trait/issues/issue-78722.stderr index 3642000597f92..109bda0c5cd60 100644 --- a/tests/ui/impl-trait/issues/issue-78722.stderr +++ b/tests/ui/impl-trait/issues/issue-78722.stderr @@ -12,7 +12,7 @@ error[E0271]: expected `{async block@$DIR/issue-78722.rs:10:13: 10:18}` to be a --> $DIR/issue-78722.rs:8:30 | LL | fn concrete_use() -> F { - | ^ expected `()`, found `u8` + | ^ expected `u8`, found `()` error: aborting due to 2 previous errors diff --git a/tests/ui/impl-trait/projection-mismatch-in-impl-where-clause.stderr b/tests/ui/impl-trait/projection-mismatch-in-impl-where-clause.stderr index c4ea44740662d..fa71adc63809f 100644 --- a/tests/ui/impl-trait/projection-mismatch-in-impl-where-clause.stderr +++ b/tests/ui/impl-trait/projection-mismatch-in-impl-where-clause.stderr @@ -4,7 +4,7 @@ error[E0271]: type mismatch resolving `<() as Super>::Assoc == ()` LL | fn test() -> impl Test { | ^^^^^^^^^ type mismatch resolving `<() as Super>::Assoc == ()` | -note: expected this to be `u8` +note: expected this to be `()` --> $DIR/projection-mismatch-in-impl-where-clause.rs:6:18 | LL | type Assoc = u8; diff --git a/tests/ui/issues/issue-33941.stderr b/tests/ui/issues/issue-33941.stderr index f1b6b6ba17edc..9535ea57430d0 100644 --- a/tests/ui/issues/issue-33941.stderr +++ b/tests/ui/issues/issue-33941.stderr @@ -20,10 +20,10 @@ error[E0271]: expected `Iter<'_, _, _>` to be an iterator that yields `&_`, but --> $DIR/issue-33941.rs:6:14 | LL | for _ in HashMap::new().iter().cloned() {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `(&_, &_)`, found `&_` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `&_`, found `(&_, &_)` | - = note: expected tuple `(&_, &_)` - found reference `&_` + = note: expected reference `&_` + found tuple `(&_, &_)` = note: required for `Cloned>` to implement `Iterator` = note: required for `Cloned>` to implement `IntoIterator` diff --git a/tests/ui/issues/issue-67039-unsound-pin-partialeq.stderr b/tests/ui/issues/issue-67039-unsound-pin-partialeq.stderr index 1ea2d48b474d7..9164e4696eb34 100644 --- a/tests/ui/issues/issue-67039-unsound-pin-partialeq.stderr +++ b/tests/ui/issues/issue-67039-unsound-pin-partialeq.stderr @@ -2,10 +2,10 @@ error[E0271]: type mismatch resolving ` as Deref>::Target == Rc --> $DIR/issue-67039-unsound-pin-partialeq.rs:25:29 | LL | let _ = Pin::new(Apple) == Rc::pin(Apple); - | ^^ expected `Apple`, found `Rc` + | ^^ expected `Rc`, found `Apple` | - = note: expected struct `Apple` - found struct `Rc` + = note: expected struct `Rc` + found struct `Apple` = note: required for `Pin` to implement `PartialEq>>` error: aborting due to 1 previous error diff --git a/tests/ui/lint/issue-106991.stderr b/tests/ui/lint/issue-106991.stderr index 4704e9ef8355f..9b4fab68102e2 100644 --- a/tests/ui/lint/issue-106991.stderr +++ b/tests/ui/lint/issue-106991.stderr @@ -2,7 +2,7 @@ error[E0271]: expected `foo` to be a fn item that returns `i32`, but it returns --> $DIR/issue-106991.rs:5:13 | LL | fn bar() -> impl Iterator { - | ^^^^^^^^^^^^^^^^^^^^^^^^^ expected `()`, found `i32` + | ^^^^^^^^^^^^^^^^^^^^^^^^^ expected `i32`, found `()` | = note: required for `Map>, for<'a> fn(&'a mut Vec) {foo}>` to implement `Iterator` diff --git a/tests/ui/suggestions/trait-hidden-method.stderr b/tests/ui/suggestions/trait-hidden-method.stderr index 5dec20718467c..729523cde55e5 100644 --- a/tests/ui/suggestions/trait-hidden-method.stderr +++ b/tests/ui/suggestions/trait-hidden-method.stderr @@ -8,14 +8,14 @@ error[E0271]: expected `Box` to be an iterator that yields `u32`, --> $DIR/trait-hidden-method.rs:3:32 | LL | pub fn i_can_has_iterator() -> impl Iterator { - | ^^^^^^^^^^^^^^^^^^^^^^^^^ expected associated type, found `u32` + | ^^^^^^^^^^^^^^^^^^^^^^^^^ expected `u32`, found associated type ... LL | Box::new(1..=10) as Box | ------------------------------------- return type was inferred to be `Box` here | - = note: expected associated type `::Item` - found type `u32` - = help: consider constraining the associated type `::Item` to `u32` or calling a method that returns `::Item` + = note: expected type `u32` + found associated type `::Item` + = help: consider constraining the associated type `::Item` to `u32` = note: for more information, visit https://doc.rust-lang.org/book/ch19-03-advanced-traits.html error: aborting due to 2 previous errors diff --git a/tests/ui/traits/next-solver/async.fail.stderr b/tests/ui/traits/next-solver/async.fail.stderr index e47da338736f4..bc89842d16a14 100644 --- a/tests/ui/traits/next-solver/async.fail.stderr +++ b/tests/ui/traits/next-solver/async.fail.stderr @@ -2,12 +2,10 @@ error[E0271]: expected `{async block@$DIR/async.rs:12:17: 12:22}` to be a future --> $DIR/async.rs:12:17 | LL | needs_async(async {}); - | ----------- ^^^^^^^^ expected `()`, found `i32` + | ----------- ^^^^^^^^ expected `i32`, found `()` | | | required by a bound introduced by this call | - = note: expected unit type `()` - found type `i32` note: required by a bound in `needs_async` --> $DIR/async.rs:8:31 | diff --git a/tests/ui/traits/next-solver/more-object-bound.stderr b/tests/ui/traits/next-solver/more-object-bound.stderr index 043cbdff9ab9d..39849d4c865e9 100644 --- a/tests/ui/traits/next-solver/more-object-bound.stderr +++ b/tests/ui/traits/next-solver/more-object-bound.stderr @@ -2,19 +2,14 @@ error[E0271]: type mismatch resolving ` as SuperTrait>:: --> $DIR/more-object-bound.rs:12:5 | LL | fn transmute(x: A) -> B { - | - - - | | | - | | expected type parameter - | | found type parameter + | - - expected type parameter + | | | found type parameter - | expected type parameter LL | foo::>(x) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected type parameter `A`, found type parameter `B` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected type parameter `B`, found type parameter `A` | - = note: expected type parameter `A` - found type parameter `B` - = note: a type parameter was expected, but a different one was found; you might be missing a type parameter or trait bound - = note: for more information, visit https://doc.rust-lang.org/book/ch10-02-traits.html#traits-as-parameters + = note: expected type parameter `B` + found type parameter `A` = note: a type parameter was expected, but a different one was found; you might be missing a type parameter or trait bound = note: for more information, visit https://doc.rust-lang.org/book/ch10-02-traits.html#traits-as-parameters = note: required because it appears within the type `dyn Trait` diff --git a/tests/ui/try-block/try-block-bad-type.stderr b/tests/ui/try-block/try-block-bad-type.stderr index 6c41b42dc643f..e9e6255cd2a34 100644 --- a/tests/ui/try-block/try-block-bad-type.stderr +++ b/tests/ui/try-block/try-block-bad-type.stderr @@ -15,13 +15,13 @@ error[E0271]: type mismatch resolving ` as Try>::Output == &str --> $DIR/try-block-bad-type.rs:12:9 | LL | "" - | ^^ expected `i32`, found `&str` + | ^^ expected `&str`, found `i32` error[E0271]: type mismatch resolving ` as Try>::Output == ()` --> $DIR/try-block-bad-type.rs:15:39 | LL | let res: Result = try { }; - | ^ expected `i32`, found `()` + | ^ expected `()`, found `i32` error[E0277]: a `try` block must return `Result` or `Option` (or another type that implements `Try`) --> $DIR/try-block-bad-type.rs:17:25 diff --git a/tests/ui/try-block/try-block-type-error.stderr b/tests/ui/try-block/try-block-type-error.stderr index 2cdb5fdee79a1..07b1209de9d77 100644 --- a/tests/ui/try-block/try-block-type-error.stderr +++ b/tests/ui/try-block/try-block-type-error.stderr @@ -2,18 +2,13 @@ error[E0271]: type mismatch resolving ` as Try>::Output == {integer} --> $DIR/try-block-type-error.rs:10:9 | LL | 42 - | ^^ expected `f32`, found integer - | -help: use a float literal - | -LL | 42.0 - | ++ + | ^^ expected integer, found `f32` error[E0271]: type mismatch resolving ` as Try>::Output == ()` --> $DIR/try-block-type-error.rs:16:5 | LL | }; - | ^ expected `i32`, found `()` + | ^ expected `()`, found `i32` error: aborting due to 2 previous errors diff --git a/tests/ui/type-alias-impl-trait/hidden_type_mismatch.stderr b/tests/ui/type-alias-impl-trait/hidden_type_mismatch.stderr index 21d9ed9336666..b05121a489e40 100644 --- a/tests/ui/type-alias-impl-trait/hidden_type_mismatch.stderr +++ b/tests/ui/type-alias-impl-trait/hidden_type_mismatch.stderr @@ -2,18 +2,18 @@ error[E0271]: type mismatch resolving `<() as Proj>::Assoc == i32` --> $DIR/hidden_type_mismatch.rs:43:9 | LL | pub type Sep = impl Sized + std::fmt::Display; - | ------------------------------ the expected opaque type + | ------------------------------ the found opaque type ... LL | Bar { inner: 1i32, _marker: () } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type mismatch resolving `<() as Proj>::Assoc == i32` | -note: expected this to be `Sep` +note: expected this to be `i32` --> $DIR/hidden_type_mismatch.rs:20:22 | LL | type Assoc = sus::Sep; | ^^^^^^^^ - = note: expected opaque type `Sep` - found type `i32` + = note: expected type `i32` + found opaque type `Sep` note: required for `Bar<()>` to implement `Copy` --> $DIR/hidden_type_mismatch.rs:32:39 | diff --git a/tests/ui/type-alias-impl-trait/issue-94429.stderr b/tests/ui/type-alias-impl-trait/issue-94429.stderr index 4c2020becbe32..f41b781f963d2 100644 --- a/tests/ui/type-alias-impl-trait/issue-94429.stderr +++ b/tests/ui/type-alias-impl-trait/issue-94429.stderr @@ -2,7 +2,7 @@ error[E0271]: type mismatch resolving `<{coroutine@$DIR/issue-94429.rs:18:9: 18: --> $DIR/issue-94429.rs:15:26 | LL | fn run(&mut self) -> Self::Coro { - | ^^^^^^^^^^ expected integer, found `()` + | ^^^^^^^^^^ expected `()`, found integer error: aborting due to 1 previous error From 6d8815887caf82ac7d75086286cf2a94354a7b62 Mon Sep 17 00:00:00 2001 From: GnomedDev Date: Wed, 16 Oct 2024 09:33:48 +0100 Subject: [PATCH 04/13] Remove TODO in proc_macro now `const_refs_to_static` is stable --- library/proc_macro/src/bridge/client.rs | 23 +++++++---------------- library/proc_macro/src/bridge/server.rs | 8 ++++---- 2 files changed, 11 insertions(+), 20 deletions(-) diff --git a/library/proc_macro/src/bridge/client.rs b/library/proc_macro/src/bridge/client.rs index 5a1086527a127..f6d4825c67b24 100644 --- a/library/proc_macro/src/bridge/client.rs +++ b/library/proc_macro/src/bridge/client.rs @@ -18,17 +18,10 @@ macro_rules! define_client_handles { $(pub(super) $ity: AtomicU32,)* } - impl HandleCounters { - // FIXME(eddyb) use a reference to the `static COUNTERS`, instead of - // a wrapper `fn` pointer, once `const fn` can reference `static`s. - extern "C" fn get() -> &'static Self { - static COUNTERS: HandleCounters = HandleCounters { - $($oty: AtomicU32::new(1),)* - $($ity: AtomicU32::new(1),)* - }; - &COUNTERS - } - } + static COUNTERS: HandleCounters = HandleCounters { + $($oty: AtomicU32::new(1),)* + $($ity: AtomicU32::new(1),)* + }; $( pub(crate) struct $oty { @@ -259,9 +252,7 @@ pub(crate) fn is_available() -> bool { /// and forcing the use of APIs that take/return `S::TokenStream`, server-side. #[repr(C)] pub struct Client { - // FIXME(eddyb) use a reference to the `static COUNTERS`, instead of - // a wrapper `fn` pointer, once `const fn` can reference `static`s. - pub(super) get_handle_counters: extern "C" fn() -> &'static HandleCounters, + pub(super) handle_counters: &'static HandleCounters, pub(super) run: extern "C" fn(BridgeConfig<'_>) -> Buffer, @@ -346,7 +337,7 @@ fn run_client DecodeMut<'a, 's, ()>, R: Encode<()>>( impl Client { pub const fn expand1(f: impl Fn(crate::TokenStream) -> crate::TokenStream + Copy) -> Self { Client { - get_handle_counters: HandleCounters::get, + handle_counters: &COUNTERS, run: super::selfless_reify::reify_to_extern_c_fn_hrt_bridge(move |bridge| { run_client(bridge, |input| f(crate::TokenStream(Some(input))).0) }), @@ -360,7 +351,7 @@ impl Client<(crate::TokenStream, crate::TokenStream), crate::TokenStream> { f: impl Fn(crate::TokenStream, crate::TokenStream) -> crate::TokenStream + Copy, ) -> Self { Client { - get_handle_counters: HandleCounters::get, + handle_counters: &COUNTERS, run: super::selfless_reify::reify_to_extern_c_fn_hrt_bridge(move |bridge| { run_client(bridge, |(input, input2)| { f(crate::TokenStream(Some(input)), crate::TokenStream(Some(input2))).0 diff --git a/library/proc_macro/src/bridge/server.rs b/library/proc_macro/src/bridge/server.rs index 692b6038a3872..97e5a603c3ac9 100644 --- a/library/proc_macro/src/bridge/server.rs +++ b/library/proc_macro/src/bridge/server.rs @@ -400,10 +400,10 @@ impl client::Client { S: Server, S::TokenStream: Default, { - let client::Client { get_handle_counters, run, _marker } = *self; + let client::Client { handle_counters, run, _marker } = *self; run_server( strategy, - get_handle_counters(), + handle_counters, server, as Types>::TokenStream::mark(input), run, @@ -426,10 +426,10 @@ impl client::Client<(crate::TokenStream, crate::TokenStream), crate::TokenStream S: Server, S::TokenStream: Default, { - let client::Client { get_handle_counters, run, _marker } = *self; + let client::Client { handle_counters, run, _marker } = *self; run_server( strategy, - get_handle_counters(), + handle_counters, server, ( as Types>::TokenStream::mark(input), From 62aa8f0740a5a847482df50ae457f22113630719 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 19 Sep 2024 13:56:24 -0700 Subject: [PATCH 05/13] compiler: Embed consensus in `lint::types::improper_ctypes` Extracting this logic into a module makes it easier to write down, and more importantly, later find, the actual decisions we've made. --- compiler/rustc_lint/src/types.rs | 42 +++++---------- .../rustc_lint/src/types/improper_ctypes.rs | 51 +++++++++++++++++++ 2 files changed, 65 insertions(+), 28 deletions(-) create mode 100644 compiler/rustc_lint/src/types/improper_ctypes.rs diff --git a/compiler/rustc_lint/src/types.rs b/compiler/rustc_lint/src/types.rs index eef1f0d133ae4..48e9e78db1e6f 100644 --- a/compiler/rustc_lint/src/types.rs +++ b/compiler/rustc_lint/src/types.rs @@ -3,7 +3,6 @@ use std::ops::ControlFlow; use rustc_data_structures::fx::FxHashSet; use rustc_errors::DiagMessage; -use rustc_hir::def::CtorKind; use rustc_hir::{is_range_literal, Expr, ExprKind, Node}; use rustc_middle::bug; use rustc_middle::ty::layout::{IntegerExt, LayoutOf, SizeSkeleton}; @@ -19,6 +18,8 @@ use rustc_target::spec::abi::Abi as SpecAbi; use tracing::debug; use {rustc_ast as ast, rustc_attr as attr, rustc_hir as hir}; +mod improper_ctypes; + use crate::lints::{ AmbiguousWidePointerComparisons, AmbiguousWidePointerComparisonsAddrMetadataSuggestion, AmbiguousWidePointerComparisonsAddrSuggestion, AtomicOrderingFence, AtomicOrderingLoad, @@ -1405,38 +1406,23 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> { }; } - // non_exhaustive suggests it is possible that someone might break ABI - // see: https://github.com/rust-lang/rust/issues/44109#issuecomment-537583344 - // so warn on complex enums being used outside their crate - let nonexhaustive_nonlocal_ffi = - def.is_variant_list_non_exhaustive() && !def.did().is_local(); + use improper_ctypes::{ + check_non_exhaustive_variant, non_local_and_non_exhaustive, + }; + let non_local_def = non_local_and_non_exhaustive(def); // Check the contained variants. - for variant in def.variants() { - // but only warn about really_tagged_union reprs, - // exempt enums with unit ctors like C's (like rust-bindgen) - if nonexhaustive_nonlocal_ffi - && !matches!(variant.ctor_kind(), Some(CtorKind::Const)) - { - return FfiUnsafe { - ty, - reason: fluent::lint_improper_ctypes_non_exhaustive, - help: None, - }; - }; - let is_non_exhaustive = variant.is_field_list_non_exhaustive(); - if is_non_exhaustive && !variant.def_id.is_local() { - return FfiUnsafe { - ty, - reason: fluent::lint_improper_ctypes_non_exhaustive_variant, - help: None, - }; - } + let ret = def.variants().iter().try_for_each(|variant| { + check_non_exhaustive_variant(non_local_def, variant) + .map_break(|reason| FfiUnsafe { ty, reason, help: None })?; match self.check_variant_for_ffi(acc, ty, def, variant, args) { - FfiSafe => (), - r => return r, + FfiSafe => ControlFlow::Continue(()), + r => ControlFlow::Break(r), } + }); + if let ControlFlow::Break(result) = ret { + return result; } FfiSafe diff --git a/compiler/rustc_lint/src/types/improper_ctypes.rs b/compiler/rustc_lint/src/types/improper_ctypes.rs new file mode 100644 index 0000000000000..1030101c545da --- /dev/null +++ b/compiler/rustc_lint/src/types/improper_ctypes.rs @@ -0,0 +1,51 @@ +use std::ops::ControlFlow; + +use rustc_errors::DiagMessage; +use rustc_hir::def::CtorKind; +use rustc_middle::ty; + +use crate::fluent_generated as fluent; + +/// Check a variant of a non-exhaustive enum for improper ctypes +/// +/// We treat `#[non_exhaustive] enum` as "ensure that code will compile if new variants are added". +/// This includes linting, on a best-effort basis. There are valid additions that are unlikely. +/// +/// Adding a data-carrying variant to an existing C-like enum that is passed to C is "unlikely", +/// so we don't need the lint to account for it. +/// e.g. going from enum Foo { A, B, C } to enum Foo { A, B, C, D(u32) }. +pub(crate) fn check_non_exhaustive_variant( + non_local_def: bool, + variant: &ty::VariantDef, +) -> ControlFlow { + // non_exhaustive suggests it is possible that someone might break ABI + // see: https://github.com/rust-lang/rust/issues/44109#issuecomment-537583344 + // so warn on complex enums being used outside their crate + if non_local_def { + // which is why we only warn about really_tagged_union reprs from https://rust.tf/rfc2195 + // with an enum like `#[repr(u8)] enum Enum { A(DataA), B(DataB), }` + // but exempt enums with unit ctors like C's (e.g. from rust-bindgen) + if variant_has_complex_ctor(variant) { + return ControlFlow::Break(fluent::lint_improper_ctypes_non_exhaustive); + } + } + + let non_exhaustive_variant_fields = variant.is_field_list_non_exhaustive(); + if non_exhaustive_variant_fields && !variant.def_id.is_local() { + return ControlFlow::Break(fluent::lint_improper_ctypes_non_exhaustive_variant); + } + + ControlFlow::Continue(()) +} + +fn variant_has_complex_ctor(variant: &ty::VariantDef) -> bool { + // CtorKind::Const means a "unit" ctor + !matches!(variant.ctor_kind(), Some(CtorKind::Const)) +} + +// non_exhaustive suggests it is possible that someone might break ABI +// see: https://github.com/rust-lang/rust/issues/44109#issuecomment-537583344 +// so warn on complex enums being used outside their crate +pub(crate) fn non_local_and_non_exhaustive(def: ty::AdtDef<'_>) -> bool { + def.is_variant_list_non_exhaustive() && !def.did().is_local() +} From aa299a9bdfc64b02404d05fe1984fd1571aa224d Mon Sep 17 00:00:00 2001 From: clubby789 Date: Sat, 19 Oct 2024 10:05:07 +0000 Subject: [PATCH 06/13] Add codegen test for branchy bool match --- .../issues/issue-108395-branchy-bool-match.rs | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 tests/codegen/issues/issue-108395-branchy-bool-match.rs diff --git a/tests/codegen/issues/issue-108395-branchy-bool-match.rs b/tests/codegen/issues/issue-108395-branchy-bool-match.rs new file mode 100644 index 0000000000000..24f5c0f663532 --- /dev/null +++ b/tests/codegen/issues/issue-108395-branchy-bool-match.rs @@ -0,0 +1,27 @@ +//@ compile-flags: -O -Zmerge-functions=disabled +//! Test for . Check that +//! matching on two bools with wildcards does not produce branches. +#![crate_type = "lib"] + +// CHECK-LABEL: @wildcard( +#[no_mangle] +pub fn wildcard(a: u16, b: u16, v: u16) -> u16 { + // CHECK-NOT: br + match (a == v, b == v) { + (true, false) => 0, + (false, true) => u16::MAX, + _ => 1 << 15, // half + } +} + +// CHECK-LABEL: @exhaustive( +#[no_mangle] +pub fn exhaustive(a: u16, b: u16, v: u16) -> u16 { + // CHECK-NOT: br + match (a == v, b == v) { + (true, false) => 0, + (false, true) => u16::MAX, + (true, true) => 1 << 15, + (false, false) => 1 << 15, + } +} From d84114690bc0241792e50e386250d8ebd48884f0 Mon Sep 17 00:00:00 2001 From: klensy Date: Sat, 19 Oct 2024 13:05:42 +0300 Subject: [PATCH 07/13] replace STATX_ALL with (STATX_BASIC_STATS | STATX_BTIME) as former is deprecated --- library/std/src/sys/pal/unix/fs.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/library/std/src/sys/pal/unix/fs.rs b/library/std/src/sys/pal/unix/fs.rs index 567577b2b4d23..f1f843a5f7ae7 100644 --- a/library/std/src/sys/pal/unix/fs.rs +++ b/library/std/src/sys/pal/unix/fs.rs @@ -189,7 +189,7 @@ cfg_has_statx! {{ // See: https://github.com/rust-lang/rust/issues/65662 // // FIXME what about transient conditions like `ENOMEM`? - let err2 = cvt(statx(0, ptr::null(), 0, libc::STATX_ALL, ptr::null_mut())) + let err2 = cvt(statx(0, ptr::null(), 0, libc::STATX_BASIC_STATS | libc::STATX_BTIME, ptr::null_mut())) .err() .and_then(|e| e.raw_os_error()); if err2 == Some(libc::EFAULT) { @@ -910,7 +910,7 @@ impl DirEntry { fd, name, libc::AT_SYMLINK_NOFOLLOW | libc::AT_STATX_SYNC_AS_STAT, - libc::STATX_ALL, + libc::STATX_BASIC_STATS | libc::STATX_BTIME, ) } { return ret; } @@ -1194,7 +1194,7 @@ impl File { fd, c"".as_ptr() as *const c_char, libc::AT_EMPTY_PATH | libc::AT_STATX_SYNC_AS_STAT, - libc::STATX_ALL, + libc::STATX_BASIC_STATS | libc::STATX_BTIME, ) } { return ret; } @@ -1767,7 +1767,7 @@ pub fn stat(p: &Path) -> io::Result { libc::AT_FDCWD, p.as_ptr(), libc::AT_STATX_SYNC_AS_STAT, - libc::STATX_ALL, + libc::STATX_BASIC_STATS | libc::STATX_BTIME, ) } { return ret; } @@ -1786,7 +1786,7 @@ pub fn lstat(p: &Path) -> io::Result { libc::AT_FDCWD, p.as_ptr(), libc::AT_SYMLINK_NOFOLLOW | libc::AT_STATX_SYNC_AS_STAT, - libc::STATX_ALL, + libc::STATX_BASIC_STATS | libc::STATX_BTIME, ) } { return ret; } From d82a21f9adf840a48c5ddee3dbc474259d0bc1d3 Mon Sep 17 00:00:00 2001 From: clubby789 Date: Sat, 19 Oct 2024 11:11:33 +0000 Subject: [PATCH 08/13] Warn on redundant `--cfg` directive when revisions are used --- src/tools/compiletest/src/runtest.rs | 14 +++++++++++++- tests/ui/precondition-checks/layout.rs | 2 -- tests/ui/sanitizer/cfg.rs | 12 ++++++------ .../core-std-import-order-issue-83564.no_std.fixed | 2 +- .../core-std-import-order-issue-83564.rs | 2 +- .../core-std-import-order-issue-83564.std.fixed | 2 +- 6 files changed, 22 insertions(+), 12 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index f0452008304b4..5b8a96a54c2df 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -468,7 +468,19 @@ impl<'test> TestCx<'test> { if let Some(revision) = self.revision { let normalized_revision = normalize_revision(revision); - cmd.args(&["--cfg", &normalized_revision]); + let cfg_arg = ["--cfg", &normalized_revision]; + let arg = format!("--cfg={normalized_revision}"); + if self + .props + .compile_flags + .windows(2) + .any(|args| args == cfg_arg || args[0] == arg || args[1] == arg) + { + panic!( + "error: redundant cfg argument `{normalized_revision}` is already created by the revision" + ); + } + cmd.args(cfg_arg); } if !self.props.no_auto_check_cfg { diff --git a/tests/ui/precondition-checks/layout.rs b/tests/ui/precondition-checks/layout.rs index 4fd1bbc4a994b..4ee66cc932886 100644 --- a/tests/ui/precondition-checks/layout.rs +++ b/tests/ui/precondition-checks/layout.rs @@ -2,8 +2,6 @@ //@ compile-flags: -Copt-level=3 -Cdebug-assertions=no -Zub-checks=yes //@ error-pattern: unsafe precondition(s) violated: Layout::from_size_align_unchecked requires //@ revisions: toolarge badalign -//@[toolarge] compile-flags: --cfg toolarge -//@[badalign] compile-flags: --cfg badalign fn main() { unsafe { diff --git a/tests/ui/sanitizer/cfg.rs b/tests/ui/sanitizer/cfg.rs index b1ba17d57139c..7b8f285e41a8a 100644 --- a/tests/ui/sanitizer/cfg.rs +++ b/tests/ui/sanitizer/cfg.rs @@ -5,19 +5,19 @@ //@ revisions: address cfi kcfi leak memory thread //@compile-flags: -Ctarget-feature=-crt-static //@[address]needs-sanitizer-address -//@[address]compile-flags: -Zsanitizer=address --cfg address +//@[address]compile-flags: -Zsanitizer=address //@[cfi]needs-sanitizer-cfi -//@[cfi]compile-flags: -Zsanitizer=cfi --cfg cfi +//@[cfi]compile-flags: -Zsanitizer=cfi //@[cfi]compile-flags: -Clto -Ccodegen-units=1 //@[kcfi]needs-llvm-components: x86 -//@[kcfi]compile-flags: -Zsanitizer=kcfi --cfg kcfi --target x86_64-unknown-none +//@[kcfi]compile-flags: -Zsanitizer=kcfi --target x86_64-unknown-none //@[kcfi]compile-flags: -C panic=abort //@[leak]needs-sanitizer-leak -//@[leak]compile-flags: -Zsanitizer=leak --cfg leak +//@[leak]compile-flags: -Zsanitizer=leak //@[memory]needs-sanitizer-memory -//@[memory]compile-flags: -Zsanitizer=memory --cfg memory +//@[memory]compile-flags: -Zsanitizer=memory //@[thread]needs-sanitizer-thread -//@[thread]compile-flags: -Zsanitizer=thread --cfg thread +//@[thread]compile-flags: -Zsanitizer=thread #![feature(cfg_sanitize, no_core, lang_items)] #![crate_type="lib"] diff --git a/tests/ui/suggestions/core-std-import-order-issue-83564.no_std.fixed b/tests/ui/suggestions/core-std-import-order-issue-83564.no_std.fixed index 02d667d984421..b7b94a051212d 100644 --- a/tests/ui/suggestions/core-std-import-order-issue-83564.no_std.fixed +++ b/tests/ui/suggestions/core-std-import-order-issue-83564.no_std.fixed @@ -4,7 +4,7 @@ // For some reason, Rust 2018 or higher is required to reproduce the bug. //@ run-rustfix //@ revisions: no_std std -//@ [no_std]compile-flags: --cfg=no_std -C panic=abort +//@ [no_std]compile-flags: -C panic=abort #![cfg_attr(no_std, no_std)] use core::num::NonZero; diff --git a/tests/ui/suggestions/core-std-import-order-issue-83564.rs b/tests/ui/suggestions/core-std-import-order-issue-83564.rs index 5bb5bfe176ba8..4cfc9a6bf7425 100644 --- a/tests/ui/suggestions/core-std-import-order-issue-83564.rs +++ b/tests/ui/suggestions/core-std-import-order-issue-83564.rs @@ -4,7 +4,7 @@ // For some reason, Rust 2018 or higher is required to reproduce the bug. //@ run-rustfix //@ revisions: no_std std -//@ [no_std]compile-flags: --cfg=no_std -C panic=abort +//@ [no_std]compile-flags: -C panic=abort #![cfg_attr(no_std, no_std)] fn main() { diff --git a/tests/ui/suggestions/core-std-import-order-issue-83564.std.fixed b/tests/ui/suggestions/core-std-import-order-issue-83564.std.fixed index 3492b42c685f9..84c7c19d19e25 100644 --- a/tests/ui/suggestions/core-std-import-order-issue-83564.std.fixed +++ b/tests/ui/suggestions/core-std-import-order-issue-83564.std.fixed @@ -4,7 +4,7 @@ // For some reason, Rust 2018 or higher is required to reproduce the bug. //@ run-rustfix //@ revisions: no_std std -//@ [no_std]compile-flags: --cfg=no_std -C panic=abort +//@ [no_std]compile-flags: -C panic=abort #![cfg_attr(no_std, no_std)] use std::num::NonZero; From fa478239b19ccf1a455985fec7577d7091a7620c Mon Sep 17 00:00:00 2001 From: usamoi Date: Sat, 19 Oct 2024 22:30:41 +0800 Subject: [PATCH 09/13] use tracked_path in rustc_fluent_macro --- compiler/rustc_fluent_macro/src/fluent.rs | 4 ++-- compiler/rustc_fluent_macro/src/lib.rs | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_fluent_macro/src/fluent.rs b/compiler/rustc_fluent_macro/src/fluent.rs index ead72e2a0e19a..d4604c27e6da3 100644 --- a/compiler/rustc_fluent_macro/src/fluent.rs +++ b/compiler/rustc_fluent_macro/src/fluent.rs @@ -8,6 +8,7 @@ use fluent_syntax::ast::{ Attribute, Entry, Expression, Identifier, InlineExpression, Message, Pattern, PatternElement, }; use fluent_syntax::parser::ParserError; +use proc_macro::tracked_path::path; use proc_macro::{Diagnostic, Level, Span}; use proc_macro2::TokenStream; use quote::quote; @@ -99,8 +100,7 @@ pub(crate) fn fluent_messages(input: proc_macro::TokenStream) -> proc_macro::Tok let crate_name = Ident::new(&crate_name, resource_str.span()); - // As this macro also outputs an `include_str!` for this file, the macro will always be - // re-executed when the file changes. + path(absolute_ftl_path.to_str().unwrap()); let resource_contents = match read_to_string(absolute_ftl_path) { Ok(resource_contents) => resource_contents, Err(e) => { diff --git a/compiler/rustc_fluent_macro/src/lib.rs b/compiler/rustc_fluent_macro/src/lib.rs index 6e5add24bcc4f..3ad51fa1e64d0 100644 --- a/compiler/rustc_fluent_macro/src/lib.rs +++ b/compiler/rustc_fluent_macro/src/lib.rs @@ -6,6 +6,7 @@ #![feature(proc_macro_diagnostic)] #![feature(proc_macro_span)] #![feature(rustdoc_internals)] +#![feature(track_path)] #![warn(unreachable_pub)] // tidy-alphabetical-end From ad2a649aab3e26f466f8fecd0f170e2e7e546af0 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 19 Oct 2024 10:36:56 -0400 Subject: [PATCH 10/13] Remove unnecessary constness --- .../src/hir_ty_lowering/bounds.rs | 1 - .../src/hir_ty_lowering/mod.rs | 60 +++++-------------- 2 files changed, 16 insertions(+), 45 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs index 8f7ca089c9184..6c7d521ebcfb5 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs @@ -516,7 +516,6 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { self_ty, trait_segment, false, - ty::BoundConstness::NotConst, ); // SUBTLE: As noted at the end of `try_append_return_type_notation_params` diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs index d760acf53bdd8..8ad9687b23338 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs @@ -336,14 +336,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { def_id: DefId, item_segment: &hir::PathSegment<'tcx>, ) -> GenericArgsRef<'tcx> { - let (args, _) = self.lower_generic_args_of_path( - span, - def_id, - &[], - item_segment, - None, - ty::BoundConstness::NotConst, - ); + let (args, _) = self.lower_generic_args_of_path(span, def_id, &[], item_segment, None); if let Some(c) = item_segment.args().constraints.first() { prohibit_assoc_item_constraint(self, c, Some((def_id, item_segment, span))); } @@ -392,7 +385,6 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { parent_args: &[ty::GenericArg<'tcx>], segment: &hir::PathSegment<'tcx>, self_ty: Option>, - constness: ty::BoundConstness, ) -> (GenericArgsRef<'tcx>, GenericArgCountResult) { // If the type is parameterized by this region, then replace this // region with the current anon region binding (in other words, @@ -415,7 +407,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { assert!(self_ty.is_none()); } - let mut arg_count = check_generic_arg_count( + let arg_count = check_generic_arg_count( self, def_id, segment, @@ -573,16 +565,6 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { } } } - if let ty::BoundConstness::Const | ty::BoundConstness::ConstIfConst = constness - && generics.has_self - && !tcx.is_const_trait(def_id) - { - let reported = self.dcx().emit_err(crate::errors::ConstBoundForNonConstTrait { - span, - modifier: constness.as_str(), - }); - arg_count.correct = Err(GenericArgCountMismatch { reported, invalid_args: vec![] }); - } let mut args_ctx = GenericArgsCtxt { lowerer: self, @@ -614,14 +596,8 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { parent_args: GenericArgsRef<'tcx>, ) -> GenericArgsRef<'tcx> { debug!(?span, ?item_def_id, ?item_segment); - let (args, _) = self.lower_generic_args_of_path( - span, - item_def_id, - parent_args, - item_segment, - None, - ty::BoundConstness::NotConst, - ); + let (args, _) = + self.lower_generic_args_of_path(span, item_def_id, parent_args, item_segment, None); if let Some(c) = item_segment.args().constraints.first() { prohibit_assoc_item_constraint(self, c, Some((item_def_id, item_segment, span))); } @@ -647,7 +623,6 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { self_ty, trait_ref.path.segments.last().unwrap(), true, - ty::BoundConstness::NotConst, ) } @@ -700,9 +675,17 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { &[], trait_segment, Some(self_ty), - constness, ); + if let ty::BoundConstness::Const | ty::BoundConstness::ConstIfConst = constness + && !self.tcx().is_const_trait(trait_def_id) + { + self.dcx().emit_err(crate::errors::ConstBoundForNonConstTrait { + span: trait_ref.path.span, + modifier: constness.as_str(), + }); + } + let tcx = self.tcx(); let bound_vars = tcx.late_bound_vars(trait_ref.hir_ref_id); debug!(?bound_vars); @@ -762,19 +745,11 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { self_ty: Ty<'tcx>, trait_segment: &hir::PathSegment<'tcx>, is_impl: bool, - // FIXME(effects): Move all host param things in HIR ty lowering to AST lowering. - constness: ty::BoundConstness, ) -> ty::TraitRef<'tcx> { self.complain_about_internal_fn_trait(span, trait_def_id, trait_segment, is_impl); - let (generic_args, _) = self.lower_generic_args_of_path( - span, - trait_def_id, - &[], - trait_segment, - Some(self_ty), - constness, - ); + let (generic_args, _) = + self.lower_generic_args_of_path(span, trait_def_id, &[], trait_segment, Some(self_ty)); if let Some(c) = trait_segment.args().constraints.first() { prohibit_assoc_item_constraint(self, c, Some((trait_def_id, trait_segment, span))); } @@ -1542,7 +1517,6 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { item_def_id: DefId, trait_segment: &hir::PathSegment<'tcx>, item_segment: &hir::PathSegment<'tcx>, - constness: ty::BoundConstness, ) -> Ty<'tcx> { let tcx = self.tcx(); @@ -1555,7 +1529,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { debug!(?self_ty); let trait_ref = - self.lower_mono_trait_ref(span, trait_def_id, self_ty, trait_segment, false, constness); + self.lower_mono_trait_ref(span, trait_def_id, self_ty, trait_segment, false); debug!(?trait_ref); let item_args = @@ -1918,7 +1892,6 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { def_id, &path.segments[path.segments.len() - 2], path.segments.last().unwrap(), - ty::BoundConstness::NotConst, ) } Res::PrimTy(prim_ty) => { @@ -2151,7 +2124,6 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { &[], &hir::PathSegment::invalid(), None, - ty::BoundConstness::NotConst, ); tcx.at(span).type_of(def_id).instantiate(tcx, args) } From d1fa49b2e66c343210c413b68ed57f150b7b89d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jalil=20David=20Salam=C3=A9=20Messina?= Date: Sat, 19 Oct 2024 17:48:06 +0200 Subject: [PATCH 11/13] feat(rustdoc-json-types): introduce rustc-hash feature This allows the public `rustdoc-types` crate to expose this feature easily and allows consumers of the crate to get the performance advantages from doing so. The reasoning for this was discussed on [Zulip][1] Changes: - Make `rustc-hash` optional but default to including it - Rename all occurrences of `FxHashMap` to `HashMap`. - Feature gate the import and rename the imported `FxHashMap` to `HashMap` - Introduce a type alias `FxHashMap` which resolves to the currently used `HashMap` (`rustc_hash::FxHashMap` or `std::collections::HashMap`) for use in `src/librustdoc`. [1]: https://rust-lang.zulipchat.com/#narrow/channel/266220-t-rustdoc/topic/rustc-hash.20and.20performance.20of.20rustdoc-types --- src/rustdoc-json-types/Cargo.toml | 5 ++++- src/rustdoc-json-types/lib.rs | 15 ++++++++++----- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/rustdoc-json-types/Cargo.toml b/src/rustdoc-json-types/Cargo.toml index d3548036d4c72..7f7cd3672b786 100644 --- a/src/rustdoc-json-types/Cargo.toml +++ b/src/rustdoc-json-types/Cargo.toml @@ -6,9 +6,12 @@ edition = "2021" [lib] path = "lib.rs" +[features] +default = ["rustc-hash"] + [dependencies] serde = { version = "1.0", features = ["derive"] } -rustc-hash = "1.1.0" +rustc-hash = { version = "1.1.0", optional = true } [dev-dependencies] serde_json = "1.0" diff --git a/src/rustdoc-json-types/lib.rs b/src/rustdoc-json-types/lib.rs index b0bedab495f7b..c4e142342a85e 100644 --- a/src/rustdoc-json-types/lib.rs +++ b/src/rustdoc-json-types/lib.rs @@ -3,11 +3,16 @@ //! These types are the public API exposed through the `--output-format json` flag. The [`Crate`] //! struct is the root of the JSON blob and all other items are contained within. +#[cfg(not(feature = "rustc-hash"))] +use std::collections::HashMap; use std::path::PathBuf; -pub use rustc_hash::FxHashMap; +#[cfg(feature = "rustc-hash")] +use rustc_hash::FxHashMap as HashMap; use serde::{Deserialize, Serialize}; +pub type FxHashMap = HashMap; // re-export for use in src/librustdoc + /// The version of JSON output that this crate represents. /// /// This integer is incremented with every breaking change to the API, @@ -30,11 +35,11 @@ pub struct Crate { pub includes_private: bool, /// A collection of all items in the local crate as well as some external traits and their /// items that are referenced locally. - pub index: FxHashMap, + pub index: HashMap, /// Maps IDs to fully qualified paths and other info helpful for generating links. - pub paths: FxHashMap, + pub paths: HashMap, /// Maps `crate_id` of items to a crate name and html_root_url if it exists. - pub external_crates: FxHashMap, + pub external_crates: HashMap, /// A single version number to be used in the future when making backwards incompatible changes /// to the JSON output. pub format_version: u32, @@ -95,7 +100,7 @@ pub struct Item { /// Some("") if there is some documentation but it is empty (EG `#[doc = ""]`). pub docs: Option, /// This mapping resolves [intra-doc links](https://github.com/rust-lang/rfcs/blob/master/text/1946-intra-rustdoc-links.md) from the docstring to their IDs - pub links: FxHashMap, + pub links: HashMap, /// Stringified versions of the attributes on this item (e.g. `"#[inline]"`) pub attrs: Vec, /// Information about the item’s deprecation, if present. From 9989b1b0d5a96de4af9ef81a626dad1e396b87d3 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 19 Oct 2024 11:13:11 -0400 Subject: [PATCH 12/13] Use PredicateFilter instead of OnlySelfBounds --- compiler/rustc_hir_analysis/src/bounds.rs | 15 +++- .../src/collect/predicates_of.rs | 40 +-------- .../src/hir_ty_lowering/bounds.rs | 83 +++++++++---------- .../src/hir_ty_lowering/dyn_compatibility.rs | 6 +- .../src/hir_ty_lowering/mod.rs | 12 ++- 5 files changed, 61 insertions(+), 95 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/bounds.rs b/compiler/rustc_hir_analysis/src/bounds.rs index 8947e7a22167b..7f4ab352ef20b 100644 --- a/compiler/rustc_hir_analysis/src/bounds.rs +++ b/compiler/rustc_hir_analysis/src/bounds.rs @@ -9,7 +9,7 @@ use rustc_middle::ty::{self, Ty, TyCtxt, Upcast}; use rustc_span::Span; use rustc_span::def_id::DefId; -use crate::hir_ty_lowering::OnlySelfBounds; +use crate::hir_ty_lowering::PredicateFilter; /// Collects together a list of type bounds. These lists of bounds occur in many places /// in Rust's syntax: @@ -52,7 +52,7 @@ impl<'tcx> Bounds<'tcx> { span: Span, polarity: ty::PredicatePolarity, constness: ty::BoundConstness, - only_self_bounds: OnlySelfBounds, + predicate_filter: PredicateFilter, ) { let clause = ( bound_trait_ref @@ -72,9 +72,18 @@ impl<'tcx> Bounds<'tcx> { // FIXME(effects): Lift this out of `push_trait_bound`, and move it somewhere else. // Perhaps moving this into `lower_poly_trait_ref`, just like we lower associated // type bounds. - if !tcx.features().effects || only_self_bounds.0 { + if !tcx.features().effects { return; } + match predicate_filter { + PredicateFilter::SelfOnly | PredicateFilter::SelfThatDefines(_) => { + return; + } + PredicateFilter::All | PredicateFilter::SelfAndAssociatedTypeBounds => { + // Ok. + } + } + // For `T: ~const Tr` or `T: const Tr`, we need to add an additional bound on the // associated type of `` and make sure that the effect is compatible. let compat_val = match (tcx.def_kind(defining_def_id), constness) { diff --git a/compiler/rustc_hir_analysis/src/collect/predicates_of.rs b/compiler/rustc_hir_analysis/src/collect/predicates_of.rs index a87b29b3093d2..421ba40aa88ce 100644 --- a/compiler/rustc_hir_analysis/src/collect/predicates_of.rs +++ b/compiler/rustc_hir_analysis/src/collect/predicates_of.rs @@ -16,7 +16,7 @@ use crate::bounds::Bounds; use crate::collect::ItemCtxt; use crate::constrained_generic_params as cgp; use crate::delegation::inherit_predicates_for_delegation_item; -use crate::hir_ty_lowering::{HirTyLowerer, OnlySelfBounds, PredicateFilter, RegionInferReason}; +use crate::hir_ty_lowering::{HirTyLowerer, PredicateFilter, RegionInferReason}; /// Returns a list of all type predicates (explicit and implicit) for the definition with /// ID `def_id`. This includes all predicates returned by `explicit_predicates_of`, plus @@ -270,7 +270,7 @@ fn gather_explicit_predicates_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Gen bound_pred.bounds.iter(), &mut bounds, bound_vars, - OnlySelfBounds(false), + PredicateFilter::All, ); predicates.extend(bounds.clauses(tcx)); effects_min_tys.extend(bounds.effects_min_tys()); @@ -825,20 +825,6 @@ impl<'tcx> ItemCtxt<'tcx> { continue; }; - // Subtle: If we're collecting `SelfAndAssociatedTypeBounds`, then we - // want to only consider predicates with `Self: ...`, but we don't want - // `OnlySelfBounds(true)` since we want to collect the nested associated - // type bound as well. - let (only_self_bounds, assoc_name) = match filter { - PredicateFilter::All | PredicateFilter::SelfAndAssociatedTypeBounds => { - (OnlySelfBounds(false), None) - } - PredicateFilter::SelfOnly => (OnlySelfBounds(true), None), - PredicateFilter::SelfThatDefines(assoc_name) => { - (OnlySelfBounds(true), Some(assoc_name)) - } - }; - let bound_ty = if predicate.is_param_bound(param_def_id.to_def_id()) { ty } else if matches!(filter, PredicateFilter::All) { @@ -850,31 +836,13 @@ impl<'tcx> ItemCtxt<'tcx> { let bound_vars = self.tcx.late_bound_vars(predicate.hir_id); self.lowerer().lower_poly_bounds( bound_ty, - predicate.bounds.iter().filter(|bound| { - assoc_name - .map_or(true, |assoc_name| self.bound_defines_assoc_item(bound, assoc_name)) - }), + predicate.bounds.iter(), &mut bounds, bound_vars, - only_self_bounds, + filter, ); } bounds.clauses(self.tcx).collect() } - - #[instrument(level = "trace", skip(self))] - fn bound_defines_assoc_item(&self, b: &hir::GenericBound<'_>, assoc_name: Ident) -> bool { - match b { - hir::GenericBound::Trait(poly_trait_ref) => { - let trait_ref = &poly_trait_ref.trait_ref; - if let Some(trait_did) = trait_ref.trait_def_id() { - self.tcx.trait_may_define_assoc_item(trait_did, assoc_name) - } else { - false - } - } - _ => false, - } - } } diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs index 8f7ca089c9184..171acbcef86d0 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs @@ -19,9 +19,7 @@ use tracing::{debug, instrument}; use super::errors::GenericsArgsErrExtend; use crate::bounds::Bounds; use crate::errors; -use crate::hir_ty_lowering::{ - AssocItemQSelf, HirTyLowerer, OnlySelfBounds, PredicateFilter, RegionInferReason, -}; +use crate::hir_ty_lowering::{AssocItemQSelf, HirTyLowerer, PredicateFilter, RegionInferReason}; impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { /// Add a `Sized` bound to the `bounds` if appropriate. @@ -150,11 +148,25 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { hir_bounds: I, bounds: &mut Bounds<'tcx>, bound_vars: &'tcx ty::List, - only_self_bounds: OnlySelfBounds, + predicate_filter: PredicateFilter, ) where 'tcx: 'hir, { for hir_bound in hir_bounds { + // In order to avoid cycles, when we're lowering `SelfThatDefines`, + // we skip over any traits that don't define the given associated type. + + if let PredicateFilter::SelfThatDefines(assoc_name) = predicate_filter { + if let Some(trait_ref) = hir_bound.trait_ref() + && let Some(trait_did) = trait_ref.trait_def_id() + && self.tcx().trait_may_define_assoc_item(trait_did, assoc_name) + { + // Okay + } else { + continue; + } + } + match hir_bound { hir::GenericBound::Trait(poly_trait_ref) => { let (constness, polarity) = match poly_trait_ref.modifiers { @@ -179,7 +191,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { polarity, param_ty, bounds, - only_self_bounds, + predicate_filter, ); } hir::GenericBound::Outlives(lifetime) => { @@ -213,37 +225,16 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { &self, param_ty: Ty<'tcx>, hir_bounds: &[hir::GenericBound<'tcx>], - filter: PredicateFilter, + predicate_filter: PredicateFilter, ) -> Bounds<'tcx> { let mut bounds = Bounds::default(); - let only_self_bounds = match filter { - PredicateFilter::All | PredicateFilter::SelfAndAssociatedTypeBounds => { - OnlySelfBounds(false) - } - PredicateFilter::SelfOnly | PredicateFilter::SelfThatDefines(_) => OnlySelfBounds(true), - }; - self.lower_poly_bounds( param_ty, - hir_bounds.iter().filter(|bound| match filter { - PredicateFilter::All - | PredicateFilter::SelfOnly - | PredicateFilter::SelfAndAssociatedTypeBounds => true, - PredicateFilter::SelfThatDefines(assoc_name) => { - if let Some(trait_ref) = bound.trait_ref() - && let Some(trait_did) = trait_ref.trait_def_id() - && self.tcx().trait_may_define_assoc_item(trait_did, assoc_name) - { - true - } else { - false - } - } - }), + hir_bounds.iter(), &mut bounds, ty::List::empty(), - only_self_bounds, + predicate_filter, ); debug!(?bounds); @@ -267,7 +258,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { bounds: &mut Bounds<'tcx>, duplicates: &mut FxIndexMap, path_span: Span, - only_self_bounds: OnlySelfBounds, + predicate_filter: PredicateFilter, ) -> Result<(), ErrorGuaranteed> { let tcx = self.tcx(); @@ -444,21 +435,23 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { // Lower a constraint like `Item: Debug` as found in HIR bound `T: Iterator` // to a bound involving a projection: `::Item: Debug`. hir::AssocItemConstraintKind::Bound { bounds: hir_bounds } => { - // NOTE: If `only_self_bounds` is true, do NOT expand this associated type bound into - // a trait predicate, since we only want to add predicates for the `Self` type. - if !only_self_bounds.0 { - let projection_ty = projection_term - .map_bound(|projection_term| projection_term.expect_ty(self.tcx())); - // Calling `skip_binder` is okay, because `lower_bounds` expects the `param_ty` - // parameter to have a skipped binder. - let param_ty = Ty::new_alias(tcx, ty::Projection, projection_ty.skip_binder()); - self.lower_poly_bounds( - param_ty, - hir_bounds.iter(), - bounds, - projection_ty.bound_vars(), - only_self_bounds, - ); + match predicate_filter { + PredicateFilter::SelfOnly | PredicateFilter::SelfThatDefines(_) => {} + PredicateFilter::All | PredicateFilter::SelfAndAssociatedTypeBounds => { + let projection_ty = projection_term + .map_bound(|projection_term| projection_term.expect_ty(self.tcx())); + // Calling `skip_binder` is okay, because `lower_bounds` expects the `param_ty` + // parameter to have a skipped binder. + let param_ty = + Ty::new_alias(tcx, ty::Projection, projection_ty.skip_binder()); + self.lower_poly_bounds( + param_ty, + hir_bounds.iter(), + bounds, + projection_ty.bound_vars(), + predicate_filter, + ); + } } } } diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs index 98822eec2ac4d..2cf97e290605b 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs @@ -20,7 +20,7 @@ use tracing::{debug, instrument}; use super::HirTyLowerer; use crate::bounds::Bounds; use crate::hir_ty_lowering::{ - GenericArgCountMismatch, GenericArgCountResult, OnlySelfBounds, RegionInferReason, + GenericArgCountMismatch, GenericArgCountResult, PredicateFilter, RegionInferReason, }; impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { @@ -55,9 +55,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { ty::PredicatePolarity::Positive, dummy_self, &mut bounds, - // True so we don't populate `bounds` with associated type bounds, even - // though they're disallowed from object types. - OnlySelfBounds(true), + PredicateFilter::SelfOnly, ) { potential_assoc_types.extend(cur_potential_assoc_types); } diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs index d760acf53bdd8..f5d9d186f1348 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs @@ -64,9 +64,6 @@ use crate::require_c_abi_if_c_variadic; #[derive(Debug)] pub struct GenericPathSegment(pub DefId, pub usize); -#[derive(Copy, Clone, Debug)] -pub struct OnlySelfBounds(pub bool); - #[derive(Copy, Clone, Debug)] pub enum PredicateFilter { /// All predicates may be implied by the trait. @@ -76,7 +73,8 @@ pub enum PredicateFilter { SelfOnly, /// Only traits that reference `Self: ..` and define an associated type - /// with the given ident are implied by the trait. + /// with the given ident are implied by the trait. This mode exists to + /// side-step query cycles when lowering associated types. SelfThatDefines(Ident), /// Only traits that reference `Self: ..` and their associated type bounds. @@ -683,7 +681,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { polarity: ty::PredicatePolarity, self_ty: Ty<'tcx>, bounds: &mut Bounds<'tcx>, - only_self_bounds: OnlySelfBounds, + predicate_filter: PredicateFilter, ) -> GenericArgCountResult { let trait_def_id = trait_ref.trait_def_id().unwrap_or_else(|| FatalError.raise()); let trait_segment = trait_ref.path.segments.last().unwrap(); @@ -720,7 +718,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { span, polarity, constness, - only_self_bounds, + predicate_filter, ); let mut dup_constraints = FxIndexMap::default(); @@ -744,7 +742,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { bounds, &mut dup_constraints, constraint.span, - only_self_bounds, + predicate_filter, ); // Okay to ignore `Err` because of `ErrorGuaranteed` (see above). } From 70746d078e0a3dbef1ea96dc94c927c39ab97eab Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 16 Oct 2024 12:17:21 -0400 Subject: [PATCH 13/13] Make sure that outer opaques capture inner opaques's lifetimes even with precise capturing syntax --- compiler/rustc_ast_lowering/src/item.rs | 2 +- compiler/rustc_ast_lowering/src/lib.rs | 79 +++++-------------- .../src/lifetime_collector.rs | 53 ++++++++++--- .../precise-capturing/capturing-implicit.rs | 15 ++++ .../capturing-implicit.stderr | 22 ++++++ 5 files changed, 102 insertions(+), 69 deletions(-) create mode 100644 tests/ui/impl-trait/precise-capturing/capturing-implicit.rs create mode 100644 tests/ui/impl-trait/precise-capturing/capturing-implicit.stderr diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index ce744cc56e1ab..7416a1e39eb53 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -1574,7 +1574,7 @@ impl<'hir> LoweringContext<'_, 'hir> { .collect(); // Introduce extra lifetimes if late resolution tells us to. - let extra_lifetimes = self.resolver.take_extra_lifetime_params(parent_node_id); + let extra_lifetimes = self.resolver.extra_lifetime_params(parent_node_id); params.extend(extra_lifetimes.into_iter().filter_map(|(ident, node_id, res)| { self.lifetime_res_to_generic_param( ident, diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index 00eafeb4d844d..4d8d22e09d922 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -268,8 +268,8 @@ impl ResolverAstLowering { /// /// The extra lifetimes that appear from the parenthesized `Fn`-trait desugaring /// should appear at the enclosing `PolyTraitRef`. - fn take_extra_lifetime_params(&mut self, id: NodeId) -> Vec<(Ident, NodeId, LifetimeRes)> { - self.extra_lifetime_params_map.remove(&id).unwrap_or_default() + fn extra_lifetime_params(&mut self, id: NodeId) -> Vec<(Ident, NodeId, LifetimeRes)> { + self.extra_lifetime_params_map.get(&id).cloned().unwrap_or_default() } } @@ -885,7 +885,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { let mut generic_params: Vec<_> = self .lower_generic_params_mut(generic_params, hir::GenericParamSource::Binder) .collect(); - let extra_lifetimes = self.resolver.take_extra_lifetime_params(binder); + let extra_lifetimes = self.resolver.extra_lifetime_params(binder); debug!(?extra_lifetimes); generic_params.extend(extra_lifetimes.into_iter().filter_map(|(ident, node_id, res)| { self.lifetime_res_to_generic_param(ident, node_id, res, hir::GenericParamSource::Binder) @@ -1495,62 +1495,25 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { // frequently opened issues show. let opaque_ty_span = self.mark_span_with_reason(DesugaringKind::OpaqueTy, span, None); - let captured_lifetimes_to_duplicate = if let Some(args) = - // We only look for one `use<...>` syntax since we syntactially reject more than one. - bounds.iter().find_map( - |bound| match bound { - ast::GenericBound::Use(a, _) => Some(a), - _ => None, - }, - ) { - // We'll actually validate these later on; all we need is the list of - // lifetimes to duplicate during this portion of lowering. - args.iter() - .filter_map(|arg| match arg { - PreciseCapturingArg::Lifetime(lt) => Some(*lt), - PreciseCapturingArg::Arg(..) => None, - }) - // Add in all the lifetimes mentioned in the bounds. We will error - // them out later, but capturing them here is important to make sure - // they actually get resolved in resolve_bound_vars. - .chain(lifetime_collector::lifetimes_in_bounds(self.resolver, bounds)) - .collect() - } else { - match origin { - hir::OpaqueTyOrigin::TyAlias { .. } => { - // type alias impl trait and associated type position impl trait were - // decided to capture all in-scope lifetimes, which we collect for - // all opaques during resolution. - self.resolver - .take_extra_lifetime_params(opaque_ty_node_id) - .into_iter() - .map(|(ident, id, _)| Lifetime { id, ident }) - .collect() - } - hir::OpaqueTyOrigin::FnReturn { in_trait_or_impl, .. } => { - if in_trait_or_impl.is_some() - || self.tcx.features().lifetime_capture_rules_2024 - || span.at_least_rust_2024() - { - // return-position impl trait in trait was decided to capture all - // in-scope lifetimes, which we collect for all opaques during resolution. - self.resolver - .take_extra_lifetime_params(opaque_ty_node_id) - .into_iter() - .map(|(ident, id, _)| Lifetime { id, ident }) - .collect() - } else { - // in fn return position, like the `fn test<'a>() -> impl Debug + 'a` - // example, we only need to duplicate lifetimes that appear in the - // bounds, since those are the only ones that are captured by the opaque. - lifetime_collector::lifetimes_in_bounds(self.resolver, bounds) - } - } - hir::OpaqueTyOrigin::AsyncFn { .. } => { - unreachable!("should be using `lower_async_fn_ret_ty`") - } + // Whether this opaque always captures lifetimes in scope. + // Right now, this is all RPITIT and TAITs, and when `lifetime_capture_rules_2024` + // is enabled. We don't check the span of the edition, since this is done + // on a per-opaque basis to account for nested opaques. + let always_capture_in_scope = match origin { + _ if self.tcx.features().lifetime_capture_rules_2024 => true, + hir::OpaqueTyOrigin::TyAlias { .. } => true, + hir::OpaqueTyOrigin::FnReturn { in_trait_or_impl, .. } => in_trait_or_impl.is_some(), + hir::OpaqueTyOrigin::AsyncFn { .. } => { + unreachable!("should be using `lower_coroutine_fn_ret_ty`") } }; + let captured_lifetimes_to_duplicate = lifetime_collector::lifetimes_for_opaque( + self.resolver, + always_capture_in_scope, + opaque_ty_node_id, + bounds, + span, + ); debug!(?captured_lifetimes_to_duplicate); // Feature gate for RPITIT + use<..> @@ -1920,7 +1883,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { let captured_lifetimes = self .resolver - .take_extra_lifetime_params(opaque_ty_node_id) + .extra_lifetime_params(opaque_ty_node_id) .into_iter() .map(|(ident, id, _)| Lifetime { id, ident }) .collect(); diff --git a/compiler/rustc_ast_lowering/src/lifetime_collector.rs b/compiler/rustc_ast_lowering/src/lifetime_collector.rs index fe64160fb4dcf..8d47c856bdd24 100644 --- a/compiler/rustc_ast_lowering/src/lifetime_collector.rs +++ b/compiler/rustc_ast_lowering/src/lifetime_collector.rs @@ -1,5 +1,7 @@ use rustc_ast::visit::{self, BoundKind, LifetimeCtxt, Visitor}; -use rustc_ast::{GenericBounds, Lifetime, NodeId, PathSegment, PolyTraitRef, Ty, TyKind}; +use rustc_ast::{ + GenericBound, GenericBounds, Lifetime, NodeId, PathSegment, PolyTraitRef, Ty, TyKind, +}; use rustc_data_structures::fx::FxIndexSet; use rustc_hir::def::{DefKind, LifetimeRes, Res}; use rustc_middle::span_bug; @@ -10,14 +12,41 @@ use rustc_span::symbol::{Ident, kw}; use super::ResolverAstLoweringExt; struct LifetimeCollectVisitor<'ast> { - resolver: &'ast ResolverAstLowering, + resolver: &'ast mut ResolverAstLowering, + always_capture_in_scope: bool, current_binders: Vec, collected_lifetimes: FxIndexSet, } impl<'ast> LifetimeCollectVisitor<'ast> { - fn new(resolver: &'ast ResolverAstLowering) -> Self { - Self { resolver, current_binders: Vec::new(), collected_lifetimes: FxIndexSet::default() } + fn new(resolver: &'ast mut ResolverAstLowering, always_capture_in_scope: bool) -> Self { + Self { + resolver, + always_capture_in_scope, + current_binders: Vec::new(), + collected_lifetimes: FxIndexSet::default(), + } + } + + fn visit_opaque(&mut self, opaque_ty_node_id: NodeId, bounds: &'ast GenericBounds, span: Span) { + // If we're edition 2024 or within a TAIT or RPITIT, *and* there is no + // `use<>` statement to override the default capture behavior, then + // capture all of the in-scope lifetimes. + if (self.always_capture_in_scope || span.at_least_rust_2024()) + && bounds.iter().all(|bound| !matches!(bound, GenericBound::Use(..))) + { + for (ident, id, _) in self.resolver.extra_lifetime_params(opaque_ty_node_id) { + self.record_lifetime_use(Lifetime { id, ident }); + } + } + + // We also recurse on the bounds to make sure we capture all the lifetimes + // mentioned in the bounds. These may disagree with the `use<>` list, in which + // case we will error on these later. We will also recurse to visit any + // nested opaques, which may *implicitly* capture lifetimes. + for bound in bounds { + self.visit_param_bound(bound, BoundKind::Bound); + } } fn record_lifetime_use(&mut self, lifetime: Lifetime) { @@ -99,6 +128,9 @@ impl<'ast> Visitor<'ast> for LifetimeCollectVisitor<'ast> { self.record_elided_anchor(t.id, t.span); visit::walk_ty(self, t); } + TyKind::ImplTrait(opaque_ty_node_id, bounds) => { + self.visit_opaque(*opaque_ty_node_id, bounds, t.span) + } _ => { visit::walk_ty(self, t); } @@ -106,13 +138,14 @@ impl<'ast> Visitor<'ast> for LifetimeCollectVisitor<'ast> { } } -pub(crate) fn lifetimes_in_bounds( - resolver: &ResolverAstLowering, +pub(crate) fn lifetimes_for_opaque( + resolver: &mut ResolverAstLowering, + always_capture_in_scope: bool, + opaque_ty_node_id: NodeId, bounds: &GenericBounds, + span: Span, ) -> FxIndexSet { - let mut visitor = LifetimeCollectVisitor::new(resolver); - for bound in bounds { - visitor.visit_param_bound(bound, BoundKind::Bound); - } + let mut visitor = LifetimeCollectVisitor::new(resolver, always_capture_in_scope); + visitor.visit_opaque(opaque_ty_node_id, bounds, span); visitor.collected_lifetimes } diff --git a/tests/ui/impl-trait/precise-capturing/capturing-implicit.rs b/tests/ui/impl-trait/precise-capturing/capturing-implicit.rs new file mode 100644 index 0000000000000..5ef8542d862e7 --- /dev/null +++ b/tests/ui/impl-trait/precise-capturing/capturing-implicit.rs @@ -0,0 +1,15 @@ +//@ edition: 2024 +//@ compile-flags: -Zunstable-options + +#![feature(rustc_attrs)] +#![feature(type_alias_impl_trait)] +#![rustc_variance_of_opaques] + +fn foo(x: &()) -> impl IntoIterator + use<> { + //~^ ERROR ['_: o] + //~| ERROR ['_: o] + //~| ERROR `impl Trait` captures lifetime parameter + [*x] +} + +fn main() {} diff --git a/tests/ui/impl-trait/precise-capturing/capturing-implicit.stderr b/tests/ui/impl-trait/precise-capturing/capturing-implicit.stderr new file mode 100644 index 0000000000000..b14ed20bd367c --- /dev/null +++ b/tests/ui/impl-trait/precise-capturing/capturing-implicit.stderr @@ -0,0 +1,22 @@ +error: `impl Trait` captures lifetime parameter, but it is not mentioned in `use<...>` precise captures list + --> $DIR/capturing-implicit.rs:8:11 + | +LL | fn foo(x: &()) -> impl IntoIterator + use<> { + | ^ -------------------------------------------- lifetime captured due to being mentioned in the bounds of the `impl Trait` + | | + | this lifetime parameter is captured + +error: ['_: o] + --> $DIR/capturing-implicit.rs:8:19 + | +LL | fn foo(x: &()) -> impl IntoIterator + use<> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: ['_: o] + --> $DIR/capturing-implicit.rs:8:44 + | +LL | fn foo(x: &()) -> impl IntoIterator + use<> { + | ^^^^^^^^^^ + +error: aborting due to 3 previous errors +