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

parse: fuse associated and extern items up to defaultness #69194

Merged
merged 16 commits into from
Feb 19, 2020

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Feb 15, 2020

Language changes:

  • The grammar of extern type aliases is unified with associated ones, and becomes:

    TypeItem = "type" ident generics {":" bounds}? where_clause {"=" type}? ";" ;

    Semantic restrictions (ast_validation) are added to forbid any parameters in generics, any bounds in bounds, and any predicates in where_clause, as well as the presence of a type expression (= u8).

    (Work still remains to fuse this with free type aliases, but this can be done later.)

  • The grammar of constants and static items (free, associated, and extern) now permits the absence of an expression, and becomes:

    GlobalItem = {"const" {ident | "_"} | "static" "mut"? ident} {"=" expr}? ";" ;
    • A semantic restriction is added to enforce the presence of the expression (the body).
    • A semantic restriction is added to reject const _ in associated contexts.

Together, these changes allow us to fuse the grammar of associated items and extern items up to defaultness which is the main goal of the PR.


We are now very close to fully fusing the entirely of item parsing and their ASTs. To progress further, we must make a decision: should we parse e.g. default use foo::bar; and whatnot? Accepting that is likely easiest from a parsing perspective, as it does not require using look-ahead, but it is perhaps not too onerous to only accept it for fns (and all their various qualifiers), consts, statics, and types.

r? @petrochenkov

@Centril Centril added T-lang Relevant to the language team, which will review and decide on the PR/issue. relnotes Marks issues that should be documented in the release notes of the next release. labels Feb 15, 2020
@Centril Centril added this to the 1.43 milestone Feb 15, 2020
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 15, 2020
@petrochenkov
Copy link
Contributor

The grammar of extern items now recognize const items.
The grammar of associated items now recognize static items.

This is excessive, IMO.
The goal is unify with free items as well, does that mean we'll syntactically accept e.g. struct inside traits or extern blocks? I don't think so.
We can parse them (for better diagnostics) while still issuing immediate errors in the parser.

@Centril
Copy link
Contributor Author

Centril commented Feb 15, 2020

Fair, but if we want to unify the parsing functions, we need to at least make the AST changes so that the *Kinds can be merged. I'll move the respective errors from validation to parsing as a "filtering" step.

@petrochenkov
Copy link
Contributor

Here's how I see the end state here

enum ItemKind {
    Fn(FnCommon),
    Const(ConstCommon),
    TyAlias(TyAliasCommon),
    Mac(MacCommon),
    Static(StaticCommon),
    ... // Others
}

enum AssocItemKind {
    Fn(FnCommon),
    Const(ConstCommon),
    TyAlias(TyAliasCommon),
    Mac(MacCommon),
}

enum ForeignItemKind {
    Fn(FnCommon),
    TyAlias(TyAliasCommon),
    Mac(MacCommon),
    Static(StaticCommon),
}

, modulo the *Common naming.

All *Common things share data structures and parsing code, but ItemKind, AssocItemKind and ForeignItemKind are still different enums because they support different subsets of items inside them at parse time (at least in the near future).

@petrochenkov
Copy link
Contributor

Basically we can use a parse_item routine for free items in all cases, but then filter and re-package its results.

src/libsyntax/ast.rs Show resolved Hide resolved
src/libsyntax/ast.rs Outdated Show resolved Hide resolved
src/libsyntax/ast.rs Outdated Show resolved Hide resolved
src/libsyntax/ast.rs Outdated Show resolved Hide resolved
@Centril
Copy link
Contributor Author

Centril commented Feb 15, 2020

I would much prefer to have a single ItemKind and do filtering on that as opposed to repackaging, as I think that will lead to more code sharing in terms of pretty printing, visitors, and whatnot compared to having different kinds (and thus needing matches for each form in terms of visiting, pprust, etc.).

At any rate, it would be easiest to move towards merging into ItemKind and then diverge later than to try to add those *Common structs "in-flight".

@petrochenkov
Copy link
Contributor

@Centril

