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

Tracking issue for RFC 2565, "Attributes in formal function parameter position" #60406

Closed
4 tasks done
Centril opened this issue Apr 30, 2019 · 17 comments
Closed
4 tasks done
Assignees
Labels
B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. B-RFC-implemented Blocker: Approved by a merged RFC and implemented. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. F-param_attrs `#![feature(param_attrs)]` T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@Centril
Copy link
Contributor

Centril commented Apr 30, 2019

This is a tracking issue for the RFC "Attributes in formal function parameter position" (rust-lang/rfcs#2565).

Steps:

Unresolved questions:

None.

This issue has been assigned to @c410-f3r via this comment.

@Centril Centril added B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. T-lang Relevant to the language team, which will review and decide on the PR/issue. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. labels Apr 30, 2019
@c410-f3r
Copy link
Contributor

c410-f3r commented May 1, 2019

Wow! I've always wanted to contribute to Rustc itself and this is a nice opportunity. I will try to implement this feature with some mentoring.

@Centril
Copy link
Contributor Author

Centril commented May 2, 2019

@petrochenkov You are familiar with most of this, right? Could you perhaps write up some instructions?

bors added a commit that referenced this issue Jun 12, 2019
Allow attributes in formal function parameters

Implements #60406.

This is my first contribution to the compiler and since this is a large and complex project, I am not fully aware of the consequences of the changes I have made.

**TODO**

- [x] Forbid some built-in attributes.
- [x] Expand cfg/cfg_attr
@Centril
Copy link
Contributor Author

Centril commented Jun 12, 2019

Partial implementation completed as of 2019-06-12. Implementation was done in #60669 by @c410-f3r and the PR was reviewed by @petrochenkov and @Centril.

#61238 still needs to be done for a full implementation.

@Centril Centril added B-RFC-implemented Blocker: Approved by a merged RFC and implemented. B-unstable Blocker: Implemented in the nightly compiler and unstable. labels Jun 12, 2019
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Jul 16, 2019
Chapter for `param_attrs`

Most the information was taken from the RFC.

cc rust-lang#60406
Centril added a commit to Centril/rust that referenced this issue Jul 28, 2019
@Centril
Copy link
Contributor Author

Centril commented Jul 29, 2019

#61238 was fixed on the 2019-07-29 in #61856. The PR was written by @c410-f3r and reviewed by @matthewjasper, @petrochenkov, and @oli-obk.

The RFC is now fully implemented.

Setting the earliest date for a stabilization report to 6 weeks from now which is on 2019-09-09.

@c410-f3r

This comment has been minimized.

@Centril

This comment has been minimized.

@Centril Centril added the F-param_attrs `#![feature(param_attrs)]` label Aug 2, 2019
Centril added a commit to Centril/rust that referenced this issue Aug 2, 2019
@Centril
Copy link
Contributor Author

Centril commented Aug 3, 2019

A bug #63210 was filed on 2019-08-02 wherein the attributes on formal parameters would not be passed to macros. The issue was forgetting to call the relevant method in fn print_arg in the pretty printer. In #63212, written by @Centril on 2019-08-02 and reviewed by @davidtwco, the issue aforementioned was fixed.

@theduke
Copy link
Contributor

theduke commented Aug 18, 2019

Would love for this feature to hit stable in 1.39.

With a stabilization report at the beginning of September, that should be possible, @Centril ?
(Assuming the documentation is taken care of in time)

@Centril
Copy link
Contributor Author

Centril commented Aug 18, 2019

Seems plausible; 1.39 branches on the 26th; - 10 days for FCP - some time to write a report.

@c410-f3r
Copy link
Contributor

If possible, I would like to write the report, as well as the stabilization PR and remaining documentation

@Centril
Copy link
Contributor Author

Centril commented Aug 18, 2019

@c410-f3r Sure. Here's a report you can use as a template: #61682 (comment). I'd like to review and vet the report first however before it is published on a stabilization PR.

@nikomatsakis
Copy link
Contributor

@rustbot assign @c410-f3r

@rustbot rustbot self-assigned this Aug 29, 2019
@Centril
Copy link
Contributor Author

Centril commented Aug 29, 2019

We discussed this on the language team meeting and found that we should indeed move ahead with stabilizing it.

@Centril
Copy link
Contributor Author

Centril commented Aug 29, 2019

The stabilization proposal is up: #64010.

Centril added a commit to Centril/rust that referenced this issue Sep 5, 2019
… r=nikomatsakis

Harden `param_attrs` test wrt. usage of a proc macro `#[attr]`

The `param-attrs-builtin-attrs.rs` test file uses the `#[test]` attribute which should cover this but `#[test]` isn't a proc macro attribute so we add another test to be on the safe side. This intends to address rust-lang#64010 (comment).

r? @nikomatsakis

cc @c410-f3r @petrochenkov
cc rust-lang#60406
Centril added a commit to Centril/rust that referenced this issue Sep 5, 2019
… r=nikomatsakis

Harden `param_attrs` test wrt. usage of a proc macro `#[attr]`

The `param-attrs-builtin-attrs.rs` test file uses the `#[test]` attribute which should cover this but `#[test]` isn't a proc macro attribute so we add another test to be on the safe side. This intends to address rust-lang#64010 (comment).

r? @nikomatsakis

cc @c410-f3r @petrochenkov
cc rust-lang#60406
bors added a commit that referenced this issue Sep 21, 2019
Stabilize `param_attrs` in Rust 1.39.0

# Stabilization proposal

I propose that we stabilize `#![feature(param_attrs)]`.

Tracking issue: #60406
Version: 1.39 (2019-09-26 => beta, 2019-11-07 => stable).

## What is stabilized

It is now possible to add outer attributes like `#[cfg(..)]` on formal parameters of functions, closures, and function pointer types. For example:

```rust
fn len(
    #[cfg(windows)] slice: &[u16],
    #[cfg(not(windows))] slice: &[u8],
) -> usize {
    slice.len()
}
```

## What isn't stabilized

* Documentation comments like `/// Doc` on parameters.

* Code expansion of a user-defined `#[proc_macro_attribute]` macro used on parameters.

* Built-in attributes other than `cfg`, `cfg_attr`, `allow`, `warn`, `deny`, and `forbid`. Currently, only the lints `unused_variables` and `unused_mut` have effect and may be controlled on parameters.

## Motivation

The chief motivations for stabilizing `param_attrs` include:

* Finer conditional compilation with `#[cfg(..)]` and linting control of variables.

* Richer macro DSLs created by users.

* External tools and compiler internals can take advantage of the additional information that the parameters provide.

For more examples, see the [RFC][rfc motivation].

## Reference guide

In the grammar of function and function pointer, the grammar of variadic tails (`...`) and parameters are changed respectively from:

```rust
FnParam = { pat:Pat ":" }? ty:Type;
VaradicTail = "...";
```

into:

```rust
FnParam = OuterAttr* { pat:Pat ":" }? ty:Type;
VaradicTail = OuterAttr* "...";
```

The grammar of a closure parameter is changed from:

```rust
ClosureParam = pat:Pat { ":" ty:Type }?;
```

into:

```rust
ClosureParam = OuterAttr* pat:Pat { ":" ty:Type }?;
```

More generally, where there's a list of formal (value) parameters separated or terminated by `,` and delimited by `(` and `)`. Each parameter in that list may optionally be prefixed by `OuterAttr+`.

Note that in all cases, `OuterAttr*` applies to the whole parameter and not just the pattern. This distinction matters in pretty printing and in turn for macros.

## History

* On 2018-10-15, @Robbepop proposes [RFC 2565][rfc], "Attributes in formal function parameter position".

* On 2019-04-30, [RFC 2565][rfc] is merged and the tracking issue is made.

* On 2019-06-12, a partial implementation was completed. The implementation was done in [#60669][60669] by @c410-f3r and the PR was reviewed by @petrochenkov and @Centril.

* On 2019-07-29, [#61238][61238] was fixed in [#61856][61856]. The issue fixed was that lint attributes on function args had no effect. The PR was written by @c410-f3r and reviewed by @matthewjasper, @petrochenkov, and @oli-obk.

* On 2019-08-02, a bug [#63210][63210] was filed wherein the attributes on formal parameters would not be passed to macros. The issue was about forgetting to call the relevant method in `fn print_arg` in the pretty printer. In [#63212][63212], written by @Centril on 2019-08-02 and reviewed by @davidtwco, the issue aforementioned was fixed.

* This PR stabilizes `param_attrs`.

## Tests

* [On Rust 2018, attributes aren't permitted on function parameters without a pattern in trait definitions.](https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2565-param-attrs/param-attrs-2018.rs)

* [All attributes that should be allowed. This includes `cfg`, `cfg_attr`, and lints check attributes.](https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2565-param-attrs/param-attrs-allowed.rs)

* [Built-in attributes, which should be forbidden, e.g., `#[test]`, are.](https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2565-param-attrs/param-attrs-builtin-attrs.rs)

* [`cfg` and `cfg_attr` are properly evaluated.](https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2565-param-attrs/param-attrs-cfg.rs)

* [`unused_mut`](https://github.com/rust-lang/rust/blob/46f405ec4d7c6bf16fc2eaafe7541019f1da2996/src/test/ui/rfc-2565-param-attrs/param-attrs-cfg.rs) and [`unused_variables`](https://github.com/rust-lang/rust/blob/master/src/test/ui/lint/lint-unused-variables.rs) are correctly applied to parameter patterns.

* [Pretty printing takes formal parameter attributes into account.](https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2565-param-attrs/param-attrs-pretty.rs)

## Possible future work

* Custom attributes inside function parameters aren't currently supported but it is something being worked on internally.

* Since documentation comments are syntactic sugar for `#[doc(...)]`, it is possible to allow literal `/// Foo` comments on function parameters.

[rfc motivation]: https://github.com/rust-lang/rfcs/blob/master/text/2565-formal-function-parameter-attributes.md#motivation
[rfc]: rust-lang/rfcs#2565
[60669]: #60669
[61856]: #61856
[63210]: #63210
[61238]: #61238
[63212]: #63212

This report is a collaborative work with @Centril.
Centril added a commit to Centril/rust that referenced this issue Sep 21, 2019
Stabilize `param_attrs` in Rust 1.39.0

# Stabilization proposal

I propose that we stabilize `#![feature(param_attrs)]`.

Tracking issue: rust-lang#60406
Version: 1.39 (2019-09-26 => beta, 2019-11-07 => stable).

## What is stabilized

It is now possible to add outer attributes like `#[cfg(..)]` on formal parameters of functions, closures, and function pointer types. For example:

```rust
fn len(
    #[cfg(windows)] slice: &[u16],
    #[cfg(not(windows))] slice: &[u8],
) -> usize {
    slice.len()
}
```

## What isn't stabilized

* Documentation comments like `/// Doc` on parameters.

* Code expansion of a user-defined `#[proc_macro_attribute]` macro used on parameters.

* Built-in attributes other than `cfg`, `cfg_attr`, `allow`, `warn`, `deny`, and `forbid`. Currently, only the lints `unused_variables` and `unused_mut` have effect and may be controlled on parameters.

## Motivation

The chief motivations for stabilizing `param_attrs` include:

* Finer conditional compilation with `#[cfg(..)]` and linting control of variables.

* Richer macro DSLs created by users.

* External tools and compiler internals can take advantage of the additional information that the parameters provide.

For more examples, see the [RFC][rfc motivation].

## Reference guide

In the grammar of function and function pointer, the grammar of variadic tails (`...`) and parameters are changed respectively from:

```rust
FnParam = { pat:Pat ":" }? ty:Type;
VaradicTail = "...";
```

into:

```rust
FnParam = OuterAttr* { pat:Pat ":" }? ty:Type;
VaradicTail = OuterAttr* "...";
```

The grammar of a closure parameter is changed from:

```rust
ClosureParam = pat:Pat { ":" ty:Type }?;
```

into:

```rust
ClosureParam = OuterAttr* pat:Pat { ":" ty:Type }?;
```

More generally, where there's a list of formal (value) parameters separated or terminated by `,` and delimited by `(` and `)`. Each parameter in that list may optionally be prefixed by `OuterAttr+`.

Note that in all cases, `OuterAttr*` applies to the whole parameter and not just the pattern. This distinction matters in pretty printing and in turn for macros.

## History

* On 2018-10-15, @Robbepop proposes [RFC 2565][rfc], "Attributes in formal function parameter position".

* On 2019-04-30, [RFC 2565][rfc] is merged and the tracking issue is made.

* On 2019-06-12, a partial implementation was completed. The implementation was done in [rust-lang#60669][60669] by @c410-f3r and the PR was reviewed by @petrochenkov and @Centril.

* On 2019-07-29, [rust-lang#61238][61238] was fixed in [rust-lang#61856][61856]. The issue fixed was that lint attributes on function args had no effect. The PR was written by @c410-f3r and reviewed by @matthewjasper, @petrochenkov, and @oli-obk.

* On 2019-08-02, a bug [rust-lang#63210][63210] was filed wherein the attributes on formal parameters would not be passed to macros. The issue was about forgetting to call the relevant method in `fn print_arg` in the pretty printer. In [rust-lang#63212][63212], written by @Centril on 2019-08-02 and reviewed by @davidtwco, the issue aforementioned was fixed.

* This PR stabilizes `param_attrs`.

## Tests

* [On Rust 2018, attributes aren't permitted on function parameters without a pattern in trait definitions.](https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2565-param-attrs/param-attrs-2018.rs)

* [All attributes that should be allowed. This includes `cfg`, `cfg_attr`, and lints check attributes.](https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2565-param-attrs/param-attrs-allowed.rs)

* [Built-in attributes, which should be forbidden, e.g., `#[test]`, are.](https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2565-param-attrs/param-attrs-builtin-attrs.rs)

* [`cfg` and `cfg_attr` are properly evaluated.](https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2565-param-attrs/param-attrs-cfg.rs)

* [`unused_mut`](https://github.com/rust-lang/rust/blob/46f405ec4d7c6bf16fc2eaafe7541019f1da2996/src/test/ui/rfc-2565-param-attrs/param-attrs-cfg.rs) and [`unused_variables`](https://github.com/rust-lang/rust/blob/master/src/test/ui/lint/lint-unused-variables.rs) are correctly applied to parameter patterns.

* [Pretty printing takes formal parameter attributes into account.](https://github.com/rust-lang/rust/blob/master/src/test/ui/rfc-2565-param-attrs/param-attrs-pretty.rs)

## Possible future work

* Custom attributes inside function parameters aren't currently supported but it is something being worked on internally.

* Since documentation comments are syntactic sugar for `#[doc(...)]`, it is possible to allow literal `/// Foo` comments on function parameters.

[rfc motivation]: https://github.com/rust-lang/rfcs/blob/master/text/2565-formal-function-parameter-attributes.md#motivation
[rfc]: rust-lang/rfcs#2565
[60669]: rust-lang#60669
[61856]: rust-lang#61856
[63210]: rust-lang#63210
[61238]: rust-lang#61238
[63212]: rust-lang#63212

This report is a collaborative work with @Centril.
davidpdrsn added a commit to davidpdrsn/juniper-from-schema that referenced this issue Sep 21, 2019
Fixes #55

This isn't complete yet. This patch currently removes support for having
descriptions on field arguments. That is because getting those to work
[requires funky stuff][funky] in the juniper proc macro invocation.

While it could totally be made to work I would rather wait since it
requires a bit of refactoring and since it should work seamlessly once
[RFC 2564](rust-lang/rust#60406) is stable,
which seems to happen in 1.39.

[funky]: https://docs.rs/juniper_codegen/0.13.2/juniper_codegen/attr.object.html#customization-documentation-renaming-
davidpdrsn added a commit to davidpdrsn/juniper-from-schema that referenced this issue Sep 21, 2019
Fixes #55

This isn't complete yet. This patch currently removes support for having
descriptions on field arguments. That is because getting those to work
[requires funky stuff][funky] in the juniper proc macro invocation.

While it could totally be made to work I would rather wait since it
requires a bit of refactoring and since it should work seamlessly once
[RFC 2564](rust-lang/rust#60406) is stable,
which seems to happen in 1.39.

[funky]: https://docs.rs/juniper_codegen/0.13.2/juniper_codegen/attr.object.html#customization-documentation-renaming-
davidpdrsn added a commit to davidpdrsn/juniper-from-schema that referenced this issue Sep 21, 2019
Fixes #55

This isn't complete yet. This patch currently removes support for having
descriptions on field arguments. That is because getting those to work
[requires funky stuff][funky] in the juniper proc macro invocation.

While it could totally be made to work I would rather wait since it
requires a bit of refactoring and since it should work seamlessly once
[RFC 2564](rust-lang/rust#60406) is stable,
which seems to happen in 1.39.

[funky]: https://docs.rs/juniper_codegen/0.13.2/juniper_codegen/attr.object.html#customization-documentation-renaming-
davidpdrsn added a commit to davidpdrsn/juniper-from-schema that referenced this issue Sep 21, 2019
Fixes #55

This isn't complete yet. This patch currently removes support for having
descriptions on field arguments. That is because getting those to work
[requires funky stuff][funky] in the juniper proc macro invocation.

While it could totally be made to work I would rather wait since it
requires a bit of refactoring and since it should work seamlessly once
[RFC 2564](rust-lang/rust#60406) is stable,
which seems to happen in 1.39.

[funky]: https://docs.rs/juniper_codegen/0.13.2/juniper_codegen/attr.object.html#customization-documentation-renaming-
@Centril
Copy link
Contributor Author

Centril commented Sep 21, 2019

Stabilization is done; documentation will be done in the reference. Closing as nothing remains to do.

@Centril Centril closed this as completed Sep 21, 2019
@c410-f3r
Copy link
Contributor

It was a long journey and I would like to thank @Robbepop for the RFC and all the PR reviewers, specially @petrochenkov and @Centril.

@theduke
Copy link
Contributor

theduke commented Sep 21, 2019

Also thanks from me, this is much needed for improving proc macro ergonomics in juniper

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. B-RFC-implemented Blocker: Approved by a merged RFC and implemented. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. F-param_attrs `#![feature(param_attrs)]` T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants