Tracking Issue for Restrictions

[tmandry]

View all comments

This is a tracking issue for the RFC "Restrictions" (rust-lang/rfcs#3323).
The feature gates for the issue are:

• #![feature(impl_restriction)]
• #![feature(mut_restriction)]

About tracking issues

Tracking issues are used to record the overall progress of implementation.
They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Steps

• Implement the RFC
  • impl restrictions
    • Parse impl restrictions #152943
    • impl restriction lowering #153556
    • Semantic checks of impl restrictions #154661
  • mut restrictions
  • r-a support
    • feat: Support impl and mut restrictions rust-analyzer#22022
  • rustdoc support
• Adjust documentation (see instructions on rustc-dev-guide)
• Stabilization PR (see instructions on rustc-dev-guide)

Unresolved Questions

• What should the order of keywords be, including ordering with unsafe fields?
  Decided in Decide on keyword order for impl, mut restrictions #155222; group with visibility. Remaining question of overall syntax remains.
• Should a simpler syntax be provided for common cases? For instance, sealed or readonly. A
  different syntax altogether could be used as well.
  • Also see Restrictions rfcs#3323 (comment).
• Should an "unnecessary restriction" lint be introduced? It would fire when the restriction is as
  strict or less strict than the visibility. This warning could also be used for pub(self).
  • Does this necessarily have to be decided as part of this RFC?
• How will restrictions work with macro_rules! matchers? There is currently a vis matcher, but
  it is likely unwise to add a new matcher for each restriction.
  • The proposed syntax cannot be added to the vis matcher, as it does not current restrict the
    tokens that can follow. For this reason, it could break existing code, such as the following
    example.

    macro_rules! foo {
        ($v:vis impl(crate) trait Foo) => {}
    }
    foo!(pub impl(crate) trait Foo);

  • A restriction matcher could work, but restrictions are not the same everywhere.
  • mut_restriction and impl_restriction are relatively long.
  • This is a non-issue if the syntax ends up being attribute-based (or sugar for attributes).
• What is the interaction between stability and restrictions?
  • Suggestion: Visibility is an inherent part of the item; restrictions should be as well. Metadata
    can be added in the future indicating when an item had its restriction lifted, if applicable.
    The design for this is left to the language team as necessary. A decision does not need to be
    made prior to stabilization, as stability attributes are not stable in their own right.
• Should the in syntax be permitted for restrictions? Including it is consistent with the existing
  syntax for visibility. Further, the lack of inclusion would lead to continued use of the
  workaround for impl. For mut, there is no workaround. The syntax is not used often for
  visibility, but it is very useful when it is used.
• Should struct expressions be disallowed?
  • Where would it be desirable to prohibit mutability after construction, but still permit
    construction with unchecked values?

Implementation history

• Implement RFC 3323: restrictions #106074 — closed in favor of a piecemeal approach
• Introduce impl restrictions to AST, lower to rustc_middle #141754 — closed in favor of GSoC
• Parse impl restrictions #152943
• impl restriction lowering #153556
• Semantic checks of impl restrictions #154661

[jhpratt]

NB: My mostly-complete implementation uses #![feature(impl_restriction)] and #![feature(mut_restriction)], not a single shared feature.

[SUPERCILEX]

Thanks for keeping the features separate! :)

[rinarakaki]

The whole this RFC is trying to solve is admittedly complicated in terms both of the concepts and the syntax but I found this RFC might relate to what I have proposed and provide with what I wanted: https://internals.rust-lang.org/t/structs-field-level-mutability-control-at-instantiation-not-at-definition/16061. The scoped mutability syntax included.

[jhpratt]

This is not the place to rehash the RFC, expect for the listed unresolved questions. Also, saying you "found" the RFC is misleading — you are the one who created the thread.

[rpjohnst]

I would like to sketch a holistic mental model to shed some light on the unresolved questions: this, along with some other features like #[non_exhaustive], type-alias-impl-trait, safe transmute, unsafe fields, and private impls, are all fundamentally about making visibility more granular. Visibility is our one big, orthogonal tool for enforcing invariants, so keeping things under that umbrella keeps the language feeling clean and composable. A beginner encountering any variation on visibility for the first time should be directed to our documentation on this approach.

Under this model, the RFC splits up the existing visibility of fields into 1) the visibility of their value and address (read) and 2) the visibility of their construction and mutable place exprs (write). Similarly, it splits up the existing visibility of traits into 1) the visibility of the trait as a bound and 2) the visibility of its "construction" (impl-ability). (This comment on the RFC thread also notes that construction is fundamentally a whole-type property, not a per-field property.)

#[non_exhaustive] splits out the visibility of a struct's construction, and an enum's full set of variants. Type-alias-impl-trait splits out the visibility of the concrete type, as I noted on Zulip. Safe transmute is trying to consider the visibility of a type's layout. Unsafe fields would hang proof obligations off of the same "facet of visibility" of the field as this RFC's mut. And private impls would split out the visibility of an impl from the visibility of its Self type.

To me, this model implies answers to a few of the unresolved questions, and some future possibilities to consider:

• First, syntax: we should go with the pub(..)-based design in spite of the concern about enum variant fields, neatly resolving the shorthand and macro matcher questions. We lose very little with post-expansion error checking, where we can give more meaningful errors anyway. Consider the current error message for pub on an enum variant field: "unnecessary visibility qualifier, pub not permitted here because it's implied." Also consider the cited prior art: the fully-general form of read-only properties in e.g. C# uses private set { .. }- a visibility modifier.
• As noted in the RFC thread, controlling both mutable places and construction with mut, yet not supporting anything like mut(nowhere), makes for a rather false analogy with mut on bindings. We should resolve the struct expression question either by giving construction its own type-level visibility facet aligned with the existing #[non_exhaustive], or else by renaming mut to something that more fully encompasses what it does.
• If we establish that these features work in terms of granular visibility, that gives us a useful precedent and vocabulary to guide future additions like visibility on trait items or enum variant fields, locally-transparent type-alias-impl-trait, safe transmute across crates, unsafe fields, etc. We can explicitly consider the way visibility is divided up and how that relates to semver.

---

However, having given my best shot above at making this proposal make sense to myself, I am concerned that the RFC was merged without sufficient motivation over existing practice. The cost of new syntax like this, especially when there are already idiomatic alternatives, weighs much more to me than having all these knobs available. Indeed, there were several well-reasoned comments in this direction, before and during the FCP (which I missed).

For instance, the suggestion in rust-lang/rfcs#3323 (comment) and rust-lang/rfcs#3323 (comment) to reduce the feature's surface area based on more concrete examples. Attributes are already our main "escape valve" for new and increasingly niche syntax- this would address all the syntactic open questions even more effectively than the pub(..) I advocated for above. The first comment also suggested splitting up the RFC specifically because sealed traits are much more strongly motivated than read-only fields.

Or consider rust-lang/rfcs#3323 (comment) which makes a strong case that the mut half of this RFC may not meaningfully improve the status quo. The response didn't point to any better examples, and merely handwaved that it might help in some cases with borrow checking. So on one hand we've got hand-written or macro-generated getters (including borrow-splitting ones or view types), and on the other we've got the RFC's new syntax which can't fully replace it or offer any truly new capabilities. This actually came up again during FCP!

[jhpratt]

I understand what you're saying with regard to a mental model, but I have to disagree with a fair amount.

we should go with the pub(..)-based design in spite of the concern about enum variant fields, neatly resolving the shorthand and macro matcher questions. We lose very little with post-expansion error checking, where we can give more meaningful errors anyway.

The concern is not about enum variant fields. It is about having what is permitted as part of visibility being different in different places. For example, you couldn't do pub(mut in crate) trait Foo {} for obvious reasons. I strongly believe that if something is a visibility, it should be the same everywhere. I cannot immediately think of any other construct that has such significant differences depending on where it is used. Further, I believe that a macro author should be able to reject invalid code upon invocation, not just deferring errors to AST validation. After all, what if the macro author doesn't want to permit a restriction?

As noted in the RFC thread, controlling both mutable places and construction with mut, yet not supporting anything like mut(nowhere), makes for a rather false analogy with mut on bindings. We should resolve the struct expression question either by giving construction its own type-level visibility facet aligned with the existing #[non_exhaustive], or else by renaming mut to something that more fully encompasses what it does.

I have no objection to full immutability. Without rereading the full RFC PR, I'm fairly certain I asked at least once for potential syntax, and no one provided anything that could be agreed upon. I would personally prefer mut(!), following the ! type.

For struct expressions, there is a key question that you did not address.

Where would it be desirable to prohibit mutability after construction, but still permit
construction with unchecked values?

Unless there is a use case here, I see no reason to permit it. If you have a suggestion for a keyword other than mut, I'm all ears.

If we establish that these features work in terms of granular visibility, that gives us a useful precedent and vocabulary to guide future additions like visibility on trait items or enum variant fields, locally-transparent type-alias-impl-trait, safe transmute across crates, unsafe fields, etc. We can explicitly consider the way visibility is divided up and how that relates to semver.

I believe that process-wise, this should require significant public discussion and possibly an FCP of some sort. Suggesting a mental model is one thing, but making decisions based off of it is another.

For instance, the suggestion in rust-lang/rfcs#3323 (comment) and rust-lang/rfcs#3323 (comment) to reduce the feature's surface area based on more concrete examples.

I addressed those links in the comments immediately following them.

The first comment also suggested splitting up the RFC specifically because sealed traits are much more strongly motivated than read-only fields.

If even one lang team member wanted it split, it would have been. I stated this repeatedly. If you have an issue with the RFC not being split, the time has passed. It was accepted as one, and as such both restrictions have been accepted (minus the unresolved questions). There is absolutely no reason to believe that the outcome would have been any different were the RFC split in two.

Since you mention view types, however, the "handwaving" you're referring to (language that I find borderline rude) is identical to that of view types. They are complementary, and would have the same effect on splitting borrow checking.

[jhpratt]

As a general note to anyone, I have a nearly complete implementation of restrictions on my fork of rustc. There is only one outstanding bug (impl restrictions not being enforced for traits from external crates). Aside from that, a PR should be up soon. For anyone that has concerns about syntax, you'll be able to play around with it on nightly soon.

(in a separate comment because it's not in response to the prior comment)

[jhpratt]

Mostly-complete implementation is up: #106074

Any assistance with figuring out the one bug is appreciated!

[dtolnay]

Are restrictions proposed for the mutability of statics and extern statics? I didn't find this addressed one way or the other in the RFC text or RFC PR discussion.

pub static mut(in …) ASDF: usize = 0;

extern "C" {
    pub static mut(in …) JKL: usize;
}

[jhpratt]

@dtolnay No one brought this up at any point. I think it's a reasonable enough extension that it shouldn't require another RFC. Once the current implementation PR is merged, I can look into it.