Skip to content

Commit

Permalink
Rollup merge of #117988 - estebank:issue-106020, r=cjgillot
Browse files Browse the repository at this point in the history
Handle attempts to have multiple `cfg`d tail expressions

When encountering code that seems like it might be trying to have multiple tail expressions depending on `cfg` information, suggest alternatives that will success to parse.

```rust
fn foo() -> String {
    #[cfg(feature = "validation")]
    [1, 2, 3].iter().map(|c| c.to_string()).collect::<String>()
    #[cfg(not(feature = "validation"))]
    String::new()
}
```

```
error: expected `;`, found `#`
  --> $DIR/multiple-tail-expr-behind-cfg.rs:5:64
   |
LL |     #[cfg(feature = "validation")]
   |     ------------------------------ only `;` terminated statements or tail expressions are allowed after this attribute
LL |     [1, 2, 3].iter().map(|c| c.to_string()).collect::<String>()
   |                                                                ^ expected `;` here
LL |     #[cfg(not(feature = "validation"))]
   |     - unexpected token
   |
help: add `;` here
   |
LL |     [1, 2, 3].iter().map(|c| c.to_string()).collect::<String>();
   |                                                                +
help: alternatively, consider surrounding the expression with a block
   |
LL |     { [1, 2, 3].iter().map(|c| c.to_string()).collect::<String>() }
   |     +                                                             +
help: it seems like you are trying to provide different expressions depending on `cfg`, consider using `if cfg!(..)`
   |
LL ~     if cfg!(feature = "validation") {
LL ~         [1, 2, 3].iter().map(|c| c.to_string()).collect::<String>()
LL ~     } else if cfg!(not(feature = "validation")) {
LL ~         String::new()
LL +     }
   |
```

Fix #106020.

r? `@oli-obk`
  • Loading branch information
compiler-errors authored Nov 20, 2023
2 parents 6d33e90 + a16722d commit e6a3ca0
Show file tree
Hide file tree
Showing 4 changed files with 183 additions and 0 deletions.
96 changes: 96 additions & 0 deletions compiler/rustc_parse/src/parser/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use crate::errors::{

use crate::fluent_generated as fluent;
use crate::parser;
use crate::parser::attr::InnerAttrPolicy;
use rustc_ast as ast;
use rustc_ast::ptr::P;
use rustc_ast::token::{self, Delimiter, Lit, LitKind, TokenKind};
Expand Down Expand Up @@ -723,6 +724,101 @@ impl<'a> Parser<'a> {
Err(err)
}

pub(super) fn attr_on_non_tail_expr(&self, expr: &Expr) {
// Missing semicolon typo error.
let span = self.prev_token.span.shrink_to_hi();
let mut err = self.sess.create_err(ExpectedSemi {
span,
token: self.token.clone(),
unexpected_token_label: Some(self.token.span),
sugg: ExpectedSemiSugg::AddSemi(span),
});
let attr_span = match &expr.attrs[..] {
[] => unreachable!(),
[only] => only.span,
[first, rest @ ..] => {
for attr in rest {
err.span_label(attr.span, "");
}
first.span
}
};
err.span_label(
attr_span,
format!(
"only `;` terminated statements or tail expressions are allowed after {}",
if expr.attrs.len() == 1 { "this attribute" } else { "these attributes" },
),
);
if self.token == token::Pound
&& self.look_ahead(1, |t| t.kind == token::OpenDelim(Delimiter::Bracket))
{
// We have
// #[attr]
// expr
// #[not_attr]
// other_expr
err.span_label(span, "expected `;` here");
err.multipart_suggestion(
"alternatively, consider surrounding the expression with a block",
vec![
(expr.span.shrink_to_lo(), "{ ".to_string()),
(expr.span.shrink_to_hi(), " }".to_string()),
],
Applicability::MachineApplicable,
);
let mut snapshot = self.create_snapshot_for_diagnostic();
if let [attr] = &expr.attrs[..]
&& let ast::AttrKind::Normal(attr_kind) = &attr.kind
&& let [segment] = &attr_kind.item.path.segments[..]
&& segment.ident.name == sym::cfg
&& let Ok(next_attr) = snapshot.parse_attribute(InnerAttrPolicy::Forbidden(None))
&& let ast::AttrKind::Normal(next_attr_kind) = next_attr.kind
&& let [next_segment] = &next_attr_kind.item.path.segments[..]
&& segment.ident.name == sym::cfg
&& let Ok(next_expr) = snapshot.parse_expr()
{
// We have for sure
// #[cfg(..)]
// expr
// #[cfg(..)]
// other_expr
// So we suggest using `if cfg!(..) { expr } else if cfg!(..) { other_expr }`.
let margin = self.sess.source_map().span_to_margin(next_expr.span).unwrap_or(0);
let sugg = vec![
(attr.span.with_hi(segment.span().hi()), "if cfg!".to_string()),
(
attr_kind.item.args.span().unwrap().shrink_to_hi().with_hi(attr.span.hi()),
" {".to_string(),
),
(expr.span.shrink_to_lo(), " ".to_string()),
(
next_attr.span.with_hi(next_segment.span().hi()),
"} else if cfg!".to_string(),
),
(
next_attr_kind
.item
.args
.span()
.unwrap()
.shrink_to_hi()
.with_hi(next_attr.span.hi()),
" {".to_string(),
),
(next_expr.span.shrink_to_lo(), " ".to_string()),
(next_expr.span.shrink_to_hi(), format!("\n{}}}", " ".repeat(margin))),
];
err.multipart_suggestion(
"it seems like you are trying to provide different expressions depending on \
`cfg`, consider using `if cfg!(..)`",
sugg,
Applicability::MachineApplicable,
);
}
}
err.emit();
}
fn check_too_many_raw_str_terminators(&mut self, err: &mut Diagnostic) -> bool {
let sm = self.sess.source_map();
match (&self.prev_token.kind, &self.token.kind) {
Expand Down
14 changes: 14 additions & 0 deletions compiler/rustc_parse/src/parser/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,20 @@ impl<'a> Parser<'a> {
let mut add_semi_to_stmt = false;

match &mut stmt.kind {
// Expression without semicolon.
StmtKind::Expr(expr)
if classify::expr_requires_semi_to_be_stmt(expr)
&& !expr.attrs.is_empty()
&& ![token::Eof, token::Semi, token::CloseDelim(Delimiter::Brace)]
.contains(&self.token.kind) =>
{
// The user has written `#[attr] expr` which is unsupported. (#106020)
self.attr_on_non_tail_expr(&expr);
// We already emitted an error, so don't emit another type error
let sp = expr.span.to(self.prev_token.span);
*expr = self.mk_expr_err(sp);
}

// Expression without semicolon.
StmtKind::Expr(expr)
if self.token != token::Eof && classify::expr_requires_semi_to_be_stmt(expr) =>
Expand Down
19 changes: 19 additions & 0 deletions tests/ui/parser/attribute/multiple-tail-expr-behind-cfg.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#![feature(stmt_expr_attributes)]

fn foo() -> String {
#[cfg(feature = "validation")]
[1, 2, 3].iter().map(|c| c.to_string()).collect::<String>() //~ ERROR expected `;`, found `#`
#[cfg(not(feature = "validation"))]
String::new()
}

fn bar() -> String {
#[attr]
[1, 2, 3].iter().map(|c| c.to_string()).collect::<String>() //~ ERROR expected `;`, found `#`
#[attr] //~ ERROR cannot find attribute `attr` in this scope
String::new()
}

fn main() {
println!("{}", foo());
}
54 changes: 54 additions & 0 deletions tests/ui/parser/attribute/multiple-tail-expr-behind-cfg.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
error: expected `;`, found `#`
--> $DIR/multiple-tail-expr-behind-cfg.rs:5:64
|
LL | #[cfg(feature = "validation")]
| ------------------------------ only `;` terminated statements or tail expressions are allowed after this attribute
LL | [1, 2, 3].iter().map(|c| c.to_string()).collect::<String>()
| ^ expected `;` here
LL | #[cfg(not(feature = "validation"))]
| - unexpected token
|
help: add `;` here
|
LL | [1, 2, 3].iter().map(|c| c.to_string()).collect::<String>();
| +
help: alternatively, consider surrounding the expression with a block
|
LL | { [1, 2, 3].iter().map(|c| c.to_string()).collect::<String>() }
| + +
help: it seems like you are trying to provide different expressions depending on `cfg`, consider using `if cfg!(..)`
|
LL ~ if cfg!(feature = "validation") {
LL ~ [1, 2, 3].iter().map(|c| c.to_string()).collect::<String>()
LL ~ } else if cfg!(not(feature = "validation")) {
LL ~ String::new()
LL + }
|

error: expected `;`, found `#`
--> $DIR/multiple-tail-expr-behind-cfg.rs:12:64
|
LL | #[attr]
| ------- only `;` terminated statements or tail expressions are allowed after this attribute
LL | [1, 2, 3].iter().map(|c| c.to_string()).collect::<String>()
| ^ expected `;` here
LL | #[attr]
| - unexpected token
|
help: add `;` here
|
LL | [1, 2, 3].iter().map(|c| c.to_string()).collect::<String>();
| +
help: alternatively, consider surrounding the expression with a block
|
LL | { [1, 2, 3].iter().map(|c| c.to_string()).collect::<String>() }
| + +

error: cannot find attribute `attr` in this scope
--> $DIR/multiple-tail-expr-behind-cfg.rs:13:7
|
LL | #[attr]
| ^^^^

error: aborting due to 3 previous errors

0 comments on commit e6a3ca0

Please sign in to comment.