Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect missing ; on methods with return type () #42850

Merged
merged 6 commits into from
Jun 28, 2017
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 39 additions & 5 deletions src/librustc/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -594,8 +594,12 @@ impl<'hir> Map<'hir> {
/// last good node id we found. Note that reaching the crate root (id == 0),
/// is not an error, since items in the crate module have the crate root as
/// parent.
fn walk_parent_nodes<F>(&self, start_id: NodeId, found: F) -> Result<NodeId, NodeId>
where F: Fn(&Node<'hir>) -> bool
fn walk_parent_nodes<F, F2>(&self,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Meh. This is fine for now, but it'd be nice to make this an iterator, I think.

start_id: NodeId,
found: F,
bail_early: F2)
-> Result<NodeId, NodeId>
where F: Fn(&Node<'hir>) -> bool, F2: Fn(&Node<'hir>) -> bool
{
let mut id = start_id;
loop {
Expand All @@ -616,6 +620,8 @@ impl<'hir> Map<'hir> {
Some(ref node) => {
if found(node) {
return Ok(parent_node);
} else if bail_early(node) {
return Err(parent_node);
}
}
None => {
Expand All @@ -626,6 +632,34 @@ impl<'hir> Map<'hir> {
}
}

pub fn get_return_block(&self, id: NodeId) -> Option<NodeId> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment explaining what this function does would be good. Ideally with an example.

let match_fn = |node: &Node| {
match *node {
NodeItem(_) |
NodeForeignItem(_) |
NodeTraitItem(_) |
NodeImplItem(_) => true,
_ => false,
}
};
let match_non_returning_block = |node: &Node| {
match *node {
NodeExpr(ref expr) => {
match expr.node {
ExprWhile(..) | ExprLoop(..) => true,
_ => false,
}
}
_ => false,
}
};

match self.walk_parent_nodes(id, match_fn, match_non_returning_block) {
Ok(id) => Some(id),
Err(_) => None,
}
}

/// Retrieve the NodeId for `id`'s parent item, or `id` itself if no
/// parent item is in this map. The "parent item" is the closest parent node
/// in the AST which is recorded by the map and is an item, either an item
Expand All @@ -637,7 +671,7 @@ impl<'hir> Map<'hir> {
NodeTraitItem(_) |
NodeImplItem(_) => true,
_ => false,
}) {
}, |_| false) {
Ok(id) => id,
Err(id) => id,
}
Expand All @@ -649,7 +683,7 @@ impl<'hir> Map<'hir> {
let id = match self.walk_parent_nodes(id, |node| match *node {
NodeItem(&Item { node: Item_::ItemMod(_), .. }) => true,
_ => false,
}) {
}, |_| false) {
Ok(id) => id,
Err(id) => id,
};
Expand All @@ -668,7 +702,7 @@ impl<'hir> Map<'hir> {
NodeImplItem(_) |
NodeBlock(_) => true,
_ => false,
}) {
}, |_| false) {
Ok(id) => Some(id),
Err(_) => None,
}
Expand Down
4 changes: 3 additions & 1 deletion src/librustc/traits/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1088,7 +1088,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
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");
}
Expand Down Expand Up @@ -1133,6 +1133,8 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
but not on the corresponding trait method",
predicate));
}
ObligationCauseCode::ReturnType(_) |
ObligationCauseCode::BlockTailExpression(_) => (),
}
}

Expand Down
53 changes: 32 additions & 21 deletions src/librustc/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,65 +118,76 @@ 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,
trait_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)]
Expand Down
44 changes: 17 additions & 27 deletions src/librustc/traits/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,8 @@ impl<'a, 'tcx> Lift<'tcx> for traits::ObligationCauseCode<'a> {
super::AssignmentLhsSized => Some(super::AssignmentLhsSized),
super::StructInitializerSized => Some(super::StructInitializerSized),
super::VariableType(id) => Some(super::VariableType(id)),
super::ReturnType => Some(super::ReturnType),
super::ReturnType(id) => Some(super::ReturnType(id)),
super::SizedReturnType => Some(super::SizedReturnType),
super::RepeatVec => Some(super::RepeatVec),
super::FieldSized => Some(super::FieldSized),
super::ConstSized => Some(super::ConstSized),
Expand All @@ -213,34 +214,19 @@ impl<'a, 'tcx> Lift<'tcx> for traits::ObligationCauseCode<'a> {
lint_id: lint_id,
})
}
super::ExprAssignable => {
Some(super::ExprAssignable)
}
super::ExprAssignable => Some(super::ExprAssignable),
super::MatchExpressionArm { arm_span, source } => {
Some(super::MatchExpressionArm { arm_span: arm_span,
source: source })
}
super::IfExpression => {
Some(super::IfExpression)
}
super::IfExpressionWithNoElse => {
Some(super::IfExpressionWithNoElse)
}
super::EquatePredicate => {
Some(super::EquatePredicate)
}
super::MainFunctionType => {
Some(super::MainFunctionType)
}
super::StartFunctionType => {
Some(super::StartFunctionType)
}
super::IntrinsicType => {
Some(super::IntrinsicType)
}
super::MethodReceiver => {
Some(super::MethodReceiver)
}
super::IfExpression => Some(super::IfExpression),
super::IfExpressionWithNoElse => Some(super::IfExpressionWithNoElse),
super::EquatePredicate => Some(super::EquatePredicate),
super::MainFunctionType => Some(super::MainFunctionType),
super::StartFunctionType => Some(super::StartFunctionType),
super::IntrinsicType => Some(super::IntrinsicType),
super::MethodReceiver => Some(super::MethodReceiver),
super::BlockTailExpression(id) => Some(super::BlockTailExpression(id)),
}
}
}
Expand Down Expand Up @@ -492,12 +478,14 @@ impl<'tcx> TypeFoldable<'tcx> for traits::ObligationCauseCode<'tcx> {
super::AssignmentLhsSized |
super::StructInitializerSized |
super::VariableType(_) |
super::ReturnType |
super::ReturnType(_) |
super::SizedReturnType |
super::ReturnNoExpression |
super::RepeatVec |
super::FieldSized |
super::ConstSized |
super::SharedStatic |
super::BlockTailExpression(_) |
super::CompareImplMethodObligation { .. } => self.clone(),

super::ProjectionWf(proj) => super::ProjectionWf(proj.fold_with(folder)),
Expand Down Expand Up @@ -537,12 +525,14 @@ impl<'tcx> TypeFoldable<'tcx> for traits::ObligationCauseCode<'tcx> {
super::AssignmentLhsSized |
super::StructInitializerSized |
super::VariableType(_) |
super::ReturnType |
super::ReturnType(_) |
super::SizedReturnType |
super::ReturnNoExpression |
super::RepeatVec |
super::FieldSized |
super::ConstSized |
super::SharedStatic |
super::BlockTailExpression(_) |
super::CompareImplMethodObligation { .. } => false,

super::ProjectionWf(proj) => proj.visit_with(visitor),
Expand Down
12 changes: 12 additions & 0 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,18 @@ impl<'tcx> TyS<'tcx> {
_ => false,
}
}

pub fn is_suggestable(&self) -> bool {
match self.sty {
TypeVariants::TyAnon(..) |
TypeVariants::TyFnDef(..) |
TypeVariants::TyFnPtr(..) |
TypeVariants::TyDynamic(..) |
TypeVariants::TyClosure(..) |
TypeVariants::TyProjection(..) => false,
_ => true,
}
}
}

impl<'a, 'gcx, 'tcx> HashStable<StableHashingContext<'a, 'gcx, 'tcx>> for ty::TyS<'tcx> {
Expand Down
7 changes: 6 additions & 1 deletion src/librustc_errors/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,12 @@ impl Emitter for EmitterWriter {
// don't display multiline suggestions as labels
sugg.substitution_parts[0].substitutions[0].find('\n').is_none() {
let substitution = &sugg.substitution_parts[0].substitutions[0];
let msg = format!("help: {} `{}`", sugg.msg, substitution);
let msg = if substitution.len() == 0 {
// This substitution is only removal, don't show it
format!("help: {}", sugg.msg)
} else {
format!("help: {} `{}`", sugg.msg, substitution)
};
primary_span.push_span_label(sugg.substitution_spans().next().unwrap(), msg);
} else {
// if there are multiple suggestions, print them all in full
Expand Down
18 changes: 18 additions & 0 deletions src/librustc_typeck/check/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1162,6 +1162,24 @@ impl<'gcx, 'tcx, 'exprs, E> CoerceMany<'gcx, 'tcx, 'exprs, E>
"`return;` in a function whose return type is not `()`");
db.span_label(cause.span, "return type is not ()");
}
ObligationCauseCode::BlockTailExpression(blk_id) => {
db = fcx.report_mismatched_types(cause, expected, found, err);

let expr = expression.unwrap_or_else(|| {
span_bug!(cause.span,
"supposed to be part of a block tail expression, but the \
expression is empty");
});
fcx.suggest_mismatched_types_on_tail(&mut db, expr,
expected, found,
cause.span, blk_id);
}
ObligationCauseCode::ReturnType(ret_id) => {
db = fcx.report_mismatched_types(cause, expected, found, err);
if let Some((fn_decl, _)) = fcx.get_fn_decl(ret_id) {
fcx.point_to_type_requirement(&mut db, &fn_decl, expected);
}
}
_ => {
db = fcx.report_mismatched_types(cause, expected, found, err);
}
Expand Down
17 changes: 12 additions & 5 deletions src/librustc_typeck/check/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,21 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
}
}

pub fn demand_coerce(&self, expr: &hir::Expr, checked_ty: Ty<'tcx>, expected: Ty<'tcx>) {
if let Some(mut err) = self.demand_coerce_diag(expr, checked_ty, expected) {
err.emit();
}
}

// Checks that the type of `expr` can be coerced to `expected`.
//
// NB: This code relies on `self.diverges` to be accurate. In
// particular, assignments to `!` will be permitted if the
// diverges flag is currently "always".
pub fn demand_coerce(&self,
expr: &hir::Expr,
checked_ty: Ty<'tcx>,
expected: Ty<'tcx>) {
pub fn demand_coerce_diag(&self,
expr: &hir::Expr,
checked_ty: Ty<'tcx>,
expected: Ty<'tcx>) -> Option<DiagnosticBuilder<'tcx>> {
let expected = self.resolve_type_vars_with_obligations(expected);

if let Err(e) = self.try_coerce(expr, checked_ty, self.diverges.get(), expected) {
Expand All @@ -105,8 +111,9 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
self.get_best_match(&suggestions).join("\n")));
}
}
err.emit();
return Some(err);
}
None
}

fn format_method_suggestion(&self, method: &AssociatedItem) -> String {
Expand Down
Loading