Make privacy visitor use types more (instead of HIR) #113671

oli-obk · 2023-07-13T19:37:29Z

This is a prerequisite to normalizing projections, as otherwise we have too many invalid bound vars (hir_ty_to_ty is creating types that have bound vars, but no binder).

The commits are still chaotic, I'm gonna clean them up, but I just wanted to let you know about the general direction and wondering if we could land this before adding normalization, as normalization is where behavioral changes happen, and I'd like to keep that part as minimal as possible.

context can be found on zulip

rustbot · 2023-07-13T19:37:38Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @petrochenkov (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

rustbot · 2023-07-13T19:37:41Z

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

petrochenkov · 2023-07-13T20:50:49Z

cc @Bryanskiy

compiler/rustc_privacy/src/lib.rs

petrochenkov · 2023-07-14T15:11:50Z

(I also don't understand all the new binder play here, but I assume it's not something complex, and you know what you are doing.)

petrochenkov · 2023-07-14T15:12:48Z

wondering if we could land this before adding normalization, as normalization is where behavioral changes happen, and I'd like to keep that part as minimal as possible.

Yes, that's quite reasonable.

oli-obk · 2023-07-14T17:59:19Z

(I also don't understand all the new binder play here, but I assume it's not something complex, and you know what you are doing.)

I'll pull it into a separate commit. It's not necessary until we do normalization, and it's indeed just a mechanical refactoring to not lose the binders

oli-obk · 2023-07-18T08:26:52Z

@rustbot blocked on #113126

petrochenkov · 2023-09-01T15:17:47Z

#113126 has landed.
@rustbot author

oli-obk · 2023-09-06T09:07:48Z

@rustbot ready

not sure about the last commit, but I added it to show why I'm special casing projections

compiler/rustc_privacy/src/lib.rs

oli-obk · 2024-02-08T12:28:57Z

@bors r=petrochenkov

bors · 2024-02-08T12:28:59Z

📌 Commit d80d7ea has been approved by petrochenkov

It is now in the queue for this repository.

…ochenkov Make privacy visitor use types more (instead of HIR) r? `@petrochenkov` This is a prerequisite to normalizing projections, as otherwise we have too many invalid bound vars (hir_ty_to_ty is creating types that have bound vars, but no binder). The commits are still chaotic, I'm gonna clean them up, but I just wanted to let you know about the general direction and wondering if we could land this before adding normalization, as normalization is where behavioral changes happen, and I'd like to keep that part as minimal as possible. [context can be found on zulip](https://rust-lang.zulipchat.com/#narrow/stream/315482-t-compiler.2Fetc.2Fopaque-types/topic/weak.20type.20aliases.20and.20privacy)

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#113026 (Introduce `run-make` V2 infrastructure, a `run_make_support` library and port over 2 tests as example) - rust-lang#113671 (Make privacy visitor use types more (instead of HIR)) - rust-lang#120308 (core/time: avoid divisions in Duration::new) - rust-lang#120693 (Invert diagnostic lints.) - rust-lang#120704 (A drive-by rewrite of `give_region_a_name()`) - rust-lang#120809 (Use `transmute_unchecked` in `NonZero::new`.) - rust-lang#120817 (Fix more `ty::Error` ICEs in MIR passes) - rust-lang#120828 (Fix `ErrorGuaranteed` unsoundness with stash/steal.) - rust-lang#120831 (Startup objects disappearing from sysroot) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#113671 (Make privacy visitor use types more (instead of HIR)) - rust-lang#120308 (core/time: avoid divisions in Duration::new) - rust-lang#120693 (Invert diagnostic lints.) - rust-lang#120704 (A drive-by rewrite of `give_region_a_name()`) - rust-lang#120809 (Use `transmute_unchecked` in `NonZero::new`.) - rust-lang#120817 (Fix more `ty::Error` ICEs in MIR passes) - rust-lang#120828 (Fix `ErrorGuaranteed` unsoundness with stash/steal.) - rust-lang#120831 (Startup objects disappearing from sysroot) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#113671 - oli-obk:normalize_weak_tys, r=petrochenkov Make privacy visitor use types more (instead of HIR) r? ``@petrochenkov`` This is a prerequisite to normalizing projections, as otherwise we have too many invalid bound vars (hir_ty_to_ty is creating types that have bound vars, but no binder). The commits are still chaotic, I'm gonna clean them up, but I just wanted to let you know about the general direction and wondering if we could land this before adding normalization, as normalization is where behavioral changes happen, and I'd like to keep that part as minimal as possible. [context can be found on zulip](https://rust-lang.zulipchat.com/#narrow/stream/315482-t-compiler.2Fetc.2Fopaque-types/topic/weak.20type.20aliases.20and.20privacy)

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#113671 (Make privacy visitor use types more (instead of HIR)) - rust-lang#120308 (core/time: avoid divisions in Duration::new) - rust-lang#120693 (Invert diagnostic lints.) - rust-lang#120704 (A drive-by rewrite of `give_region_a_name()`) - rust-lang#120809 (Use `transmute_unchecked` in `NonZero::new`.) - rust-lang#120817 (Fix more `ty::Error` ICEs in MIR passes) - rust-lang#120828 (Fix `ErrorGuaranteed` unsoundness with stash/steal.) - rust-lang#120831 (Startup objects disappearing from sysroot) r? `@ghost` `@rustbot` modify labels: rollup

…=compiler-errors Clean up AstConv Split off from rust-lang#120926 to make it only contain the renaming & (doc) comment updates. Any changes other than that which have accumulated over time are now part of this PR. Let's be disciplined ;) Inspired by rust-lang#120926 (comment). --- * Remove `hir_trait_to_predicates` * Unused since rust-lang#113671 * Inline `create_args_for_ast_trait_ref` * Only had a single call site * Having it as a separate method didn't gain us anything * Use an if-let guard somewhere to avoid unwrapping * Avoid explicit trait object lifetimes * More legible, stylistic-only (the updated code is 100% semantically identical) * Use explicitly elided lifetimes in impl headers, they get elaborated to distinct lifetimes * Make use of [object lifetime defaulting](https://doc.rust-lang.org/reference/lifetime-elision.html#default-trait-object-lifetimes) for a trait object type inside of a reference type somewhere * Use preexisting dedicated method `ItemCtxt::to_ty` over `<dyn AstConv<'_>>::ast_ty_to_ty` * Use preexisting dedicated method `AstConv::astconv` over explicit coercions * Simplify the function signature of `create_args_for_ast_path` and of `check_generic_arg_count` * In both cases redundant information was passed rendering the call sites verbose and confusing * No perf impact (tested in [rust-lang#120926](rust-lang#120926)) * Move diagnostic method `report_ambiguous_associated_type` from `astconv` to `astconv::errors` * The submodule `errors` exists specifically for that purpose * Use it to keep the main module clean & short

Rollup merge of rust-lang#122527 - fmease:clean-up-hir-ty-lowering, r=compiler-errors Clean up AstConv Split off from rust-lang#120926 to make it only contain the renaming & (doc) comment updates. Any changes other than that which have accumulated over time are now part of this PR. Let's be disciplined ;) Inspired by rust-lang#120926 (comment). --- * Remove `hir_trait_to_predicates` * Unused since rust-lang#113671 * Inline `create_args_for_ast_trait_ref` * Only had a single call site * Having it as a separate method didn't gain us anything * Use an if-let guard somewhere to avoid unwrapping * Avoid explicit trait object lifetimes * More legible, stylistic-only (the updated code is 100% semantically identical) * Use explicitly elided lifetimes in impl headers, they get elaborated to distinct lifetimes * Make use of [object lifetime defaulting](https://doc.rust-lang.org/reference/lifetime-elision.html#default-trait-object-lifetimes) for a trait object type inside of a reference type somewhere * Use preexisting dedicated method `ItemCtxt::to_ty` over `<dyn AstConv<'_>>::ast_ty_to_ty` * Use preexisting dedicated method `AstConv::astconv` over explicit coercions * Simplify the function signature of `create_args_for_ast_path` and of `check_generic_arg_count` * In both cases redundant information was passed rendering the call sites verbose and confusing * No perf impact (tested in [rust-lang#120926](rust-lang#120926)) * Move diagnostic method `report_ambiguous_associated_type` from `astconv` to `astconv::errors` * The submodule `errors` exists specifically for that purpose * Use it to keep the main module clean & short

…piler-errors Remove obsolete parameter `speculative` from `instantiate_poly_trait_ref` In rust-lang#122527 I totally missed that `speculative` has become obsolete with the removal of `hir_trait_to_predicates` / due to rust-lang#113671. Fixes rust-lang#114635. r? `@compiler-errors`

Rollup merge of rust-lang#122577 - fmease:speculative-say-what, r=compiler-errors Remove obsolete parameter `speculative` from `instantiate_poly_trait_ref` In rust-lang#122527 I totally missed that `speculative` has become obsolete with the removal of `hir_trait_to_predicates` / due to rust-lang#113671. Fixes rust-lang#114635. r? `@compiler-errors`

Refactor type visitor walking r? `@petrochenkov` pulling out the uncontroversial parts of rust-lang/rust#113671

Fix validation when lowering `?` trait bounds Pass the unlowered (`rustc_hir`) polarity to `lower_poly_trait_ref`. This allows us to actually *validate* that generic args are actually valid on `?Trait` paths. This actually regressed in rust-lang#113671 because that PR changed the behavior where we were inadvertently re-lowering paths as `BoundPolarity::Positive`, which was also coincidentally the only place we were enforcing the generics on `?Trait` paths were correct.

Rollup merge of rust-lang#132209 - compiler-errors:modifiers, r=fmease Fix validation when lowering `?` trait bounds Pass the unlowered (`rustc_hir`) polarity to `lower_poly_trait_ref`. This allows us to actually *validate* that generic args are actually valid on `?Trait` paths. This actually regressed in rust-lang#113671 because that PR changed the behavior where we were inadvertently re-lowering paths as `BoundPolarity::Positive`, which was also coincidentally the only place we were enforcing the generics on `?Trait` paths were correct.

rustbot assigned petrochenkov Jul 13, 2023

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 Jul 13, 2023

petrochenkov mentioned this pull request Jul 13, 2023

private_in_public lint triggered for pub associated type computed using non-pub trait #45713

Open

petrochenkov reviewed Jul 14, 2023

View reviewed changes

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 Jul 14, 2023

This comment was marked as resolved.

Sign in to view

oli-obk force-pushed the normalize_weak_tys branch 2 times, most recently from d9726ca to 565f573 Compare July 18, 2023 07:23

oli-obk 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 Jul 18, 2023

rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 18, 2023

This comment was marked as resolved.

Sign in to view

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Sep 1, 2023

oli-obk force-pushed the normalize_weak_tys branch from 565f573 to 8ce14f0 Compare September 5, 2023 08:57

rustbot 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 Sep 6, 2023

petrochenkov reviewed Sep 7, 2023

View reviewed changes

compiler/rustc_privacy/src/lib.rs Outdated Show resolved Hide resolved

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 8, 2024

matthiaskrgr mentioned this pull request Feb 9, 2024

Rollup of 9 pull requests #120837

Closed

matthiaskrgr mentioned this pull request Feb 9, 2024

Rollup of 8 pull requests #120843

Merged

bors merged commit f41d0d9 into rust-lang:master Feb 9, 2024
11 checks passed

rustbot added this to the 1.78.0 milestone Feb 9, 2024

This was referenced Feb 27, 2024

Rename astconv::AstConv and related items rust-lang/compiler-team#723

Closed

[MCP 723] Rename astconv::AstConv and related items #120926

Merged

fmease mentioned this pull request Mar 15, 2024

Clean up AstConv #122527

Merged

fmease mentioned this pull request Mar 16, 2024

Remove obsolete parameter speculative from instantiate_poly_trait_ref #122577

Merged

Noratrieb mentioned this pull request Mar 31, 2024

regression: type is now private? #123288

Closed

lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Apr 7, 2024

Auto merge of #117076 - oli-obk:privacy_visitor_types, r=petrochenkov

66ec6c1

Refactor type visitor walking r? `@petrochenkov` pulling out the uncontroversial parts of rust-lang/rust#113671

RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 27, 2024

Auto merge of #117076 - oli-obk:privacy_visitor_types, r=petrochenkov

894e4d7

Refactor type visitor walking r? `@petrochenkov` pulling out the uncontroversial parts of rust-lang/rust#113671

compiler-errors mentioned this pull request Oct 27, 2024

Fix validation when lowering ? trait bounds #132209

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make privacy visitor use types more (instead of HIR) #113671

Make privacy visitor use types more (instead of HIR) #113671

oli-obk commented Jul 13, 2023 •

edited

Loading

rustbot commented Jul 13, 2023

rustbot commented Jul 13, 2023

petrochenkov commented Jul 13, 2023

petrochenkov commented Jul 14, 2023

petrochenkov commented Jul 14, 2023

oli-obk commented Jul 14, 2023

This comment was marked as resolved.

oli-obk commented Jul 18, 2023 •

edited

Loading

This comment was marked as resolved.

petrochenkov commented Sep 1, 2023

oli-obk commented Sep 6, 2023

oli-obk commented Feb 8, 2024

bors commented Feb 8, 2024

Make privacy visitor use types more (instead of HIR) #113671

Make privacy visitor use types more (instead of HIR) #113671

Conversation

oli-obk commented Jul 13, 2023 • edited Loading

rustbot commented Jul 13, 2023

rustbot commented Jul 13, 2023

petrochenkov commented Jul 13, 2023

petrochenkov commented Jul 14, 2023

petrochenkov commented Jul 14, 2023

oli-obk commented Jul 14, 2023

This comment was marked as resolved.

oli-obk commented Jul 18, 2023 • edited Loading

This comment was marked as resolved.

petrochenkov commented Sep 1, 2023

oli-obk commented Sep 6, 2023

oli-obk commented Feb 8, 2024

bors commented Feb 8, 2024

oli-obk commented Jul 13, 2023 •

edited

Loading

oli-obk commented Jul 18, 2023 •

edited

Loading