Skip to content

Commit

Permalink
Support HIR wf checking for function signatures
Browse files Browse the repository at this point in the history
During function type-checking, we normalize any associated types in
the function signature (argument types + return type), and then
create WF obligations for each of the normalized types. The HIR wf code
does not currently support this case, so any errors that we get have
imprecise spans.

This commit extends `ObligationCauseCode::WellFormed` to support
recording a function parameter, allowing us to get the corresponding
HIR type if an error occurs. Function typechecking is modified to
pass this information during signature normalization and WF checking.
The resulting code is fairly verbose, due to the fact that we can
no longer normalize the entire signature with a single function call.

As part of the refactoring, we now perform HIR-based WF checking
for several other 'typed items' (statics, consts, and inherent impls).

As a result, WF and projection errors in a function signature now
have a precise span, which points directly at the responsible type.
If a function signature is constructed via a macro, this will allow
the error message to point at the code 'most responsible' for the error
(e.g. a user-supplied macro argument).
  • Loading branch information
Aaron1011 committed Jul 20, 2021
1 parent da7d405 commit db0324e
Show file tree
Hide file tree
Showing 28 changed files with 240 additions and 91 deletions.
5 changes: 4 additions & 1 deletion compiler/rustc_middle/src/hir/map/mod.rs
Expand Up @@ -29,7 +29,10 @@ fn fn_decl<'hir>(node: Node<'hir>) -> Option<&'hir FnDecl<'hir>> {
Node::Item(Item { kind: ItemKind::Fn(sig, _, _), .. })
| Node::TraitItem(TraitItem { kind: TraitItemKind::Fn(sig, _), .. })
| Node::ImplItem(ImplItem { kind: ImplItemKind::Fn(sig, _), .. }) => Some(&sig.decl),
Node::Expr(Expr { kind: ExprKind::Closure(_, fn_decl, ..), .. }) => Some(fn_decl),
Node::Expr(Expr { kind: ExprKind::Closure(_, fn_decl, ..), .. })
| Node::ForeignItem(ForeignItem { kind: ForeignItemKind::Fn(fn_decl, ..), .. }) => {
Some(fn_decl)
}
_ => None,
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/query/mod.rs
Expand Up @@ -1722,7 +1722,7 @@ rustc_queries! {
/// span) for an *existing* error. Therefore, it is best-effort, and may never handle
/// all of the cases that the normal `ty::Ty`-based wfcheck does. This is fine,
/// because the `ty::Ty`-based wfcheck is always run.
query diagnostic_hir_wf_check(key: (ty::Predicate<'tcx>, hir::HirId)) -> Option<traits::ObligationCause<'tcx>> {
query diagnostic_hir_wf_check(key: (ty::Predicate<'tcx>, traits::WellFormedLoc)) -> Option<traits::ObligationCause<'tcx>> {
eval_always
no_hash
desc { "performing HIR wf-checking for predicate {:?} at item {:?}", key.0, key.1 }
Expand Down
34 changes: 28 additions & 6 deletions compiler/rustc_middle/src/traits/mod.rs
Expand Up @@ -16,7 +16,7 @@ use crate::ty::{self, AdtKind, Ty, TyCtxt};
use rustc_data_structures::sync::Lrc;
use rustc_errors::{Applicability, DiagnosticBuilder};
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::Constness;
use rustc_span::symbol::Symbol;
use rustc_span::{Span, DUMMY_SP};
Expand Down Expand Up @@ -327,17 +327,39 @@ pub enum ObligationCauseCode<'tcx> {
/// If `X` is the concrete type of an opaque type `impl Y`, then `X` must implement `Y`
OpaqueType,

/// Well-formed checking. If a `HirId` is provided,
/// it is used to perform HIR-based wf checking if an error
/// occurs, in order to generate a more precise error message.
/// Well-formed checking. If a `WellFormedLoc` is provided,
/// then it will be used to eprform HIR-based wf checking
/// after an error occurs, in order to generate a more precise error span.
/// This is purely for diagnostic purposes - it is always
/// correct to use `MiscObligation` instead
WellFormed(Option<hir::HirId>),
/// correct to use `MiscObligation` instead, or to specify
/// `WellFormed(None)`
WellFormed(Option<WellFormedLoc>),

/// From `match_impl`. The cause for us having to match an impl, and the DefId we are matching against.
MatchImpl(Lrc<ObligationCauseCode<'tcx>>, DefId),
}

/// The 'location' at which we try to perform HIR-based wf checking.
/// This information is used to obtain an `hir::Ty`, which
/// we can walk in order to obtain precise spans for any
/// 'nested' types (e.g. `Foo` in `Option<Foo>`).
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, HashStable)]
pub enum WellFormedLoc {
/// Use the type of the provided definition.
Ty(LocalDefId),
/// Use the type of the parameter of the provided function.
/// We cannot use `hir::Param`, since the function may
/// not have a body (e.g. a trait method definition)
Param {
/// The function to lookup the parameter in
function: LocalDefId,
/// The index of the parameter to use.
/// Parameters are indexed from 0, with the return type
/// being the last 'parameter'
param_idx: u16,
},
}

impl ObligationCauseCode<'_> {
// Return the base obligation, ignoring derived obligations.
pub fn peel_derives(&self) -> &Self {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/context.rs
Expand Up @@ -1682,7 +1682,7 @@ nop_list_lift! {bound_variable_kinds; ty::BoundVariableKind => ty::BoundVariable
// This is the impl for `&'a InternalSubsts<'a>`.
nop_list_lift! {substs; GenericArg<'a> => GenericArg<'tcx>}

CloneLiftImpls! { for<'tcx> { Constness, } }
CloneLiftImpls! { for<'tcx> { Constness, traits::WellFormedLoc, } }

pub mod tls {
use super::{ptr_eq, GlobalCtxt, TyCtxt};
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_query_impl/src/keys.rs
@@ -1,9 +1,9 @@
//! Defines the set of legal keys that can be used in queries.

use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, LOCAL_CRATE};
use rustc_hir::HirId;
use rustc_middle::infer::canonical::Canonical;
use rustc_middle::mir;
use rustc_middle::traits;
use rustc_middle::ty::fast_reject::SimplifiedType;
use rustc_middle::ty::subst::{GenericArg, SubstsRef};
use rustc_middle::ty::{self, Ty, TyCtxt};
Expand Down Expand Up @@ -397,7 +397,7 @@ impl<'tcx> Key for (DefId, Ty<'tcx>, SubstsRef<'tcx>, ty::ParamEnv<'tcx>) {
}
}

impl<'tcx> Key for (ty::Predicate<'tcx>, HirId) {
impl<'tcx> Key for (ty::Predicate<'tcx>, traits::WellFormedLoc) {
#[inline(always)]
fn query_crate_is_local(&self) -> bool {
true
Expand Down
Expand Up @@ -242,11 +242,11 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
SelectionError::Unimplemented => {
// If this obligation was generated as a result of well-formed checking, see if we
// can get a better error message by performing HIR-based well formed checking.
if let ObligationCauseCode::WellFormed(Some(wf_hir_id)) =
if let ObligationCauseCode::WellFormed(Some(wf_loc)) =
root_obligation.cause.code.peel_derives()
{
if let Some(cause) =
self.tcx.diagnostic_hir_wf_check((obligation.predicate, *wf_hir_id))
self.tcx.diagnostic_hir_wf_check((obligation.predicate, wf_loc.clone()))
{
obligation.cause = cause;
span = obligation.cause.span;
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs
Expand Up @@ -40,6 +40,7 @@ use rustc_trait_selection::opaque_types::InferCtxtExt as _;
use rustc_trait_selection::traits::error_reporting::InferCtxtExt as _;
use rustc_trait_selection::traits::{
self, ObligationCause, ObligationCauseCode, StatementAsExpression, TraitEngine, TraitEngineExt,
WellFormedLoc,
};

use std::collections::hash_map::Entry;
Expand Down Expand Up @@ -419,13 +420,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&self,
span: Span,
value: T,
hir_id: hir::HirId,
loc: WellFormedLoc,
) -> T
where
T: TypeFoldable<'tcx>,
{
self.inh.normalize_associated_types_in_with_cause(
ObligationCause::new(span, self.body_id, ObligationCauseCode::WellFormed(Some(hir_id))),
ObligationCause::new(span, self.body_id, ObligationCauseCode::WellFormed(Some(loc))),
self.param_env,
value,
)
Expand Down
72 changes: 62 additions & 10 deletions compiler/rustc_typeck/src/check/wfcheck.rs
Expand Up @@ -22,8 +22,9 @@ use rustc_span::symbol::{sym, Ident, Symbol};
use rustc_span::Span;
use rustc_trait_selection::opaque_types::may_define_opaque_type;
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
use rustc_trait_selection::traits::{self, ObligationCause, ObligationCauseCode};
use rustc_trait_selection::traits::{self, ObligationCause, ObligationCauseCode, WellFormedLoc};

use std::convert::TryInto;
use std::iter;
use std::ops::ControlFlow;

Expand Down Expand Up @@ -386,7 +387,7 @@ fn check_associated_item(
span: Span,
sig_if_method: Option<&hir::FnSig<'_>>,
) {
let code = ObligationCauseCode::WellFormed(Some(item_id));
let code = ObligationCauseCode::WellFormed(Some(WellFormedLoc::Ty(item_id.expect_owner())));
for_id(tcx, item_id, span).with_fcx(|fcx| {
let item = fcx.tcx.associated_item(fcx.tcx.hir().local_def_id(item_id));

Expand All @@ -400,7 +401,11 @@ fn check_associated_item(
match item.kind {
ty::AssocKind::Const => {
let ty = fcx.tcx.type_of(item.def_id);
let ty = fcx.normalize_associated_types_in_wf(span, ty, item_id);
let ty = fcx.normalize_associated_types_in_wf(
span,
ty,
WellFormedLoc::Ty(item_id.expect_owner()),
);
fcx.register_wf_obligation(ty.into(), span, code.clone());
}
ty::AssocKind::Fn => {
Expand All @@ -422,7 +427,11 @@ fn check_associated_item(
}
if item.defaultness.has_value() {
let ty = fcx.tcx.type_of(item.def_id);
let ty = fcx.normalize_associated_types_in_wf(span, ty, item_id);
let ty = fcx.normalize_associated_types_in_wf(
span,
ty,
WellFormedLoc::Ty(item_id.expect_owner()),
);
fcx.register_wf_obligation(ty.into(), span, code.clone());
}
}
Expand Down Expand Up @@ -621,7 +630,11 @@ fn check_item_type(tcx: TyCtxt<'_>, item_id: hir::HirId, ty_span: Span, allow_fo

for_id(tcx, item_id, ty_span).with_fcx(|fcx| {
let ty = tcx.type_of(tcx.hir().local_def_id(item_id));
let item_ty = fcx.normalize_associated_types_in_wf(ty_span, ty, item_id);
let item_ty = fcx.normalize_associated_types_in_wf(
ty_span,
ty,
WellFormedLoc::Ty(item_id.expect_owner()),
);

let mut forbid_unsized = true;
if allow_foreign_ty {
Expand All @@ -634,7 +647,7 @@ fn check_item_type(tcx: TyCtxt<'_>, item_id: hir::HirId, ty_span: Span, allow_fo
fcx.register_wf_obligation(
item_ty.into(),
ty_span,
ObligationCauseCode::WellFormed(Some(item_id)),
ObligationCauseCode::WellFormed(Some(WellFormedLoc::Ty(item_id.expect_owner()))),
);
if forbid_unsized {
fcx.register_bound(
Expand Down Expand Up @@ -684,7 +697,9 @@ fn check_impl<'tcx>(
fcx.register_wf_obligation(
self_ty.into(),
ast_self_ty.span,
ObligationCauseCode::WellFormed(Some(item.hir_id())),
ObligationCauseCode::WellFormed(Some(WellFormedLoc::Ty(
item.hir_id().expect_owner(),
))),
);
}
}
Expand Down Expand Up @@ -901,11 +916,48 @@ fn check_fn_or_method<'fcx, 'tcx>(
implied_bounds: &mut Vec<Ty<'tcx>>,
) {
let sig = fcx.tcx.liberate_late_bound_regions(def_id, sig);
let sig = fcx.normalize_associated_types_in(span, sig);

for (&input_ty, ty) in iter::zip(sig.inputs(), hir_decl.inputs) {
fcx.register_wf_obligation(input_ty.into(), ty.span, ObligationCauseCode::WellFormed(None));
// Normalize the input and output types one at a time, using a different
// `WellFormedLoc` for each. We cannot call `normalize_associated_types`
// on the entire `FnSig`, since this would use the same `WellFormedLoc`
// for each type, preventing the HIR wf check from generating
// a nice error message.
let ty::FnSig { mut inputs_and_output, c_variadic, unsafety, abi } = sig;
inputs_and_output =
fcx.tcx.mk_type_list(inputs_and_output.iter().enumerate().map(|(i, ty)| {
fcx.normalize_associated_types_in_wf(
span,
ty,
WellFormedLoc::Param {
function: def_id.expect_local(),
// Note that the `param_idx` of the output type is
// one greater than the index of the last input type.
param_idx: i.try_into().unwrap(),
},
)
}));
// Manually call `normalize_assocaited_types_in` on the other types
// in `FnSig`. This ensures that if the types of these fields
// ever change to include projections, we will start normalizing
// them automatically.
let sig = ty::FnSig {
inputs_and_output,
c_variadic: fcx.normalize_associated_types_in(span, c_variadic),
unsafety: fcx.normalize_associated_types_in(span, unsafety),
abi: fcx.normalize_associated_types_in(span, abi),
};

for (i, (&input_ty, ty)) in iter::zip(sig.inputs(), hir_decl.inputs).enumerate() {
fcx.register_wf_obligation(
input_ty.into(),
ty.span,
ObligationCauseCode::WellFormed(Some(WellFormedLoc::Param {
function: def_id.expect_local(),
param_idx: i.try_into().unwrap(),
})),
);
}

implied_bounds.extend(sig.inputs());

fcx.register_wf_obligation(
Expand Down

0 comments on commit db0324e

Please sign in to comment.