Skip to content

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

Closed
Closed
(Modules) Tracking issue for crate as a visibility modifier#53120
Labels
A-visibilityArea: Visibility / privacyB-RFC-approvedBlocker: Approved by a merged RFC but not yet implemented.B-RFC-implementedBlocker: Approved by a merged RFC and implemented but not stabilized.B-unstableBlocker: Implemented in the nightly compiler and unstable.C-tracking-issueCategory: An issue tracking the progress of sth. like the implementation of an RFCS-tracking-design-concernsStatus: There are blocking design concerns.T-langRelevant to the language teamdisposition-closeThis PR / issue is in PFCP or FCP with a disposition to close it.finished-final-comment-periodThe final comment period is finished for this PR / Issue.

Description

@Centril
Centril
opened on Aug 6, 2018

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.

Unresolved questions:

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

Activity

  1. Centril
    added
    B-RFC-approvedBlocker: Approved by a merged RFC but not yet implemented.
    T-langRelevant to the language team
    C-tracking-issueCategory: An issue tracking the progress of sth. like the implementation of an RFC
    on Aug 6, 2018
  2. Centril
    added this to the Rust 2018 RC milestone on Aug 6, 2018

  3. Centril commented on Aug 6, 2018

    @Centril
    Centril
    on Aug 6, 2018
    ContributorAuthor

    Comments about crate as a visibility modifier:

    Parsing ambiguity

    Unnatural / Confusion / Not improvement

    A good idea

    pub(extern) instead

    Bikeshed

    An early preview

    Dedicated thread

  4. Centril commented on Aug 6, 2018

    @Centril
    Centril
    on Aug 6, 2018
    ContributorAuthor

    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.

  5. lilianmoraru commented on Aug 6, 2018

    @lilianmoraru
    lilianmoraru
    on Aug 6, 2018

    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.

  6. rpjohnst commented on Aug 6, 2018

    @rpjohnst
    rpjohnst
    on Aug 6, 2018
    Contributor

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

  7. Centril commented on Aug 6, 2018

    @Centril
    Centril
    on Aug 6, 2018
    ContributorAuthor

    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.

  8. matklad commented on Aug 6, 2018

    @matklad
    matklad
    on Aug 6, 2018
    Contributor

    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!.

  9. johnthagen commented on Aug 6, 2018

    @johnthagen
    johnthagen
    on Aug 6, 2018
    Contributor

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

    1. 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.
    2. 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.

  10. seanmonstar commented on Aug 6, 2018

    @seanmonstar
    seanmonstar
    on Aug 6, 2018
    Contributor

    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).

  11. johnthagen commented on Aug 6, 2018

    @johnthagen
    johnthagen
    on Aug 6, 2018
    Contributor

    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?

  12. rpjohnst commented on Aug 7, 2018

    @rpjohnst
    rpjohnst
    on Aug 7, 2018
    Contributor

    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.

  13. glaebhoerl commented on Aug 7, 2018

    @glaebhoerl
    glaebhoerl
    on Aug 7, 2018
    Contributor

    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.

  14. 112 remaining items

  15. bors
    added a commit that references this issue on May 21, 2022

    Auto merge of rust-lang#97239 - jhpratt:remove-crate-vis, r=joshtriplett

    4f372b1
  16. rfcbot
    added
    finished-final-comment-periodThe final comment period is finished for this PR / Issue.
    and removed
    final-comment-periodIn the final comment period and will be merged soon unless new substantive objections are raised.
    on May 21, 2022

  17. rfcbot commented on May 21, 2022

    @rfcbot
    rfcbot
    on May 21, 2022

    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.

  18. rfcbot
    added
    to-announceAnnounce this issue on triage meeting
    on May 21, 2022

  19. jhpratt commented on May 21, 2022

    @jhpratt
    jhpratt
    on May 21, 2022
    Member

    PR up for removing the feature itself: #97254

  20. Dylan-DPC
    added a commit that references this issue on May 23, 2022

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

    b73f1c7
  21. apiraino
    removed
    to-announceAnnounce this issue on triage meeting
    on May 27, 2022
  22. flip1995
    added a commit that references this issue on Jun 4, 2022

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

    77d2b10

  23. joshtriplett commented on Jul 16, 2022

    @joshtriplett
    joshtriplett
    on Jul 16, 2022
    Member

    Removed in #97254 .

  24. joshtriplett
    closed this as completedon Jul 16, 2022
  25. bors
    added a commit that references this issue on Jul 18, 2023

    Auto merge of #15287 - lowr:internal/remove-crate-vis, r=lnicola

    3759c41
  26. kevinmehall
    added 2 commits that reference this issue on Oct 11, 2023

    Remove support for `crate` visibility modifier

    8d11048

    Remove support for `crate` visibility modifier

    d51f5c4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-visibilityArea: Visibility / privacyB-RFC-approvedBlocker: Approved by a merged RFC but not yet implemented.B-RFC-implementedBlocker: Approved by a merged RFC and implemented but not stabilized.B-unstableBlocker: Implemented in the nightly compiler and unstable.C-tracking-issueCategory: An issue tracking the progress of sth. like the implementation of an RFCS-tracking-design-concernsStatus: There are blocking design concerns.T-langRelevant to the language teamdisposition-closeThis PR / issue is in PFCP or FCP with a disposition to close it.finished-final-comment-periodThe final comment period is finished for this PR / Issue.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions