Stabilize pub(restricted) #40556

cramertj · 2017-03-15T20:49:22Z

rust-highfive · 2017-03-15T20:49:33Z

(rust_highfive has picked a reviewer for you, use r? to override)

retep998 · 2017-03-15T21:09:30Z

Shouldn't we first just stabilize pub(restricted) so people have time to switch to it before we turn private in public into a hard error?

cramertj · 2017-03-15T21:12:56Z

@retep998 That seems reasonable to me.

cramertj · 2017-03-15T21:28:11Z

@retep998 I'll have a new commit up in a minute with that change.

petrochenkov · 2017-03-15T21:52:24Z

src/librustc_privacy/lib.rs

-                                               format!("private type `{}` in public \
-                                                        interface (error E0446)", ty));
-                    }
+                    let mut err = struct_span_err!(self.tcx.sess, self.span, E0446,


private_in_public can't be turned into an error in the near future unfortunately.
self.tcx.sess.features.borrow().pub_restricted can be well approximated with is_pub_restricted(vis) && is_pub_restricted(self.required_visibility) where is_pub_restricted = not pub && not private.
The idea is to always report an error and not warning when pub(something) is involved.

What do you mean by not private? Visibility is either Public, Restricted(DefId), or Invisible (reserved for private external items).

You can use hir::Visibility for this (as opposed to ty::Visibility).
For vis HIR visibility is immediately available (item.vis) , for self.required_visibility you can pass it from PrivateItemsInPublicInterfacesVisitor::visit_item to here somehow.

Even for hir::Visibility, I still only see Public, Crate, Restricted {...}, and Inherited, no Private. Am I missing something obvious?

Inherited is private (i.e. no any explicit pub)

sfackler · 2017-03-16T00:51:34Z

Beta nominating since I think @aturon was interested in landing this in 1.17.

nikomatsakis · 2017-03-16T16:47:42Z

I'm not inclined to backport a stabilization -- or at least not this one.

nikomatsakis · 2017-03-16T16:51:06Z

@bors r+

bors · 2017-03-16T16:51:07Z

📌 Commit 28626ca has been approved by nikomatsakis

nikomatsakis · 2017-03-16T16:51:26Z

Note: doc pr is here: rust-lang/reference#12

petrochenkov · 2017-03-16T17:14:40Z

Right now this PR turns all private-in-public errors for pub(restricted) into warnings (since they are not in the "old set").
I'd prefer @cramertj to implement #40556 (comment) before merging.

@bors r-

nikomatsakis · 2017-03-16T17:52:21Z

Hmm. I see. OK, that probably makes sense.

strega-nil · 2017-03-16T20:06:46Z

I disagree with this stabilization. This is a poor implementation of pub(restricted) which has too much power that isn't all that useful for real code, while also being ugly.

nikomatsakis · 2017-03-16T20:41:48Z

Removing beta-nominated. Seems like there is no need to backport a stabilization, no great urgency.

strega-nil · 2017-03-17T07:03:44Z

Okay, I remove my disagreement, actually (sorry). I read more of the reasoning behind the need for in path, and I can agree that it is useful. (sorry sorry sorry)

petrochenkov · 2017-03-17T08:10:55Z

@cramertj
A simpler and more precise solution - just walk all the crate's HIR and check if pub(restricted) is ever used.
This can be done in advance even if no errors happen, (almost) no-op HIR walk is super fast and won't affect performance (and this is all temporary anyway).

cramertj · 2017-03-17T08:34:23Z

@petrochenkov I was just finishing up piping &'tcx hir::Visibility through the whole thing and shaking my head at how much churn wound up being necessary (also made me realize how much I'm dying for static rvalue promotion :) ). This sounds like a much better idea.

Just to make sure I understood you: the process is to add a new visitor which runs in librustc_privacy/lib.rs::check_crate and returns a bool of whether or not pub_restricted was used. If it was, all "private in public" warnings become errors?

petrochenkov · 2017-03-17T10:15:52Z

Just to make sure I understood you: the process is to add a new visitor which runs in librustc_privacy/lib.rs::check_crate and returns a bool of whether or not pub_restricted was used. If it was, all "private in public" warnings become errors?

Exactly.
That would be equivalent to the previous behavior (modulo unused #![feature(pub_restricted)]s).

nikomatsakis · 2017-03-20T18:47:56Z

r? @petrochenkov -- since you have something specific in mind =)

petrochenkov · 2017-03-20T19:26:42Z

@bors r+

bors · 2017-03-20T19:26:43Z

📌 Commit 60c1c96 has been approved by petrochenkov

…r=petrochenkov Stabilize pub(restricted) Fix rust-lang#32409

Rollup of 10 pull requests - Successful merges: #40229, #40312, #40332, #40502, #40556, #40576, #40667, #40671, #40681, #40685 - Failed merges:

bors · 2017-03-21T19:35:49Z

⌛ Testing commit 60c1c96 with merge 58c701f...

@petrochenkov

support pub(restricted) in thread_local! (round 2) Resurrected #40984 now that the issue blocking it was fixed. Original description: `pub(restricted)` was stabilized in #40556 so let's go! Here is a [playground](https://play.rust-lang.org/?gist=f55f32f164a6ed18c219fec8f8293b98&version=nightly&backtrace=1). I changed the interface of `__thread_local_inner!`, which is supposedly unstable but this is not checked for macros (#34097 cc @petrochenkov @jseyfried), so this may be an issue.

