Skip to content

Commit 279f0a7

Browse files
estebankestebank
committed
review comments
1 parent 9a514b9 commit 279f0a7

File tree

7 files changed

+200
-115
lines changed

7 files changed

+200
-115
lines changed

‎src/libsyntax/ast.rs

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1254,17 +1254,26 @@ pub enum ExprKind {
12541254
}
12551255

12561256
impl ExprKind {
1257-
/// Whether this expression can appear in a contst argument without being surrounded by braces.
1257+
/// Whether this expression can appear applied to a `const` parameter without being surrounded
1258+
/// by braces.
12581259
///
12591260
/// Only used in error recovery.
1260-
pub(crate) fn is_valid_const_on_its_own(&self) -> bool {
1261+
crate fn is_valid_const_on_its_own(&self) -> bool {
1262+
fn is_const_lit(kind: &ExprKind) -> bool {
1263+
// These are the only literals that can be negated as a bare `const` argument.
1264+
match kind {
1265+
ExprKind::Lit(Lit { node: LitKind::Int(..), ..}) |
1266+
ExprKind::Lit(Lit { node: LitKind::Float(..), ..}) |
1267+
ExprKind::Lit(Lit { node: LitKind::FloatUnsuffixed(..), ..}) => true,
1268+
_ => false,
1269+
}
1270+
}
1271+
12611272
match self {
1262-
ExprKind::Tup(_) |
1263-
ExprKind::Lit(_) |
1264-
ExprKind::Type(..) |
1265-
ExprKind::Path(..) |
1266-
ExprKind::Unary(..) |
1273+
ExprKind::Lit(_) | // `foo::<42>()`
12671274
ExprKind::Err => true,
1275+
// `foo::<-42>()`
1276+
ExprKind::Unary(UnOp::Neg, expr) if is_const_lit(&expr.node) => true,
12681277
_ => false,
12691278
}
12701279
}

‎src/libsyntax/parse/parser/expr.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,9 @@ impl<'a> Parser<'a> {
355355
(self.restrictions.contains(Restrictions::STMT_EXPR) &&
356356
!classify::expr_requires_semi_to_be_stmt(e)) ||
357357
(self.restrictions.contains(Restrictions::CONST_EXPR_RECOVERY) &&
358-
(self.token == token::Lt || self.token == token::Gt || self.token == token::Comma))
358+
// `<` is necessary here to avoid cases like `foo::< 1 < 3 >()` where we'll fallback
359+
// to a regular parse error without recovery or suggestions.
360+
[token::Lt, token::Gt, token::Comma].contains(&self.token.kind))
359361
}
360362

361363
fn is_at_start_of_range_notation_rhs(&self) -> bool {

‎src/libsyntax/parse/parser/path.rs

Lines changed: 74 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
1-
use super::{Parser, PResult, Restrictions, TokenType};
1+
use super::{P, Parser, PResult, Restrictions, TokenType};
22

33
use crate::{maybe_whole, ThinVec};
4-
use crate::ast::{self, QSelf, Path, PathSegment, Ident, ParenthesizedArgs, AngleBracketedArgs};
5-
use crate::ast::{AnonConst, GenericArg, AssocTyConstraint, AssocTyConstraintKind, BlockCheckMode};
4+
use crate::ast::{
5+
self, AngleBracketedArgs, AnonConst, AssocTyConstraint, AssocTyConstraintKind, BlockCheckMode,
6+
Expr, GenericArg, Ident, ParenthesizedArgs, Path, PathSegment, QSelf,
7+
};
68
use crate::parse::token::{self, Token};
79
use crate::source_map::{Span, BytePos};
810
use crate::symbol::kw;
@@ -413,54 +415,37 @@ impl<'a> Parser<'a> {
413415
span,
414416
});
415417
assoc_ty_constraints.push(span);
418+
} else if [
419+
token::Not,
420+
token::OpenDelim(token::Paren),
421+
].contains(&self.token.kind) && self.look_ahead(1, |t| t.is_lit() || t.is_bool_lit()) {
422+
// Parse bad `const` argument. `!` is only allowed here to go through
423+
// `recover_bare_const_expr` for better diagnostics when encountering
424+
// `foo::<!false>()`. `(` is allowed for the case `foo::<(1, 2, 3)>()`.
425+
426+
// This can't possibly be a valid const arg, it is likely missing braces.
427+
let value = AnonConst {
428+
id: ast::DUMMY_NODE_ID,
429+
value: self.recover_bare_const_expr()?,
430+
};
431+
args.push(GenericArg::Const(value));
432+
misplaced_assoc_ty_constraints.append(&mut assoc_ty_constraints);
416433
} else if self.check_const_arg() {
417-
// Parse const argument.
434+
// Parse `const` argument.
435+
436+
// `const` arguments that don't require surrunding braces would have a length of
437+
// one token, so anything that *isn't* surrounded by braces and is not
438+
// immediately followed by `,` or `>` is not a valid `const` argument.
418439
let invalid = self.look_ahead(1, |t| t != &token::Lt && t != &token::Comma);
440+
419441
let expr = if let token::OpenDelim(token::Brace) = self.token.kind {
442+
// Parse `const` argument surrounded by braces.
420443
self.parse_block_expr(
421444
None, self.token.span, BlockCheckMode::Default, ThinVec::new()
422445
)?
423446
} else if invalid {
424447
// This can't possibly be a valid const arg, it is likely missing braces.
425-
let snapshot = self.clone();
426-
match self.parse_expr_res(Restrictions::CONST_EXPR_RECOVERY, None) {
427-
Ok(expr) => {
428-
if self.token == token::Comma || self.token == token::Gt {
429-
// We parsed the whole const argument successfully without braces.
430-
if !expr.node.is_valid_const_on_its_own() {
431-
// But it wasn't a literal, so we emit a custom error and
432-
// suggest the appropriate code.
433-
let msg =
434-
"complex const arguments must be surrounded by braces";
435-
let appl = Applicability::MachineApplicable;
436-
self.span_fatal(expr.span, msg)
437-
.multipart_suggestion(
438-
"surround this const argument in braces",
439-
vec![
440-
(expr.span.shrink_to_lo(), "{ ".to_string()),
441-
(expr.span.shrink_to_hi(), " }".to_string()),
442-
],
443-
appl,
444-
)
445-
.emit();
446-
}
447-
expr
448-
} else {
449-
// We parsed *some* expression, but it isn't the whole argument
450-
// so we can't ensure it was a const argument with missing braces.
451-
// Roll-back and emit a regular parser error.
452-
mem::replace(self, snapshot);
453-
self.parse_literal_maybe_minus()?
454-
}
455-
}
456-
Err(mut err) => {
457-
// We couldn't parse an expression successfully.
458-
// Roll-back, hide the error and emit a regular parser error.
459-
err.cancel();
460-
mem::replace(self, snapshot);
461-
self.parse_literal_maybe_minus()?
462-
}
463-
}
448+
self.recover_bare_const_expr()?
464449
} else if self.token.is_ident() {
465450
// FIXME(const_generics): to distinguish between idents for types and consts,
466451
// we should introduce a GenericArg::Ident in the AST and distinguish when
@@ -513,4 +498,50 @@ impl<'a> Parser<'a> {
513498

514499
Ok((args, constraints))
515500
}
501+
502+
fn recover_bare_const_expr(&mut self) -> PResult<'a, P<Expr>> {
503+
let snapshot = self.clone();
504+
debug!("recover_bare_const_expr {:?}", self.token);
505+
match self.parse_expr_res(Restrictions::CONST_EXPR_RECOVERY, None) {
506+
Ok(expr) => {
507+
debug!("recover_bare_const_expr expr {:?} {:?}", expr, expr.node);
508+
if let token::Comma | token::Gt = self.token.kind {
509+
// We parsed the whole const argument successfully without braces.
510+
debug!("recover_bare_const_expr ok");
511+
if !expr.node.is_valid_const_on_its_own() {
512+
// But it wasn't a literal, so we emit a custom error and
513+
// suggest the appropriate code. `foo::<-1>()` is valid but gets parsed
514+
// here, so we need to gate the error only for invalid cases.
515+
self.span_fatal(
516+
expr.span,
517+
"complex const arguments must be surrounded by braces",
518+
).multipart_suggestion(
519+
"surround this const argument in braces",
520+
vec![
521+
(expr.span.shrink_to_lo(), "{ ".to_string()),
522+
(expr.span.shrink_to_hi(), " }".to_string()),
523+
],
524+
Applicability::MachineApplicable,
525+
).emit();
526+
}
527+
Ok(expr)
528+
} else {
529+
debug!("recover_bare_const_expr not");
530+
// We parsed *some* expression, but it isn't the whole argument
531+
// so we can't ensure it was a const argument with missing braces.
532+
// Roll-back and emit a regular parser error.
533+
mem::replace(self, snapshot);
534+
self.parse_literal_maybe_minus()
535+
}
536+
}
537+
Err(mut err) => {
538+
debug!("recover_bare_const_expr err");
539+
// We couldn't parse an expression successfully.
540+
// Roll-back, hide the error and emit a regular parser error.
541+
err.cancel();
542+
mem::replace(self, snapshot);
543+
self.parse_literal_maybe_minus()
544+
}
545+
}
546+
}
516547
}

‎src/test/ui/const-generics/const-expression-parameter.rs

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,40 @@
11
#![feature(const_generics)]
22
//~^ WARN the feature `const_generics` is incomplete and may cause the compiler to crash
33

4-
fn i32_identity<const X: i32>() -> i32 {
5-
5
6-
}
4+
fn foo<const X: i32>() -> i32 { 5 }
75

8-
fn foo_a() {
9-
i32_identity::<-1>(); // ok
10-
}
6+
fn baz<const X: i32, const Y: i32>() { }
117

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

16-
fn foo_c() {
17-
i32_identity::< -1 >(); // ok
18-
}
10+
fn bat<const X: (i32, i32, i32)>() {}
1911

20-
fn foo_d() {
21-
i32_identity::<1 + 2, 3 + 4>();
12+
fn main() {
13+
foo::<-1>(); // ok
14+
foo::<1 + 2>(); //~ ERROR complex const arguments must be surrounded by braces
15+
foo::< -1 >(); // ok
16+
foo::<1 + 2, 3 + 4>();
2217
//~^ ERROR complex const arguments must be surrounded by braces
2318
//~| ERROR complex const arguments must be surrounded by braces
2419
//~| ERROR wrong number of const arguments: expected 1, found 2
25-
}
20+
foo::<5>(); // ok
2621

27-
fn baz<const X: i32, const Y: i32>() -> i32 {
28-
42
29-
}
30-
31-
fn foo_e() {
22+
baz::<-1, -2>(); // ok
3223
baz::<1 + 2, 3 + 4>();
3324
//~^ ERROR complex const arguments must be surrounded by braces
3425
//~| ERROR complex const arguments must be surrounded by braces
26+
baz::< -1 , 2 >(); // ok
27+
baz::< -1 , "2" >(); //~ ERROR mismatched types
28+
29+
bat::<(1, 2, 3)>(); //~ ERROR complex const arguments must be surrounded by braces
30+
bat::<(1, 2)>();
31+
//~^ ERROR complex const arguments must be surrounded by braces
32+
//~| ERROR mismatched types
33+
34+
bar::<false>(); // ok
35+
bar::<!false>(); //~ ERROR complex const arguments must be surrounded by braces
3536
}
3637

37-
fn main() {
38-
i32_identity::<5>(); // ok
38+
fn parse_err_1() {
39+
bar::< 3 < 4 >(); //~ ERROR expected one of `,` or `>`, found `<`
3940
}

‎src/test/ui/const-generics/const-expression-parameter.stderr

Lines changed: 77 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,35 @@
11
error: complex const arguments must be surrounded by braces
2-
--> $DIR/const-expression-parameter.rs:13:20
2+
--> $DIR/const-expression-parameter.rs:14:11
33
|
4-
LL | i32_identity::<1 + 2>();
5-
| ^^^^^
4+
LL | foo::<1 + 2>();
5+
| ^^^^^
66
help: surround this const argument in braces
77
|
8-
LL | i32_identity::<{ 1 + 2 }>();
9-
| ^ ^
8+
LL | foo::<{ 1 + 2 }>();
9+
| ^ ^
1010

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

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

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

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

51+
error: complex const arguments must be surrounded by braces
52+
--> $DIR/const-expression-parameter.rs:29:11
53+
|
54+
LL | bat::<(1, 2, 3)>();
55+
| ^^^^^^^^^
56+
help: surround this const argument in braces
57+
|
58+
LL | bat::<{ (1, 2, 3) }>();
59+
| ^ ^
60+
61+
error: complex const arguments must be surrounded by braces
62+
--> $DIR/const-expression-parameter.rs:30:11
63+
|
64+
LL | bat::<(1, 2)>();
65+
| ^^^^^^
66+
help: surround this const argument in braces
67+
|
68+
LL | bat::<{ (1, 2) }>();
69+
| ^ ^
70+
71+
error: complex const arguments must be surrounded by braces
72+
--> $DIR/const-expression-parameter.rs:35:11
73+
|
74+
LL | bar::<!false>();
75+
| ^^^^^^
76+
help: surround this const argument in braces
77+
|
78+
LL | bar::<{ !false }>();
79+
| ^ ^
80+
81+
error: expected one of `,` or `>`, found `<`
82+
--> $DIR/const-expression-parameter.rs:39:14
83+
|
84+
LL | bar::< 3 < 4 >();
85+
| ^ expected one of `,` or `>` here
86+
5187
warning: the feature `const_generics` is incomplete and may cause the compiler to crash
5288
--> $DIR/const-expression-parameter.rs:1:12
5389
|
@@ -57,11 +93,30 @@ LL | #![feature(const_generics)]
5793
= note: `#[warn(incomplete_features)]` on by default
5894

5995
error[E0107]: wrong number of const arguments: expected 1, found 2
60-
--> $DIR/const-expression-parameter.rs:21:27
96+
--> $DIR/const-expression-parameter.rs:16:18
97+
|
98+
LL | foo::<1 + 2, 3 + 4>();
99+
| ^^^^^ unexpected const argument
100+
101+
error[E0308]: mismatched types
102+
--> $DIR/const-expression-parameter.rs:27:17
103+
|
104+
LL | baz::< -1 , "2" >();
105+
| ^^^ expected i32, found reference
106+
|
107+
= note: expected type `i32`
108+
found type `&'static str`
109+
110+
error[E0308]: mismatched types
111+
--> $DIR/const-expression-parameter.rs:30:11
112+
|
113+
LL | bat::<(1, 2)>();
114+
| ^^^^^^ expected a tuple with 3 elements, found one with 2 elements
61115
|
62-
LL | i32_identity::<1 + 2, 3 + 4>();
63-
| ^^^^^ unexpected const argument
116+
= note: expected type `(i32, i32, i32)`
117+
found type `(i32, i32)`
64118

65-
error: aborting due to 6 previous errors
119+
error: aborting due to 12 previous errors
66120

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

0 commit comments

Comments
 (0)