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

Rollup of 9 pull requests #64658

Merged
merged 23 commits into from
Sep 21, 2019
Merged

Rollup of 9 pull requests #64658

merged 23 commits into from
Sep 21, 2019

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Sep 21, 2019

Successful merges:

Failed merges:

r? @ghost

c410-f3r and others added 23 commits September 11, 2019 10:13
Sync to rust-lang/rust branch master
This would follow the same coding style as all the other match arms in this file.
It should be fine for Rust ABIs to involve any Rust type.
It was using the snippet from the "use" span, which often renders the
same, but with closures that snippet is on the start of the closure
where the value is captured. We should be using the snippet from the
span where it was moved into the `for` loop, which is `move_span`.
rust stat should call libc stat
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.
Document From trait for LhsExpr in parser

Add doc for From trait for converting P<Expr> and Option<ThinVec<Attribute>> to LhsExpr

As part of issue rust-lang#51430 (cc @skade).

Both of these should just be moving an address and setting a discriminant in an enum. The main thing I'm not sure about is whether it's worth documenting the branch in the From<Option<ThinVec<Attribute>>. As far as I can tell it doesn't seem like it is optimized away (although if the discriminant happened to work out you could just copy the pointer and the discriminant which might be cheaper, but that's not guaranteed). So it seems like if it's being called often, it's doubling the number of possible branch mispredictions on this Option, which could be a significant cost.

Let me know if there's anything that needs fixing and I'll get to it as soon as possible!
factor out pluralisation remains after rust-lang#64280

there are two case that doesn't not match the original macro pattern at [here](https://github.com/rust-lang/rust/blob/master/src/librustc_lint/unused.rs#L146) and [here](https://github.com/rust-lang/rust/blob/master/src/libsyntax/parse/diagnostics.rs#L539) as the provided param is already a bool or the check condition is not `x != 1`, so I change the macro accept a boolean expr instead of number to fit all the cases.

@Centril  please review

Fixes rust-lang#64238.
remove the extra comma after the match arm

This would follow the same coding style as all the other match arms in this file.
Exempt extern "Rust" from improper_ctypes

It should be fine for Rust ABIs to involve any Rust type.

Fixes rust-lang#64593.
…rkor

Fix the span used to suggest avoiding for-loop moves

It was using the snippet from the "use" span, which often renders the
same, but with closures that snippet is on the start of the closure
where the value is captured. We should be using the snippet from the
span where it was moved into the `for` loop, which is `move_span`.

Fixes rust-lang#64559.
@Centril
Copy link
Contributor Author

Centril commented Sep 21, 2019

@bors r+ p=9 rollup=never

@bors
Copy link
Contributor

bors commented Sep 21, 2019

📌 Commit 97ca073 has been approved by Centril

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Sep 21, 2019
@bors
Copy link
Contributor

bors commented Sep 21, 2019

⌛ Testing commit 97ca073 with merge ed8b708...

bors added a commit that referenced this pull request Sep 21, 2019
Rollup of 9 pull requests

Successful merges:

 - #64010 (Stabilize `param_attrs` in Rust 1.39.0)
 - #64136 (Document From trait for LhsExpr in parser)
 - #64342 (factor out pluralisation remains after #64280)
 - #64347 (Add long error explanation for E0312)
 - #64621 (Add Compatibility Notes to RELEASES.md for 1.38.0)
 - #64632 (remove the extra comma after the match arm)
 - #64640 (No home directory on vxWorks)
 - #64641 (Exempt extern "Rust" from improper_ctypes)
 - #64642 (Fix the span used to suggest avoiding for-loop moves)

Failed merges:

r? @ghost
@bors
Copy link
Contributor

bors commented Sep 21, 2019

☀️ Test successful - checks-azure
Approved by: Centril
Pushing ed8b708 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants