Skip to content

Update pub(in path) for 2018. - #480

Merged
matthewjasper merged 2 commits into
rust-lang:masterfrom
ehuss:pub-in-path-2018
Dec 17, 2018
Merged

matthewjasper merged 2 commits into
rust-lang:masterfrom
ehuss:pub-in-path-2018

Conversation

@ehuss

@ehuss ehuss commented Dec 10, 2018

Copy link
Copy Markdown
Contributor

When updating paths, I neglected to add a note that visibility paths must
be absolute.

When updating paths, I neglected to add a note that visibility paths must
be absolute.
Comment thread src/visibility-and-privacy.md Outdated
to `pub(in self)`.

> **Edition Differences**: Starting with the 2018 edition, paths for
> `pub(in path)` must start with `crate`, `self`, or `super`.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this just a consequence of the path having to be absolute?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but there is nothing else that explicitly calls that out for visibility paths. The only other part is the "use declarations", which only talk about the requirements in use declarations. Or are you saying it should be worded differently?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't realise how the reference described paths in use items. The wording here is probably fine, but I think that there should be some wording on how paths that start with an identifier are resolved in the 2015 edition.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, good catch!

@matthewjasper
matthewjasper merged commit 2a487bd into rust-lang:master Dec 17, 2018
@matthewjasper

Copy link
Copy Markdown
Contributor

Thanks!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants