Skip to content

Commit 8d98877

Browse files
committed
Implement a new wfcheck to replace the old wf; this new code only issues
warnings. It also checks more conditions than the old code. Keep the old wf code around unchanged so that we can continue to issue errors for the cases where we used to report errors. As part of this, remove the where-clauses-must-reference-parameter rule, which is easily circumvented.
1 parent ad47bd8 commit 8d98877

File tree

9 files changed

+854
-145
lines changed

9 files changed

+854
-145
lines changed

src/librustc_typeck/check/method/confirm.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,9 @@ impl<'a,'tcx> ConfirmContext<'a,'tcx> {
432432
traits::ObligationCause::misc(self.span, self.fcx.body_id),
433433
method_predicates);
434434

435-
self.fcx.add_default_region_param_bounds(
435+
// this is a projection from a trait reference, so we have to
436+
// make sure that the trait reference inputs are well-formed.
437+
self.fcx.add_wf_bounds(
436438
all_substs,
437439
self.call_expr);
438440
}

src/librustc_typeck/check/method/probe.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,8 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> {
480480
ty::Predicate::Equate(..) |
481481
ty::Predicate::Projection(..) |
482482
ty::Predicate::RegionOutlives(..) |
483+
ty::Predicate::WellFormed(..) |
484+
ty::Predicate::ObjectSafe(..) |
483485
ty::Predicate::TypeOutlives(..) => {
484486
None
485487
}

src/librustc_typeck/check/mod.rs

Lines changed: 84 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ use middle::infer;
9090
use middle::infer::type_variable;
9191
use middle::pat_util::{self, pat_id_map};
9292
use middle::privacy::{AllPublic, LastMod};
93-
use middle::region::{self, CodeExtent};
93+
use middle::region::{self};
9494
use middle::subst::{self, Subst, Substs, VecPerParamSpace, ParamSpace, TypeSpace};
9595
use middle::traits::{self, report_fulfillment_errors};
9696
use middle::ty::{FnSig, GenericPredicates, TypeScheme};
@@ -133,7 +133,8 @@ pub mod coercion;
133133
pub mod demand;
134134
pub mod method;
135135
mod upvar;
136-
pub mod wf;
136+
mod wf;
137+
mod wfcheck;
137138
mod cast;
138139
mod closure;
139140
mod callee;
@@ -382,25 +383,47 @@ impl<'a, 'tcx> Visitor<'tcx> for CheckItemBodiesVisitor<'a, 'tcx> {
382383
}
383384
}
384385

385-
pub fn check_item_types(ccx: &CrateCtxt) {
386+
pub fn check_wf_old(ccx: &CrateCtxt) {
387+
// FIXME(#25759). The new code below is much more reliable but (for now)
388+
// only generates warnings. So as to ensure that we continue
389+
// getting errors where we used to get errors, we run the old wf
390+
// code first and abort if it encounters any errors. If no abort
391+
// comes, we run the new code and issue warnings.
386392
let krate = ccx.tcx.map.krate();
387393
let mut visit = wf::CheckTypeWellFormedVisitor::new(ccx);
388394
visit::walk_crate(&mut visit, krate);
389395

390396
// If types are not well-formed, it leads to all manner of errors
391397
// downstream, so stop reporting errors at this point.
392398
ccx.tcx.sess.abort_if_errors();
399+
}
393400

394-
let mut visit = CheckItemTypesVisitor { ccx: ccx };
401+
pub fn check_wf_new(ccx: &CrateCtxt) {
402+
let krate = ccx.tcx.map.krate();
403+
let mut visit = wfcheck::CheckTypeWellFormedVisitor::new(ccx);
395404
visit::walk_crate(&mut visit, krate);
396405

406+
// If types are not well-formed, it leads to all manner of errors
407+
// downstream, so stop reporting errors at this point.
397408
ccx.tcx.sess.abort_if_errors();
409+
}
398410

411+
pub fn check_item_types(ccx: &CrateCtxt) {
412+
let krate = ccx.tcx.map.krate();
413+
let mut visit = CheckItemTypesVisitor { ccx: ccx };
414+
visit::walk_crate(&mut visit, krate);
415+
ccx.tcx.sess.abort_if_errors();
416+
}
417+
418+
pub fn check_item_bodies(ccx: &CrateCtxt) {
419+
let krate = ccx.tcx.map.krate();
399420
let mut visit = CheckItemBodiesVisitor { ccx: ccx };
400421
visit::walk_crate(&mut visit, krate);
401422

402423
ccx.tcx.sess.abort_if_errors();
424+
}
403425

426+
pub fn check_drop_impls(ccx: &CrateCtxt) {
404427
for drop_method_did in ccx.tcx.destructors.borrow().iter() {
405428
if drop_method_did.krate == ast::LOCAL_CRATE {
406429
let drop_impl_did = ccx.tcx.map.get_parent_did(drop_method_did.node);
@@ -586,7 +609,7 @@ fn check_fn<'a, 'tcx>(ccx: &'a CrateCtxt<'a, 'tcx>,
586609

587610
if let ty::FnConverging(ret_ty) = ret_ty {
588611
fcx.require_type_is_sized(ret_ty, decl.output.span(), traits::ReturnType);
589-
fn_sig_tys.push(ret_ty);
612+
fn_sig_tys.push(ret_ty); // FIXME(#25759) just take implied bounds from the arguments
590613
}
591614

592615
debug!("fn-sig-map: fn_id={} fn_sig_tys={:?}",
@@ -600,6 +623,14 @@ fn check_fn<'a, 'tcx>(ccx: &'a CrateCtxt<'a, 'tcx>,
600623

601624
// Add formal parameters.
602625
for (arg_ty, input) in arg_tys.iter().zip(&decl.inputs) {
626+
// The type of the argument must be well-formed.
627+
//
628+
// NB -- this is now checked in wfcheck, but that
629+
// currently only results in warnings, so we issue an
630+
// old-style WF obligation here so that we still get the
631+
// errors that we used to get.
632+
fcx.register_old_wf_obligation(arg_ty, input.ty.span, traits::MiscObligation);
633+
603634
// Create type variables for each argument.
604635
pat_util::pat_bindings(
605636
&tcx.def_map,
@@ -1507,10 +1538,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
15071538
pub fn to_ty(&self, ast_t: &ast::Ty) -> Ty<'tcx> {
15081539
let t = ast_ty_to_ty(self, self, ast_t);
15091540

1510-
let mut bounds_checker = wf::BoundsChecker::new(self,
1511-
self.body_id,
1512-
None);
1513-
bounds_checker.check_ty(t, ast_t.span);
1541+
// Generally speaking, we must check that types entered by the
1542+
// user are well-formed. This is not true for `_`, since those
1543+
// types are generated by inference. Now, you might think that
1544+
// we could as well generate a WF obligation -- but
1545+
// unfortunately that breaks code like `foo as *const _`,
1546+
// because those type variables wind up being unconstrained
1547+
// until very late. Nasty. Probably it'd be best to refactor
1548+
// that code path, but that's tricky because of
1549+
// defaults. Argh!
1550+
match ast_t.node {
1551+
ast::TyInfer => { }
1552+
_ => { self.register_wf_obligation(t, ast_t.span, traits::MiscObligation); }
1553+
}
15141554

15151555
t
15161556
}
@@ -1629,15 +1669,38 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
16291669
fulfillment_cx.register_region_obligation(ty, region, cause);
16301670
}
16311671

1632-
pub fn add_default_region_param_bounds(&self,
1633-
substs: &Substs<'tcx>,
1634-
expr: &ast::Expr)
1672+
/// Registers an obligation for checking later, during regionck, that the type `ty` must
1673+
/// outlive the region `r`.
1674+
pub fn register_wf_obligation(&self,
1675+
ty: Ty<'tcx>,
1676+
span: Span,
1677+
code: traits::ObligationCauseCode<'tcx>)
1678+
{
1679+
// WF obligations never themselves fail, so no real need to give a detailed cause:
1680+
let cause = traits::ObligationCause::new(span, self.body_id, code);
1681+
self.register_predicate(traits::Obligation::new(cause, ty::Predicate::WellFormed(ty)));
1682+
}
1683+
1684+
pub fn register_old_wf_obligation(&self,
1685+
ty: Ty<'tcx>,
1686+
span: Span,
1687+
code: traits::ObligationCauseCode<'tcx>)
1688+
{
1689+
// Registers an "old-style" WF obligation that uses the
1690+
// implicator code. This is basically a buggy version of
1691+
// `register_wf_obligation` that is being kept around
1692+
// temporarily just to help with phasing in the newer rules.
1693+
//
1694+
// FIXME(#27579) all uses of this should be migrated to register_wf_obligation eventually
1695+
let cause = traits::ObligationCause::new(span, self.body_id, code);
1696+
self.register_region_obligation(ty, ty::ReEmpty, cause);
1697+
}
1698+
1699+
/// Registers obligations that all types appearing in `substs` are well-formed.
1700+
pub fn add_wf_bounds(&self, substs: &Substs<'tcx>, expr: &ast::Expr)
16351701
{
16361702
for &ty in &substs.types {
1637-
let default_bound = ty::ReScope(CodeExtent::from_node_id(expr.id));
1638-
let cause = traits::ObligationCause::new(expr.span, self.body_id,
1639-
traits::MiscObligation);
1640-
self.register_region_obligation(ty, default_bound, cause);
1703+
self.register_wf_obligation(ty, expr.span, traits::MiscObligation);
16411704
}
16421705
}
16431706

@@ -2477,7 +2540,8 @@ fn check_argument_types<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
24772540
Expectation::rvalue_hint(fcx.tcx(), ty)
24782541
});
24792542

2480-
check_expr_with_unifier(fcx, &**arg,
2543+
check_expr_with_unifier(fcx,
2544+
&**arg,
24812545
expected.unwrap_or(ExpectHasType(formal_ty)),
24822546
NoPreference, || {
24832547
// 2. Coerce to the most detailed type that could be coerced
@@ -3362,7 +3426,9 @@ fn check_expr_with_unifier<'a, 'tcx, F>(fcx: &FnCtxt<'a, 'tcx>,
33623426

33633427
// We always require that the type provided as the value for
33643428
// a type parameter outlives the moment of instantiation.
3365-
constrain_path_type_parameters(fcx, expr);
3429+
fcx.opt_node_ty_substs(expr.id, |item_substs| {
3430+
fcx.add_wf_bounds(&item_substs.substs, expr);
3431+
});
33663432
}
33673433
ast::ExprInlineAsm(ref ia) => {
33683434
for &(_, ref input) in &ia.inputs {
@@ -3903,14 +3969,6 @@ pub fn resolve_ty_and_def_ufcs<'a, 'b, 'tcx>(fcx: &FnCtxt<'b, 'tcx>,
39033969
}
39043970
}
39053971

3906-
fn constrain_path_type_parameters(fcx: &FnCtxt,
3907-
expr: &ast::Expr)
3908-
{
3909-
fcx.opt_node_ty_substs(expr.id, |item_substs| {
3910-
fcx.add_default_region_param_bounds(&item_substs.substs, expr);
3911-
});
3912-
}
3913-
39143972
impl<'tcx> Expectation<'tcx> {
39153973
/// Provide an expectation for an rvalue expression given an *optional*
39163974
/// hint, which is not required for type safety (the resulting type might

0 commit comments

Comments
 (0)