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: unify item parsing & filter illegal item kinds #69366

Merged
merged 17 commits into from Feb 24, 2020

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Feb 22, 2020

This PR fully unifies item parsing into a single fn parse_item_common method which produces Option<Item>. The Item is then mapped into ForeignItem and AssocItem as necessary by transforming the *Kind and converting contextually bad variants into None, thereby filtering them away.

The PR does not yet unmerge the definition of ForeignItemKind from AssocItemKind. I've left that as future work as it didn't feel like this parser-focused PR would be the best one to deal with it. Changes to the AST data structures are instead kept to a reasonable minimum.

Based on #69361.

Fixes #48137.
RELNOTES: Now, item macro fragments can be interpolated into impl, trait, and extern contexts. See src/test/ui/parser/issue-48137-macros-cannot-interpolate-impl-items.rs for the relevant test.

r? @petrochenkov
cc @estebank

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 22, 2020
src/libsyntax/ast.rs Outdated Show resolved Hide resolved
src/librustc_parse/parser/item.rs Outdated Show resolved Hide resolved
src/librustc_parse/parser/item.rs Outdated Show resolved Hide resolved
src/librustc_parse/parser/item.rs Outdated Show resolved Hide resolved
src/test/ui/parser/default-on-wrong-item-kind.stderr Outdated Show resolved Hide resolved
@petrochenkov
Copy link
Contributor

This almost fixes #48137 (which is the end goal here), except that #69366 (comment) needs to be fixed and maybe_whole!(NtItem) needs to be moved inside parse_item_common.

@petrochenkov
Copy link
Contributor

LGTM in general, except I disagree with placing Defaultness into Item and the defaultness parsing rules not aligning with #69366 (comment).

@petrochenkov
Copy link
Contributor

petrochenkov commented Feb 22, 2020

The super-goal is making parse_stmt the basic routine instead of parse_item and handling stuff like lets and other copypaste from function bodies at module level gracefully :)

@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 22, 2020
@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. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 23, 2020
@bors

This comment has been minimized.

src/librustc_parse/parser/item.rs Outdated Show resolved Hide resolved
src/librustc_parse/parser/item.rs Outdated Show resolved Hide resolved
src/librustc_parse/parser/item.rs Outdated Show resolved Hide resolved
@petrochenkov
Copy link
Contributor

Left a few wording nits regarding incomplete sentences in primary messages.

@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 23, 2020
@Centril
Copy link
Contributor Author

Centril commented Feb 23, 2020

Also, I threw in a small refactoring 0564b71 which I had not pushed yet.

@petrochenkov
Copy link
Contributor

Also, I threw in a small refactoring 0564b71 which I had not pushed yet.

Yeah, I've seen it.

@Centril Centril added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 23, 2020
@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Feb 23, 2020

📌 Commit d7f39d5613026024eade9fb4377c4b7be4a1d71b 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 23, 2020
@bors

This comment has been minimized.

@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 Feb 23, 2020
@Centril
Copy link
Contributor Author

Centril commented Feb 24, 2020

@bors r=petrochenkov

@bors
Copy link
Contributor

bors commented Feb 24, 2020

📌 Commit 1c75f5a 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 Feb 24, 2020
@bors
Copy link
Contributor

bors commented Feb 24, 2020

⌛ Testing commit 1c75f5a with merge 79cd224...

@bors
Copy link
Contributor

bors commented Feb 24, 2020

☀️ Test successful - checks-azure
Approved by: petrochenkov
Pushing 79cd224 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 24, 2020
@bors bors merged commit 79cd224 into rust-lang:master Feb 24, 2020
@rust-highfive
Copy link
Collaborator

📣 Toolstate changed by #69366!

Tested on commit 79cd224.
Direct link to PR: #69366

💔 clippy-driver on windows: test-pass → build-fail (cc @mcarton @oli-obk @Manishearth @flip1995 @yaahc @phansch @llogiq).
💔 clippy-driver on linux: test-pass → build-fail (cc @mcarton @oli-obk @Manishearth @flip1995 @yaahc @phansch @llogiq).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request Feb 24, 2020
Tested on commit rust-lang/rust@79cd224.
Direct link to PR: <rust-lang/rust#69366>

💔 clippy-driver on windows: test-pass → build-fail (cc @mcarton @oli-obk @Manishearth @flip1995 @yaahc @phansch @llogiq).
💔 clippy-driver on linux: test-pass → build-fail (cc @mcarton @oli-obk @Manishearth @flip1995 @yaahc @phansch @llogiq).
JohnTitor added a commit to JohnTitor/rust-clippy that referenced this pull request Feb 24, 2020
bors added a commit to rust-lang/rust-clippy that referenced this pull request Feb 24, 2020
@Centril Centril deleted the unified-items branch February 24, 2020 12:52
bors added a commit that referenced this pull request Feb 24, 2020
Update Clippy from 8fbb23f to fc5d0cc

Fixes #69419

```
Fix false positive in `missing_const_for_fn`
Rustup to #69366
Add new lint [`wildcard imports`]. Add suggestion to [`enum_glob_use`]
Add new lint `lossy_float_literal` to detect lossy whole number float literals
```
bors added a commit that referenced this pull request Feb 25, 2020
Update Clippy from 8fbb23f to fc5d0cc

Fixes #69419

```
Fix false positive in `missing_const_for_fn`
Rustup to #69366
Add new lint [`wildcard imports`]. Add suggestion to [`enum_glob_use`]
Add new lint `lossy_float_literal` to detect lossy whole number float literals
```
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 25, 2020
syntax: Remove `Nt(Impl,Trait,Foreign)Item`

Follow-up to rust-lang#69366.
r? @Centril
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 26, 2020
syntax: Remove `Nt(Impl,Trait,Foreign)Item`

Follow-up to rust-lang#69366.
r? @Centril
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
merged-by-bors This PR was explicitly merged by bors. 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.

macros can't interpolate impl items (or really parse them at all)
4 participants