Skip to content
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

Disallow ambiguous attributes on expressions #124099

Merged
merged 1 commit into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions compiler/rustc_parse/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,8 @@ parse_or_pattern_not_allowed_in_let_binding = top-level or-patterns are not allo
parse_out_of_range_hex_escape = out of range hex escape
.label = must be a character in the range [\x00-\x7f]

parse_outer_attr_ambiguous = ambiguous outer attributes

parse_outer_attr_explanation = outer attributes, like `#[test]`, annotate the item following them

parse_outer_attribute_not_allowed_on_if_else = outer attributes are not allowed on `if` and `else` branches
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_parse/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,15 @@ pub(crate) struct OuterAttributeNotAllowedOnIfElse {
pub attributes: Span,
}

#[derive(Diagnostic)]
#[diag(parse_outer_attr_ambiguous)]
pub(crate) struct AmbiguousOuterAttributes {
#[primary_span]
pub span: Span,
#[subdiagnostic]
pub sugg: WrapInParentheses,
}

#[derive(Diagnostic)]
#[diag(parse_missing_in_in_for_loop)]
pub(crate) struct MissingInInForLoop {
Expand Down
32 changes: 19 additions & 13 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,9 @@ impl<'a> Parser<'a> {
this.parse_expr_assoc_with(prec + prec_adjustment, LhsExpr::NotYetParsed)
})?;

let span = self.mk_expr_sp(&lhs, lhs_span, rhs.span);
self.error_ambiguous_outer_attrs(&lhs, lhs_span, rhs.span);
let span = lhs_span.to(rhs.span);

lhs = match op {
AssocOp::Add
| AssocOp::Subtract
Expand Down Expand Up @@ -426,6 +428,18 @@ impl<'a> Parser<'a> {
});
}

fn error_ambiguous_outer_attrs(&self, lhs: &P<Expr>, lhs_span: Span, rhs_span: Span) {
if let Some(attr) = lhs.attrs.iter().find(|a| a.style == AttrStyle::Outer) {
self.dcx().emit_err(errors::AmbiguousOuterAttributes {
span: attr.span.to(rhs_span),
sugg: errors::WrapInParentheses::Expression {
left: attr.span.shrink_to_lo(),
right: lhs_span.shrink_to_hi(),
},
});
}
}

/// Possibly translate the current token to an associative operator.
/// The method does not advance the current token.
///
Expand Down Expand Up @@ -506,7 +520,8 @@ impl<'a> Parser<'a> {
None
};
let rhs_span = rhs.as_ref().map_or(cur_op_span, |x| x.span);
let span = self.mk_expr_sp(&lhs, lhs.span, rhs_span);
self.error_ambiguous_outer_attrs(&lhs, lhs.span, rhs_span);
let span = lhs.span.to(rhs_span);
let limits =
if op == AssocOp::DotDot { RangeLimits::HalfOpen } else { RangeLimits::Closed };
let range = self.mk_range(Some(lhs), rhs, limits);
Expand Down Expand Up @@ -722,7 +737,8 @@ impl<'a> Parser<'a> {
expr_kind: fn(P<Expr>, P<Ty>) -> ExprKind,
) -> PResult<'a, P<Expr>> {
let mk_expr = |this: &mut Self, lhs: P<Expr>, rhs: P<Ty>| {
this.mk_expr(this.mk_expr_sp(&lhs, lhs_span, rhs.span), expr_kind(lhs, rhs))
this.error_ambiguous_outer_attrs(&lhs, lhs_span, rhs.span);
this.mk_expr(lhs_span.to(rhs.span), expr_kind(lhs, rhs))
};

// Save the state of the parser before parsing type normally, in case there is a
Expand Down Expand Up @@ -3807,16 +3823,6 @@ impl<'a> Parser<'a> {
self.mk_expr(span, ExprKind::Err(guar))
}

/// Create expression span ensuring the span of the parent node
/// is larger than the span of lhs and rhs, including the attributes.
fn mk_expr_sp(&self, lhs: &P<Expr>, lhs_span: Span, rhs_span: Span) -> Span {
lhs.attrs
.iter()
.find(|a| a.style == AttrStyle::Outer)
.map_or(lhs_span, |a| a.span)
.to(rhs_span)
}

fn collect_tokens_for_expr(
&mut self,
attrs: AttrWrapper,
Expand Down
6 changes: 3 additions & 3 deletions src/tools/clippy/tests/ui/cfg_attr_rustfmt.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ fn foo(

fn skip_on_statements() {
#[rustfmt::skip]
5+3;
{ 5+3; }
}

#[rustfmt::skip]
Expand All @@ -33,11 +33,11 @@ mod foo {
#[clippy::msrv = "1.29"]
fn msrv_1_29() {
#[cfg_attr(rustfmt, rustfmt::skip)]
1+29;
{ 1+29; }
}

#[clippy::msrv = "1.30"]
fn msrv_1_30() {
#[rustfmt::skip]
1+30;
{ 1+30; }
}
6 changes: 3 additions & 3 deletions src/tools/clippy/tests/ui/cfg_attr_rustfmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ fn foo(

fn skip_on_statements() {
#[cfg_attr(rustfmt, rustfmt::skip)]
5+3;
{ 5+3; }
}

#[cfg_attr(rustfmt, rustfmt_skip)]
Expand All @@ -33,11 +33,11 @@ mod foo {
#[clippy::msrv = "1.29"]
fn msrv_1_29() {
#[cfg_attr(rustfmt, rustfmt::skip)]
1+29;
{ 1+29; }
}

#[clippy::msrv = "1.30"]
fn msrv_1_30() {
#[cfg_attr(rustfmt, rustfmt::skip)]
1+30;
{ 1+30; }
}
4 changes: 2 additions & 2 deletions src/tools/rustfmt/tests/source/attrib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,8 @@ type Os = NoSource;
// #3313
fn stmt_expr_attributes() {
let foo ;
#[must_use]
foo = false ;
(#[must_use]
foo) = false ;
}

// #3509
Expand Down
4 changes: 2 additions & 2 deletions src/tools/rustfmt/tests/target/attrib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,8 @@ type Os = NoSource;
// #3313
fn stmt_expr_attributes() {
let foo;
#[must_use]
foo = false;
(#[must_use]
foo) = false;
}

// #3509
Expand Down
18 changes: 9 additions & 9 deletions tests/pretty/ast-stmt-expr-attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,17 @@ fn syntax() {
let _ = #[attr] ();
let _ = #[attr] (#[attr] 0,);
let _ = #[attr] (#[attr] 0, 0);
let _ = #[attr] 0 + #[attr] 0;
let _ = #[attr] 0 / #[attr] 0;
let _ = #[attr] 0 & #[attr] 0;
let _ = #[attr] 0 % #[attr] 0;
let _ = (#[attr] 0) + #[attr] 0;
let _ = (#[attr] 0) / #[attr] 0;
let _ = (#[attr] 0) & #[attr] 0;
let _ = (#[attr] 0) % #[attr] 0;
let _ = #[attr] (0 + 0);
let _ = #[attr] !0;
let _ = #[attr] -0;
let _ = #[attr] false;
let _ = #[attr] 0;
let _ = #[attr] 'c';
let _ = #[attr] x as Y;
let _ = (#[attr] x) as Y;
let _ = #[attr] (x as Y);
let _ =
#[attr] while true {
Expand Down Expand Up @@ -88,18 +88,18 @@ fn syntax() {
let _ = ();
foo
};
let _ = #[attr] x = y;
let _ = (#[attr] x) = y;
let _ = #[attr] (x = y);
let _ = #[attr] x += y;
let _ = (#[attr] x) += y;
let _ = #[attr] (x += y);
let _ = #[attr] foo.bar;
let _ = (#[attr] foo).bar;
let _ = #[attr] foo.0;
let _ = (#[attr] foo).0;
let _ = #[attr] foo[bar];
let _ = (#[attr] foo)[bar];
let _ = #[attr] 0..#[attr] 0;
let _ = #[attr] 0..;
let _ = (#[attr] 0)..#[attr] 0;
let _ = (#[attr] 0)..;
let _ = #[attr] (0..0);
let _ = #[attr] (0..);
let _ = #[attr] (..0);
Expand Down
12 changes: 6 additions & 6 deletions tests/pretty/stmt_expr_attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,13 +148,13 @@ fn _11() {
let _ = #[rustc_dummy] (0);
let _ = #[rustc_dummy] (0,);
let _ = #[rustc_dummy] (0, 0);
let _ = #[rustc_dummy] 0 + #[rustc_dummy] 0;
let _ = (#[rustc_dummy] 0) + #[rustc_dummy] 0;
let _ = #[rustc_dummy] !0;
let _ = #[rustc_dummy] -0i32;
let _ = #[rustc_dummy] false;
let _ = #[rustc_dummy] 'c';
let _ = #[rustc_dummy] 0;
let _ = #[rustc_dummy] 0 as usize;
let _ = (#[rustc_dummy] 0) as usize;
let _ =
#[rustc_dummy] while false {
#![rustc_dummy]
Expand Down Expand Up @@ -214,8 +214,8 @@ fn _11() {
#![rustc_dummy]
};
let mut x = 0;
let _ = #[rustc_dummy] x = 15;
let _ = #[rustc_dummy] x += 15;
let _ = (#[rustc_dummy] x) = 15;
let _ = (#[rustc_dummy] x) += 15;
let s = Foo { data: () };
let _ = #[rustc_dummy] s.data;
let _ = (#[rustc_dummy] s).data;
Expand All @@ -225,8 +225,8 @@ fn _11() {
let v = vec!(0);
let _ = #[rustc_dummy] v[0];
let _ = (#[rustc_dummy] v)[0];
let _ = #[rustc_dummy] 0..#[rustc_dummy] 0;
let _ = #[rustc_dummy] 0..;
let _ = (#[rustc_dummy] 0)..#[rustc_dummy] 0;
let _ = (#[rustc_dummy] 0)..;
let _ = #[rustc_dummy] (0..0);
let _ = #[rustc_dummy] (0..);
let _ = #[rustc_dummy] (..0);
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/lint/unused/unused-doc-comments-edge-cases.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ fn doc_comment_between_if_else(num: u8) -> bool {
}

fn doc_comment_on_expr(num: u8) -> bool {
/// useless doc comment
(/// useless doc comment
//~^ ERROR: attributes on expressions are experimental
//~| ERROR: unused doc comment
num == 3
num) == 3
}

fn doc_comment_on_expr_field() -> bool {
Expand Down
14 changes: 7 additions & 7 deletions tests/ui/lint/unused/unused-doc-comments-edge-cases.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ LL | else {
| ^^^^ expected expression

error[E0658]: attributes on expressions are experimental
--> $DIR/unused-doc-comments-edge-cases.rs:23:5
--> $DIR/unused-doc-comments-edge-cases.rs:23:6
|
LL | /// useless doc comment
| ^^^^^^^^^^^^^^^^^^^^^^^
LL | (/// useless doc comment
| ^^^^^^^^^^^^^^^^^^^^^^^
|
= note: see issue #15701 <https://github.com/rust-lang/rust/issues/15701> for more information
= help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable
Expand All @@ -32,12 +32,12 @@ LL | #![deny(unused_doc_comments)]
| ^^^^^^^^^^^^^^^^^^^

error: unused doc comment
--> $DIR/unused-doc-comments-edge-cases.rs:23:5
--> $DIR/unused-doc-comments-edge-cases.rs:23:6
|
LL | /// useless doc comment
| ^^^^^^^^^^^^^^^^^^^^^^^
LL | (/// useless doc comment
| ^^^^^^^^^^^^^^^^^^^^^^^
...
LL | num == 3
LL | num) == 3
| --- rustdoc does not generate documentation for expressions
|
= help: use `//` for a plain comment
Expand Down
15 changes: 15 additions & 0 deletions tests/ui/parser/attribute/attr-binary-expr-ambigous.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//@ run-rustfix
#![feature(stmt_expr_attributes)]
#![allow(unused_assignments, unused_attributes)]

fn main() {
let mut x = (#[deprecated] 1) + 2; //~ ERROR ambiguous

(#[deprecated] x) = 4; //~ ERROR ambiguous

x = (#[deprecated] 5) as i32; //~ ERROR ambiguous

let _r = (#[deprecated] 1)..6; //~ ERROR ambiguous

println!("{}", x);
}
15 changes: 15 additions & 0 deletions tests/ui/parser/attribute/attr-binary-expr-ambigous.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//@ run-rustfix
#![feature(stmt_expr_attributes)]
#![allow(unused_assignments, unused_attributes)]

fn main() {
let mut x = #[deprecated] 1 + 2; //~ ERROR ambiguous

#[deprecated] x = 4; //~ ERROR ambiguous

x = #[deprecated] 5 as i32; //~ ERROR ambiguous

let _r = #[deprecated] 1..6; //~ ERROR ambiguous

println!("{}", x);
}
46 changes: 46 additions & 0 deletions tests/ui/parser/attribute/attr-binary-expr-ambigous.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
error: ambiguous outer attributes
--> $DIR/attr-binary-expr-ambigous.rs:6:17
|
LL | let mut x = #[deprecated] 1 + 2;
| ^^^^^^^^^^^^^^^^^^^
|
help: wrap the expression in parentheses
|
LL | let mut x = (#[deprecated] 1) + 2;
| + +

error: ambiguous outer attributes
--> $DIR/attr-binary-expr-ambigous.rs:8:5
|
LL | #[deprecated] x = 4;
| ^^^^^^^^^^^^^^^^^^^
|
help: wrap the expression in parentheses
|
LL | (#[deprecated] x) = 4;
| + +

error: ambiguous outer attributes
--> $DIR/attr-binary-expr-ambigous.rs:10:9
|
LL | x = #[deprecated] 5 as i32;
| ^^^^^^^^^^^^^^^^^^^^^^
|
help: wrap the expression in parentheses
|
LL | x = (#[deprecated] 5) as i32;
| + +

error: ambiguous outer attributes
--> $DIR/attr-binary-expr-ambigous.rs:12:14
|
LL | let _r = #[deprecated] 1..6;
| ^^^^^^^^^^^^^^^^^^
|
help: wrap the expression in parentheses
|
LL | let _r = (#[deprecated] 1)..6;
| + +

error: aborting due to 4 previous errors

3 changes: 1 addition & 2 deletions tests/ui/proc-macro/issue-81555.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,5 @@ use test_macros::identity_attr;
fn main() {
let _x;
let y = ();
#[identity_attr]
_x = y;
(#[identity_attr] _x) = y;
}
Loading