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

Tracking issue for stmt_expr_attributes: Add attributes to expressions, etc. #15701

Open
brson opened this Issue Jul 15, 2014 · 85 comments

Comments

Projects
None yet
@brson
Contributor

brson commented Jul 15, 2014

Tracking RFC 40: rust-lang/rfcs#16

@brson

This comment has been minimized.

Contributor

brson commented Jul 16, 2014

cc @huonw

@liigo

This comment has been minimized.

Contributor

liigo commented Dec 30, 2014

ping. will this be implemented before 1.0?

@kmcallister

This comment has been minimized.

Contributor

kmcallister commented Jan 14, 2015

The RFC suggests

The sort of things one could do with other arbitrary annotations are

#[allowed_unsafe_actions(ffi)]
#[audited="2014-04-22"]
unsafe { ... }

and then have an external tool that checks that that unsafe block's only unsafe actions are FFI, or a tool that lists blocks that have been changed since the last audit or haven't been audited ever.

which inspired me to make a plugin for cryptographically signing unsafe blocks. It would be great to put those signatures on the blocks themselves rather than the enclosing function.

@alexcrichton alexcrichton added the T-lang label Aug 11, 2015

@jonas-schievink

This comment has been minimized.

Contributor

jonas-schievink commented Sep 6, 2015

The RFC says:

Attributes bind tighter than any operator, that is #[attr] x op y is always parsed as (#[attr] x) op y.

Does this mean that #[attr] x.f() should be parsed as (#[attr] x).f()? And should #[a] f() be the same as (#[a] f)()?

And what happens if an attribute is applied to a syntax sugar construct that is later expanded, for example #[attr] in <place> { <expr> }? There isn't really an obvious place for the attribute in the desugared form, so not allowing them seems more sensible.

@Kimundi

This comment has been minimized.

Member

Kimundi commented Nov 5, 2015

Just a heads up that I'm currently in the process of implementing this RFC.

@dgrunwald

This comment has been minimized.

Contributor

dgrunwald commented Nov 5, 2015

I think the obvious choice is to give attributes the same operator precedence as unary operators.
So #[attr] x.f() would be parsed as #[attr] (x.f()), but #[attr] x + f will be (#[attr] x) + f.

@Kimundi

This comment has been minimized.

Member

Kimundi commented Nov 6, 2015

Yeah, thats how I'm doing it in my WIP branch right now.

bors added a commit that referenced this issue Dec 3, 2015

Auto merge of #29850 - Kimundi:attributes_that_make_a_statement, r=pn…
…kfelix

See rust-lang/rfcs#16 and #15701

- Added syntax support for attributes on expressions and all syntax nodes in statement position.
- Extended `#[cfg]` folder to allow removal of statements, and
of expressions in optional positions like expression lists and trailing
block expressions.
- Extended lint checker to recognize lint levels on expressions and
locals.
- As per RFC, attributes are not yet accepted on `if` expressions.

Examples:
  ```rust
let x = y;
{
        ...
}
assert_eq!((1, #[cfg(unset)] 2, 3), (1, 3));

let FOO = 0;
```

Implementation wise, there are a few rough corners and open questions:
- The parser work ended up a bit ugly.
- The pretty printer change was based mostly on guessing.
- Similar to the `if` case, there are some places in the grammar where a new `Expr` node starts,
  but where it seemed weird to accept attributes and hence the parser doesn't. This includes:
  - const expressions in patterns
  - in the middle of an postfix operator chain (that is, after `.`, before indexing, before calls)
  - on range expressions, since `#[attr] x .. y` parses as  `(#[attr] x) .. y`, which is inconsistent with
    `#[attr] .. y` which would parse as `#[attr] (.. y)`
- Attributes are added as additional `Option<Box<Vec<Attribute>>>` fields in expressions and locals.
- Memory impact has not been measured yet.
- A cfg-away trailing expression in a block does not currently promote the previous `StmtExpr` in a block to a new trailing expr. That is to say, this won't work:
```rust
let x = {
    #[cfg(foo)]
    Foo { data: x }
    #[cfg(not(foo))]
    Foo { data: y }
};
```
- One-element tuples can have their inner expression removed to become Unit, but just Parenthesis can't. Eg, `(#[cfg(unset)] x,) == ()` but `(#[cfg(unset)] x) == error`. This seemed reasonable to me since tuples and unit are type constructors, but could probably be argued either way.
- Attributes on macro nodes are currently unconditionally dropped during macro expansion, which seemed fine since macro disappear at that point?
- Attributes on `ast::ExprParens` will be prepend-ed to the inner expression in the hir folder.
- The work on pretty printer tests for this did trigger, but not fix errors regarding macros:
  - expression `foo![]` prints as `foo!()`
  - expression `foo!{}` prints as `foo!()`
  - statement `foo![];` prints as `foo!();`
  - statement `foo!{};` prints as `foo!();`
  - statement `foo!{}` triggers a `None` unwrap ICE.

bors added a commit that referenced this issue Dec 4, 2015

Auto merge of #29850 - Kimundi:attributes_that_make_a_statement, r=pn…
…kfelix

See rust-lang/rfcs#16 and #15701

- Added syntax support for attributes on expressions and all syntax nodes in statement position.
- Extended `#[cfg]` folder to allow removal of statements, and
of expressions in optional positions like expression lists and trailing
block expressions.
- Extended lint checker to recognize lint levels on expressions and
locals.
- As per RFC, attributes are not yet accepted on `if` expressions.

Examples:
  ```rust
let x = y;
{
        ...
}
assert_eq!((1, #[cfg(unset)] 2, 3), (1, 3));

let FOO = 0;
```

Implementation wise, there are a few rough corners and open questions:
- The parser work ended up a bit ugly.
- The pretty printer change was based mostly on guessing.
- Similar to the `if` case, there are some places in the grammar where a new `Expr` node starts,
  but where it seemed weird to accept attributes and hence the parser doesn't. This includes:
  - const expressions in patterns
  - in the middle of an postfix operator chain (that is, after `.`, before indexing, before calls)
  - on range expressions, since `#[attr] x .. y` parses as  `(#[attr] x) .. y`, which is inconsistent with
    `#[attr] .. y` which would parse as `#[attr] (.. y)`
- Attributes are added as additional `Option<Box<Vec<Attribute>>>` fields in expressions and locals.
- Memory impact has not been measured yet.
- A cfg-away trailing expression in a block does not currently promote the previous `StmtExpr` in a block to a new trailing expr. That is to say, this won't work:
```rust
let x = {
    #[cfg(foo)]
    Foo { data: x }
    #[cfg(not(foo))]
    Foo { data: y }
};
```
- One-element tuples can have their inner expression removed to become Unit, but just Parenthesis can't. Eg, `(#[cfg(unset)] x,) == ()` but `(#[cfg(unset)] x) == error`. This seemed reasonable to me since tuples and unit are type constructors, but could probably be argued either way.
- Attributes on macro nodes are currently unconditionally dropped during macro expansion, which seemed fine since macro disappear at that point?
- Attributes on `ast::ExprParens` will be prepend-ed to the inner expression in the hir folder.
- The work on pretty printer tests for this did trigger, but not fix errors regarding macros:
  - expression `foo![]` prints as `foo!()`
  - expression `foo!{}` prints as `foo!()`
  - statement `foo![];` prints as `foo!();`
  - statement `foo!{};` prints as `foo!();`
  - statement `foo!{}` triggers a `None` unwrap ICE.
@huonw

This comment has been minimized.

Member

huonw commented Jan 5, 2016

@Kimundi, is there a reason that #29850 doesn't close this?

@Kimundi

This comment has been minimized.

Member

Kimundi commented Jan 7, 2016

Not as far as I can rember, probably just forgot about it. :)

@Kimundi Kimundi closed this Jan 7, 2016

@comex

This comment has been minimized.

Contributor

comex commented Feb 25, 2016

FYI, the stmt_expr_attributes feature gate still mentions this issue number.

@bluss

This comment has been minimized.

Contributor

bluss commented Apr 10, 2016

Tracking issues are open until the feature is stable, aren't they?

@bluss bluss reopened this Apr 10, 2016

matklad added a commit to intellij-rust/intellij-rust that referenced this issue Apr 18, 2016

(GRAM): support attributes on statements.
Not stable, but used in the compiler.

Tracking issue: rust-lang/rust#15701
@msjyoo

This comment has been minimized.

msjyoo commented Jul 10, 2016

By what Rust version will this feature gate be allowed on the stable channel? I couldn't find any chart / matrix with the list of them.

@Kimundi

This comment has been minimized.

Member

Kimundi commented Mar 9, 2017

@nikomatsakis: I'd be fine with that as a initial "punting on it" restriction, but I have the feeling that with a sufficiently advanced macro and attribute system we could have a few nice syntactic opportunities in the future.

I'm mainly thinking of what I'd call "attribute-based macros" - syntatic transformations that start off as an attribute on a pice of normal code.

As an example, I could imagine an attribute to turn a lambda expression into an AST similar to what is possible in C# with lambdas to create LINQ queries:

let x: Expression = #[linq] |x, y| x + y;

While I don't see this becoming possible or stable anytime soon, I would at least like to hold open the door for possibilities like this.


(Similarly, I still hold hope for getting ride of the [] in attributes in at least some circumstances)

@comex

This comment has been minimized.

Contributor

comex commented Mar 9, 2017

@Kimundi What would the advantage be of that over linq!(|x, y| x + y)? They both require about the same amount of typing, although I guess yours looks a little less 'intrusive'.

@Kimundi

This comment has been minimized.

Member

Kimundi commented Mar 10, 2017

@comex:

One advantage I can see is that in the attribute case you would still get Rust syntax checking and potentially highlighting for the expression, while in the latter macro case you explicitly left Rust standard grammar, and thus the compiler or editor won't be able to check it as such.

The other advantage is indeed just mainly that it (subjectively) would look nicer.

I realize that these are weak reasons though, and that this might spawn a heated debate of what macros and syntax extensions should or should not be able to do or be used for. ;)

@mbrubeck mbrubeck changed the title from Add attributes to expressions, etc. to Tracking issue for stmt_expr_attributes: Add attributes to expressions, etc. May 1, 2017

@Ygg01

This comment has been minimized.

Ygg01 commented Jun 23, 2017

@comex Also people dislike using linq!(...), since rustfmt for example just doesn't format macros at all.

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Jul 17, 2017

@Kimundi

One advantage I can see is that in the attribute case you would still get Rust syntax checking and potentially highlighting for the expression, while in the latter macro case you explicitly left Rust standard grammar, and thus the compiler or editor won't be able to check it as such.

Interesting. I think we do anticipate, at least when decorating functions, that we'd accept a superset of Rust grammar within decorated functions, so that you can do things like add an await keyword and the like, but I guess that there are advantages to enforcing Rust grammar more strictly as well. But that seems a bit like a false comfort -- in particular, until you see the expansion, you can't e.g. do name resolution etc necessarily.

@liigo

This comment has been minimized.

Contributor

liigo commented Jul 18, 2017

An idea of attributes pair like xml?
#[foo..] expr/stmt/blocks #[/foo]

@nagisa

This comment has been minimized.

Contributor

nagisa commented Aug 4, 2017

Relevant issue #43494

I still think it is a good idea to work on stabilising different groups of expressions, starting with the most trivial and useful of all -- blocks, so e.g. { #![cfg(...)] ... } could be used.

We can then think how and if we want to stabilise the other sort of statement/expression attributes.

@liigo that’s just a

#[foo] {
    exprs/stmts/blocks
}
// or
{ #![foo] 
    exprs/stmts/blocks
}
@solson

This comment has been minimized.

Member

solson commented Aug 4, 2017

I still think it is a good idea to work on stabilising different groups of expressions, starting with the most trivial and useful of all -- blocks, so e.g. { #![cfg(...)] ... } could be used.

This form is already stable:

We can also already use #[] directly on statements. Only expression attributes remain unstable and undecided.

@nagisa

This comment has been minimized.

Contributor

nagisa commented Aug 4, 2017

@solson It is only stable for blocks in statement position.

@solson

This comment has been minimized.

Member

solson commented Aug 5, 2017

Ah, fair point. In that case, I would think it an odd decision to stabilize just blocks in expression position next, instead of all expressions. I think we already came up with reasonable rules for parsing above, and we should have an FCP about implementing and stabilizing for all expressions if there are no major objections.

@jonhoo

This comment has been minimized.

Contributor

jonhoo commented Nov 30, 2017

To make this easier to find for others, this issue (afaict) is why we can't have cfg attributes on if statements:

#[cfg(test)]
if true {
    println!("testing");
}

fails to compile with

attributes are not yet allowed on `if` expressions

Perhaps it'd be useful to make that error include a reference to this issue?

@xftroxgpx

This comment has been minimized.

xftroxgpx commented Feb 27, 2018

This seems to be the proper place to raise this: (if not, let me know)

Why is the {} block necessary for cfg attributes on macro calls ?

and I'm generalizing that based on only this one example using the println! macro and this clippy lint print_stdout so, I might be wrong in my loaded question assumptions. (in which case, please correct me)

#![feature(plugin)]
#![plugin(clippy)]

#![deny(print_stdout)]

fn main() {

    #[allow(print_stdout)] {
        println!("Hello, world!"); // all good now
    }

    {
        #![allow(print_stdout)]
        println!("Hello, world!"); // all good
    }

    #[allow(print_stdout)]
    println!("Hello, world!"); //error: use of `println!`
}

It feels like there's a multi-line behind-the-scenes expansion of println!. Or is there something clippy does wrong? (I tried asking on IRC...)

What is actually happening and will this be supported when this issue is closed?

  1. $ cargo new --bin proj1
  2. Add line in Cargo.toml for clippy
[dependencies]
clippy = "*"
  1. replace src/main.rs contents with the above example
  2. $ cargo build
error: use of `println!`
  --> /tmp/proj1/src/main.rs:18:5
   |
18 |     println!("Hello, world!"); //error: use of `println!`
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
note: lint level defined here
  --> /tmp/proj1/src/main.rs:4:9
   |
4  | #![deny(print_stdout)]
   |         ^^^^^^^^^^^^
   = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.187/index.html#print_stdout

error: aborting due to previous error

error: Could not compile `proj1`.

To learn more, run the command again with --verbose.
@eddyb

This comment has been minimized.

Member

eddyb commented Feb 28, 2018

Your first two examples are identical, the second one uses #! so it applies to the enclosing block instead of the macro invocation. Maybe you know this already but it tripped me up a bit since I didn't initially notice.

@xftroxgpx

This comment has been minimized.

xftroxgpx commented Feb 28, 2018

@eddyb sorry :D I didn't mean to include that one initially, but the first I've seen of it was here in this thread, in these two comments from above:
#15701 (comment)
#15701 (comment)

and I thought it was kinda clever and wanted to have it added, for completion(so to have all known ways to add attrs to macro calls - in my case), though I initially wanted to include only first and third examples only

@eddyb

This comment has been minimized.

Member

eddyb commented Mar 1, 2018

Well, neither of the first two examples attach an attribute to the macro invocation, but to a block surrounding it - maybe macro expansion just discards the attribute in the third example?
cc @jseyfried

zackmdavis added a commit to zackmdavis/rust that referenced this issue Mar 11, 2018

in which the fn-must-use codepath is prevented from panicking on closure
The must-use lint needs the DefId of called functions and method
receivers in order to look for a `#[must_use]` attribute, but this would
ICE (!) if a called function was actually a closure (with a non-unit
return value). Instead, let's be specific that we want a `Def::Fn`,
rather than blithely assuming that we can get the DefId of a qpath.

Supporting must-use closures doesn't seem like a priority, but could
conceivably be added in the future if desired (conditional on the
statement and expression attributes (rust-lang#15701) story being amicable).

zackmdavis added a commit to zackmdavis/rust that referenced this issue Apr 26, 2018

in which the fn-must-use codepath is prevented from panicking on closure
The must-use lint needs the DefId of called functions and method
receivers in order to look for a `#[must_use]` attribute, but this would
ICE (!) if a called function was actually a closure (with a non-unit
return value). Instead, let's be specific that we want a `Def::Fn`,
rather than blithely assuming that we can get the DefId of a qpath.

Supporting must-use closures doesn't seem like a priority, but could
conceivably be added in the future if desired (conditional on the
statement and expression attributes (rust-lang#15701) story being amicable).

zackmdavis added a commit to zackmdavis/rust that referenced this issue Apr 28, 2018

in which the fn-must-use codepath is prevented from panicking on closure
The must-use lint needs the DefId of called functions and method
receivers in order to look for a `#[must_use]` attribute, but this would
ICE (!) if a called function was actually a closure (with a non-unit
return value). Instead, let's be specific that we want a `Def::Fn`,
rather than blithely assuming that we can get the DefId of a qpath.

Supporting must-use closures doesn't seem like a priority, but could
conceivably be added in the future if desired (conditional on the
statement and expression attributes (rust-lang#15701) story being amicable).

zackmdavis added a commit to zackmdavis/rust that referenced this issue Apr 28, 2018

in which the fn-must-use codepath is prevented from panicking on closure
The must-use lint needs the DefId of called functions and method
receivers in order to look for a `#[must_use]` attribute, but this would
ICE (!) if a called function was actually a closure (with a non-unit
return value). Instead, let's be specific that we want a `Def::Fn`,
rather than blithely assuming that we can get the DefId of a qpath.

Supporting must-use closures doesn't seem like a priority, but could
conceivably be added in the future if desired (conditional on the
statement and expression attributes (rust-lang#15701) story being amicable).

zackmdavis added a commit to zackmdavis/rust that referenced this issue Apr 29, 2018

in which the fn-must-use codepath is prevented from panicking on closure
The must-use lint needs the DefId of called functions and method
receivers in order to look for a `#[must_use]` attribute, but this would
ICE (!) if a called function was actually a closure (with a non-unit
return value). Instead, let's be specific that we want a `Def::Fn`,
rather than blithely assuming that we can get the DefId of a qpath.

Supporting must-use closures doesn't seem like a priority, but could
conceivably be added in the future if desired (conditional on the
statement and expression attributes (rust-lang#15701) story being amicable).

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue May 3, 2018

in which the fn-must-use codepath is prevented from panicking on closure
The must-use lint needs the DefId of called functions and method
receivers in order to look for a `#[must_use]` attribute, but this would
ICE (!) if a called function was actually a closure (with a non-unit
return value). Instead, let's be specific that we want a `Def::Fn`,
rather than blithely assuming that we can get the DefId of a qpath.

Supporting must-use closures doesn't seem like a priority, but could
conceivably be added in the future if desired (conditional on the
statement and expression attributes (rust-lang#15701) story being amicable).
@goffrie

This comment has been minimized.

Contributor

goffrie commented May 22, 2018

Is it expected that attributes on function arguments are allowed?

fn main() {
    drop(#[cfg(never)] 1, #[cfg(not(never))] 2); // compiles fine on stable
}

edit: never mind, I see it was mentioned in #15701 (comment)

@pnkfelix

This comment has been minimized.

Member

pnkfelix commented Sep 22, 2018

@goffrie my theory (which I documented on #32796) is that such attributes were accidentally stabilized.

@bluss

This comment has been minimized.

Contributor

bluss commented Dec 6, 2018

Same with #[inline] on closures, compiles fine on Rust 1.28 - 1.31 when the closure expression is a function parameter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment