Implement RFC 3323: restrictions #106074

jhpratt · 2022-12-23T00:29:35Z

This PR implements RFC 3323, which proposed impl and mut restrictions. Both are implemented here.

The current state of the PR is insufficient to merge. mut restrictions are implemented without any known bugs. impl restrictions, however, are not currently enforced on external traits — they are only enforced on same-crate traits. I do not know why this is the case, and would appreciate some help in narrowing down what needs to change to fix this. This is the only known issue with the implementation, and is why this PR is currently marked as a draft.

~~Once the bug is fixed, I'll also need to clean up the commit history. The current history is due to having it implemented while the RFC was still in progress.~~

CI will fail for a number of reasons. I still have TODO in the code as opposed to FIXME (because it needs to be done before this gets merged). I haven't added the tracking issue yet. And the bug will also cause a few errors by itself.

~~I am currently refactoring some code and cleaning up the commit history. After that is done, this PR will be ready to merge.~~

cc #105077

rustbot · 2022-12-23T00:29:41Z

r? @eholk

(rustbot has picked a reviewer for you, use r? to override)

rustbot · 2022-12-23T00:29:44Z

Error: Label B-rfc-approved can only be set by Rust team members

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

joshtriplett · 2022-12-23T01:33:34Z

Reassigning to T-compiler, as this doesn't need further T-lang review yet.

compiler/rustc_parse/src/parser/mod.rs

bors · 2022-12-29T20:37:10Z

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

compiler/rustc_restrictions/src/impl_restriction.rs

compiler/rustc_metadata/src/rmeta/decoder.rs

bors · 2023-07-19T00:41:38Z

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

estebank · 2023-07-19T15:17:11Z

compiler/rustc_error_codes/src/error_codes/E0742.md

@@ -1,5 +1,5 @@
-Visibility is restricted to a module which isn't an ancestor of the current
-item.
+Visibility or restriction is restricted to a module which isn't an ancestor of


Maybe

Suggested change

Visibility or restriction is restricted to a module which isn't an ancestor of

Visibility or restriction is limited to a module which isn't an ancestor of

to avoid repetition?

apiraino · 2023-08-24T09:58:37Z

I'll reassign to author(s) to rebase and address a few comments but tbh I'm not sure about who should have the ball right now. Feel free to request a review with @rustbot ready, thanks!

@rustbot author

petrochenkov · 2023-08-24T10:20:00Z

When it gets to actual landing it will be better to split this PR into separate PRs for individual sub-features, it's too large right now.

WaffleLapkin · 2023-08-24T20:25:53Z

Agree with @petrochenkov, one of the reasons I'm procrastinating reviewing this (sorry!) is that this PR adds a lot. Maybe an approach closer to what we are doing with the explicit tail calls experiment (a PR per level — one for parsing, one for HIR, ...; maybe a PR "per feature" would make more sense here, I'm not sure) would be a bit easier to manage.

jhpratt · 2023-08-24T20:41:29Z

No worries on the delay — I know it's a large PR. I can definitely split it up into multiple PRs when I get the chance. That may be a little while, unfortunately, as I've got a few different things going on.

compiler-errors · 2023-09-20T20:10:19Z

For the record: feel free to follow the new nightly style policy and open a separate formatting PR against rustfmt once this has landed in the compiler.

Dylan-DPC · 2024-02-05T18:31:23Z

@jhpratt any updates on this?

jhpratt · 2024-02-05T18:32:29Z

@Dylan-DPC Not at the moment. I have splitting it up on my personal backlog.

Dylan-DPC · 2024-07-25T17:32:57Z

@jhpratt any updates on this? It's just collecting conflicts at the moment :D

WaffleLapkin · 2024-07-25T21:04:23Z

r? compiler
(I know this is waiting on author, but I do not plan to review anymore, so rerolling)

compiler-errors · 2024-07-25T21:06:00Z

I can review this but I'd rather we split it as agreed above (not to seem repetitive, and obviously take your time).

jhpratt · 2024-07-25T21:22:47Z

The update would be that I'm quite busy, unfortunately. Believe me when I say this is something that I still want to happen 😄

Between work, RustConf prep, and other open source things, this isn't happening in the next month. However, I hope to get around to it within the next few months. It'll likely be a series of PRs, each being an incremental step towards the full implementation that this PR is.

Dylan-DPC · 2025-01-26T12:53:03Z

@jhpratt any updates on this?

jhpratt · 2025-01-27T00:08:38Z

@Dylan-DPC I'll be working on it in the coming weeks 😄

…items (#14516) Found this while reviewing PR rust-lang/rust#138384: See rust-lang/rust#138384 (comment) in which I suggested a FIXME to be added that I'm now fixing in this PR. --- Before this PR, `redundant_pub_crate` computed a broken primary Span when flagging items (`hir::Item`s) that never bear a name (`ForeignMod`, `GlobalAsm`, `Impl`, `Use(UseKind::Glob)`, `Use(UseKind::ListStem)`). Namely, it created a span whose high byte index is `DUMMY_SP.hi()` which is quite broken (`DUMMY_SP` is synonymous with `0..0` wrt. the entire `SourceMap` meaning it points at the start of the very first source file in the `SourceMap`). Why did this happen? Before PR rust-lang/rust#138384, the offending line looked like `let span = item.span.with_hi(item.ident.span.hi());`. For nameless items, `item.ident` used to be set to `Ident(sym::empty, DUMMY_SP)`. This is where the `DUMMY_SP` came from. The code means to compute a "shorter item span" that doesn't include the "body" of items, only the "head" (similar to `TyCtxt::def_span`). <details><summary>Examples of Clippy's butchered output on master</summary> ```rs #![deny(clippy::redundant_pub_crate)] mod m5 { pub mod m5_1 {} pub(crate) use m5_1::*; } ``` ``` error: pub(crate) import inside private module --> file.rs:1:1 | 1 | / #![deny(clippy::redundant_pub_crate)] 2 | | 3 | | mod m5 { 4 | | pub mod m5_1 {} 5 | | pub(crate) use m5_1::*; | | ^---------- help: consider using: `pub` | |____| | ``` Or if the `SourceMap` contains multiple files (notice how it leaks `clippy.toml`!): ``` error: pub(crate) import inside private module --> /home/fmease/programming/rust/clippy/clippy.toml:1:1 | 1 | / avoid-breaking-exported-api = false 2 | | 3 | | check-inconsistent-struct-field-initializers = true ... | 6 | | | |_^ | ::: file.rs:6:5 | 6 | pub(crate) use m5_1::{*}; // Glob | ---------- help: consider using: `pub` | ``` </details> --- **Note**: Currently, the only nameless item kind that can also have a visibility is `Use(UseKind::{Glob,ListStem})`. Thus I'm just falling back to the entire item's Span which wouldn't be that verbose. However, in the future Rust will feature impl restrictions (like `pub(crate) impl Trait for Type {}`, see [RFC 3323](https://rust-lang.github.io/rfcs/3323-restrictions.html) and rust-lang/rust#106074). Using `item.span` for these would be quite bad (it would include all associated items). Should I add a FIXME? --- changelog: [`redundant_pub_crate`]: Fix the code highlighting for nameless items.

rustbot assigned eholk Dec 23, 2022

jhpratt mentioned this pull request Dec 23, 2022

Tracking Issue for Restrictions #105077

Open

10 tasks

This comment was marked as resolved.

Sign in to view

joshtriplett added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Dec 23, 2022

This comment has been minimized.

Sign in to view

Noratrieb reviewed Dec 23, 2022

View reviewed changes

compiler/rustc_parse/src/parser/mod.rs Outdated Show resolved Hide resolved

compiler-errors reviewed Jan 4, 2023

View reviewed changes

compiler/rustc_restrictions/src/impl_restriction.rs Outdated Show resolved Hide resolved

rust-cloud-vms bot force-pushed the restrictions branch from a2b72b7 to 7b21778 Compare January 5, 2023 05:13

rustbot added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic labels Jan 5, 2023

This comment has been minimized.

Sign in to view

compiler-errors reviewed Jan 5, 2023

View reviewed changes

compiler/rustc_metadata/src/rmeta/decoder.rs Outdated Show resolved Hide resolved

rust-cloud-vms bot force-pushed the restrictions branch 2 times, most recently from 0f88945 to c0c8d21 Compare January 5, 2023 23:53

This comment has been minimized.

Sign in to view

estebank reviewed Jul 19, 2023

View reviewed changes

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-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 24, 2023

fmease mentioned this pull request Feb 13, 2024

rustdoc: generic constant expressions not displayed (and where clauses using said gces) #120994

Open

fmease mentioned this pull request Apr 11, 2024

Add an expression for direct access to an enum's discriminant rust-lang/rfcs#3607

Open

rustbot assigned pnkfelix and unassigned WaffleLapkin Jul 25, 2024

compiler-errors assigned compiler-errors and unassigned pnkfelix Jul 25, 2024

alex-semenyuk 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 12, 2024

JohnCSimon 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 29, 2024

fmease mentioned this pull request Mar 12, 2025

Move hir::Item::ident into hir::ItemKind. #138384

Merged

fmease mentioned this pull request Apr 1, 2025

Fix the primary span of redundant_pub_crate when flagging nameless items rust-lang/rust-clippy#14516

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement RFC 3323: restrictions #106074

Implement RFC 3323: restrictions #106074

jhpratt commented Dec 23, 2022 •

edited

Loading

rustbot commented Dec 23, 2022

rustbot commented Dec 23, 2022

This comment was marked as resolved.

joshtriplett commented Dec 23, 2022

This comment has been minimized.

bors commented Dec 29, 2022

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bors commented Jul 19, 2023

estebank Jul 19, 2023

apiraino commented Aug 24, 2023

petrochenkov commented Aug 24, 2023

WaffleLapkin commented Aug 24, 2023

jhpratt commented Aug 24, 2023

compiler-errors commented Sep 20, 2023

Dylan-DPC commented Feb 5, 2024

jhpratt commented Feb 5, 2024

Dylan-DPC commented Jul 25, 2024

WaffleLapkin commented Jul 25, 2024

compiler-errors commented Jul 25, 2024

jhpratt commented Jul 25, 2024 •

edited

Loading

Dylan-DPC commented Jan 26, 2025

jhpratt commented Jan 27, 2025

	Visibility or restriction is restricted to a module which isn't an ancestor of
	Visibility or restriction is limited to a module which isn't an ancestor of

Implement RFC 3323: restrictions #106074

Are you sure you want to change the base?

Implement RFC 3323: restrictions #106074

Conversation

jhpratt commented Dec 23, 2022 • edited Loading

rustbot commented Dec 23, 2022

rustbot commented Dec 23, 2022

This comment was marked as resolved.

joshtriplett commented Dec 23, 2022

This comment has been minimized.

bors commented Dec 29, 2022

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

bors commented Jul 19, 2023

estebank Jul 19, 2023

Choose a reason for hiding this comment

apiraino commented Aug 24, 2023

petrochenkov commented Aug 24, 2023

WaffleLapkin commented Aug 24, 2023

jhpratt commented Aug 24, 2023

compiler-errors commented Sep 20, 2023

Dylan-DPC commented Feb 5, 2024

jhpratt commented Feb 5, 2024

Dylan-DPC commented Jul 25, 2024

WaffleLapkin commented Jul 25, 2024

compiler-errors commented Jul 25, 2024

jhpratt commented Jul 25, 2024 • edited Loading

Dylan-DPC commented Jan 26, 2025

jhpratt commented Jan 27, 2025

jhpratt commented Dec 23, 2022 •

edited

Loading

jhpratt commented Jul 25, 2024 •

edited

Loading