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

Do not emit type errors on recovered blocks #46732

Merged
merged 2 commits into from Dec 22, 2017
Jump to file or symbol
Failed to load files and symbols.
+112 −41
Diff settings

Always

Just for now

Copy path View file
@@ -359,18 +359,21 @@ lifetime elision rules (see below).
Here are some simple examples of where you'll run into this error:
```compile_fail,E0106
struct Foo { x: &bool } // error
struct Foo<'a> { x: &'a bool } // correct
struct Foo1 { x: &bool }
// ^ expected lifetime parameter
struct Foo2<'a> { x: &'a bool } // correct
struct Bar { x: Foo }
^^^ expected lifetime parameter
struct Bar<'a> { x: Foo<'a> } // correct
struct Bar1 { x: Foo2 }
// ^^^^ expected lifetime parameter
struct Bar2<'a> { x: Foo2<'a> } // correct
enum Bar { A(u8), B(&bool), } // error
enum Bar<'a> { A(u8), B(&'a bool), } // correct
enum Baz1 { A(u8), B(&bool), }
// ^ expected lifetime parameter
enum Baz2<'a> { A(u8), B(&'a bool), } // correct
type MyStr = &str; // error
type MyStr<'a> = &'a str; // correct
type MyStr1 = &str;
// ^ expected lifetime parameter
type MyStr2<'a> = &'a str; // correct
```
Lifetime elision is a special, limited kind of inference for lifetimes in
Copy path View file
@@ -1835,6 +1835,7 @@ impl<'a> LoweringContext<'a> {
rules: self.lower_block_check_mode(&b.rules),
span: b.span,
targeted_by_break,
recovered: b.recovered,
})
}
@@ -2691,6 +2692,7 @@ impl<'a> LoweringContext<'a> {
rules: hir::DefaultBlock,
span,
targeted_by_break: false,
recovered: blk.recovered,
});
P(self.expr_block(blk, ThinVec::new()))
}
@@ -3507,6 +3509,7 @@ impl<'a> LoweringContext<'a> {
rules: hir::DefaultBlock,
span,
targeted_by_break: false,
recovered: false,
}
}
@@ -3610,6 +3613,7 @@ impl<'a> LoweringContext<'a> {
stmts,
expr: Some(expr),
targeted_by_break: false,
recovered: false,
});
self.expr_block(block, attrs)
}
Copy path View file
@@ -625,6 +625,11 @@ pub struct Block {
/// currently permitted in Rust itself, but it is generated as
/// part of `catch` statements.
pub targeted_by_break: bool,
/// If true, don't emit return value type errors as the parser had
/// to recover from a parse error so this block will not have an
/// appropriate type. A parse error will have been emitted so the
/// compilation will never succeed if this is true.
pub recovered: bool,
}
#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash)]
Copy path View file
@@ -378,12 +378,14 @@ impl<'gcx> HashStable<StableHashingContext<'gcx>> for hir::Block {
rules,
span,
targeted_by_break,
recovered,
} = *self;
stmts.hash_stable(hcx, hasher);
expr.hash_stable(hcx, hasher);
rules.hash_stable(hcx, hasher);
span.hash_stable(hcx, hasher);
recovered.hash_stable(hcx, hasher);
targeted_by_break.hash_stable(hcx, hasher);
}
}
Copy path View file
@@ -729,6 +729,7 @@ impl<'a> fold::Folder for ReplaceBodyWithLoop<'a> {
fn fold_block(&mut self, b: P<ast::Block>) -> P<ast::Block> {
fn expr_to_block(rules: ast::BlockCheckMode,
recovered: bool,
e: Option<P<ast::Expr>>,
sess: &Session) -> P<ast::Block> {
P(ast::Block {
@@ -744,20 +745,21 @@ impl<'a> fold::Folder for ReplaceBodyWithLoop<'a> {
rules,
id: sess.next_node_id(),
span: syntax_pos::DUMMY_SP,
recovered,
})
}
if !self.within_static_or_const {
let empty_block = expr_to_block(BlockCheckMode::Default, None, self.sess);
let empty_block = expr_to_block(BlockCheckMode::Default, false, None, self.sess);
let loop_expr = P(ast::Expr {
node: ast::ExprKind::Loop(empty_block, None),
id: self.sess.next_node_id(),
span: syntax_pos::DUMMY_SP,
attrs: ast::ThinVec::new(),
});
expr_to_block(b.rules, Some(loop_expr), self.sess)
expr_to_block(b.rules, b.recovered, Some(loop_expr), self.sess)
} else {
fold::noop_fold_block(b, self)
@@ -4279,7 +4279,12 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
//
// #41425 -- label the implicit `()` as being the
// "found type" here, rather than the "expected type".
if !self.diverges.get().always() {
//
// #44579 -- if the block was recovered during parsing,
// the type would be nonsensical and it is not worth it
// to perform the type check, so we avoid generating the
// diagnostic output.
if !self.diverges.get().always() && !blk.recovered {
coerce.coerce_forced_unit(self, &self.misc(blk.span), &mut |err| {
if let Some(expected_ty) = expected.only_has_type(self) {
self.consider_hint_about_removing_semicolon(blk,
Copy path View file
@@ -468,6 +468,7 @@ pub struct Block {
/// Distinguishes between `unsafe { ... }` and `{ ... }`
pub rules: BlockCheckMode,
pub span: Span,
pub recovered: bool,
}
#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash)]
Copy path View file
@@ -594,6 +594,7 @@ impl<'a> AstBuilder for ExtCtxt<'a> {
id: ast::DUMMY_NODE_ID,
rules: BlockCheckMode::Default,
span,
recovered: false,
})
}
Copy path View file
@@ -851,11 +851,12 @@ fn noop_fold_bounds<T: Folder>(bounds: TyParamBounds, folder: &mut T)
}
pub fn noop_fold_block<T: Folder>(b: P<Block>, folder: &mut T) -> P<Block> {
b.map(|Block {id, stmts, rules, span}| Block {
b.map(|Block {id, stmts, rules, span, recovered}| Block {
id: folder.new_id(id),
stmts: stmts.move_flat_map(|s| folder.fold_stmt(s).into_iter()),
rules,
span: folder.new_span(span),
recovered,
})
}
Copy path View file
@@ -931,6 +931,7 @@ mod tests {
id: ast::DUMMY_NODE_ID,
rules: ast::BlockCheckMode::Default, // no idea
span: sp(15,21),
recovered: false,
})),
vis: ast::Visibility::Inherited,
span: sp(0,21)})));
Copy path View file
@@ -4371,13 +4371,15 @@ impl<'a> Parser<'a> {
/// Precondition: already parsed the '{'.
fn parse_block_tail(&mut self, lo: Span, s: BlockCheckMode) -> PResult<'a, P<Block>> {
let mut stmts = vec![];
let mut recovered = false;
while !self.eat(&token::CloseDelim(token::Brace)) {
let stmt = match self.parse_full_stmt(false) {
Err(mut err) => {
err.emit();
self.recover_stmt_(SemiColonMode::Ignore, BlockMode::Break);
self.recover_stmt_(SemiColonMode::Ignore, BlockMode::Ignore);
self.eat(&token::CloseDelim(token::Brace));
recovered = true;
break;
}
Ok(stmt) => stmt,
@@ -4396,12 +4398,13 @@ impl<'a> Parser<'a> {
id: ast::DUMMY_NODE_ID,
rules: s,
span: lo.to(self.prev_span),
recovered,
}))
}
/// Parse a statement, including the trailing semicolon.
pub fn parse_full_stmt(&mut self, macro_legacy_warnings: bool) -> PResult<'a, Option<Stmt>> {
let mut stmt = match self.parse_stmt_(macro_legacy_warnings) {
let mut stmt = match self.parse_stmt_without_recovery(macro_legacy_warnings)? {

This comment has been minimized.

@petrochenkov

petrochenkov Dec 16, 2017

Contributor

This seems to be a change independent from adding the recovered flag to blocks?
From what I see in the test diffs I'd keep this recovery in place.

This comment has been minimized.

@estebank

estebank Dec 17, 2017

Contributor

The recovery happens in the callee already, and using parse_stmt_ swallows errors that we do want to see in parse_block_tail.

Some(stmt) => stmt,
None => return Ok(None),
};
@@ -158,5 +158,6 @@ fn call_intrinsic(cx: &ExtCtxt,
id: ast::DUMMY_NODE_ID,
rules: ast::BlockCheckMode::Unsafe(ast::CompilerGenerated),
span,
recovered: false,
}))
}
@@ -11,5 +11,4 @@
fn main () {
let sr: Vec<(u32, _, _) = vec![]; //~ ERROR expected one of `,` or `>`, found `=`
let sr2: Vec<(u32, _, _)> = sr.iter().map(|(faction, th_sender, th_receiver)| {}).collect();
//~^ ERROR cannot find value `sr` in this scope
}
@@ -16,7 +16,6 @@ fn main() {
println!("Y {}",x);
return x;
};
//~^ ERROR expected item, found `;`
caller(bar_handler);
}
@@ -11,6 +11,10 @@
// compile-flags: -Z parse-only
fn main() {
struct::foo(); //~ ERROR expected identifier
mut::baz(); //~ ERROR expected expression, found keyword `mut`
struct::foo();
//~^ ERROR expected identifier
}
fn bar() {
mut::baz();
//~^ ERROR expected expression, found keyword `mut`
}
@@ -15,5 +15,4 @@
pub fn main() {
struct Foo { x: isize }
let mut Foo { x: x } = Foo { x: 3 }; //~ ERROR: expected one of `:`, `;`, `=`, or `@`, found `{`
//~^ ERROR expected item, found `=`
}
@@ -131,6 +131,7 @@ fn iter_exprs(depth: usize, f: &mut FnMut(P<Expr>)) {
id: DUMMY_NODE_ID,
rules: BlockCheckMode::Default,
span: DUMMY_SP,
recovered: false,
});
iter_exprs(depth - 1, &mut |e| g(ExprKind::If(e, block.clone(), None)));
},
Copy path View file
@@ -17,11 +17,13 @@ pub fn main() {
0..;
..1;
0..1;
..=; //~ERROR inclusive range with no end
0..=; //~ERROR inclusive range with no end
//~^HELP bounded at the end
}
fn _foo1() {
..=1;
0..=1;
0..=; //~ERROR inclusive range with no end
//~^HELP bounded at the end
}
@@ -1,15 +1,15 @@
error[E0586]: inclusive range with no end
--> $DIR/impossible_range.rs:21:8
--> $DIR/impossible_range.rs:20:8
|
21 | ..=; //~ERROR inclusive range with no end
20 | ..=; //~ERROR inclusive range with no end
| ^
|
= help: inclusive ranges must be bounded at the end (`..=b` or `a..=b`)
error[E0586]: inclusive range with no end
--> $DIR/impossible_range.rs:22:9
--> $DIR/impossible_range.rs:27:9
|
22 | 0..=; //~ERROR inclusive range with no end
27 | 0..=; //~ERROR inclusive range with no end
| ^
|
= help: inclusive ranges must be bounded at the end (`..=b` or `a..=b`)
@@ -13,5 +13,3 @@ error: expected type, found keyword `true`
18 | foo!(true); //~ ERROR expected type, found keyword
| ^^^^ expecting a type here because of type ascription
error: aborting due to 2 previous errors
@@ -43,5 +43,3 @@ error: expected expression, found reserved keyword `typeof`
26 | m!();
| ----- in this macro invocation
error: aborting due to 4 previous errors
@@ -0,0 +1,31 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
use std::env;
pub struct Foo {
text: String
}
pub fn foo() -> Foo {
let args: Vec<String> = env::args().collect();
let text = args[1].clone();
pub Foo { text }
}
//~^^ ERROR missing `struct` for struct definition

This comment has been minimized.

@petrochenkov

petrochenkov Dec 16, 2017

Contributor

Could you add the example with fn (#46732 (comment)) as a test too?

This comment has been minimized.

@estebank

estebank Dec 17, 2017

Contributor

added

pub fn bar() -> Foo {
fn
Foo { text: "".to_string() }
}
//~^^ ERROR expected one of `(` or `<`, found `{`
fn main() {}
@@ -0,0 +1,18 @@
error: missing `struct` for struct definition
--> $DIR/recovered-block.rs:21:8
|
21 | pub Foo { text }
| ^
help: add `struct` here to parse `Foo` as a public struct
|
21 | pub struct Foo { text }
| ^^^^^^
error: expected one of `(` or `<`, found `{`
--> $DIR/recovered-block.rs:27:9
|
27 | Foo { text: "".to_string() }
| ^ expected one of `(` or `<` here
error: aborting due to 2 previous errors
@@ -16,4 +16,3 @@ fn main() {
}
//~^ ERROR: incorrect close delimiter: `}`
//~| ERROR: incorrect close delimiter: `}`
//~| ERROR: expected expression, found `)`
@@ -28,11 +28,5 @@ error: expected expression, found `;`
14 | foo(bar(;
| ^
error: expected expression, found `)`
--> $DIR/token-error-correct.rs:16:1
|
16 | }
| ^
error: aborting due to 4 previous errors
error: aborting due to 3 previous errors
Copy path View file
@@ -26,7 +26,7 @@
miri = "Broken"
# ping @Manishearth @llogiq @mcarton @oli-obk
clippy = "Testing"
clippy = "Broken"
# ping @nrc
rls = "Broken"
ProTip! Use n and p to navigate between commits in a pull request.