Skip to content

Commit

Permalink
review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
estebank committed Sep 23, 2019
1 parent 27801f7 commit 39ae744
Show file tree
Hide file tree
Showing 7 changed files with 200 additions and 115 deletions.
23 changes: 16 additions & 7 deletions src/libsyntax/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1255,17 +1255,26 @@ pub enum ExprKind {
}

impl ExprKind {
/// Whether this expression can appear in a contst argument without being surrounded by braces.
/// Whether this expression can appear applied to a `const` parameter without being surrounded
/// by braces.
///
/// Only used in error recovery.
pub(crate) fn is_valid_const_on_its_own(&self) -> bool {
crate fn is_valid_const_on_its_own(&self) -> bool {
fn is_const_lit(kind: &ExprKind) -> bool {
// These are the only literals that can be negated as a bare `const` argument.
match kind {
ExprKind::Lit(Lit { node: LitKind::Int(..), ..}) |
ExprKind::Lit(Lit { node: LitKind::Float(..), ..}) |
ExprKind::Lit(Lit { node: LitKind::FloatUnsuffixed(..), ..}) => true,
_ => false,
}
}

match self {
ExprKind::Tup(_) |
ExprKind::Lit(_) |
ExprKind::Type(..) |
ExprKind::Path(..) |
ExprKind::Unary(..) |
ExprKind::Lit(_) | // `foo::<42>()`
ExprKind::Err => true,
// `foo::<-42>()`
ExprKind::Unary(UnOp::Neg, expr) if is_const_lit(&expr.node) => true,
_ => false,
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/libsyntax/parse/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,9 @@ impl<'a> Parser<'a> {
(self.restrictions.contains(Restrictions::STMT_EXPR) &&
!classify::expr_requires_semi_to_be_stmt(e)) ||
(self.restrictions.contains(Restrictions::CONST_EXPR_RECOVERY) &&
(self.token == token::Lt || self.token == token::Gt || self.token == token::Comma))
// `<` is necessary here to avoid cases like `foo::< 1 < 3 >()` where we'll fallback
// to a regular parse error without recovery or suggestions.
[token::Lt, token::Gt, token::Comma].contains(&self.token.kind))
}

fn is_at_start_of_range_notation_rhs(&self) -> bool {
Expand Down
117 changes: 74 additions & 43 deletions src/libsyntax/parse/parser/path.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
use super::{Parser, PResult, Restrictions, TokenType};
use super::{P, Parser, PResult, Restrictions, TokenType};

use crate::{maybe_whole, ThinVec};
use crate::ast::{self, QSelf, Path, PathSegment, Ident, ParenthesizedArgs, AngleBracketedArgs};
use crate::ast::{AnonConst, GenericArg, AssocTyConstraint, AssocTyConstraintKind, BlockCheckMode};
use crate::ast::{
self, AngleBracketedArgs, AnonConst, AssocTyConstraint, AssocTyConstraintKind, BlockCheckMode,
Expr, GenericArg, Ident, ParenthesizedArgs, Path, PathSegment, QSelf,
};
use crate::parse::token::{self, Token};
use crate::source_map::{Span, BytePos};
use crate::symbol::kw;
Expand Down Expand Up @@ -412,54 +414,37 @@ impl<'a> Parser<'a> {
span,
});
assoc_ty_constraints.push(span);
} else if [
token::Not,
token::OpenDelim(token::Paren),
].contains(&self.token.kind) && self.look_ahead(1, |t| t.is_lit() || t.is_bool_lit()) {
// Parse bad `const` argument. `!` is only allowed here to go through
// `recover_bare_const_expr` for better diagnostics when encountering
// `foo::<!false>()`. `(` is allowed for the case `foo::<(1, 2, 3)>()`.

// This can't possibly be a valid const arg, it is likely missing braces.
let value = AnonConst {
id: ast::DUMMY_NODE_ID,
value: self.recover_bare_const_expr()?,
};
args.push(GenericArg::Const(value));
misplaced_assoc_ty_constraints.append(&mut assoc_ty_constraints);
} else if self.check_const_arg() {
// Parse const argument.
// Parse `const` argument.

// `const` arguments that don't require surrunding braces would have a length of
// one token, so anything that *isn't* surrounded by braces and is not
// immediately followed by `,` or `>` is not a valid `const` argument.
let invalid = self.look_ahead(1, |t| t != &token::Lt && t != &token::Comma);

let expr = if let token::OpenDelim(token::Brace) = self.token.kind {
// Parse `const` argument surrounded by braces.
self.parse_block_expr(
None, self.token.span, BlockCheckMode::Default, ThinVec::new()
)?
} else if invalid {
// This can't possibly be a valid const arg, it is likely missing braces.
let snapshot = self.clone();
match self.parse_expr_res(Restrictions::CONST_EXPR_RECOVERY, None) {
Ok(expr) => {
if self.token == token::Comma || self.token == token::Gt {
// We parsed the whole const argument successfully without braces.
if !expr.node.is_valid_const_on_its_own() {
// But it wasn't a literal, so we emit a custom error and
// suggest the appropriate code.
let msg =
"complex const arguments must be surrounded by braces";
let appl = Applicability::MachineApplicable;
self.span_fatal(expr.span, msg)
.multipart_suggestion(
"surround this const argument in braces",
vec![
(expr.span.shrink_to_lo(), "{ ".to_string()),
(expr.span.shrink_to_hi(), " }".to_string()),
],
appl,
)
.emit();
}
expr
} else {
// We parsed *some* expression, but it isn't the whole argument
// so we can't ensure it was a const argument with missing braces.
// Roll-back and emit a regular parser error.
mem::replace(self, snapshot);
self.parse_literal_maybe_minus()?
}
}
Err(mut err) => {
// We couldn't parse an expression successfully.
// Roll-back, hide the error and emit a regular parser error.
err.cancel();
mem::replace(self, snapshot);
self.parse_literal_maybe_minus()?
}
}
self.recover_bare_const_expr()?
} else if self.token.is_ident() {
// FIXME(const_generics): to distinguish between idents for types and consts,
// we should introduce a GenericArg::Ident in the AST and distinguish when
Expand Down Expand Up @@ -512,4 +497,50 @@ impl<'a> Parser<'a> {

Ok((args, constraints))
}

fn recover_bare_const_expr(&mut self) -> PResult<'a, P<Expr>> {
let snapshot = self.clone();
debug!("recover_bare_const_expr {:?}", self.token);
match self.parse_expr_res(Restrictions::CONST_EXPR_RECOVERY, None) {
Ok(expr) => {
debug!("recover_bare_const_expr expr {:?} {:?}", expr, expr.node);
if let token::Comma | token::Gt = self.token.kind {
// We parsed the whole const argument successfully without braces.
debug!("recover_bare_const_expr ok");
if !expr.node.is_valid_const_on_its_own() {
// But it wasn't a literal, so we emit a custom error and
// suggest the appropriate code. `foo::<-1>()` is valid but gets parsed
// here, so we need to gate the error only for invalid cases.
self.span_fatal(
expr.span,
"complex const arguments must be surrounded by braces",
).multipart_suggestion(
"surround this const argument in braces",
vec![
(expr.span.shrink_to_lo(), "{ ".to_string()),
(expr.span.shrink_to_hi(), " }".to_string()),
],
Applicability::MachineApplicable,
).emit();
}
Ok(expr)
} else {
debug!("recover_bare_const_expr not");
// We parsed *some* expression, but it isn't the whole argument
// so we can't ensure it was a const argument with missing braces.
// Roll-back and emit a regular parser error.
mem::replace(self, snapshot);
self.parse_literal_maybe_minus()
}
}
Err(mut err) => {
debug!("recover_bare_const_expr err");
// We couldn't parse an expression successfully.
// Roll-back, hide the error and emit a regular parser error.
err.cancel();
mem::replace(self, snapshot);
self.parse_literal_maybe_minus()
}
}
}
}
45 changes: 23 additions & 22 deletions src/test/ui/const-generics/const-expression-parameter.rs
Original file line number Diff line number Diff line change
@@ -1,39 +1,40 @@
#![feature(const_generics)]
//~^ WARN the feature `const_generics` is incomplete and may cause the compiler to crash

fn i32_identity<const X: i32>() -> i32 {
5
}
fn foo<const X: i32>() -> i32 { 5 }

fn foo_a() {
i32_identity::<-1>(); // ok
}
fn baz<const X: i32, const Y: i32>() { }

fn foo_b() {
i32_identity::<1 + 2>(); //~ ERROR complex const arguments must be surrounded by braces
}
fn bar<const X: bool>() {}

fn foo_c() {
i32_identity::< -1 >(); // ok
}
fn bat<const X: (i32, i32, i32)>() {}

fn foo_d() {
i32_identity::<1 + 2, 3 + 4>();
fn main() {
foo::<-1>(); // ok
foo::<1 + 2>(); //~ ERROR complex const arguments must be surrounded by braces
foo::< -1 >(); // ok
foo::<1 + 2, 3 + 4>();
//~^ ERROR complex const arguments must be surrounded by braces
//~| ERROR complex const arguments must be surrounded by braces
//~| ERROR wrong number of const arguments: expected 1, found 2
}
foo::<5>(); // ok

fn baz<const X: i32, const Y: i32>() -> i32 {
42
}

fn foo_e() {
baz::<-1, -2>(); // ok
baz::<1 + 2, 3 + 4>();
//~^ ERROR complex const arguments must be surrounded by braces
//~| ERROR complex const arguments must be surrounded by braces
baz::< -1 , 2 >(); // ok
baz::< -1 , "2" >(); //~ ERROR mismatched types

bat::<(1, 2, 3)>(); //~ ERROR complex const arguments must be surrounded by braces
bat::<(1, 2)>();
//~^ ERROR complex const arguments must be surrounded by braces
//~| ERROR mismatched types

bar::<false>(); // ok
bar::<!false>(); //~ ERROR complex const arguments must be surrounded by braces
}

fn main() {
i32_identity::<5>(); // ok
fn parse_err_1() {
bar::< 3 < 4 >(); //~ ERROR expected one of `,` or `>`, found `<`
}
99 changes: 77 additions & 22 deletions src/test/ui/const-generics/const-expression-parameter.stderr
Original file line number Diff line number Diff line change
@@ -1,35 +1,35 @@
error: complex const arguments must be surrounded by braces
--> $DIR/const-expression-parameter.rs:13:20
--> $DIR/const-expression-parameter.rs:14:11
|
LL | i32_identity::<1 + 2>();
| ^^^^^
LL | foo::<1 + 2>();
| ^^^^^
help: surround this const argument in braces
|
LL | i32_identity::<{ 1 + 2 }>();
| ^ ^
LL | foo::<{ 1 + 2 }>();
| ^ ^

error: complex const arguments must be surrounded by braces
--> $DIR/const-expression-parameter.rs:21:20
--> $DIR/const-expression-parameter.rs:16:11
|
LL | i32_identity::<1 + 2, 3 + 4>();
| ^^^^^
LL | foo::<1 + 2, 3 + 4>();
| ^^^^^
help: surround this const argument in braces
|
LL | i32_identity::<{ 1 + 2 }, 3 + 4>();
| ^ ^
LL | foo::<{ 1 + 2 }, 3 + 4>();
| ^ ^

error: complex const arguments must be surrounded by braces
--> $DIR/const-expression-parameter.rs:21:27
--> $DIR/const-expression-parameter.rs:16:18
|
LL | i32_identity::<1 + 2, 3 + 4>();
| ^^^^^
LL | foo::<1 + 2, 3 + 4>();
| ^^^^^
help: surround this const argument in braces
|
LL | i32_identity::<1 + 2, { 3 + 4 }>();
| ^ ^
LL | foo::<1 + 2, { 3 + 4 }>();
| ^ ^

error: complex const arguments must be surrounded by braces
--> $DIR/const-expression-parameter.rs:32:11
--> $DIR/const-expression-parameter.rs:23:11
|
LL | baz::<1 + 2, 3 + 4>();
| ^^^^^
Expand All @@ -39,7 +39,7 @@ LL | baz::<{ 1 + 2 }, 3 + 4>();
| ^ ^

error: complex const arguments must be surrounded by braces
--> $DIR/const-expression-parameter.rs:32:18
--> $DIR/const-expression-parameter.rs:23:18
|
LL | baz::<1 + 2, 3 + 4>();
| ^^^^^
Expand All @@ -48,6 +48,42 @@ help: surround this const argument in braces
LL | baz::<1 + 2, { 3 + 4 }>();
| ^ ^

error: complex const arguments must be surrounded by braces
--> $DIR/const-expression-parameter.rs:29:11
|
LL | bat::<(1, 2, 3)>();
| ^^^^^^^^^
help: surround this const argument in braces
|
LL | bat::<{ (1, 2, 3) }>();
| ^ ^

error: complex const arguments must be surrounded by braces
--> $DIR/const-expression-parameter.rs:30:11
|
LL | bat::<(1, 2)>();
| ^^^^^^
help: surround this const argument in braces
|
LL | bat::<{ (1, 2) }>();
| ^ ^

error: complex const arguments must be surrounded by braces
--> $DIR/const-expression-parameter.rs:35:11
|
LL | bar::<!false>();
| ^^^^^^
help: surround this const argument in braces
|
LL | bar::<{ !false }>();
| ^ ^

error: expected one of `,` or `>`, found `<`
--> $DIR/const-expression-parameter.rs:39:14
|
LL | bar::< 3 < 4 >();
| ^ expected one of `,` or `>` here

warning: the feature `const_generics` is incomplete and may cause the compiler to crash
--> $DIR/const-expression-parameter.rs:1:12
|
Expand All @@ -57,11 +93,30 @@ LL | #![feature(const_generics)]
= note: `#[warn(incomplete_features)]` on by default

error[E0107]: wrong number of const arguments: expected 1, found 2
--> $DIR/const-expression-parameter.rs:21:27
--> $DIR/const-expression-parameter.rs:16:18
|
LL | foo::<1 + 2, 3 + 4>();
| ^^^^^ unexpected const argument

error[E0308]: mismatched types
--> $DIR/const-expression-parameter.rs:27:17
|
LL | baz::< -1 , "2" >();
| ^^^ expected i32, found reference
|
= note: expected type `i32`
found type `&'static str`

error[E0308]: mismatched types
--> $DIR/const-expression-parameter.rs:30:11
|
LL | bat::<(1, 2)>();
| ^^^^^^ expected a tuple with 3 elements, found one with 2 elements
|
LL | i32_identity::<1 + 2, 3 + 4>();
| ^^^^^ unexpected const argument
= note: expected type `(i32, i32, i32)`
found type `(i32, i32)`

error: aborting due to 6 previous errors
error: aborting due to 12 previous errors

For more information about this error, try `rustc --explain E0107`.
Some errors have detailed explanations: E0107, E0308.
For more information about an error, try `rustc --explain E0107`.
Loading

0 comments on commit 39ae744

Please sign in to comment.