Skip to content

Commit 5f4ab3c

Browse files
committed
Various changes from feedback
- Fix typo - Elaborate on drawbacks, alternatives - Add unresolved questions - Add future possibility
1 parent e1e3983 commit 5f4ab3c

File tree

1 file changed

+39
-7
lines changed

1 file changed

+39
-7
lines changed

text/3323-restrictions.md

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ Foo::Beta { y: 5 };
284284

285285
In this example, `Foo::Alpha { x: 5 }` is allowed when it is in the same crate as `Foo`. This is
286286
because `x` is not restricted within this scope, so the field can be freely mutated. Because of
287-
this, the previous concern about upholding variants is not applicable.
287+
this, the previous concern about upholding invariants is not applicable.
288288

289289
# Reference-level explanation
290290

@@ -376,21 +376,32 @@ Trait aliases cannot be implemented. As such, there is no concern about compatib
376376
# Drawbacks
377377

378378
- Additional syntax for macros to handle
379+
- More syntax to learn
380+
- While unambiguous to parse, `trait impl(crate) Foo` could be confusing due to its similarity to
381+
`impl Foo`.
379382

380383
# Alternatives
381384

382385
- `impl` and `mut` restrictions could be attributes, similar to `#[non_exhaustive]`.
383386
- The proposed syntax could by syntactic sugar for these attributes.
384387
- Visibility could be altered to accept restrictions as a type of parameter, such as
385-
`pub(crate, mut = self)`. This does not work because restrictions are not permitted everywhere
386-
visibility is.
388+
`pub(crate, mut = self)`. This is not ideal because restrictions are not permitted everywhere
389+
visibility is. As a result, any errors would have to occur later in the compilation process than
390+
they would be with the proposed syntax. It would also mean macro authors would be unable to accept
391+
only syntax that would be valid in a given context. Further, some positions such as `enum`
392+
variants do not semantically accept a visibility, while they do accept a restriction.
387393

388394
# Prior art
389395

390396
- The [`readonly` crate] simulates immutable fields outside of the defining module. Types with this
391397
attribute cannot define `Deref`, which can be limiting. Additionally, it applies to all fields and
392398
within the defining crate. The advantages of native read-only fields relating to borrow checking
393399
also do not apply when using this crate.
400+
- The `derive-getters` and `getset` crates are derive macros that are used to generate getter
401+
methods. The latter also has the ability to derive getters. This demonstrates the usefulness of
402+
reduced syntax for common behavior. Further, `getset` allows explicitly setting the visibility of
403+
the derived methods. In this manner, it is very similar to the ability to provide a path to the
404+
`mut` restriction.
394405
- The ability to restrict implementations of a trait can be simulated by a public trait in a private
395406
module. This has the disadvantage that the trait is no longer nameable by external users,
396407
preventing its use as a generic bound. Current diagnostics, while technically correct, are
@@ -404,16 +415,35 @@ Trait aliases cannot be implemented. As such, there is no concern about compatib
404415

405416
- Should an "unnecessary restriction" lint be introduced? It would fire when the restriction is as
406417
strict or less strict than the visibility. This warning could also be used for `pub(self)`.
418+
- Does this necessarily have to be decided as part of this RFC?
407419
- How will restrictions work with `macro_rules!` matchers? There is currently a `vis` matcher, but
408-
it is likely unwise to add a new matcher for each restriction. Should the new syntax be included
409-
as part of the `vis` matcher? If so, it is important to note that restrictions are not the same
410-
everywhere, if they are valid at all.
411-
- Suggestions welcome!
420+
it is likely unwise to add a new matcher for each restriction.
421+
- The proposed syntax cannot be added to the `vis` matcher, as it does not current restrict the
422+
tokens that can follow. For this reason, it could break existing code, such as the following
423+
example.
424+
425+
```rust
426+
macro_rules! foo {
427+
($v:vis impl(crate) trait Foo) => {}
428+
}
429+
430+
foo!(pub impl(crate) trait Foo);
431+
```
432+
433+
- A `restriction` matcher could work, but restrictions are not the same everywhere.
434+
- `mut_restriction` and `impl_restriction` are relatively long.
412435
- What is the interaction between stability and restrictions?
413436
- Suggestion: Visibility is an inherent part of the item; restrictions should be as well. Metadata
414437
can be added in the future indicating when an item had its restriction lifted, if applicable.
415438
The design for this is left to the language team as necessary. A decision does _not_ need to be
416439
made prior to stabilization, as stability attributes are not stable in their own right.
440+
- Should the `in` syntax be permitted for restrictions? Including it is consistent with the existing
441+
syntax for visibility. Further, the lack of inclusion would lead to continued use of the
442+
workaround for `impl`. For `mut`, there is no workaround. The syntax is not used often for
443+
visibility, but it is very useful when it is used.
444+
- Should `struct` expressions be disallowed?
445+
- Where would it be desirable to prohibit mutability after construction, but still permit
446+
construction with unchecked values?
417447

418448
# Future possibilities
419449

@@ -431,3 +461,5 @@ Trait aliases cannot be implemented. As such, there is no concern about compatib
431461
- Syntax such as `impl(mod)` could be added for clarity as an alternative to `impl(self)`.
432462
- `impl` and `mut` could be usable without a path if deemed necessary. This behavior would be
433463
identical to omitting the keyword entirely.
464+
- `mut` could be placed on the `struct` or variant itself, which would be equivalent to having the
465+
same restriction on each field. This would avoid repetition.

0 commit comments

Comments
 (0)