I think that will lead to more code sharing in terms of pretty printing, visitors, and whatnot

Not significantly, it's basically an extra match function in printing, visitors, parsing and lowering, but instead we do not keep syntactically impossible state in AST. Yes, it doesn't allow to reduce code to absolute minimum, but on the other hand it would be nice to keep AST in check a bit in terms of its closeness to the language.

@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 Feb 15, 2020
@rust-highfive

This comment has been minimized.

@Centril Centril added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 17, 2020
@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Feb 17, 2020

📌 Commit 045b7d5 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-review Status: Awaiting review from the assignee but also interested parties. labels Feb 17, 2020
Centril added a commit to Centril/rust that referenced this pull request Feb 18, 2020
…henkov

parse: fuse associated and extern items up to defaultness

Language changes:

- The grammar of extern `type` aliases is unified with associated ones, and becomes:
  ```rust
  TypeItem = "type" ident generics {":" bounds}? where_clause {"=" type}? ";" ;
  ```

  Semantic restrictions (`ast_validation`) are added to forbid any parameters in `generics`, any bounds in `bounds`, and any predicates in `where_clause`, as well as the presence of a type expression (`= u8`).

  (Work still remains to fuse this with free `type` aliases, but this can be done later.)

- The grammar of constants and static items (free, associated, and extern) now permits the absence of an expression, and becomes:

  ```rust
  GlobalItem = {"const" {ident | "_"} | "static" "mut"? ident} {"=" expr}? ";" ;
  ```

  - A semantic restriction is added to enforce the presence of the expression (the body).
  - A semantic restriction is added to reject `const _` in associated contexts.

Together, these changes allow us to fuse the grammar of associated items and extern items up to `default`ness which is the main goal of the PR.

-----------------------

We are now very close to fully fusing the entirely of item parsing and their ASTs. To progress further, we must make a decision: should we parse e.g. `default use foo::bar;` and whatnot? Accepting that is likely easiest from a parsing perspective, as it does not require using look-ahead, but it is perhaps not too onerous to only accept it for `fn`s (and all their various qualifiers), `const`s, `static`s, and `type`s.

r? @petrochenkov
bors added a commit that referenced this pull request Feb 18, 2020
Rollup of 6 pull requests

Successful merges:

 - #69146 (Always const qualify literals by type)
 - #69159 (Select an appropriate unused lifetime name in suggestion)
 - #69194 (parse: fuse associated and extern items up to defaultness)
 - #69211 (parser: Simplify treatment of macro variables in `Parser::bump`)
 - #69217 (Do not emit note suggesting to implement operation trait to foreign type)
 - #69236 (parse: recover `mut (x @ y)` as `(mut x @ mut y)`.)

Failed merges:

r? @ghost
@bors bors merged commit 045b7d5 into rust-lang:master Feb 19, 2020
JohnTitor added a commit to JohnTitor/rust-clippy that referenced this pull request Feb 19, 2020
bors added a commit to rust-lang/rust-clippy that referenced this pull request Feb 19, 2020
@Centril Centril deleted the assoc-extern-fuse branch February 19, 2020 01:56
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 19, 2020
Changes:
````
Rustup to rust-lang#69194
Rustup to rust-lang#69181
Add `LOG2_10` and `LOG10_2` to `approx_const` lint
Clean up imports
Use `Vec::with_capacity()` as possible
needless_doctest_main: False positive for async fn
Remove use of `TyKind`.
Use `if_chain`.
Fix ICE.
Add tests and improve checks.
Add `Future` detection for `missing_errors_doc`.
````

Fixes rust-lang#69269
bors added a commit that referenced this pull request Feb 19, 2020
submodules: update clippy from b91ae16 to 2855b21

Changes:
````
Rustup to #69194
Rustup to #69181
Add `LOG2_10` and `LOG10_2` to `approx_const` lint
Clean up imports
Use `Vec::with_capacity()` as possible
needless_doctest_main: False positive for async fn
Remove use of `TyKind`.
Use `if_chain`.
Fix ICE.
Add tests and improve checks.
Add `Future` detection for `missing_errors_doc`.
````

