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

Merge TraitItem & ImplItem into AssocItem` #67131

Merged
merged 26 commits into from
Dec 20, 2019
Merged

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Dec 7, 2019

In this PR we:

  • Merge {Trait,Impl}Item{Kind?} into AssocItem{Kind?} as discussed in an empty :vis doesn't work in trait declaration #65041 (comment).

    • This is done by using the cover grammar of both forms.

    • In particular, it requires that we syntactically allow (under #[cfg(FALSE)]):

      • defaultness on trait items,

      • impl items without a body / definition (const, type, and fn),

      • and associated types in impls with bounds, e.g., type Foo: Ord;.

    • The syntactic restrictions are replaced by semantic ones in ast_validation.

  • Move syntactic restrictions around C-variadic parameters from the parser into ast_validation:

    • fns in all contexts now syntactically allow ...,

    • ... can occur anywhere in the list syntactically (fn foo(..., x: usize) {}),

    • and ... can be the sole parameter (fn foo(...) {}.

r? @petrochenkov

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 7, 2019
@rust-highfive

This comment has been minimized.

@petrochenkov
Copy link
Contributor

r=me on the implementation.

Not sure whether this needs an FCP from the lang team or the consensus in #65041 (comment) is enough.

@petrochenkov petrochenkov 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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 8, 2019
@Centril
Copy link
Contributor Author

Centril commented Dec 8, 2019

👋 @rust-lang/lang :)

@Centril
Copy link
Contributor Author

Centril commented Dec 10, 2019

I'm gonna interpret the radio silence here + general feeling about semantic vs. syntactic errors expressed in the past as "I have nothing new to add atop the existing consensus in #65041 (comment)". :)

@bors r=petrochenkov

@bors
Copy link
Contributor

bors commented Dec 10, 2019

📌 Commit bf521fb5300a194ffbd34cb70e6f4c8b8eb487eb has been approved by petrochenkov

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 10, 2019
@bors
Copy link
Contributor

bors commented Dec 10, 2019

⌛ Testing commit bf521fb5300a194ffbd34cb70e6f4c8b8eb487eb with merge 0ecb374d13d500af84e0453ffde16d81cb97b52a...

@eddyb
Copy link
Member

eddyb commented Dec 10, 2019

Could this PR allow the following (assuming delegate is some proc macro)?

struct Wrapper<T>(T);

#[delegate(self.0: T)]
impl<T> fmt::Debug for Wrapper<T> {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result;
}

Which could expand into:

struct Wrapper<T>(T);

impl<T> fmt::Debug for Wrapper<T>
    where T: fmt::Debug
{
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        fmt::Debug::fmt(&self.0, f)
    }
}

I guess you could do it before this PR too and just require an empty body or thereabouts, but being able to have an explicit ; seems slightly nicer.

@bors

This comment has been minimized.

@rust-highfive

This comment has been minimized.

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 10, 2019
@nikomatsakis
Copy link
Contributor

@Centril

I'm gonna interpret the radio silence here + general feeling about semantic vs. syntactic errors expressed in the past as "I have nothing new to add atop the existing consensus in #65041 (comment)". :)

in my case, I just hadn't gotten to the notification -- but I am indeed fine with the "language implications" here.

I didn't look too closely at the PR itself, though from a quick look it seemed good.

@petrochenkov petrochenkov 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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 11, 2019
@Centril
Copy link
Contributor Author

Centril commented Dec 11, 2019

Could this PR allow the following (assuming delegate is some proc macro)?

@eddyb Indeed, that would be allowed. :) Go forth and implement!

@Centril Centril added relnotes Marks issues that should be documented in the release notes of the next release. T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Dec 11, 2019
@Centril Centril added this to the 1.41 milestone Dec 11, 2019
@Centril
Copy link
Contributor Author

Centril commented Dec 11, 2019

The job i686-mingw-2 of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Might be spurious? I have no idea why this is happening at the moment so let's try again and see if it was.

@bors r=petrochenkov rollup=never

@Centril Centril mentioned this pull request Dec 16, 2019
@Centril Centril removed the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Dec 19, 2019
@Centril
Copy link
Contributor Author

Centril commented Dec 19, 2019

@bors r=petrochenkov

@bors
Copy link
Contributor

bors commented Dec 19, 2019

📌 Commit 054458b has been approved by petrochenkov

@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 Dec 19, 2019
@Centril Centril modified the milestones: 1.41, 1.42 Dec 19, 2019
Centril added a commit to Centril/rust that referenced this pull request Dec 20, 2019
Merge `TraitItem` & `ImplItem into `AssocItem`

In this PR we:

- Merge `{Trait,Impl}Item{Kind?}` into `AssocItem{Kind?}` as discussed in rust-lang#65041 (comment).

   - This is done by using the cover grammar of both forms.

   - In particular, it requires that we syntactically allow (under `#[cfg(FALSE)]`):

      - `default`ness on `trait` items,

      - `impl` items without a body / definition (`const`, `type`, and `fn`),

      - and associated `type`s in `impl`s with bounds, e.g., `type Foo: Ord;`.

   - The syntactic restrictions are replaced by semantic ones in `ast_validation`.

- Move syntactic restrictions around C-variadic parameters from the parser into `ast_validation`:

    - `fn`s in all contexts now syntactically allow `...`,

    - `...` can occur anywhere in the list syntactically (`fn foo(..., x: usize) {}`),

    - and `...` can be the sole parameter (`fn foo(...) {}`.

r? @petrochenkov
@Centril
Copy link
Contributor Author

Centril commented Dec 20, 2019

@bors rollup=maybe

bors added a commit that referenced this pull request Dec 20, 2019
Rollup of 5 pull requests

Successful merges:

 - #64588 (Add a raw "address of" operator)
 - #67031 (Update tokio crates to latest versions)
 - #67131 (Merge `TraitItem` & `ImplItem into `AssocItem`)
 - #67354 (Fix pointing at arg when cause is outside of call)
 - #67363 (Fix handling of wasm import modules and names)

Failed merges:

r? @ghost
@bors bors merged commit 054458b into rust-lang:master Dec 20, 2019
@Centril Centril deleted the item-merge branch December 20, 2019 21:02
Centril added a commit to Centril/rust that referenced this pull request Dec 21, 2019
 Refactor type & bounds parsing thoroughly

PR is based on rust-lang#67131 with first one from this PR being ` extract parse_ty_tuple_or_parens`.

Also fixes rust-lang#67146.

r? @estebank
Centril added a commit to Centril/rust that referenced this pull request Dec 22, 2019
 Refactor type & bounds parsing thoroughly

PR is based on rust-lang#67131 with first one from this PR being ` extract parse_ty_tuple_or_parens`.

Also fixes rust-lang#67146.

r? @estebank
Centril added a commit to Centril/rust that referenced this pull request Dec 22, 2019
 Refactor type & bounds parsing thoroughly

PR is based on rust-lang#67131 with first one from this PR being ` extract parse_ty_tuple_or_parens`.

Also fixes rust-lang#67146.

r? @estebank
calebcartwright added a commit to calebcartwright/rustfmt that referenced this pull request Jan 16, 2020
TraitItem/ImplItem were combined to `AssocItem`, along with their
respecitve `*Kind`'s, the `Method` variant was also renamed to `Fn` in
the new `AssocItemKind`

rust-lang/rust#67131
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 13, 2020
…enkov

parse: merge `fn` syntax + cleanup item parsing

Here we continue the work in rust-lang#67131 in particular to merge the grammars of `fn` items in various positions.

A list of *language level* changes (as sanctioned by the language team in rust-lang#65041 (comment) and rust-lang#67131):

- `self` parameters are now *syntactically* allowed as the first parameter irrespective of item context (and in function pointers). Instead, semantic validation (`ast_validation`) is used.

- Syntactically, `fn` items in `extern { ... }` blocks can now have bodies (`fn foo() { ... }` as opposed to `fn foo();`). As above, we use semantic restrictions instead.

- Syntactically, `fn` items in free contexts (directly in a file or a module) can now be without bodies (`fn foo();` as opposed to `fn foo() { ... }`. As above, we use semantic restrictions instead, including for non-ident parameter patterns.

- `const extern fn` feature gating is now done post-expansion such that we do not have conditional compatibilities of function qualifiers *in parsing*.

- The `FnFrontMatter` grammar becomes:
   ```rust
   Extern = "extern" StringLit ;
   FnQual = "const"? "async"? "unsafe"? Extern? ;
   FnFrontMatter = FnQual "fn" ;
   ```

   That is, all item contexts now *syntactically* allow `const async unsafe extern "C" fn` and use semantic restrictions to rule out combinations previously prevented syntactically. The semantic restrictions include in particular:

   - `fn`s in `extern { ... }` can have no qualifiers.
   - `const` and `async` cannot be combined.

- To fuse the list-of-items parsing in the 4 contexts that items are allowed, we now must permit inner attributes (`#![attr]`) inside `trait Foo { ... }` definitions. That is, we now allow e.g. `trait Foo { #![attr] }`. This was probably an oversight due to not using a uniform parsing mechanism, which we now do have (`fn parse_item_list`). The semantic support (including e.g. for linting) falls out directly from the attributes infrastructure. To ensure this, we include a test for lints.

Put together, these grammar changes allow us to substantially reduce the complexity of item parsing and its grammar. There are however some other non-language improvements that allow the compression to take place.

A list of *compiler-internal* changes (in particular noting the parser-external data-structure changes):

- We use `enum AllowPlus/RecoverQPath/AllowCVariadic { Yes, No }` in `parser/ty.rs` instead of passing around 3 different `bool`s. I felt this was necessary as it was becoming mentally taxing to track which-is-which.

- `fn visit_trait_item` and `fn visit_impl_item` are merged into `fn visit_assoc_item` which now is passed an `AssocCtxt` to check which one it is.

- We change `FnKind` to:

  ```rust
  pub enum FnKind<'a> {
      Fn(FnCtxt, Ident, &'a FnSig, &'a Visibility, Option<&'a Block>),
      Closure(&'a FnDecl, &'a Expr),
  }
  ```

  with:

  ```rust
  pub enum FnCtxt {
      Free,
      Foreign,
      Assoc(AssocCtxt),
  }
  ```

  This is then taken advantage of in tweaking the various semantic restrictions as well as in pretty printing.

- In `ItemKind::Fn`, we change `P<Block>` to `Option<P<Block>>`.

- In `ForeignItemKind::Fn`, we change `P<FnDecl>` to `FnSig` and `P<Block>` to `Option<P<Block>>`.

- We change `ast::{Unsafety, Spanned<Constness>}>` into `enum ast::{Unsafe, Const} { Yes(Span), No }` respectively. This change in formulation allow us to exclude `Span` in the case of `No`, which facilitates parsing. Moreover, we also add a `Span` to `IsAsync` which is renamed to `Async`. The new `Span`s in `Unsafety` and `Async` are then taken advantage of for better diagnostics. A reason this change was made is to have a more uniform and clear naming scheme.

  The HIR keeps the structures in AST (with those definitions moved into HIR) for now to avoid regressing perf.

- Various cleanups, bug fixes, and diagnostics improvements are made along the way. It is probably best to understand those via the diffs.

I would recommend reviewing this commit-by-commit with whitespace changes hidden.

r? @estebank @petrochenkov
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Mar 13, 2020
Version 1.42.0 (2020-03-12)
==========================

Language
--------
- [You can now use the slice pattern syntax with subslices.][67712] e.g.
  ```rust
  fn foo(words: &[&str]) {
      match words {
          ["Hello", "World", "!", ..] => println!("Hello World!"),
          ["Foo", "Bar", ..] => println!("Baz"),
          rest => println!("{:?}", rest),
      }
  }
  ```
- [You can now use `#[repr(transparent)]` on univariant `enum`s.][68122] Meaning
  that you can create an enum that has the exact layout and ABI of the type
  it contains.
- [There are some *syntax-only* changes:][67131]
   - `default` is syntactically allowed before items in `trait` definitions.
   - Items in `impl`s (i.e. `const`s, `type`s, and `fn`s) may syntactically
     leave out their bodies in favor of `;`.
   - Bounds on associated types in `impl`s are now syntactically allowed
     (e.g. `type Foo: Ord;`).
   - `...` (the C-variadic type) may occur syntactically directly as the type of
      any function parameter.

  These are still rejected *semantically*, so you will likely receive an error
  but these changes can be seen and parsed by procedural macros and
  conditional compilation.

Compiler
--------
- [Added tier 2* support for `armv7a-none-eabi`.][68253]
- [Added tier 2 support for `riscv64gc-unknown-linux-gnu`.][68339]
- [`Option::{expect,unwrap}` and
   `Result::{expect, expect_err, unwrap, unwrap_err}` now produce panic messages
   pointing to the location where they were called, rather than
   `core`'s internals. ][67887]

* Refer to Rust's [platform support page][forge-platform-support] for more
information on Rust's tiered platform support.

Libraries
---------
- [`iter::Empty<T>` now implements `Send` and `Sync` for any `T`.][68348]
- [`Pin::{map_unchecked, map_unchecked_mut}` no longer require the return type
   to implement `Sized`.][67935]
- [`io::Cursor` now derives `PartialEq` and `Eq`.][67233]
- [`Layout::new` is now `const`.][66254]
- [Added Standard Library support for `riscv64gc-unknown-linux-gnu`.][66899]


Stabilized APIs
---------------
- [`CondVar::wait_while`]
- [`CondVar::wait_timeout_while`]
- [`DebugMap::key`]
- [`DebugMap::value`]
- [`ManuallyDrop::take`]
- [`matches!`]
- [`ptr::slice_from_raw_parts_mut`]
- [`ptr::slice_from_raw_parts`]

Cargo
-----
- [You no longer need to include `extern crate proc_macro;` to be able to
  `use proc_macro;` in the `2018` edition.][cargo/7700]

Compatibility Notes
-------------------
- [`Error::description` has been deprecated, and its use will now produce a
  warning.][66919] It's recommended to use `Display`/`to_string` instead.
- [`use $crate;` inside macros is now a hard error.][37390] The compiler
  emitted forward compatibility warnings since Rust 1.14.0.
- [As previously announced, this release reduces the level of support for
  32-bit Apple targets to tier 3.][apple-32bit-drop]. This means that the
  source code is still available to build, but the targets are no longer tested
  and no release binary is distributed by the Rust project. Please refer to the
  linked blog post for more information.

[37390]: rust-lang/rust#37390
[68253]: rust-lang/rust#68253
[68348]: rust-lang/rust#68348
[67935]: rust-lang/rust#67935
[68339]: rust-lang/rust#68339
[68122]: rust-lang/rust#68122
[67712]: rust-lang/rust#67712
[67887]: rust-lang/rust#67887
[67131]: rust-lang/rust#67131
[67233]: rust-lang/rust#67233
[66899]: rust-lang/rust#66899
[66919]: rust-lang/rust#66919
[66254]: rust-lang/rust#66254
[cargo/7700]: rust-lang/cargo#7700
[`DebugMap::key`]: https://doc.rust-lang.org/stable/std/fmt/struct.DebugMap.html#method.key
[`DebugMap::value`]: https://doc.rust-lang.org/stable/std/fmt/struct.DebugMap.html#method.value
[`ManuallyDrop::take`]: https://doc.rust-lang.org/stable/std/mem/struct.ManuallyDrop.html#method.take
[`matches!`]: https://doc.rust-lang.org/stable/std/macro.matches.html
[`ptr::slice_from_raw_parts_mut`]: https://doc.rust-lang.org/stable/std/ptr/fn.slice_from_raw_parts_mut.html
[`ptr::slice_from_raw_parts`]: https://doc.rust-lang.org/stable/std/ptr/fn.slice_from_raw_parts.html
[`CondVar::wait_while`]: https://doc.rust-lang.org/stable/std/sync/struct.Condvar.html#method.wait_while
[`CondVar::wait_timeout_while`]: https://doc.rust-lang.org/stable/std/sync/struct.Condvar.html#method.wait_timeout_while
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants