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.

Sign up for GitHub

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

Jump to bottom

Accept TyCtxt instead of TyCtxtAt in Ty::is_* functions #103625

Merged
merged 3 commits into from
Oct 29, 2022
Merged

Accept TyCtxt instead of TyCtxtAt in Ty::is_* functions #103625

merged 3 commits into from
Oct 29, 2022

Conversation

WaffleLapkin
Copy link
Member

Functions in answer:

  • Ty::is_freeze
  • Ty::is_sized
  • Ty::is_unpin
  • Ty::is_copy_modulo_regions

This allows to remove a lot of useless .at(DUMMY_SP), making the code a bit nicer :3

r? @compiler-errors

@WaffleLapkin
Accept TyCtxt instead of TyCtxtAt in Ty::is_* functions
a17ccfa 
Functions in answer:

- `Ty::is_freeze`
- `Ty::is_sized`
- `Ty::is_unpin`
- `Ty::is_copy_modulo_regions`
@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. labels Oct 27, 2022
@rustbot
Copy link
Collaborator

rustbot commented Oct 27, 2022

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@WaffleLapkin
Copy link
Member Author

I was told that the span from TyCtxtAt is actually used in query engine, somehow (I don't want to untangle the macros to check how exactly...), so maybe we shouldn't do that. On the other hand, more than half of all places where .at was used, just used DUMMY_SP, so maybe it's actually fine to use TyCtxt?..

@oli-obk
Copy link
Contributor

oli-obk commented Oct 27, 2022

I was told that the span from TyCtxtAt is actually used in query engine, somehow (I don't want to untangle the macros to check how exactly...), so maybe we shouldn't do that. On the other hand, more than half of all places where .at was used, just used DUMMY_SP, so maybe it's actually fine to use TyCtxt?..

If it is used anywhere and we test that code path, you'll see it in ui tests. We have a bunch of cycle error tests, and cycles is where this shows up for users.

@rustbot
Copy link
Collaborator

rustbot commented Oct 27, 2022

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@compiler-errors
Copy link
Member

I'm not sure if this is worthwhile, and I think you'll see UI test changes when you bless the tests... 🤔

@rustbot
Copy link
Collaborator

rustbot commented Oct 27, 2022

The Miri subtree was changed

cc @rust-lang/miri

@compiler-errors
Copy link
Member

🤔 how does this not affect any of the UI tests?

@WaffleLapkin
Copy link
Member Author

Why would it? I don't think that these queries can lead to cycle errors, so the spans are never shown to the user, I think?

@compiler-errors
Copy link
Member

Hm, maybe so. In that case, @bors r+

@bors
Copy link
Collaborator

bors commented Oct 27, 2022

📌 Commit c8c4971 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-review Status: Awaiting review from the assignee but also interested parties. labels Oct 27, 2022
@WaffleLapkin WaffleLapkin mentioned this pull request Oct 27, 2022
Closed
@GuillaumeGomez GuillaumeGomez mentioned this pull request Oct 29, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 29, 2022
@bors
Auto merge of rust-lang#103727 - GuillaumeGomez:rollup-hfyxccr, r=Gui…
68c836a 
…llaumeGomez

Rollup of 8 pull requests

Successful merges:

 - rust-lang#102634 (compiletest: Refactor test rustcflags)
 - rust-lang#102721 (Prevent foreign Rust exceptions from being caught)
 - rust-lang#103415 (filter candidates in pick probe for diagnostics)
 - rust-lang#103618 (Rename some `OwnerId` fields.)
 - rust-lang#103625 (Accept `TyCtxt` instead of `TyCtxtAt` in `Ty::is_*` functions)
 - rust-lang#103653 (Add missing impl blocks for item reexported from private mod in JSON output)
 - rust-lang#103699 (Emit proper error when casting to `dyn*`)
 - rust-lang#103719 (fix typo in `try_reserve` method from `HashMap` and `HashSet`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 2414a4c into rust-lang:master Oct 29, 2022
@rustbot rustbot added this to the 1.66.0 milestone Oct 29, 2022
flip1995 pushed a commit to flip1995/rust that referenced this pull request Nov 21, 2022
@GuillaumeGomez
Rollup merge of rust-lang#103625 - WaffleLapkin:no_tyctxt_dogs_allowe…
dff041f 
…d, r=compiler-errors

Accept `TyCtxt` instead of `TyCtxtAt` in `Ty::is_*` functions

Functions in answer:

- `Ty::is_freeze`
- `Ty::is_sized`
- `Ty::is_unpin`
- `Ty::is_copy_modulo_regions`

This allows to remove a lot of useless `.at(DUMMY_SP)`, making the code a bit nicer :3

r? `@compiler-errors`
Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
@GuillaumeGomez
Rollup merge of rust-lang#103625 - WaffleLapkin:no_tyctxt_dogs_allowe…
2265d6d 
…d, r=compiler-errors

Accept `TyCtxt` instead of `TyCtxtAt` in `Ty::is_*` functions

Functions in answer:

- `Ty::is_freeze`
- `Ty::is_sized`
- `Ty::is_unpin`
- `Ty::is_copy_modulo_regions`

This allows to remove a lot of useless `.at(DUMMY_SP)`, making the code a bit nicer :3

r? `@compiler-errors`
Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
@bors
Auto merge of rust-lang#103727 - GuillaumeGomez:rollup-hfyxccr, r=Gui…
89fa3e7 
…llaumeGomez

Rollup of 8 pull requests

Successful merges:

 - rust-lang#102634 (compiletest: Refactor test rustcflags)
 - rust-lang#102721 (Prevent foreign Rust exceptions from being caught)
 - rust-lang#103415 (filter candidates in pick probe for diagnostics)
 - rust-lang#103618 (Rename some `OwnerId` fields.)
 - rust-lang#103625 (Accept `TyCtxt` instead of `TyCtxtAt` in `Ty::is_*` functions)
 - rust-lang#103653 (Add missing impl blocks for item reexported from private mod in JSON output)
 - rust-lang#103699 (Emit proper error when casting to `dyn*`)
 - rust-lang#103719 (fix typo in `try_reserve` method from `HashMap` and `HashSet`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@compiler-errors compiler-errors

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.
Projects
None yet
Milestone
1.66.0
Development

Successfully merging this pull request may close these issues.
6 participants
@WaffleLapkin @rustbot @rust-log-analyzer @oli-obk @compiler-errors @bors