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

Implement minimal, internal-only pattern types in the type system #120131

Merged
merged 7 commits into from
Apr 8, 2024

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Jan 19, 2024

rebase of #107606

You can create pattern types with std::pat::pattern_type!(ty is pat). The feature is incomplete and will panic on you if you use any pattern other than integral range patterns. The only way to create or deconstruct a pattern type is via transmute.

This PR's implementation differs from the MCP's text. Specifically

This means you could implement different traits for different pattern types with the same base type. Thus, we just forbid implementing any traits for pattern types.

is violated in this PR. The reason is that we do need impls after all in order to make them usable as fields. constants of type std::time::Nanoseconds struct are used in patterns, so the type must be structural-eq, which it only can be if you derive several traits on it. It doesn't need to be structural-eq recursively, so we can just manually implement the relevant traits on the pattern type and use the pattern type as a private field.

Waiting on:

  • move all unrelated commits into their own PRs.
  • fix niche computation (see 2db07f9)
  • add lots more tests
  • T-types MCP Pattern types types-team#126 to finish
  • some commit cleanup
  • full self-review
  • remove 61bd325, it's not necessary anymore I think.
  • make sure we never accidentally leak pattern types to user code (add stability checks or feature gate checks and appopriate tests) we don't even do this for the new float primitives
  • get approval that the scope expansion to trait impls is ok

r? @BoxyUwU

@rustbot rustbot added A-rustdoc-json Area: Rustdoc JSON backend A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative labels Jan 19, 2024
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 20, 2024
Don't forget that the lifetime on hir types is `'tcx`

This PR just tracks the `'tcx` lifetime to wherever the original objects actually have that lifetime. This code is needed for rust-lang#107606 (now rust-lang#120131) so that `ast_ty_to_ty` can invoke `lit_to_const` on an argument passed to it. Currently the argument is `&hir::Ty<'_>`, but after this PR it is `&'tcx hir::Ty<'tcx>`.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 21, 2024
Don't forget that the lifetime on hir types is `'tcx`

This PR just tracks the `'tcx` lifetime to wherever the original objects actually have that lifetime. This code is needed for rust-lang#107606 (now rust-lang#120131) so that `ast_ty_to_ty` can invoke `lit_to_const` on an argument passed to it. Currently the argument is `&hir::Ty<'_>`, but after this PR it is `&'tcx hir::Ty<'tcx>`.
@bors
Copy link
Contributor

bors commented Jan 22, 2024

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

@oli-obk oli-obk force-pushed the pattern_types_syntax branch 2 times, most recently from f11d46e to d549b8a Compare January 22, 2024 14:28
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Jan 25, 2024

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

flip1995 pushed a commit to flip1995/rust that referenced this pull request Jan 25, 2024
Don't forget that the lifetime on hir types is `'tcx`

This PR just tracks the `'tcx` lifetime to wherever the original objects actually have that lifetime. This code is needed for rust-lang#107606 (now rust-lang#120131) so that `ast_ty_to_ty` can invoke `lit_to_const` on an argument passed to it. Currently the argument is `&hir::Ty<'_>`, but after this PR it is `&'tcx hir::Ty<'tcx>`.
@oli-obk
Copy link
Contributor Author

oli-obk commented Apr 8, 2024

@bors r=compiler-errors

@bors
Copy link
Contributor

bors commented Apr 8, 2024

📌 Commit 18ff131 has been approved by compiler-errors

It is now in the queue for this repository.

@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 Apr 8, 2024
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@bors
Copy link
Contributor

bors commented Apr 8, 2024

⌛ Testing commit 18ff131 with merge 537aab7...

@bors
Copy link
Contributor

bors commented Apr 8, 2024

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 537aab7 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 8, 2024
@bors bors merged commit 537aab7 into rust-lang:master Apr 8, 2024
12 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Apr 8, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (537aab7): comparison URL.

Overall result: ❌✅ regressions and improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.7% [2.7%, 2.7%] 1
Improvements ✅
(primary)
-0.4% [-0.4%, -0.4%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.4% [-0.4%, -0.4%] 1

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.1% [2.1%, 2.1%] 1
Improvements ✅
(primary)
-2.5% [-2.5%, -2.5%] 1
Improvements ✅
(secondary)
-3.0% [-4.5%, -1.9%] 20
All ❌✅ (primary) -2.5% [-2.5%, -2.5%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 666.152s -> 669.664s (0.53%)
Artifact size: 318.23 MiB -> 318.23 MiB (0.00%)

@oli-obk oli-obk deleted the pattern_types_syntax branch April 8, 2024 19:49
@fmease fmease added the F-pattern_types `#![feature(pattern_types)]` label Apr 9, 2024
celinval added a commit to celinval/kani-dev that referenced this pull request Apr 16, 2024
  - rust-lang/rust#120131:
    Add support to `Pat` pattern type
  - rust-lang/rust#122935:
    Rename CastKind::PointerWithExposedProvenance
  - rust-lang/rust#123097:
    Adapt to changes to local_def_path_hash_to_def_id
celinval added a commit to model-checking/kani that referenced this pull request Apr 16, 2024
Related changes:
  - rust-lang/rust#118310:
    Add `Ord::cmp` for primitives as a `BinOp` in MIR
  - rust-lang/rust#120131:
    Add support to `Pat` pattern type
  - rust-lang/rust#122935:
    Rename CastKind::PointerWithExposedProvenance
  - rust-lang/rust#123097:
    Adapt to changes to local_def_path_hash_to_def_id

Resolves #3130, #3142
celinval added a commit to celinval/kani-dev that referenced this pull request Apr 17, 2024
Related changes:
  - rust-lang/rust#118310:
    Add `Ord::cmp` for primitives as a `BinOp` in MIR
  - rust-lang/rust#120131:
    Add support to `Pat` pattern type
  - rust-lang/rust#122935:
    Rename CastKind::PointerWithExposedProvenance
  - rust-lang/rust#123097:
    Adapt to changes to local_def_path_hash_to_def_id

Resolves model-checking#3130, model-checking#3142
flip1995 pushed a commit to flip1995/rust that referenced this pull request Apr 18, 2024
…iler-errors

Implement minimal, internal-only pattern types in the type system

rebase of rust-lang#107606

You can create pattern types with `std::pat::pattern_type!(ty is pat)`. The feature is incomplete and will panic on you if you use any pattern other than integral range patterns. The only way to create or deconstruct a pattern type is via `transmute`.

This PR's implementation differs from the MCP's text. Specifically

> This means you could implement different traits for different pattern types with the same base type. Thus, we just forbid implementing any traits for pattern types.

is violated in this PR. The reason is that we do need impls after all in order to make them usable as fields. constants of type `std::time::Nanoseconds` struct are used in patterns, so the type must be structural-eq, which it only can be if you derive several traits on it. It doesn't need to be structural-eq recursively, so we can just manually implement the relevant traits on the pattern type and use the pattern type as a private field.

Waiting on:

* [x] move all unrelated commits into their own PRs.
* [x] fix niche computation (see 2db07f9)
* [x] add lots more tests
* [x] T-types MCP rust-lang/types-team#126 to finish
* [x] some commit cleanup
* [x] full self-review
* [x] remove 61bd325, it's not necessary anymore I think.
* [ ] ~~make sure we never accidentally leak pattern types to user code (add stability checks or feature gate checks and appopriate tests)~~ we don't even do this for the new float primitives
* [x] get approval that [the scope expansion to trait impls](https://rust-lang.zulipchat.com/#narrow/stream/326866-t-types.2Fnominated/topic/Pattern.20types.20types-team.23126/near/427670099) is ok

r? `@BoxyUwU`
zpzigi754 pushed a commit to zpzigi754/kani that referenced this pull request May 8, 2024
Related changes:
  - rust-lang/rust#118310:
    Add `Ord::cmp` for primitives as a `BinOp` in MIR
  - rust-lang/rust#120131:
    Add support to `Pat` pattern type
  - rust-lang/rust#122935:
    Rename CastKind::PointerWithExposedProvenance
  - rust-lang/rust#123097:
    Adapt to changes to local_def_path_hash_to_def_id

Resolves model-checking#3130, model-checking#3142
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-json Area: Rustdoc JSON backend A-testsuite Area: The testsuite used to check the correctness of rustc F-pattern_types `#![feature(pattern_types)]` merged-by-bors This PR was explicitly merged by bors. PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.