Skip to content

Commit

Permalink
Handle attempts to have multiple cfgd tail expressions
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
estebank committed Nov 16, 2023
1 parent 1be1e84 commit a16722d
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 @@ -722,6 +723,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 a16722d

Please sign in to comment.