]> git.lizzy.rs Git - rust.git/commitdiff
Auto merge of #42642 - venkatagiri:issue_42312, r=nikomatsakis
authorbors <bors@rust-lang.org>
Thu, 29 Jun 2017 02:30:53 +0000 (02:30 +0000)
committerbors <bors@rust-lang.org>
Thu, 29 Jun 2017 02:30:53 +0000 (02:30 +0000)
rustc_typeck: enforce argument type is sized

closes #42312

r? @nikomatsakis

1  2 
src/librustc/traits/error_reporting.rs
src/librustc/traits/mod.rs
src/librustc_typeck/check/mod.rs

index 2f260b0b9ee1fca233a32f6a4c52a6a1441d645e,64a6a0522cdbf345b4c7c469019accd0f901ba8a..e2e3d520d4777d96ade68f088a78af54cc46b95f
@@@ -45,7 -45,8 +45,8 @@@ use syntax_pos::{DUMMY_SP, Span}
  
  impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
      pub fn report_fulfillment_errors(&self,
-                                      errors: &Vec<FulfillmentError<'tcx>>) {
+                                      errors: &Vec<FulfillmentError<'tcx>>,
+                                      body_id: Option<hir::BodyId>) {
          #[derive(Debug)]
          struct ErrorDescriptor<'tcx> {
              predicate: ty::Predicate<'tcx>,
  
          for (error, suppressed) in errors.iter().zip(is_suppressed) {
              if !suppressed {
-                 self.report_fulfillment_error(error);
+                 self.report_fulfillment_error(error, body_id);
              }
          }
      }
          false
      }
  
-     fn report_fulfillment_error(&self, error: &FulfillmentError<'tcx>) {
+     fn report_fulfillment_error(&self, error: &FulfillmentError<'tcx>,
+                                 body_id: Option<hir::BodyId>) {
          debug!("report_fulfillment_errors({:?})", error);
          match error.code {
              FulfillmentErrorCode::CodeSelectionError(ref e) => {
                  self.report_projection_error(&error.obligation, e);
              }
              FulfillmentErrorCode::CodeAmbiguity => {
-                 self.maybe_report_ambiguity(&error.obligation);
+                 self.maybe_report_ambiguity(&error.obligation, body_id);
              }
              FulfillmentErrorCode::CodeSubtypeError(ref expected_found, ref err) => {
                  self.report_mismatched_types(&error.obligation.cause,
@@@ -869,14 -871,14 +871,14 @@@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, '
  }
  
  impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
-     fn maybe_report_ambiguity(&self, obligation: &PredicateObligation<'tcx>) {
+     fn maybe_report_ambiguity(&self, obligation: &PredicateObligation<'tcx>,
+                               body_id: Option<hir::BodyId>) {
          // Unable to successfully determine, probably means
          // insufficient type information, but could mean
          // ambiguous impls. The latter *ought* to be a
          // coherence violation, so we don't report it here.
  
          let predicate = self.resolve_type_vars_if_possible(&obligation.predicate);
-         let body_id = hir::BodyId { node_id: obligation.cause.body_id };
          let span = obligation.cause.span;
  
          debug!("maybe_report_ambiguity(predicate={:?}, obligation={:?})",
                      let &SubtypePredicate { a_is_expected: _, a, b } = data.skip_binder();
                      // both must be type variables, or the other would've been instantiated
                      assert!(a.is_ty_var() && b.is_ty_var());
-                     self.need_type_info(hir::BodyId { node_id: obligation.cause.body_id },
+                     self.need_type_info(body_id,
                                          obligation.cause.span,
                                          a);
                  }
              ObligationCauseCode::VariableType(_) => {
                  err.note("all local variables must have a statically known size");
              }
 -            ObligationCauseCode::ReturnType => {
 +            ObligationCauseCode::SizedReturnType => {
                  err.note("the return type of a function must have a \
                            statically known size");
              }
                                but not on the corresponding trait method",
                               predicate));
              }
 +            ObligationCauseCode::ReturnType(_) |
 +            ObligationCauseCode::BlockTailExpression(_) => (),
          }
      }
  
index 249cf6b1e275ebb7bdef7786d7c4057d34116c67,056aad7460048939bf5c9878ab251599af0768a3..c128438aea0d48305eefb929b832f155082140c9
@@@ -118,32 -118,27 +118,32 @@@ pub enum ObligationCauseCode<'tcx> 
      /// Obligation incurred due to an object cast.
      ObjectCastObligation(/* Object type */ Ty<'tcx>),
  
 -    /// Various cases where expressions must be sized/copy/etc:
 -    AssignmentLhsSized,        // L = X implies that L is Sized
 -    StructInitializerSized,    // S { ... } must be Sized
 -    VariableType(ast::NodeId), // Type of each variable must be Sized
 -    ReturnType,                // Return type must be Sized
 -    RepeatVec,                 // [T,..n] --> T must be Copy
 -
 -    // Types of fields (other than the last) in a struct must be sized.
 +    // Various cases where expressions must be sized/copy/etc:
 +    /// L = X implies that L is Sized
 +    AssignmentLhsSized,
 +    /// S { ... } must be Sized
 +    StructInitializerSized,
 +    /// Type of each variable must be Sized
 +    VariableType(ast::NodeId),
 +    /// Return type must be Sized
 +    SizedReturnType,
 +    /// [T,..n] --> T must be Copy
 +    RepeatVec,
 +
 +    /// Types of fields (other than the last) in a struct must be sized.
      FieldSized,
  
 -    // Constant expressions must be sized.
 +    /// Constant expressions must be sized.
      ConstSized,
  
 -    // static items must have `Sync` type
 +    /// static items must have `Sync` type
      SharedStatic,
  
      BuiltinDerivedObligation(DerivedObligationCause<'tcx>),
  
      ImplDerivedObligation(DerivedObligationCause<'tcx>),
  
 -    // error derived when matching traits/impls; see ObligationCause for more details
 +    /// error derived when matching traits/impls; see ObligationCause for more details
      CompareImplMethodObligation {
          item_name: ast::Name,
          impl_item_def_id: DefId,
          lint_id: Option<ast::NodeId>,
      },
  
 -    // Checking that this expression can be assigned where it needs to be
 +    /// Checking that this expression can be assigned where it needs to be
      // FIXME(eddyb) #11161 is the original Expr required?
      ExprAssignable,
  
 -    // Computing common supertype in the arms of a match expression
 +    /// Computing common supertype in the arms of a match expression
      MatchExpressionArm { arm_span: Span,
                           source: hir::MatchSource },
  
 -    // Computing common supertype in an if expression
 +    /// Computing common supertype in an if expression
      IfExpression,
  
 -    // Computing common supertype of an if expression with no else counter-part
 +    /// Computing common supertype of an if expression with no else counter-part
      IfExpressionWithNoElse,
  
 -    // `where a == b`
 +    /// `where a == b`
      EquatePredicate,
  
 -    // `main` has wrong type
 +    /// `main` has wrong type
      MainFunctionType,
  
 -    // `start` has wrong type
 +    /// `start` has wrong type
      StartFunctionType,
  
 -    // intrinsic has wrong type
 +    /// intrinsic has wrong type
      IntrinsicType,
  
 -    // method receiver
 +    /// method receiver
      MethodReceiver,
  
 -    // `return` with no expression
 +    /// `return` with no expression
      ReturnNoExpression,
 +
 +    /// `return` with an expression
 +    ReturnType(ast::NodeId),
 +
 +    /// Block implicit return
 +    BlockTailExpression(ast::NodeId),
  }
  
  #[derive(Clone, Debug, PartialEq, Eq)]
@@@ -509,7 -498,7 +509,7 @@@ pub fn normalize_param_env_or_error<'a
          ) {
              Ok(predicates) => predicates,
              Err(errors) => {
-                 infcx.report_fulfillment_errors(&errors);
+                 infcx.report_fulfillment_errors(&errors, None);
                  // An unnormalized env is better than nothing.
                  return elaborated_env;
              }
@@@ -608,7 -597,7 +608,7 @@@ pub fn normalize_and_test_predicates<'a
      debug!("normalize_and_test_predicates(predicates={:?})",
             predicates);
  
 -    tcx.infer_ctxt().enter(|infcx| {
 +    let result = tcx.infer_ctxt().enter(|infcx| {
          let param_env = ty::ParamEnv::empty(Reveal::All);
          let mut selcx = SelectionContext::new(&infcx);
          let mut fulfill_cx = FulfillmentContext::new();
          }
  
          fulfill_cx.select_all_or_error(&infcx).is_ok()
 -    })
 +    });
 +    debug!("normalize_and_test_predicates(predicates={:?}) = {:?}",
 +           predicates, result);
 +    result
  }
  
  /// Given a trait `trait_ref`, iterates the vtable entries
index dd488f0c4fe93e4aec58ad387ad4cf27e00bbc40,dadf00944c61a0fdb78b30d152989c74266f78b6..34cf1d7f96ba02c04cc10bf49cff479383b7689d
@@@ -124,7 -124,6 +124,7 @@@ use syntax_pos::{self, BytePos, Span}
  
  use rustc::hir::intravisit::{self, Visitor, NestedVisitorMap};
  use rustc::hir::itemlikevisit::ItemLikeVisitor;
 +use rustc::hir::map::Node;
  use rustc::hir::{self, PatKind};
  use rustc::middle::lang_items;
  use rustc_back::slice;
@@@ -217,6 -216,8 +217,8 @@@ pub struct Inherited<'a, 'gcx: 'a+'tcx
      /// environment is for an item or something where the "callee" is
      /// not clear.
      implicit_region_bound: Option<ty::Region<'tcx>>,
+     body_id: Option<hir::BodyId>,
  }
  
  impl<'a, 'gcx, 'tcx> Deref for Inherited<'a, 'gcx, 'tcx> {
@@@ -605,6 -606,7 +607,7 @@@ impl<'a, 'gcx, 'tcx> Inherited<'a, 'gcx
              deferred_cast_checks: RefCell::new(Vec::new()),
              anon_types: RefCell::new(NodeMap()),
              implicit_region_bound,
+             body_id,
          }
      }
  
@@@ -978,7 -980,7 +981,7 @@@ fn check_fn<'a, 'gcx, 'tcx>(inherited: 
      *fcx.ps.borrow_mut() = UnsafetyState::function(fn_sig.unsafety, fn_id);
  
      let ret_ty = fn_sig.output();
 -    fcx.require_type_is_sized(ret_ty, decl.output.span(), traits::ReturnType);
 +    fcx.require_type_is_sized(ret_ty, decl.output.span(), traits::SizedReturnType);
      let ret_ty = fcx.instantiate_anon_types(&ret_ty);
      fcx.ret_coercion = Some(RefCell::new(CoerceMany::new(ret_ty)));
      fn_sig = fcx.tcx.mk_fn_sig(
  
      // Add formal parameters.
      for (arg_ty, arg) in fn_sig.inputs().iter().zip(&body.arguments) {
-         // The type of the argument must be well-formed.
-         //
-         // NB -- this is now checked in wfcheck, but that
-         // currently only results in warnings, so we issue an
-         // old-style WF obligation here so that we still get the
-         // errors that we used to get.
-         fcx.register_old_wf_obligation(arg_ty, arg.pat.span, traits::MiscObligation);
          // Check the pattern.
          fcx.check_pat_arg(&arg.pat, arg_ty, true);
+         // Check that argument is Sized.
+         // The check for a non-trivial pattern is a hack to avoid duplicate warnings
+         // for simple cases like `fn foo(x: Trait)`,
+         // where we would error once on the parameter as a whole, and once on the binding `x`.
+         if arg.pat.simple_name().is_none() {
+             fcx.require_type_is_sized(arg_ty, decl.output.span(), traits::MiscObligation);
+         }
          fcx.write_ty(arg.id, arg_ty);
      }
  
@@@ -1901,7 -1904,7 +1905,7 @@@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, '
  
                      // Require that the predicate holds for the concrete type.
                      let cause = traits::ObligationCause::new(span, self.body_id,
 -                                                             traits::ReturnType);
 +                                                             traits::SizedReturnType);
                      self.register_predicate(traits::Obligation::new(cause,
                                                                      self.param_env,
                                                                      predicate));
          }
      }
  
-     /// Registers an obligation for checking later, during regionck, that the type `ty` must
-     /// outlive the region `r`.
-     pub fn register_region_obligation(&self,
-                                       ty: Ty<'tcx>,
-                                       region: ty::Region<'tcx>,
-                                       cause: traits::ObligationCause<'tcx>)
-     {
-         let mut fulfillment_cx = self.fulfillment_cx.borrow_mut();
-         fulfillment_cx.register_region_obligation(ty, region, cause);
-     }
      /// Registers an obligation for checking later, during regionck, that the type `ty` must
      /// outlive the region `r`.
      pub fn register_wf_obligation(&self,
                                                          ty::Predicate::WellFormed(ty)));
      }
  
-     pub fn register_old_wf_obligation(&self,
-                                       ty: Ty<'tcx>,
-                                       span: Span,
-                                       code: traits::ObligationCauseCode<'tcx>)
-     {
-         // Registers an "old-style" WF obligation that uses the
-         // implicator code.  This is basically a buggy version of
-         // `register_wf_obligation` that is being kept around
-         // temporarily just to help with phasing in the newer rules.
-         //
-         // FIXME(#27579) all uses of this should be migrated to register_wf_obligation eventually
-         let cause = traits::ObligationCause::new(span, self.body_id, code);
-         self.register_region_obligation(ty, self.tcx.types.re_empty, cause);
-     }
      /// Registers obligations that all types appearing in `substs` are well-formed.
      pub fn add_wf_bounds(&self, substs: &Substs<'tcx>, expr: &hir::Expr)
      {
  
          match fulfillment_cx.select_all_or_error(self) {
              Ok(()) => { }
-             Err(errors) => { self.report_fulfillment_errors(&errors); }
+             Err(errors) => { self.report_fulfillment_errors(&errors, self.inh.body_id); }
          }
      }
  
      fn select_obligations_where_possible(&self) {
          match self.fulfillment_cx.borrow_mut().select_where_possible(self) {
              Ok(()) => { }
-             Err(errors) => { self.report_fulfillment_errors(&errors); }
+             Err(errors) => { self.report_fulfillment_errors(&errors, self.inh.body_id); }
          }
      }
  
                                               "check_return_expr called outside fn body"));
  
          let ret_ty = ret_coercion.borrow().expected_ty();
 -        let return_expr_ty = self.check_expr_with_hint(return_expr, ret_ty);
 +        let return_expr_ty = self.check_expr_with_hint(return_expr, ret_ty.clone());
          ret_coercion.borrow_mut()
                      .coerce(self,
 -                            &self.misc(return_expr.span),
 +                            &self.cause(return_expr.span,
 +                                        ObligationCauseCode::ReturnType(return_expr.id)),
                              return_expr,
                              return_expr_ty,
                              self.diverges.get());
              let mut coerce = ctxt.coerce.as_mut().unwrap();
              if let Some(tail_expr_ty) = tail_expr_ty {
                  let tail_expr = tail_expr.unwrap();
 +                let cause = self.cause(tail_expr.span,
 +                                       ObligationCauseCode::BlockTailExpression(blk.id));
                  coerce.coerce(self,
 -                              &self.misc(tail_expr.span),
 +                              &cause,
                                tail_expr,
                                tail_expr_ty,
                                self.diverges.get());
          ty
      }
  
 +    /// Given a `NodeId`, return the `FnDecl` of the method it is enclosed by and whether it is
 +    /// `fn main` if it is a method, `None` otherwise.
 +    pub fn get_fn_decl(&self, blk_id: ast::NodeId) -> Option<(hir::FnDecl, bool)> {
 +        // Get enclosing Fn, if it is a function or a trait method, unless there's a `loop` or
 +        // `while` before reaching it, as block tail returns are not available in them.
 +        if let Some(fn_id) = self.tcx.hir.get_return_block(blk_id) {
 +            let parent = self.tcx.hir.get(fn_id);
 +
 +            if let Node::NodeItem(&hir::Item {
 +                name, node: hir::ItemFn(ref decl, ..), ..
 +            }) = parent {
 +                decl.clone().and_then(|decl| {
 +                    // This is less than ideal, it will not present the return type span on any
 +                    // method called `main`, regardless of whether it is actually the entry point.
 +                    Some((decl, name == Symbol::intern("main")))
 +                })
 +            } else if let Node::NodeTraitItem(&hir::TraitItem {
 +                node: hir::TraitItemKind::Method(hir::MethodSig {
 +                    ref decl, ..
 +                }, ..), ..
 +            }) = parent {
 +                decl.clone().and_then(|decl| {
 +                    Some((decl, false))
 +                })
 +            } else {
 +                None
 +            }
 +        } else {
 +            None
 +        }
 +    }
 +
 +    /// On implicit return expressions with mismatched types, provide the following suggestions:
 +    ///
 +    ///  - Point out the method's return type as the reason for the expected type
 +    ///  - Possible missing semicolon
 +    ///  - Possible missing return type if the return type is the default, and not `fn main()`
 +    pub fn suggest_mismatched_types_on_tail(&self,
 +                                            err: &mut DiagnosticBuilder<'tcx>,
 +                                            expression: &'gcx hir::Expr,
 +                                            expected: Ty<'tcx>,
 +                                            found: Ty<'tcx>,
 +                                            cause_span: Span,
 +                                            blk_id: ast::NodeId) {
 +        self.suggest_missing_semicolon(err, expression, expected, cause_span);
 +
 +        if let Some((fn_decl, is_main)) = self.get_fn_decl(blk_id) {
 +            // `fn main()` must return `()`, do not suggest changing return type
 +            if !is_main {
 +                self.suggest_missing_return_type(err, &fn_decl, found);
 +            }
 +        }
 +    }
 +
 +    /// A common error is to forget to add a semicolon at the end of a block:
 +    ///
 +    /// ```
 +    /// fn foo() {
 +    ///     bar_that_returns_u32()
 +    /// }
 +    /// ```
 +    ///
 +    /// This routine checks if the return expression in a block would make sense on its own as a
 +    /// statement and the return type has been left as defaultor has been specified as `()`. If so,
 +    /// it suggests adding a semicolon.
 +    fn suggest_missing_semicolon(&self,
 +                                     err: &mut DiagnosticBuilder<'tcx>,
 +                                     expression: &'gcx hir::Expr,
 +                                     expected: Ty<'tcx>,
 +                                     cause_span: Span) {
 +        if expected.is_nil() {
 +            // `BlockTailExpression` only relevant if the tail expr would be
 +            // useful on its own.
 +            match expression.node {
 +                hir::ExprCall(..) |
 +                hir::ExprMethodCall(..) |
 +                hir::ExprIf(..) |
 +                hir::ExprWhile(..) |
 +                hir::ExprLoop(..) |
 +                hir::ExprMatch(..) |
 +                hir::ExprBlock(..) => {
 +                    let sp = cause_span.next_point();
 +                    err.span_suggestion(sp,
 +                                        "did you mean to add a semicolon here?",
 +                                        ";".to_string());
 +                }
 +                _ => (),
 +            }
 +        }
 +    }
 +
 +
 +    /// A possible error is to forget to add a return type that is needed:
 +    ///
 +    /// ```
 +    /// fn foo() {
 +    ///     bar_that_returns_u32()
 +    /// }
 +    /// ```
 +    ///
 +    /// This routine checks if the return type is left as default, the method is not part of an
 +    /// `impl` block and that it isn't the `main` method. If so, it suggests setting the return
 +    /// type.
 +    fn suggest_missing_return_type(&self,
 +                                   err: &mut DiagnosticBuilder<'tcx>,
 +                                   fn_decl: &hir::FnDecl,
 +                                   ty: Ty<'tcx>) {
 +
 +        // Only recommend changing the return type for methods that
 +        // haven't set a return type at all (and aren't `fn main()` or an impl).
 +        if let &hir::FnDecl {
 +            output: hir::FunctionRetTy::DefaultReturn(span), ..
 +        } = fn_decl {
 +            if ty.is_suggestable() {
 +                err.span_suggestion(span,
 +                                    "possibly return type missing here?",
 +                                    format!("-> {} ", ty));
 +            } else {
 +                err.span_label(span, "possibly return type missing here?");
 +            }
 +        }
 +    }
 +
 +
      /// A common error is to add an extra semicolon:
      ///
      /// ```
              hi: original_span.hi,
              ctxt: original_span.ctxt,
          };
 -        err.span_help(span_semi, "consider removing this semicolon:");
 +        err.span_suggestion(span_semi, "consider removing this semicolon", "".to_string());
      }
  
      // Instantiates the given path, which must refer to an item with the given