rust-highfive assigned nikomatsakis Mar 15, 2017

cramertj force-pushed the stabilize-pub-restricted branch from 862e42b to 376739f Compare March 15, 2017 21:18

cramertj changed the title ~~Stabilize pub(restricted) and convert private in public to a hard error~~ Stabilize pub(restricted) Mar 15, 2017

petrochenkov reviewed Mar 15, 2017

View reviewed changes

cramertj force-pushed the stabilize-pub-restricted branch from 376739f to 7447348 Compare March 15, 2017 22:23

sfackler added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Mar 16, 2017

Stabilize pub(restricted)

28626ca

cramertj force-pushed the stabilize-pub-restricted branch from 7447348 to 28626ca Compare March 16, 2017 05:39

nikomatsakis removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Mar 16, 2017

Make priv in pub hard error for crates using pub(restricted)

60c1c96

Xaeroxe mentioned this pull request Mar 18, 2017

File Structure Xaeroxe/nitro-game-engine#8

Closed

rust-highfive unassigned nikomatsakis Mar 20, 2017

rust-highfive assigned petrochenkov Mar 20, 2017

frewsxcv mentioned this pull request Mar 20, 2017

Rollup of 9 pull requests #40688

Closed

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Mar 21, 2017

Rollup merge of rust-lang#40556 - cramertj:stabilize-pub-restricted, …

17656ab

…r=petrochenkov Stabilize pub(restricted) Fix rust-lang#32409

frewsxcv mentioned this pull request Mar 21, 2017

Rollup of 10 pull requests #40693

Merged

bors added a commit that referenced this pull request Mar 21, 2017

Auto merge of #40693 - frewsxcv:rollup, r=frewsxcv

58c701f

Rollup of 10 pull requests - Successful merges: #40229, #40312, #40332, #40502, #40556, #40576, #40667, #40671, #40681, #40685 - Failed merges:

bors merged commit 60c1c96 into rust-lang:master Mar 21, 2017

cramertj deleted the stabilize-pub-restricted branch March 22, 2017 06:49

durka mentioned this pull request Apr 1, 2017

support pub(restricted) in thread_local! #40984

Closed

est31 mentioned this pull request Apr 7, 2017

pub(restricted) didn't make it into 1.17 #41146

Merged

durka mentioned this pull request Jul 12, 2017

support pub(restricted) in thread_local! (round 2) #43185

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stabilize pub(restricted) #40556

Stabilize pub(restricted) #40556

cramertj commented Mar 15, 2017

rust-highfive commented Mar 15, 2017

retep998 commented Mar 15, 2017

cramertj commented Mar 15, 2017

cramertj commented Mar 15, 2017

petrochenkov Mar 15, 2017

cramertj Mar 15, 2017

petrochenkov Mar 15, 2017 •

edited

cramertj Mar 15, 2017

petrochenkov Mar 16, 2017

sfackler commented Mar 16, 2017

nikomatsakis commented Mar 16, 2017 •

edited

nikomatsakis commented Mar 16, 2017

bors commented Mar 16, 2017

nikomatsakis commented Mar 16, 2017

petrochenkov commented Mar 16, 2017

nikomatsakis commented Mar 16, 2017

strega-nil commented Mar 16, 2017 •

edited

nikomatsakis commented Mar 16, 2017

strega-nil commented Mar 17, 2017

petrochenkov commented Mar 17, 2017

cramertj commented Mar 17, 2017 •

edited

petrochenkov commented Mar 17, 2017

nikomatsakis commented Mar 20, 2017

petrochenkov commented Mar 20, 2017

bors commented Mar 20, 2017

bors commented Mar 21, 2017

Stabilize pub(restricted) #40556

Stabilize pub(restricted) #40556

Conversation

cramertj commented Mar 15, 2017

rust-highfive commented Mar 15, 2017

retep998 commented Mar 15, 2017

cramertj commented Mar 15, 2017

cramertj commented Mar 15, 2017

petrochenkov Mar 15, 2017

Choose a reason for hiding this comment

cramertj Mar 15, 2017

Choose a reason for hiding this comment

petrochenkov Mar 15, 2017 • edited

Choose a reason for hiding this comment

cramertj Mar 15, 2017

Choose a reason for hiding this comment

petrochenkov Mar 16, 2017

Choose a reason for hiding this comment

sfackler commented Mar 16, 2017

nikomatsakis commented Mar 16, 2017 • edited

nikomatsakis commented Mar 16, 2017

bors commented Mar 16, 2017

nikomatsakis commented Mar 16, 2017

petrochenkov commented Mar 16, 2017

nikomatsakis commented Mar 16, 2017

strega-nil commented Mar 16, 2017 • edited

nikomatsakis commented Mar 16, 2017

strega-nil commented Mar 17, 2017

petrochenkov commented Mar 17, 2017

cramertj commented Mar 17, 2017 • edited

petrochenkov commented Mar 17, 2017

nikomatsakis commented Mar 20, 2017

petrochenkov commented Mar 20, 2017

bors commented Mar 20, 2017

bors commented Mar 21, 2017

petrochenkov Mar 15, 2017 •

edited

nikomatsakis commented Mar 16, 2017 •

edited

strega-nil commented Mar 16, 2017 •

edited

cramertj commented Mar 17, 2017 •

edited