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

Pretty print attributes in print_arg #63212

Merged
merged 3 commits into from
Aug 3, 2019
Merged

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Aug 2, 2019

Fixes #63210.
cc #60406

r? @petrochenkov

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 2, 2019
@Centril
Copy link
Contributor Author

Centril commented Aug 2, 2019

Ouch seems Vadim has a lot to review so r? @davidtwco cc @zackmdavis in case y'all want something to review :)

Copy link
Member

@davidtwco davidtwco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! r=me once CI passes.

@Centril
Copy link
Contributor Author

Centril commented Aug 2, 2019

@bors r=davidtwco rollup

@bors
Copy link
Contributor

bors commented Aug 2, 2019

📌 Commit d1c89d6 has been approved by davidtwco

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 2, 2019
Centril added a commit to Centril/rust that referenced this pull request Aug 2, 2019
bors added a commit that referenced this pull request Aug 2, 2019
Rollup of 7 pull requests

Successful merges:

 - #63107 (Added support for armv7-unknown-linux-gnueabi/musleabi)
 - #63121 (On `format!()` arg count mismatch provide extra info)
 - #63196 (build_helper: try less confusing method names)
 - #63206 (remove unsupported test case)
 - #63208 (Round generator sizes to a multiple of their alignment)
 - #63212 (Pretty print attributes in `print_arg`)
 - #63215 (Clarify semantics of mem::zeroed)

Failed merges:

r? @ghost
@bors bors merged commit d1c89d6 into rust-lang:master Aug 3, 2019
@bors
Copy link
Contributor

bors commented Aug 3, 2019

☔ The latest upstream changes (presumably #63228) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 3, 2019
@Centril Centril deleted the param-attrs-pretty branch August 3, 2019 07:26
bors added a commit that referenced this pull request 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 pull request 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Formal parameter attributes won't be passed into proc-macro
5 participants