Skip to content

Use newtype_index!-generated types more idiomatically#139811

Merged
bors merged 1 commit intorust-lang:masterfrom
yotamofek:pr/newtype_cleanups
Apr 15, 2025
Merged

Use newtype_index!-generated types more idiomatically#139811
bors merged 1 commit intorust-lang:masterfrom
yotamofek:pr/newtype_cleanups

Conversation

@yotamofek
Copy link
Contributor

@yotamofek yotamofek commented Apr 14, 2025

Continuation of sorts of #139674
Shouldn't affect anything, just makes some code simpler

@rustbot
Copy link
Collaborator

rustbot commented Apr 14, 2025

r? @matthewjasper

rustbot has assigned @matthewjasper.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Apr 14, 2025
@rustbot
Copy link
Collaborator

rustbot commented Apr 14, 2025

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@rust-log-analyzer

This comment has been minimized.

Sign in to view
oli-obk
oli-obk requested changes Apr 14, 2025
View reviewed changes
compiler/rustc_hir_analysis/src/check/compare_impl_item.rs
// prove below that the hidden types are well formed.
let universe = infcx.create_next_universe();
let mut idx = 0;
let mut idx = ty::BoundVar::ZERO;
Copy link
Contributor

@oli-obk oli-obk Apr 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh fun, I didn't realize newtype indices supported AddAssign<usize> or impls for other basic int types

Copy link
Contributor

@compiler-errors compiler-errors Apr 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it was just added in the followed-up PR in the description; but these newtypes impl'd Add<usize> already so it makes sense to have had it in the first place.

Copy link
Contributor Author

@yotamofek yotamofek Apr 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep.
I think they may also benefit from Add<Self> and AddAssign<Self>, but adding AddAssign<usize> was easier to justify (since Add<usize> was already impl'd, as @compiler-errors pointed out)

Copy link
Contributor

@oli-obk oli-obk Apr 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self adding doesn't make too much sense for indices imo, at least not without a good use case

Copy link
Contributor

@compiler-errors compiler-errors Apr 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I don't think indices should have Add<Self>.

Copy link
Contributor Author

@yotamofek yotamofek Apr 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought I had a good example for a use-case for Add<Self>, but I might have imagined it, and on second thought - yeah, doesn't make a lot of sense to have that impl. Not that I added it, anyway 😄

@oli-obk oli-obk assigned oli-obk and unassigned matthewjasper Apr 14, 2025
@yotamofek yotamofek force-pushed the pr/newtype_cleanups branch from 5706da5 to 4b63362 Compare April 14, 2025 16:19
@yotamofek yotamofek requested a review from oli-obk April 14, 2025 17:25
@oli-obk
Copy link
Contributor

oli-obk commented Apr 14, 2025

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Apr 14, 2025

📌 Commit 4b63362 has been approved by oli-obk

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-review Status: Awaiting review from the assignee but also interested parties. labels Apr 14, 2025
@matthiaskrgr matthiaskrgr mentioned this pull request Apr 14, 2025
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 14, 2025
@bors
Auto merge of rust-lang#139826 - matthiaskrgr:rollup-0q0qvkd, r=matth…
7a6dca1 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#139745 (Avoid unused clones in `Cloned<I>` and `Copied<I>`)
 - rust-lang#139757 (opt-dist: use executable-extension for host llvm-profdata)
 - rust-lang#139778 (Add test for issue 34834)
 - rust-lang#139783 (Use `compiletest-ignore-dir` for bootstrap self-tests)
 - rust-lang#139797 (Allow (but don't require) `#[unsafe(naked)]` so that `compiler-builtins` can upgrade to it)
 - rust-lang#139799 (Specify `--print info=file` syntax in `--help`)
 - rust-lang#139811 (Use `newtype_index!`-generated types more idiomatically)
 - rust-lang#139813 (Miri subtree update)

r? `@ghost`
`@rustbot` modify labels: rollup
Zalathar added a commit to Zalathar/rust that referenced this pull request Apr 15, 2025
@Zalathar
Rollup merge of rust-lang#139811 - yotamofek:pr/newtype_cleanups, r=o…
813d67e 
…li-obk

Use `newtype_index!`-generated types more idiomatically

Continuation of sorts of rust-lang#139674
Shouldn't affect anything, just makes some code simpler
@Zalathar Zalathar mentioned this pull request Apr 15, 2025
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 15, 2025
@bors
Auto merge of rust-lang#139838 - Zalathar:rollup-dxhgi8t, r=Zalathar
96e9535 
Rollup of 11 pull requests

Successful merges:

 - rust-lang#139669 (Overhaul `AssocItem`)
 - rust-lang#139671 (Proc macro span API redesign: Replace proc_macro::SourceFile by Span::{file, local_file})
 - rust-lang#139750 (std/thread: Use default stack size from menuconfig for NuttX)
 - rust-lang#139785 (Let CStrings be either 1 or 2 byte aligned.)
 - rust-lang#139797 (Allow (but don't require) `#[unsafe(naked)]` so that `compiler-builtins` can upgrade to it)
 - rust-lang#139798 (normalize: prefer `ParamEnv` over `AliasBound` candidates)
 - rust-lang#139799 (Specify `--print info=file` syntax in `--help`)
 - rust-lang#139811 (Use `newtype_index!`-generated types more idiomatically)
 - rust-lang#139813 (Miri subtree update)
 - rust-lang#139822 (Fix: Map EOPNOTSUPP to ErrorKind::Unsupported on Unix)
 - rust-lang#139836 (Basic tests of MPMC receiver cloning)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 15, 2025
@bors
Auto merge of rust-lang#139826 - matthiaskrgr:rollup-0q0qvkd, r=matth…
58c2dd9 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#139745 (Avoid unused clones in `Cloned<I>` and `Copied<I>`)
 - rust-lang#139757 (opt-dist: use executable-extension for host llvm-profdata)
 - rust-lang#139778 (Add test for issue 34834)
 - rust-lang#139783 (Use `compiletest-ignore-dir` for bootstrap self-tests)
 - rust-lang#139797 (Allow (but don't require) `#[unsafe(naked)]` so that `compiler-builtins` can upgrade to it)
 - rust-lang#139799 (Specify `--print info=file` syntax in `--help`)
 - rust-lang#139811 (Use `newtype_index!`-generated types more idiomatically)
 - rust-lang#139813 (Miri subtree update)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 04d1052 into rust-lang:master Apr 15, 2025
6 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 15, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 15, 2025
@rust-timer
Unrolled build for rust-lang#139811
663785b 
Rollup merge of rust-lang#139811 - yotamofek:pr/newtype_cleanups, r=oli-obk

Use `newtype_index!`-generated types more idiomatically

Continuation of sorts of rust-lang#139674
Shouldn't affect anything, just makes some code simpler
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oli-obk oli-obk Awaiting requested review from oli-obk

1 more reviewer

@compiler-errors compiler-errors compiler-errors left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@oli-obk oli-obk

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)

Projects

None yet

Milestone

1.88.0

Development

Successfully merging this pull request may close these issues.

7 participants

@yotamofek @rustbot @rust-log-analyzer @oli-obk @bors @compiler-errors @matthewjasper