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

Uplift trait ref is knowable into rustc_next_trait_solver #127437

Merged
merged 3 commits into from
Jul 8, 2024
Merged

Uplift trait ref is knowable into rustc_next_trait_solver #127437

merged 3 commits into from
Jul 8, 2024

Conversation

compiler-errors
Copy link
Member

Self-explanatory. Eliminates one more delegate method.

r? lcnr cc @fmease

@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 Jul 6, 2024
@rustbot
Copy link
Collaborator

rustbot commented Jul 6, 2024

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

lcnr
lcnr reviewed Jul 7, 2024
View reviewed changes
compiler/rustc_hir_analysis/src/collect.rs
@@ -1201,6 +1201,7 @@ fn trait_def(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::TraitDef {

let is_marker = tcx.has_attr(def_id, sym::marker);
let rustc_coinductive = tcx.has_attr(def_id, sym::rustc_coinductive);
let is_fundamental = tcx.has_attr(def_id, sym::fundamental);
Copy link
Contributor

Choose a reason for hiding this comment

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

can you change the builtin attr to be only_local if it isn't already?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope. This isn't possible without changing the way AdtKind serialization works. When we deserialize an ADT, we actually call has_attr on it, since we recreate its AdtFlags from scratch. Changing this is pretty involved and includes serializing AdtFlags directly.

lcnr
lcnr reviewed Jul 7, 2024
View reviewed changes
compiler/rustc_middle/src/ty/context.rs
@@ -522,6 +522,10 @@ impl<'tcx> Interner for TyCtxt<'tcx> {
self.is_object_safe(trait_def_id)
}

fn trait_is_fundamental(self, def_id: DefId) -> bool {
self.trait_def(def_id).is_fundamental
Copy link
Contributor

Choose a reason for hiding this comment

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

should we instead uplift trait_def? 🤔

Copy link
Member Author

@compiler-errors compiler-errors Jul 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Seems really annoying, and also ties rustc's internal repr of things to the type system. I don't think I want to do this.

lcnr
lcnr reviewed Jul 7, 2024
View reviewed changes
compiler/rustc_next_trait_solver/src/delegate.rs Outdated
use std::ops::Deref;

use rustc_type_ir::fold::TypeFoldable;
use rustc_type_ir::solve::{Certainty, Goal, NoSolution, SolverMode};
use rustc_type_ir::{self as ty, InferCtxtLike, Interner};

pub trait SolverDelegate:
Deref<Target: InferCtxtLike<Interner = <Self as SolverDelegate>::Interner>> + Sized
Deref<Target: InferCtxtLike<Interner = <Self as SolverDelegate>::Interner> + Sized> + Sized
Copy link
Contributor

Choose a reason for hiding this comment

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

what made the inner Sized bound necessary? Alternatively, should InferCtxtLike simply have a Sized super trait bound?

Copy link
Member Author

@compiler-errors compiler-errors Jul 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Since we pass &**delegate to trait_ref_is_knowable, which has a Infcx: InferCtxtLike bound, which isn't relaxed ?Sized. Changing that everywhere seems particularly annoying and I don't see the existence of an unsized InferCtxt happening soon.

I'll move the bound to that trait though.

lcnr
lcnr reviewed Jul 7, 2024
View reviewed changes
Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

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

r=me after nits

@compiler-errors compiler-errors force-pushed the uplift-trait-ref-is-knowable branch from 9dca9ad to ab27c2f Compare July 7, 2024 15:12
@compiler-errors
Copy link
Member Author

I moved the sized bound, but leaving the rest of the stuff as-is for now

@bors r=lcnr

@bors
Copy link
Collaborator

bors commented Jul 7, 2024

📌 Commit ab27c2f has been approved by lcnr

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 Jul 7, 2024
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Jul 8, 2024
@jieyouxu
Rollup merge of rust-lang#127437 - compiler-errors:uplift-trait-ref-i…
88eca34 
…s-knowable, r=lcnr

Uplift trait ref is knowable into `rustc_next_trait_solver`

Self-explanatory. Eliminates one more delegate method.

r? lcnr cc `@fmease`
@jieyouxu jieyouxu mentioned this pull request Jul 8, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 8, 2024
@bors
Auto merge of rust-lang#127475 - jieyouxu:rollup-ayualfk, r=jieyouxu
3700bb5 
Rollup of 12 pull requests

Successful merges:

 - rust-lang#113128 (Support tail calls in mir via `TerminatorKind::TailCall`)
 - rust-lang#126841 ([`macro_metavar_expr_concat`] Add support for literals)
 - rust-lang#126881 (Make `NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE` a deny-by-default lint in edition 2024)
 - rust-lang#126921 (Give VaList its own home)
 - rust-lang#127276 (rustdoc: Remove OpaqueTy)
 - rust-lang#127367 (Run alloc sync tests)
 - rust-lang#127431 (Use field ident spans directly instead of the full field span in diagnostics on local fields)
 - rust-lang#127437 (Uplift trait ref is knowable into `rustc_next_trait_solver`)
 - rust-lang#127439 (Uplift elaboration into `rustc_type_ir`)
 - rust-lang#127451 (Improve `run-make/output-type-permutations` code and improve `filename_not_in_denylist` API)
 - rust-lang#127452 (Fix intrinsic const parameter counting with `effects`)
 - rust-lang#127459 (rustdoc-json: add type/trait alias tests)

r? `@ghost`
`@rustbot` modify labels: rollup
@jieyouxu jieyouxu mentioned this pull request Jul 8, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 8, 2024
@bors
Auto merge of rust-lang#127476 - jieyouxu:rollup-16wyb0b, r=jieyouxu
7fdefb8 
Rollup of 10 pull requests

Successful merges:

 - rust-lang#126841 ([`macro_metavar_expr_concat`] Add support for literals)
 - rust-lang#126881 (Make `NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE` a deny-by-default lint in edition 2024)
 - rust-lang#126921 (Give VaList its own home)
 - rust-lang#127367 (Run alloc sync tests)
 - rust-lang#127431 (Use field ident spans directly instead of the full field span in diagnostics on local fields)
 - rust-lang#127437 (Uplift trait ref is knowable into `rustc_next_trait_solver`)
 - rust-lang#127439 (Uplift elaboration into `rustc_type_ir`)
 - rust-lang#127451 (Improve `run-make/output-type-permutations` code and improve `filename_not_in_denylist` API)
 - rust-lang#127452 (Fix intrinsic const parameter counting with `effects`)
 - rust-lang#127459 (rustdoc-json: add type/trait alias tests)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 928d71f into rust-lang:master Jul 8, 2024
6 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jul 8, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jul 8, 2024
@rust-timer
Unrolled build for rust-lang#127437
68a4aba 
Rollup merge of rust-lang#127437 - compiler-errors:uplift-trait-ref-is-knowable, r=lcnr

Uplift trait ref is knowable into `rustc_next_trait_solver`

Self-explanatory. Eliminates one more delegate method.

r? lcnr cc ``@fmease``
@compiler-errors compiler-errors mentioned this pull request Jul 8, 2024
Closed
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jul 11, 2024
@bors
Auto merge of rust-lang#127476 - jieyouxu:rollup-16wyb0b, r=jieyouxu
e82cfdc 
Rollup of 10 pull requests

Successful merges:

 - rust-lang#126841 ([`macro_metavar_expr_concat`] Add support for literals)
 - rust-lang#126881 (Make `NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE` a deny-by-default lint in edition 2024)
 - rust-lang#126921 (Give VaList its own home)
 - rust-lang#127367 (Run alloc sync tests)
 - rust-lang#127431 (Use field ident spans directly instead of the full field span in diagnostics on local fields)
 - rust-lang#127437 (Uplift trait ref is knowable into `rustc_next_trait_solver`)
 - rust-lang#127439 (Uplift elaboration into `rustc_type_ir`)
 - rust-lang#127451 (Improve `run-make/output-type-permutations` code and improve `filename_not_in_denylist` API)
 - rust-lang#127452 (Fix intrinsic const parameter counting with `effects`)
 - rust-lang#127459 (rustdoc-json: add type/trait alias tests)

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

@lcnr lcnr lcnr left review comments

Assignees

@lcnr lcnr

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.81.0
Development

Successfully merging this pull request may close these issues.
4 participants
@compiler-errors @rustbot @bors @lcnr