Fixes #69269
bors added a commit that referenced this pull request Feb 22, 2020
print vis & defaultness for nested items

Fixes #69315 which was injected by #69194.

r? @petrochenkov
cc @alexcrichton
bors added a commit that referenced this pull request Feb 23, 2020
print vis & defaultness for nested items

Fixes #69315 which was injected by #69194.

r? @petrochenkov
cc @alexcrichton
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 23, 2020
parse: allow `type Foo: Ord` syntactically

This addresses:
> (Work still remains to fuse this with free type aliases, but this can be done later.)

in rust-lang#69194.

r? @petrochenkov
bors added a commit that referenced this pull request Mar 6, 2020
ast: Unmerge structures for associated items and foreign items

Follow-up to #69194.
r? @Centril
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request May 5, 2020
Changes:
````
Rustup to rust-lang/rust#69194
Rustup to rust-lang/rust#69181
Add `LOG2_10` and `LOG10_2` to `approx_const` lint
Clean up imports
Use `Vec::with_capacity()` as possible
needless_doctest_main: False positive for async fn
Remove use of `TyKind`.
Use `if_chain`.
Fix ICE.
Add tests and improve checks.
Add `Future` detection for `missing_errors_doc`.
````

Fixes #69269
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request May 16, 2020
Pkgsrc changes:
 * Bump rust bootstrap version to 1.42.0, except for Darwin/i686 where the
   bootstrap is not (yet?) available.

Upstream changes:

Version 1.43.0 (2020-04-23)
==========================

Language
--------
- [Fixed using binary operations with `&{number}` (e.g. `&1.0`) not having
  the type inferred correctly.][68129]
- [Attributes such as `#[cfg()]` can now be used on `if` expressions.][69201]

**Syntax only changes**
- [Allow `type Foo: Ord` syntactically.][69361]
- [Fuse associated and extern items up to defaultness.][69194]
- [Syntactically allow `self` in all `fn` contexts.][68764]
- [Merge `fn` syntax + cleanup item parsing.][68728]
- [`item` macro fragments can be interpolated into `trait`s, `impl`s,
  and `extern` blocks.][69366]
  For example, you may now write:
  ```rust
  macro_rules! mac_trait {
      ($i:item) => {
          trait T { $i }
      }
  }
  mac_trait! {
      fn foo() {}
  }
  ```
These are still rejected *semantically*, so you will likely receive an error but
these changes can be seen and parsed by macros and
conditional compilation.


Compiler
--------
- [You can now pass multiple lint flags to rustc to override the previous
  flags.][67885] For example; `rustc -D unused -A unused-variables` denies
  everything in the `unused` lint group except `unused-variables` which
  is explicitly allowed. However, passing `rustc -A unused-variables -D unused` denies
  everything in the `unused` lint group **including** `unused-variables` since
  the allow flag is specified before the deny flag (and therefore overridden).
- [rustc will now prefer your system MinGW libraries over its bundled libraries
  if they are available on `windows-gnu`.][67429]
- [rustc now buffers errors/warnings printed in JSON.][69227]

Libraries
---------
- [`Arc<[T; N]>`, `Box<[T; N]>`, and `Rc<[T; N]>`, now implement
  `TryFrom<Arc<[T]>>`,`TryFrom<Box<[T]>>`, and `TryFrom<Rc<[T]>>`
  respectively.][69538] **Note** These conversions are only available when `N`
  is `0..=32`.
- [You can now use associated constants on floats and integers directly, rather
  than having to import the module.][68952] e.g. You can now write `u32::MAX` or
  `f32::NAN` with no imports.
- [`u8::is_ascii` is now `const`.][68984]
- [`String` now implements `AsMut<str>`.][68742]
- [Added the `primitive` module to `std` and `core`.][67637] This module
  reexports Rust's primitive types. This is mainly useful in macros
  where you want avoid these types being shadowed.
- [Relaxed some of the trait bounds on `HashMap` and `HashSet`.][67642]
- [`string::FromUtf8Error` now implements `Clone + Eq`.][68738]

Stabilized APIs
---------------
- [`Once::is_completed`]
- [`f32::LOG10_2`]
- [`f32::LOG2_10`]
- [`f64::LOG10_2`]
- [`f64::LOG2_10`]
- [`iter::once_with`]

Cargo
-----
- [You can now set config `[profile]`s in your `.cargo/config`, or through
  your environment.][cargo/7823]
- [Cargo will now set `CARGO_BIN_EXE_<name>` pointing to a binary's
  executable path when running integration tests or benchmarks.][cargo/7697]
  `<name>` is the name of your binary as-is e.g. If you wanted the executable
  path for a binary named `my-program`you would use
  `env!("CARGO_BIN_EXE_my-program")`.

Misc
----
- [Certain checks in the `const_err` lint were deemed unrelated to const
  evaluation][69185], and have been moved to the `unconditional_panic` and
  `arithmetic_overflow` lints.

Compatibility Notes
-------------------

- [Having trailing syntax in the `assert!` macro is now a hard error.][69548]
  This has been a warning since 1.36.0.
- [Fixed `Self` not having the correctly inferred type.][69340] This incorrectly
  led to some instances being accepted, and now correctly emits a hard error.

[69340]: rust-lang/rust#69340

Internal Only
-------------
These changes provide no direct user facing benefits, but represent significant
improvements to the internals and overall performance of `rustc` and
related tools.

- [All components are now built with `opt-level=3` instead of `2`.][67878]
- [Improved how rustc generates drop code.][67332]
- [Improved performance from `#[inline]`-ing certain hot functions.][69256]
- [traits: preallocate 2 Vecs of known initial size][69022]
- [Avoid exponential behaviour when relating types][68772]
- [Skip `Drop` terminators for enum variants without drop glue][68943]
- [Improve performance of coherence checks][68966]
- [Deduplicate types in the generator witness][68672]
- [Invert control in struct_lint_level.][68725]

[67332]: rust-lang/rust#67332
[67429]: rust-lang/rust#67429
[67637]: rust-lang/rust#67637
[67642]: rust-lang/rust#67642
[67878]: rust-lang/rust#67878
[67885]: rust-lang/rust#67885
[68129]: rust-lang/rust#68129
[68672]: rust-lang/rust#68672
[68725]: rust-lang/rust#68725
[68728]: rust-lang/rust#68728
[68738]: rust-lang/rust#68738
[68742]: rust-lang/rust#68742
[68764]: rust-lang/rust#68764
[68772]: rust-lang/rust#68772
[68943]: rust-lang/rust#68943
[68952]: rust-lang/rust#68952
[68966]: rust-lang/rust#68966
[68984]: rust-lang/rust#68984
[69022]: rust-lang/rust#69022
[69185]: rust-lang/rust#69185
[69194]: rust-lang/rust#69194
[69201]: rust-lang/rust#69201
[69227]: rust-lang/rust#69227
[69548]: rust-lang/rust#69548
[69256]: rust-lang/rust#69256
[69361]: rust-lang/rust#69361
[69366]: rust-lang/rust#69366
[69538]: rust-lang/rust#69538
[cargo/7823]: rust-lang/cargo#7823
[cargo/7697]: rust-lang/cargo#7697
[`Once::is_completed`]: https://doc.rust-lang.org/std/sync/struct.Once.html#method.is_completed
[`f32::LOG10_2`]: https://doc.rust-lang.org/std/f32/consts/constant.LOG10_2.html
[`f32::LOG2_10`]: https://doc.rust-lang.org/std/f32/consts/constant.LOG2_10.html
[`f64::LOG10_2`]: https://doc.rust-lang.org/std/f64/consts/constant.LOG10_2.html
[`f64::LOG2_10`]: https://doc.rust-lang.org/std/f64/consts/constant.LOG2_10.html
[`iter::once_with`]: https://doc.rust-lang.org/std/iter/fn.once_with.html
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.

4 participants