(Modules) Tracking issue for `crate` as a visibility modifier #53120

Centril · 2018-08-06T18:33:56Z

This is a sub-tracking issue for the RFC "Clarify and streamline paths and visibility" (rust-lang/rfcs#2126)
dealing with the question of crate as a visibility modifier.

Implement the RFC
Adjust documentation (see instructions on forge)
Stabilization PR (see instructions on forge)

Unresolved questions:

How do we parse struct Foo(crate ::bar)?

The text was updated successfully, but these errors were encountered:

Centril · 2018-08-06T19:06:33Z

Comments about crate as a visibility modifier:

Parsing ambiguity

Tracking issue for RFC 2126: Clarify and streamline paths and visibility #44660 (comment)
Tracking issue for RFC 2126: Clarify and streamline paths and visibility #44660 (comment)
Tracking issue for RFC 2126: Clarify and streamline paths and visibility #44660 (comment)
Tracking issue for RFC 2126: Clarify and streamline paths and visibility #44660 (comment)
Tracking issue for RFC 2126: Clarify and streamline paths and visibility #44660 (comment)
Tracking issue for RFC 2126: Clarify and streamline paths and visibility #44660 (comment)
Tracking issue for RFC 2126: Clarify and streamline paths and visibility #44660 (comment)
Tracking issue for RFC 2126: Clarify and streamline paths and visibility #44660 (comment)
Tracking issue for RFC 2126: Clarify and streamline paths and visibility #44660 (comment)
Tracking issue for RFC 2126: Clarify and streamline paths and visibility #44660 (comment)
Tracking issue for RFC 2126: Clarify and streamline paths and visibility #44660 (comment)
Tracking issue for RFC 2126: Clarify and streamline paths and visibility #44660 (comment)
Tracking issue for RFC 2126: Clarify and streamline paths and visibility #44660 (comment)
Tracking issue for RFC 2126: Clarify and streamline paths and visibility #44660 (comment)

Unnatural / Confusion / Not improvement

A good idea

Tracking issue for RFC 2126: Clarify and streamline paths and visibility #44660 (comment)

`pub(extern)` instead

Tracking issue for RFC 2126: Clarify and streamline paths and visibility #44660 (comment)

Bikeshed

An early preview

Dedicated thread

https://internals.rust-lang.org/t/visibility-modifiers-pub-super-vs-super/7838

Centril · 2018-08-06T19:08:19Z

Personally, I am very much in favor of crate as a visibility modifier and I share @stepancheg's comment here. I think that we should encourage smaller and stricter visibilities and crate does just that.

lilianmoraru · 2018-08-06T20:18:50Z

I'm personally ok with writing pub(crate), the intent seems very explicit.
The example given by @johnthagen is really painful to see(using crate):

use crate::menu::{Sound, Volume};

crate mod color;

...

/// A type for storing text and an associated color it should
/// be drawn as.
crate struct ColoredText {
    crate color: types::Color,
    crate text: &'static str,
}

crate mod color; especially seems confusing, you definitely need to put a bit of thought on what's happening there.

rpjohnst · 2018-08-06T21:19:24Z

Some of these examples are very C-static-esque- intuitively related but really surprisingly distinct.

Centril · 2018-08-06T21:42:41Z

The example due to @johnthagen does not read poorly to me. In fact, it reads naturally and I quite like the symmetry. It is beautiful in a way.

If the readability of:

crate struct ColoredText {
    crate color: types::Color,
    crate text: &'static str,
}

becomes a problem; then an IDE/editor that understands the syntax of Rust can highlight the crate tokens in the different positions with different colors. That should clear up the difference well I think.

matklad · 2018-08-06T22:22:35Z

crate as visibility modifier is definitely weird: it uses very rust-specific keyword for a thing which is not specific to rust. Kotlin & C# use internal for this.

I'd personally would want to reuse pub for crate-visible, and go for a more screamy syntax for world visible, like pub* or pub!.

johnthagen · 2018-08-06T23:22:36Z

It's been brought up before, but I think the root issues I see are:

crate is a noun. I think pub(crate) is long and odd looking, so I fully support replacing it with something, but it did have the public adjective associated with it, so grammatically it flowed better.
crate is now used as the anchor for "this-crate" imports, which means something different than "wherever this is defined, it's also exported visibly from this crate"

// Here `crate` means the root of this crate.
use crate::menu::{Sound, Volume};

// Here, `crate` means: export crate::game::color
// The `crate` is referring to `color`, not the root.
crate mod color;

...

/// A type for storing text and an associated color it should
/// be drawn as.
// Same potential confusion as `crate mod color`
crate struct ColoredText {
    crate color: types::Color,
    crate text: &'static str,
}

Compared to an example, using @matklad's internal from Kotlin/C#.

use crate::menu::{Sound, Volume};

internal mod color;

...

/// A type for storing text and an associated color it should
/// be drawn as.
internal struct ColoredText {
    internal color: types::Color,
    internal text: &'static str,
}

I'm not saying internal is the right keyword (Rust likes it's very short abbreviations, and int is unfortunately full of C/C++/Java confusion), but I personally think that the second example is immediately more readable.

I also think the crate visibility keyword will be confusing to people coming to Rust from other languages. If even many of us involved enough in Rust to comment on these threads are jarred, I'd have to imagine it will trip up people new to Rust as well.

seanmonstar · 2018-08-06T23:25:18Z

The slightly longer syntax of pub(crate) may not be such a big deal if weren't becoming a warning/error to have pub items that aren't reachable outside the crate. I personally wish that if I had a pub(crate) struct Foo { ... }, that the compiler could realize that all the pub fns in a impl Foo are clearly not reachable, and not bother me about it.

I find it to be just busy work currently in Rust 2015 if I ever mark a type from pub struct Foo to pub(crate) struct Foo, how the compiler then yells in all the places that some other pub fns exist using the suddenly pub(crate) type, when the problem isn't real, because the other type is also pub(crate).

johnthagen · 2018-08-06T23:26:53Z

I also find @matklad's idea of re-purposing pub as crate-public and using export or something for world-visible exports. But that may be too big a divergence for an edition?

rpjohnst · 2018-08-07T02:13:56Z

Repurposing pub as crate-public and adding a new visibility for world-public was the proposal before the current version. Such a change to existing semantics was considered too drastic even for an edition, which is why pub is now keeping its current meaning.

What was less discussed and considered, I think, was repurposing pub solely via a lint. Perhaps we could switch the lint from "warn on pub that's not accessible outside the crate" to "warn on pub that is accessible outside the crate," and add a purely optional pub(extern)/export keyword. That is, don't change any semantics, just add a lint-silencing syntax.

I suspect this would be less disruptive, based on the assumption that there are fewer world-public items than crate-public items. It would also preserve the intuitive (though subtly incorrect) meaning of pub as "export from the current module" rather than confronting everyone with visibility's true behavior.

glaebhoerl · 2018-08-07T10:58:11Z

Rust likes it's very short abbreviations, and int is unfortunately full of C/C++/Java confusion

FWIW, though it only saves two characters, if we wanted to abbreviate internal, the "right" abbreviation would probably, by analogy with external, be intern. Unfortunate that that is also a noun with a commonly understood, different meaning. Oh well.

johnthagen · 2018-08-07T11:32:41Z

@glaebhoerl intern is a good option to consider! ❤️

The symmetry with extern is really nice, and IMO would greatly reduce the potential confusion with the noun form of intern.

It's short, (only 1 more character than crate) and doesn't clash with use crate::.

The updated example would look like:

use crate::menu::{Sound, Volume};

intern mod color;

...

/// A type for storing text and an associated color it should
/// be drawn as.
intern struct ColoredText {
    intern color: types::Color,
    intern text: &'static str,
}

CasualX · 2018-08-09T20:51:09Z

I've mentioned it before, but I'm not sure what the problem is with nouns used as adjectives?

To recap: there are plenty of nouns that can be used as adjectives, eg. a house cat, a computer mouse, a computer desk, etc... A google search for english nouns used as adjectives seems to indicate there's nothing inherently wrong although not all nouns work as adjectives.

Let's try it:

crate mod hello; A crate module named hello, feels fine.
crate fn world() {} A crate function named world, feels fine.
crate struct Foo; A crate struct named Foo, feels fine.
crate enum Bar {} A crate enum named Bar, feels fine.
crate trait Baz {} A crate trait named Baz, feels fine.

crate use self::local::Foo; Ack, this one does not work, a crate use? You can read it as crate usable item named Foo. It does break the pattern.

It can also be awkward when used in front of struct members and even more in combination with crate as the root of a path.

While crate isn't perfect, I'm not convinced that 'being a noun' is a deciding factor.

UtherII · 2018-08-10T06:01:18Z

The problem is that it is very uncommon. There is no programming language I know that use nouns as type modifier.

epage · 2018-08-15T23:16:46Z

@Centril

becomes a problem; then an IDE/editor that understands the syntax of Rust can highlight the crate tokens in the different positions with different colors. That should clear up the difference well I think.

Personally, while I find features of different editors nice, I don't think we should design the language with the assumption of a sufficiently advanced editor. I felt like C# was designed this way and it was a major factor in my frustration with that language.

Centril · 2018-08-15T23:52:52Z

@epage I think crate as a visibility modifier is a good idea regardless of highlighting; I'm merely suggesting that highlighting is additional mitigation. In particular, it should be fairly trivial for any editor to highlight crate:: differently than crate field because the former is always crate + :: which is easy to check for in all cases except crate ::foo::bar (but this will be fairly rare..).

matklad · 2018-08-16T00:04:16Z

As an IDE person, I think such highlighting would add significant amount of noise for a very little amount of information, netting negative. IMO (this is highly personal, but informed by both using and implementing powerful IDEs) highlighting works best when it conveys semantic non-local information (does this usage refers to a variable that was declared with mut) and de emphasizes local "boilerplaty" aspects of code (so all keywords should be styled exactly the same).

zesterer · 2018-08-16T01:15:30Z

It seems to me that dom (i.e: domestic) is a potential candidate.

It has three letters (nice for alignment, easy to remember), it's not a noun, and - other than 'Document Object Model' - I don't think it has any particular ambiguity attached to it.

pub struct MyStruct {
    dom num: i32,
    pub msg: String,
}

Does anybody have thoughts on this?

epage · 2018-08-16T02:53:30Z

One angle on this that I've seen mentioned but couldn't find in the summary (thanks for doing that btw!) is how a shortcut fits in with the existing pub() syntax.

If pub and <something> (e.g. crate) have special meaning, it further reduces the visibility, and by extension familiarity, of pub(<something>). Whatever solution we go with, I think it should support or replace the existing machinery rather than being yet another one.

For example, if we use crate or replacement:

Should crate be extended to take on scoping restrictions (e.g. crate(<something>))?
Should we deprecate pub() so pub only has one meaning?

Considering this and my understanding of the goal (clarify public API from internal API), led me to recreate @vitiral's idea of pub(extern).

Fits within existing machinery
imo improves the existing machinery by making pub a shortcut of pub(<something>) rather than being a special case
If the public API is significantly smaller than the private API, then we've weighted the syntax the right way.
But might confuse people coming from other languages where public means it might be in your public API.

epage · 2018-08-16T03:06:56Z

RE Impact on encapsulation

One benefit of the existing pub system is encapsulation. The easy-path is to expose API only one-level up. This makes it easier to have things public to parts of a crate but private to the whole create.

While there will still be pub(super), having a shortcut for pub(crate) will push people in the direction of using it more, encouraging people to not encapsulate their APIs.

I suspect this isn't an issue because of the culture of small crates.

But in considering this, it gives me another iteration on my above comment on pub(extern)

pub should be a shortcut for pub(super)
pub(extern) is required for your public API.

I previously brought up the concern of people transitioning from other languages. This better aligns with them.

More closely matches how public works in various languages
Some languages tend to have a distinct mechanism for public API, so this is explainable to them.

imo this is the best of all worlds. So tear it apart and help me understand why not :)

parasyte · 2018-08-16T03:13:27Z

I still hate the pub(foo) syntax. Hyperbolically, it looks like it can't decide if it's a function call or a mashup of multiple keywords. We don't use let(mut) or for(in) so what's the deal with this one?

vitiral · 2018-08-16T03:52:41Z

@parasyte pub<foo> for the win! After all, isn't it a type of visibility?

lu-zero · 2018-08-16T04:13:46Z

pub<crate> or pub(crate) feel better indeed.

ralfbiedert · 2018-08-16T08:23:59Z

Some thoughts from someone who changed camp:

At first I was very opposed to crate and thought "this is ruining the nice pub".

I then tried it side-by-side in some of my projects and let it sink in.

Frankly, after a few days I couldn't stand looking at pub(X) anymore, it feels clunky in comparison, and harder to read.

Initially I feared there might be (visual) ambiguity; but to me the opposite happened: If I see crate now I know it's, well, "crate stuff". Whether that is importing modules or declaring visibility. What exactly is in the overwhelming majority of cases very clear from the context (sort of like ambiguity in English).

I can see there might be still be residual "harder" (visual) ambiguity in some cases, but I wouldn't want to trade that back for what now feels like an massive quantitative readability win (as in: "source code lines that require less visual tokenization / effort vs. source code lines that became more ambiguous" ).

From that angle crate - intern (or any other asymmetry) would also feel like a step back.

Having said this, I don't know about parsing ambiguity. If I had to pick one, I'd much rather have a good story around "crate means crate stuff" than a good story around "crate ::foo::bar just works".

rfcbot · 2022-05-11T17:38:13Z

Team member @joshtriplett has proposed to close this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

rfcbot · 2022-05-11T17:42:14Z

🔔 This is now entering its final comment period, as per the review above. 🔔

jhpratt · 2022-05-11T17:44:18Z

I presume if/when the FCP completes, it would be acceptable to remove the crate visibility modifier from rustc?

joshtriplett · 2022-05-20T06:01:07Z

@jhpratt Yes. And in the meantime it's probably a good idea to start preparing PRs to remove the usage of it.

jhpratt · 2022-05-20T06:10:55Z

Ok. I can, at the least, remove all usage of it in the main repositories. That should be quite easy. If I have time, I will also remove the internals of it. If someone else feels the urge to do that, though, feel free; just let me know so I won't.

jhpratt · 2022-05-21T01:11:51Z

Large (understatement) PR up removing all use of the crate visibility modifier: #97239. FCP should complete tomorrow, and it would be great to have that merged sooner rather than later. Merge conflicts will pop up constantly if it's not.

Remove `crate` visibility modifier FCP to remove this syntax is just about complete in rust-lang#53120. Once it completes, this should be merged ASAP to avoid merge conflicts. The first two commits remove usage of the feature in this repository, while the last removes the feature itself.

rfcbot · 2022-05-21T17:52:35Z

The final comment period, with a disposition to close, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

jhpratt · 2022-05-21T17:55:28Z

PR up for removing the feature itself: #97254

Remove feature: `crate` visibility modifier FCP completed in rust-lang#53120.

joshtriplett · 2022-07-16T22:01:54Z

Removed in #97254 .

internal: remove `crate` visibility modifier This PR removes `crate` as a now-unaccepted experimental visibility modifier from our parser. This feature has been [unaccepted] and [removed] from rustc more than a year ago, so I don't think this removal affects anyone. [unaccepted]: rust-lang/rust#53120 (comment) [removed]: rust-lang/rust#97239

Dropped in rustc: rust-lang/rust#53120 (comment)

Centril 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. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC labels Aug 6, 2018

Centril added this to the Rust 2018 RC milestone Aug 6, 2018

Centril mentioned this issue Aug 6, 2018

Tracking issue for RFC 2126: Clarify and streamline paths and visibility #44660

Closed

11 tasks

Centril mentioned this issue Aug 8, 2018

Backfill contents since 1.0 rust-lang/edition-guide#75

Merged

rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-close This PR / issue is in PFCP or FCP with a disposition to close it. labels May 11, 2022

rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels May 11, 2022

jhpratt mentioned this issue May 21, 2022

Remove crate visibility modifier #97239

Merged

rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels May 21, 2022

rfcbot added the to-announce Announce this issue on triage meeting label May 21, 2022

jhpratt mentioned this issue May 21, 2022

Remove feature: crate visibility modifier #97254

Merged

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue May 23, 2022

Rollup merge of rust-lang#97254 - jhpratt:remove-crate-vis, r=cjgillot

b73f1c7

Remove feature: `crate` visibility modifier FCP completed in rust-lang#53120.

JohnTitor mentioned this issue May 25, 2022

Mention about removal of crate visibility specifier rust-lang/rfcs#3273

Merged

apiraino removed the to-announce Announce this issue on triage meeting label May 27, 2022

flip1995 pushed a commit to flip1995/rust that referenced this issue Jun 4, 2022

Rollup merge of rust-lang#97254 - jhpratt:remove-crate-vis, r=cjgillot

77d2b10

Remove feature: `crate` visibility modifier FCP completed in rust-lang#53120.

joshtriplett closed this as completed Jul 16, 2022

dtolnay mentioned this issue Jan 30, 2023

Delete Visibility::Crate and VisCrate dtolnay/syn#1339

Merged

lowr mentioned this issue Jul 15, 2023

internal: remove crate visibility modifier rust-lang/rust-analyzer#15287

Merged

kevinmehall added a commit to kevinmehall/rust-peg that referenced this issue Oct 11, 2023

Remove support for crate visibility modifier

8d11048

Dropped in rustc: rust-lang/rust#53120 (comment)

kevinmehall added a commit to kevinmehall/rust-peg that referenced this issue Oct 11, 2023

Remove support for crate visibility modifier

d51f5c4

Dropped in rustc: rust-lang/rust#53120 (comment)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(Modules) Tracking issue for `crate` as a visibility modifier #53120

(Modules) Tracking issue for `crate` as a visibility modifier #53120

Centril commented Aug 6, 2018 •

edited

Loading

Centril commented Aug 6, 2018 •

edited

Loading

Centril commented Aug 6, 2018

lilianmoraru commented Aug 6, 2018 •

edited

Loading

rpjohnst commented Aug 6, 2018

Centril commented Aug 6, 2018 •

edited

Loading

matklad commented Aug 6, 2018

johnthagen commented Aug 6, 2018 •

edited

Loading

seanmonstar commented Aug 6, 2018

johnthagen commented Aug 6, 2018

rpjohnst commented Aug 7, 2018 •

edited

Loading

glaebhoerl commented Aug 7, 2018

johnthagen commented Aug 7, 2018

CasualX commented Aug 9, 2018 •

edited

Loading

UtherII commented Aug 10, 2018

epage commented Aug 15, 2018

Centril commented Aug 15, 2018

matklad commented Aug 16, 2018

zesterer commented Aug 16, 2018 •

edited

Loading

epage commented Aug 16, 2018 •

edited

Loading

epage commented Aug 16, 2018

parasyte commented Aug 16, 2018

vitiral commented Aug 16, 2018

lu-zero commented Aug 16, 2018

ralfbiedert commented Aug 16, 2018 •

edited

Loading

rfcbot commented May 11, 2022 •

edited by scottmcm

Loading

rfcbot commented May 11, 2022

jhpratt commented May 11, 2022

joshtriplett commented May 20, 2022

jhpratt commented May 20, 2022

jhpratt commented May 21, 2022 •

edited

Loading

rfcbot commented May 21, 2022

jhpratt commented May 21, 2022

joshtriplett commented Jul 16, 2022

(Modules) Tracking issue for crate as a visibility modifier #53120

(Modules) Tracking issue for crate as a visibility modifier #53120

Comments

Centril commented Aug 6, 2018 • edited Loading

Centril commented Aug 6, 2018 • edited Loading

Parsing ambiguity

Unnatural / Confusion / Not improvement

A good idea

pub(extern) instead

Bikeshed

An early preview

Dedicated thread

Centril commented Aug 6, 2018

lilianmoraru commented Aug 6, 2018 • edited Loading

rpjohnst commented Aug 6, 2018

Centril commented Aug 6, 2018 • edited Loading

matklad commented Aug 6, 2018

johnthagen commented Aug 6, 2018 • edited Loading

seanmonstar commented Aug 6, 2018

johnthagen commented Aug 6, 2018

rpjohnst commented Aug 7, 2018 • edited Loading

glaebhoerl commented Aug 7, 2018

johnthagen commented Aug 7, 2018

CasualX commented Aug 9, 2018 • edited Loading

UtherII commented Aug 10, 2018

epage commented Aug 15, 2018

Centril commented Aug 15, 2018

matklad commented Aug 16, 2018

zesterer commented Aug 16, 2018 • edited Loading

epage commented Aug 16, 2018 • edited Loading

epage commented Aug 16, 2018

parasyte commented Aug 16, 2018

vitiral commented Aug 16, 2018

lu-zero commented Aug 16, 2018

ralfbiedert commented Aug 16, 2018 • edited Loading

rfcbot commented May 11, 2022 • edited by scottmcm Loading

rfcbot commented May 11, 2022

jhpratt commented May 11, 2022

joshtriplett commented May 20, 2022

jhpratt commented May 20, 2022

jhpratt commented May 21, 2022 • edited Loading

rfcbot commented May 21, 2022

jhpratt commented May 21, 2022

joshtriplett commented Jul 16, 2022

(Modules) Tracking issue for `crate` as a visibility modifier #53120

(Modules) Tracking issue for `crate` as a visibility modifier #53120

Centril commented Aug 6, 2018 •

edited

Loading

Centril commented Aug 6, 2018 •

edited

Loading

`pub(extern)` instead

lilianmoraru commented Aug 6, 2018 •

edited

Loading

Centril commented Aug 6, 2018 •

edited

Loading

johnthagen commented Aug 6, 2018 •

edited

Loading

rpjohnst commented Aug 7, 2018 •

edited

Loading

CasualX commented Aug 9, 2018 •

edited

Loading

zesterer commented Aug 16, 2018 •

edited

Loading

epage commented Aug 16, 2018 •

edited

Loading

ralfbiedert commented Aug 16, 2018 •

edited

Loading

rfcbot commented May 11, 2022 •

edited by scottmcm

Loading

jhpratt commented May 21, 2022 •

edited

Loading