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.

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

Tracking issue for pub(restricted) privacy (RFC #1422) #32409

Closed
6 tasks done
nikomatsakis opened this issue Mar 21, 2016 · 139 comments
Closed
6 tasks done

Tracking issue for pub(restricted) privacy (RFC #1422) #32409

nikomatsakis opened this issue Mar 21, 2016 · 139 comments
Labels
B-RFC-implemented Blocker: Approved by a merged RFC and implemented. B-unstable Blocker: Implemented in the nightly compiler and unstable. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Mar 21, 2016

Tracking issue for rust-lang/rfcs#1422

RFC text

Milestones:

@nikomatsakis nikomatsakis added B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. T-lang Relevant to the language team, which will review and decide on the PR/issue. B-unstable Blocker: Implemented in the nightly compiler and unstable. labels Mar 21, 2016
@jonas-schievink
Copy link
Contributor

You probably meant rust-lang/rfcs#1422

@nikomatsakis
Copy link
Contributor Author

@jonas-schievink indeed. :)

@petrochenkov
Copy link
Contributor

How to parse struct S(pub (std::marker::Send + std::marker::Sync));? It's currently legal.

If we try to parse std::marker::Send as a part of pub(restricted) based on is_path_start(std) == true, then we end up in the middle of a type sum after parsing an arbitrary number of tokens with only a Path in hands and need to recover.

Similar cases with simpler disambiguation:

struct Z(pub (u8, u8));
struct W(pub (u8));

Note that pub(path) is a strict subset of pub TYPE (modulo pub(crate)), so the former can be parsed as the latter and reinterpreted later if necessary.

I suppose everything here can be disambiguated, but I'm concerned about the conflict pub(path) vs pub TYPE in general and how it affects formal properties of Rust grammar.

@nikomatsakis
Copy link
Contributor Author

@petrochenkov sigh. a very good point. @pnkfelix and I were chatting-- one obvious (but not particularly appealing) option would be to say that tuple structs just can't use pub(restricted) paths. This would be better if we had the notion that one could write struct Z { pub 0: (u8, u8) } to use the more explicit name.

We considered various other synaxes one could use:

  • pub{path}
  • pub(in path)
  • pub in path

One consideration is that I expect most people will want to either make fields pub -- meaning, as public as the struct itself -- or private. So perhaps it's ok that you can't use tuple structs to express more flexible things -- they're kind of a convenience shorthand as is. But it is unfortunate however you slice it.

@Stebalien
Copy link
Contributor

For reference, @nikomatsakis is referring to #1506.

@nikomatsakis
Copy link
Contributor Author

On Sun, Apr 3, 2016 at 5:52 AM, Vadim Petrochenkov <notifications@github.com

wrote:

Note that pub(path) is a strict subset of pub TYPE (modulo pub(crate)),
so the former can be parsed as the latter and reinterpreted later if
necessary.

Hmm, I suppose that's true. (Or might be true)

bors added a commit that referenced this issue Apr 6, 2016
[breaking-batch] Add support for `pub(restricted)` syntax in the AST

This PR allows the AST to represent the `pub(restricted)` syntax from RFC 1422 (cc #32409).

More specifically, it makes `ast::Visibility` non-`Copy` and adds two new variants, `Visibility::Crate` for `pub(crate)` and `Visitibility::Restricted { path: P<Path>, id: NodeId }` for `pub(path)`.

plugin-[breaking-change] cc #31645
r? @pnkfelix
Manishearth added a commit to Manishearth/rust that referenced this issue Apr 7, 2016
…tsakis

Lay groundwork for RFC 1422  and improve `PrivateItemsInPublicInterfacesVisitor`

This PR lays groundwork for RFC 1422 (cc rust-lang#32409) and improves `PrivateItemsInPublicInterfacesVisitor`. More specifically, it
 - Refactors away `hir::Visibility::inherit_from`, the semantics of which are obsolete.
 - Makes `hir::Visibility` non-`Copy` so that we will be able to add new variants to represent `pub(restricted)` (for example, `Visibility::Restricted(Path)`).
 - Adds a new `Copy` type `ty::Visibility` that represents a visibility value, i.e. a characterization of where an item is accessible. This is able to represent `pub(restricted)` visibilities.
 - Improves `PrivateItemsInPublicInterfacesVisitor` so that it checks for items in an interface that are less visible than the interface. This fixes rust-lang#30079 but doesn't change any other behavior.

r? @nikomatsakis
bors added a commit that referenced this issue Apr 16, 2016
Implement `pub(restricted)` privacy (RFC 1422)

This implements `pub(restricted)` privacy from RFC 1422 (cc #32409) behind a feature gate.

`pub(restricted)` paths currently cannot use re-exported modules both for simplicity of implementation and for future compatibility with RFC 1560 (cf #31783).

r? @nikomatsakis
@nikomatsakis
Copy link
Contributor Author

So in the @rust-lang/lang meeting we discussed this problem, as well as the solution implemented in #33100. Our conclusion was that we really ought to explore the cover grammar approach described earlier by @petrochenkov here:

Note that pub(path) is a strict subset of pub TYPE (modulo pub(crate)), so the former can be parsed as the latter and reinterpreted later if necessary.

It is basically the only approach that lets restricted paths and tuple structs be fully integrated, without any artificial restrictions (like needing to convert to {} form, or needing to introduce an extra set of parens). It's a shame that the most natural grammar would no longer be LL(k) (nor LR(k)), but given that such an obvious cover grammar exists, that shouldn't cause a lot of problems in practice.

bors added a commit that referenced this issue Apr 28, 2016
…matsakis

Parse `pub(restricted)` visibilities on tuple struct fields

Parse `pub(restricted)` on tuple struct fields (cc #32409).

r? @nikomatsakis
@nikomatsakis
Copy link
Contributor Author

The ambiguous syntax has come up some more on rust-lang/rfcs#1575, where it interferes with the idea of having a "visibility matcher". Some proposals were made for alternative syntax options there:

  • pub{path}
  • pub@path
  • pub(in path)

Most of these were covered (and rejected) in my prior comment, but it is true that the lookahead requirement is onerous.

@Stebalien
Copy link
Contributor

We could always just use another keyword (scope, scoped, restrict, restricted, vis, etc...):

  • restricted(path)
  • restricted path

And all related variants...

@nrc nrc added B-RFC-implemented Blocker: Approved by a merged RFC and implemented. and removed B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. labels Aug 29, 2016
@cramertj
Copy link
Member

Just to clarify with everyone: super is a path, so both pub(super) and pub(in super) are valid?

@withoutboats
Copy link
Contributor

Yes

frewsxcv added a commit to frewsxcv/rust that referenced this issue Mar 21, 2017
@cramertj
Copy link
Member

This probably shouldn't have been closed since the documentation hasn't landed yet.

@eddyb eddyb reopened this Mar 23, 2017
frewsxcv added a commit to frewsxcv/rust that referenced this issue Mar 23, 2017
…nkov

Add diagnostic for incorrect `pub (restriction)`

Given the following statement

```rust
pub (a) fn afn() {}
```

Provide the following diagnostic:

```rust
error: incorrect restriction in `pub`
  --> file.rs:15:1
   |
15 | pub (a) fn afn() {}
   |     ^^^
   |
   = help: some valid visibility restrictions are:
           `pub(crate)`: visible only on the current crate
           `pub(super)`: visible only in the current module's parent
           `pub(in path::to::module)`: visible only on the specified path
help: to make this visible only to module `a`, add `in` before the path:
   | pub (in a) fn afn() {}
```

Follow up to rust-lang#40340, fix rust-lang#40599, cc rust-lang#32409.
anatol pushed a commit to anatol/steed that referenced this issue Mar 31, 2017
@steveklabnik
Copy link
Member

I believe this is only waiting on docs, and I believe all the docs have made it. Can this be stabilized?

@petrochenkov
Copy link
Contributor

This was stabilized a month ago, the only remaining bit is the Rust-by-example PR that's not merged yet.
I think it's time to close this.

@steveklabnik
Copy link
Member

Ah excellent; I was going to say, sad if it had just missed a release.

@cramertj
Copy link
Member

I thought it did miss the release-- it was beta nominated for a bit, but that got cancelled IIRC.

@cramertj
Copy link
Member

Just checked in the playground-- it stabilizes in 1.18.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-RFC-implemented Blocker: Approved by a merged RFC and implemented. B-unstable Blocker: Implemented in the nightly compiler and unstable. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests