From 91b117bdc74e29c0f046c4eff623d8ed89414ac9 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 20 Aug 2024 11:35:05 -0400 Subject: [PATCH 1/4] Use equality when relating formal and expected type in arg checking (cherry picked from commit c3f9c4f4d4bbc83c7de79a09c7ec0e7fda8efc5e) --- .../rustc_hir_typeck/src/fn_ctxt/checks.rs | 9 ++++----- .../coercion/constrain-expectation-in-arg.rs | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 5 deletions(-) create mode 100644 tests/ui/coercion/constrain-expectation-in-arg.rs diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs index eebb0217990df..16d65726128c3 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs @@ -292,21 +292,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let coerce_error = self.coerce(provided_arg, checked_ty, coerced_ty, AllowTwoPhase::Yes, None).err(); - if coerce_error.is_some() { return Compatibility::Incompatible(coerce_error); } - // 3. Check if the formal type is a supertype of the checked one - // and register any such obligations for future type checks - let supertype_error = self.at(&self.misc(provided_arg.span), self.param_env).sup( + // 3. Check if the formal type is actually equal to the checked one + // and register any such obligations for future type checks. + let formal_ty_error = self.at(&self.misc(provided_arg.span), self.param_env).eq( DefineOpaqueTypes::Yes, formal_input_ty, coerced_ty, ); // If neither check failed, the types are compatible - match supertype_error { + match formal_ty_error { Ok(InferOk { obligations, value: () }) => { self.register_predicates(obligations); Compatibility::Compatible diff --git a/tests/ui/coercion/constrain-expectation-in-arg.rs b/tests/ui/coercion/constrain-expectation-in-arg.rs new file mode 100644 index 0000000000000..858c3a0bdb572 --- /dev/null +++ b/tests/ui/coercion/constrain-expectation-in-arg.rs @@ -0,0 +1,19 @@ +//@ check-pass + +trait Trait { + type Item; +} + +struct Struct, B> { + pub field: A, +} + +fn identity(x: T) -> T { + x +} + +fn test, B>(x: &Struct) { + let x: &Struct<_, _> = identity(x); +} + +fn main() {} From 824971f5ba6f5fed99a107ea61cec479aaa541a9 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 25 Aug 2024 12:45:58 -0400 Subject: [PATCH 2/4] Inline expected_inputs_for_expected_output into check_argument_types/check_expr_struct_fields (cherry picked from commit 95b9ecd6d671637e9e3db55ed31d06882d3cad4d) --- compiler/rustc_hir_typeck/src/callee.rs | 21 ++----- compiler/rustc_hir_typeck/src/expr.rs | 25 +++++--- .../rustc_hir_typeck/src/fn_ctxt/_impl.rs | 39 +------------ .../rustc_hir_typeck/src/fn_ctxt/checks.rs | 58 ++++++++++++++----- .../coercion/constrain-expectation-in-arg.rs | 5 ++ 5 files changed, 71 insertions(+), 77 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/callee.rs b/compiler/rustc_hir_typeck/src/callee.rs index a4eec5f05a8ff..9863d0364498e 100644 --- a/compiler/rustc_hir_typeck/src/callee.rs +++ b/compiler/rustc_hir_typeck/src/callee.rs @@ -503,18 +503,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let fn_sig = self.instantiate_binder_with_fresh_vars(call_expr.span, infer::FnCall, fn_sig); let fn_sig = self.normalize(call_expr.span, fn_sig); - // Call the generic checker. - let expected_arg_tys = self.expected_inputs_for_expected_output( - call_expr.span, - expected, - fn_sig.output(), - fn_sig.inputs(), - ); self.check_argument_types( call_expr.span, call_expr, fn_sig.inputs(), - expected_arg_tys, + fn_sig.output(), + expected, arg_exprs, fn_sig.c_variadic, TupleArgumentsFlag::DontTupleArguments, @@ -866,19 +860,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // don't know the full details yet (`Fn` vs `FnMut` etc), but we // do know the types expected for each argument and the return // type. - - let expected_arg_tys = self.expected_inputs_for_expected_output( - call_expr.span, - expected, - fn_sig.output(), - fn_sig.inputs(), - ); - self.check_argument_types( call_expr.span, call_expr, fn_sig.inputs(), - expected_arg_tys, + fn_sig.output(), + expected, arg_exprs, fn_sig.c_variadic, TupleArgumentsFlag::TupleArguments, diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index 1362d3626efd4..f0d47e584ac28 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -1673,15 +1673,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ) { let tcx = self.tcx; - let expected_inputs = - self.expected_inputs_for_expected_output(span, expected, adt_ty, &[adt_ty]); - let adt_ty_hint = if let Some(expected_inputs) = expected_inputs { - expected_inputs.get(0).cloned().unwrap_or(adt_ty) - } else { - adt_ty - }; - // re-link the regions that EIfEO can erase. - self.demand_eqtype(span, adt_ty_hint, adt_ty); + let adt_ty = self.resolve_vars_with_obligations(adt_ty); + let adt_ty_hint = expected.only_has_type(self).and_then(|expected| { + self.fudge_inference_if_ok(|| { + let ocx = ObligationCtxt::new(self); + ocx.sup(&self.misc(span), self.param_env, expected, adt_ty)?; + if !ocx.select_where_possible().is_empty() { + return Err(TypeError::Mismatch); + } + Ok(self.resolve_vars_if_possible(adt_ty)) + }) + .ok() + }); + if let Some(adt_ty_hint) = adt_ty_hint { + // re-link the variables that the fudging above can create. + self.demand_eqtype(span, adt_ty_hint, adt_ty); + } let ty::Adt(adt, args) = adt_ty.kind() else { span_bug!(span, "non-ADT passed to check_expr_struct_fields"); diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs index 97c27680959f0..19f7950287f93 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs @@ -20,7 +20,6 @@ use rustc_infer::infer::canonical::{Canonical, OriginalQueryValues, QueryRespons use rustc_infer::infer::{DefineOpaqueTypes, InferResult}; use rustc_lint::builtin::SELF_CONSTRUCTOR_FROM_OUTER_ITEM; use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability}; -use rustc_middle::ty::error::TypeError; use rustc_middle::ty::fold::TypeFoldable; use rustc_middle::ty::visit::{TypeVisitable, TypeVisitableExt}; use rustc_middle::ty::{ @@ -36,7 +35,7 @@ use rustc_span::Span; use rustc_target::abi::FieldIdx; use rustc_trait_selection::error_reporting::infer::need_type_info::TypeAnnotationNeeded; use rustc_trait_selection::traits::{ - self, NormalizeExt, ObligationCauseCode, ObligationCtxt, StructurallyNormalizeExt, + self, NormalizeExt, ObligationCauseCode, StructurallyNormalizeExt, }; use tracing::{debug, instrument}; @@ -689,42 +688,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { vec![ty_error; len] } - /// Unifies the output type with the expected type early, for more coercions - /// and forward type information on the input expressions. - #[instrument(skip(self, call_span), level = "debug")] - pub(crate) fn expected_inputs_for_expected_output( - &self, - call_span: Span, - expected_ret: Expectation<'tcx>, - formal_ret: Ty<'tcx>, - formal_args: &[Ty<'tcx>], - ) -> Option>> { - let formal_ret = self.resolve_vars_with_obligations(formal_ret); - let ret_ty = expected_ret.only_has_type(self)?; - - let expect_args = self - .fudge_inference_if_ok(|| { - let ocx = ObligationCtxt::new(self); - - // Attempt to apply a subtyping relationship between the formal - // return type (likely containing type variables if the function - // is polymorphic) and the expected return type. - // No argument expectations are produced if unification fails. - let origin = self.misc(call_span); - ocx.sup(&origin, self.param_env, ret_ty, formal_ret)?; - if !ocx.select_where_possible().is_empty() { - return Err(TypeError::Mismatch); - } - - // Record all the argument types, with the args - // produced from the above subtyping unification. - Ok(Some(formal_args.iter().map(|&ty| self.resolve_vars_if_possible(ty)).collect())) - }) - .unwrap_or_default(); - debug!(?formal_args, ?formal_ret, ?expect_args, ?expected_ret); - expect_args - } - pub(crate) fn resolve_lang_item_path( &self, lang_item: hir::LangItem, diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs index 16d65726128c3..bdf84f332166d 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs @@ -17,6 +17,7 @@ use rustc_hir_analysis::hir_ty_lowering::HirTyLowerer; use rustc_index::IndexVec; use rustc_infer::infer::{DefineOpaqueTypes, InferOk, TypeTrace}; use rustc_middle::ty::adjustment::AllowTwoPhase; +use rustc_middle::ty::error::TypeError; use rustc_middle::ty::visit::TypeVisitableExt; use rustc_middle::ty::{self, IsSuggestable, Ty, TyCtxt}; use rustc_middle::{bug, span_bug}; @@ -25,7 +26,7 @@ use rustc_span::symbol::{kw, Ident}; use rustc_span::{sym, Span, DUMMY_SP}; use rustc_trait_selection::error_reporting::infer::{FailureCode, ObligationCauseExt}; use rustc_trait_selection::infer::InferCtxtExt; -use rustc_trait_selection::traits::{self, ObligationCauseCode, SelectionContext}; +use rustc_trait_selection::traits::{self, ObligationCauseCode, ObligationCtxt, SelectionContext}; use tracing::debug; use {rustc_ast as ast, rustc_hir as hir}; @@ -124,6 +125,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { }; if let Err(guar) = has_error { let err_inputs = self.err_args(args_no_rcvr.len(), guar); + let err_output = Ty::new_error(self.tcx, guar); let err_inputs = match tuple_arguments { DontTupleArguments => err_inputs, @@ -134,28 +136,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { sp, expr, &err_inputs, - None, + err_output, + NoExpectation, args_no_rcvr, false, tuple_arguments, method.ok().map(|method| method.def_id), ); - return Ty::new_error(self.tcx, guar); + return err_output; } let method = method.unwrap(); - // HACK(eddyb) ignore self in the definition (see above). - let expected_input_tys = self.expected_inputs_for_expected_output( - sp, - expected, - method.sig.output(), - &method.sig.inputs()[1..], - ); self.check_argument_types( sp, expr, &method.sig.inputs()[1..], - expected_input_tys, + method.sig.output(), + expected, args_no_rcvr, method.sig.c_variadic, tuple_arguments, @@ -175,8 +172,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { call_expr: &'tcx hir::Expr<'tcx>, // Types (as defined in the *signature* of the target function) formal_input_tys: &[Ty<'tcx>], - // More specific expected types, after unifying with caller output types - expected_input_tys: Option>>, + formal_output: Ty<'tcx>, + // Expected output from the parent expression or statement + expectation: Expectation<'tcx>, // The expressions for each provided argument provided_args: &'tcx [hir::Expr<'tcx>], // Whether the function is variadic, for example when imported from C @@ -210,6 +208,40 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); } + // First, let's unify the formal method signature with the expectation eagerly. + // We use this to guide coercion inference; it's output is "fudged" which means + // any remaining type variables are assigned to new, unrelated variables. This + // is because the inference guidance here is only speculative. + let formal_output = self.resolve_vars_with_obligations(formal_output); + let expected_input_tys: Option> = expectation + .only_has_type(self) + .and_then(|expected_output| { + self.fudge_inference_if_ok(|| { + let ocx = ObligationCtxt::new(self); + + // Attempt to apply a subtyping relationship between the formal + // return type (likely containing type variables if the function + // is polymorphic) and the expected return type. + // No argument expectations are produced if unification fails. + let origin = self.misc(call_span); + ocx.sup(&origin, self.param_env, expected_output, formal_output)?; + if !ocx.select_where_possible().is_empty() { + return Err(TypeError::Mismatch); + } + + // Record all the argument types, with the args + // produced from the above subtyping unification. + Ok(Some( + formal_input_tys + .iter() + .map(|&ty| self.resolve_vars_if_possible(ty)) + .collect(), + )) + }) + .ok() + }) + .unwrap_or_default(); + let mut err_code = E0061; // If the arguments should be wrapped in a tuple (ex: closures), unwrap them here diff --git a/tests/ui/coercion/constrain-expectation-in-arg.rs b/tests/ui/coercion/constrain-expectation-in-arg.rs index 858c3a0bdb572..c515dedc4bb4d 100644 --- a/tests/ui/coercion/constrain-expectation-in-arg.rs +++ b/tests/ui/coercion/constrain-expectation-in-arg.rs @@ -1,5 +1,10 @@ //@ check-pass +// Regression test for for #129286. +// Makes sure that we don't have unconstrained type variables that come from +// bivariant type parameters due to the way that we construct expectation types +// when checking call expressions in HIR typeck. + trait Trait { type Item; } From 3cb89a05adcd26618a2705170279637dcf1db3a7 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 12 Sep 2024 09:07:44 -0400 Subject: [PATCH 3/4] Don't call extern_crate when local crate name is the same as a dependency and we have a trait error (cherry picked from commit 9d5d03b7dec8a87c81528e9a1310313fa3f0111e) --- .../traits/fulfillment_errors.rs | 1 + .../foreign_struct_trait_unimplemented.rs | 1 + .../foreign_struct_trait_unimplemented.rs | 15 ++++++++++++ .../foreign_struct_trait_unimplemented.stderr | 23 +++++++++++++++++++ 4 files changed, 40 insertions(+) create mode 100644 tests/ui/typeck/auxiliary/foreign_struct_trait_unimplemented.rs create mode 100644 tests/ui/typeck/foreign_struct_trait_unimplemented.rs create mode 100644 tests/ui/typeck/foreign_struct_trait_unimplemented.stderr 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 3fdfca50dceba..91b197b92b1b3 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 @@ -1671,6 +1671,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { let name = self.tcx.crate_name(trait_def_id.krate); let spans: Vec<_> = [trait_def_id, found_type] .into_iter() + .filter(|def_id| def_id.krate != LOCAL_CRATE) .filter_map(|def_id| self.tcx.extern_crate(def_id.krate)) .map(|data| { let dependency = if data.dependency_of == LOCAL_CRATE { diff --git a/tests/ui/typeck/auxiliary/foreign_struct_trait_unimplemented.rs b/tests/ui/typeck/auxiliary/foreign_struct_trait_unimplemented.rs new file mode 100644 index 0000000000000..097a269e8ee6b --- /dev/null +++ b/tests/ui/typeck/auxiliary/foreign_struct_trait_unimplemented.rs @@ -0,0 +1 @@ +pub struct B; diff --git a/tests/ui/typeck/foreign_struct_trait_unimplemented.rs b/tests/ui/typeck/foreign_struct_trait_unimplemented.rs new file mode 100644 index 0000000000000..8ac56bac9b0a6 --- /dev/null +++ b/tests/ui/typeck/foreign_struct_trait_unimplemented.rs @@ -0,0 +1,15 @@ +//@ aux-build:foreign_struct_trait_unimplemented.rs + +extern crate foreign_struct_trait_unimplemented; + +pub trait Test {} + +struct A; +impl Test for A {} + +fn needs_test(_: impl Test) {} + +fn main() { + needs_test(foreign_struct_trait_unimplemented::B); + //~^ ERROR the trait bound `B: Test` is not satisfied +} diff --git a/tests/ui/typeck/foreign_struct_trait_unimplemented.stderr b/tests/ui/typeck/foreign_struct_trait_unimplemented.stderr new file mode 100644 index 0000000000000..b9bb97548f67c --- /dev/null +++ b/tests/ui/typeck/foreign_struct_trait_unimplemented.stderr @@ -0,0 +1,23 @@ +error[E0277]: the trait bound `B: Test` is not satisfied + --> $DIR/foreign_struct_trait_unimplemented.rs:13:16 + | +LL | needs_test(foreign_struct_trait_unimplemented::B); + | ---------- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Test` is not implemented for `B` + | | + | required by a bound introduced by this call + | +help: there are multiple different versions of crate `foreign_struct_trait_unimplemented` in the dependency graph + --> $DIR/foreign_struct_trait_unimplemented.rs:3:1 + | +LL | extern crate foreign_struct_trait_unimplemented; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ one version of crate `foreign_struct_trait_unimplemented` is used here, as a direct dependency of the current crate + = help: you can use `cargo tree` to explore your dependency tree +note: required by a bound in `needs_test` + --> $DIR/foreign_struct_trait_unimplemented.rs:10:23 + | +LL | fn needs_test(_: impl Test) {} + | ^^^^ required by this bound in `needs_test` + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0277`. From 1ddfc902b8595057888320176bb2d2561d9dd8fa Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Wed, 18 Sep 2024 15:01:22 -0700 Subject: [PATCH 4/4] bootstrap: Set the dylib path when building books with rustdoc The library path is needed when the toolchain has been configured with `[rust] rpath = false`. Otherwise, building the reference book will get an error when it tries to run rustdoc, like: rustdoc: error while loading shared libraries: librustc_driver-2ec457c3b8826b72.so (cherry picked from commit de4c8975aaa05063129196e470d3dcf7558f19b1) --- src/bootstrap/src/core/build_steps/doc.rs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/doc.rs b/src/bootstrap/src/core/build_steps/doc.rs index ffb617c642baf..8511533c5443c 100644 --- a/src/bootstrap/src/core/build_steps/doc.rs +++ b/src/bootstrap/src/core/build_steps/doc.rs @@ -63,7 +63,7 @@ macro_rules! book { src: builder.src.join($path), parent: Some(self), languages: $lang.into(), - rustdoc: None, + rustdoc_compiler: None, }) } } @@ -113,7 +113,7 @@ impl Step for UnstableBook { src: builder.md_doc_out(self.target).join("unstable-book"), parent: Some(self), languages: vec![], - rustdoc: None, + rustdoc_compiler: None, }) } } @@ -125,7 +125,7 @@ struct RustbookSrc { src: PathBuf, parent: Option

, languages: Vec<&'static str>, - rustdoc: Option, + rustdoc_compiler: Option, } impl Step for RustbookSrc

{ @@ -157,7 +157,9 @@ impl Step for RustbookSrc

{ let _ = fs::remove_dir_all(&out); let mut rustbook_cmd = builder.tool_cmd(Tool::Rustbook); - if let Some(mut rustdoc) = self.rustdoc { + + if let Some(compiler) = self.rustdoc_compiler { + let mut rustdoc = builder.rustdoc(compiler); rustdoc.pop(); let old_path = env::var_os("PATH").unwrap_or_default(); let new_path = @@ -165,6 +167,7 @@ impl Step for RustbookSrc

{ .expect("could not add rustdoc to PATH"); rustbook_cmd.env("PATH", new_path); + builder.add_rustc_lib_path(compiler, &mut rustbook_cmd); } rustbook_cmd.arg("build").arg(&src).arg("-d").arg(&out).run(builder); @@ -240,7 +243,7 @@ impl Step for TheBook { src: absolute_path.clone(), parent: Some(self), languages: vec![], - rustdoc: None, + rustdoc_compiler: None, }); // building older edition redirects @@ -253,7 +256,7 @@ impl Step for TheBook { // treat the other editions as not having a parent. parent: Option::::None, languages: vec![], - rustdoc: None, + rustdoc_compiler: None, }); } @@ -1218,7 +1221,7 @@ impl Step for RustcBook { src: out_base, parent: Some(self), languages: vec![], - rustdoc: None, + rustdoc_compiler: None, }); } } @@ -1252,16 +1255,15 @@ impl Step for Reference { // This is needed for generating links to the standard library using // the mdbook-spec plugin. builder.ensure(compile::Std::new(self.compiler, builder.config.build)); - let rustdoc = builder.rustdoc(self.compiler); // Run rustbook/mdbook to generate the HTML pages. builder.ensure(RustbookSrc { target: self.target, name: "reference".to_owned(), src: builder.src.join("src/doc/reference"), + rustdoc_compiler: Some(self.compiler), parent: Some(self), languages: vec![], - rustdoc: Some(rustdoc), }); } }