Skip to content

Fix private module loophole in the 'private type in public item' check #23290

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 1 commit into from
Mar 18, 2015
Merged

Fix private module loophole in the 'private type in public item' check #23290

merged 1 commit into from
Mar 18, 2015

Conversation

nrc
Copy link
Member

@nrc nrc commented Mar 11, 2015

Closes #22261

r? @nikomatsakis

(+ a new test coming soon...)

@nrc
Copy link
Member Author

nrc commented Mar 11, 2015

Now with test

@nrc
Copy link
Member Author

nrc commented Mar 11, 2015

testing, ignore this

triage: I-nominated (1.0 beta)

@rust-highfive rust-highfive added this to the 1.0 beta milestone Mar 11, 2015
@nrc nrc removed the I-nominated label Mar 11, 2015
@nrc nrc removed this from the 1.0 beta milestone Mar 11, 2015
@nrc
Copy link
Member Author

nrc commented Mar 11, 2015

triage: I-nominated

@rust-highfive rust-highfive added this to the 1.0 beta milestone Mar 11, 2015
@nrc nrc removed the I-nominated label Mar 11, 2015
@nrc nrc removed this from the 1.0 beta milestone Mar 11, 2015
@nrc
Copy link
Member Author

nrc commented Mar 11, 2015

triage: I-nominated

1 similar comment
@nrc
Copy link
Member Author

nrc commented Mar 11, 2015

triage: I-nominated

@nrc
Copy link
Member Author

nrc commented Mar 11, 2015

triage: I-nominated (1.0 beta)

@rust-highfive rust-highfive added this to the 1.0 beta milestone Mar 11, 2015
@nrc nrc removed the I-nominated label Mar 11, 2015
@nrc nrc removed this from the 1.0 beta milestone Mar 11, 2015
@nrc
Copy link
Member Author

nrc commented Mar 11, 2015

triage: P-high

@rust-highfive rust-highfive added the P-medium Medium priority label Mar 11, 2015
@rust-highfive rust-highfive added this to the 1.0 beta milestone Mar 11, 2015
@nrc nrc removed this from the 1.0 beta milestone Mar 11, 2015
@nrc
Copy link
Member Author

nrc commented Mar 11, 2015

triage: I-nominated

@rust-highfive rust-highfive removed the P-medium Medium priority label Mar 11, 2015
@nrc
Copy link
Member Author

nrc commented Mar 11, 2015

triage P-high

@rust-highfive rust-highfive added the P-medium Medium priority label Mar 11, 2015
@nrc
Copy link
Member Author

nrc commented Mar 11, 2015

triage: I-nominated

@rust-highfive rust-highfive removed the P-medium Medium priority label Mar 11, 2015
@nrc
Copy link
Member Author

nrc commented Mar 11, 2015

triage P-high

@rust-highfive rust-highfive added P-medium Medium priority and removed I-nominated P-low Low priority labels Mar 11, 2015
@nrc nrc removed the P-medium Medium priority label Mar 11, 2015
@bors
Copy link
Collaborator

bors commented Mar 12, 2015

☔ The latest upstream changes (presumably #23265) made this pull request unmergeable. Please resolve the merge conflicts.

@nrc
Copy link
Member Author

nrc commented Mar 15, 2015

rebased

nikomatsakis
nikomatsakis reviewed Mar 16, 2015
View reviewed changes
src/librustc_privacy/lib.rs
visit::walk_impl_item(self, impl_item);
match impl_item.node {
ast::MethodImplItem(..)
if self.exported_items.contains(&impl_item.id) ||
Copy link
Contributor

Choose a reason for hiding this comment

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

can you maybe add a comment about what's going on here? I'm not sure I understand, though I presume this is related to the rules about privacy and impls?

@nikomatsakis
Copy link
Contributor

r+ modulo nits

@nrc
Copy link
Member Author

nrc commented Mar 17, 2015

@bors r=nmatsakis 8ef2fb5237ec49ea8d27b868a29743693a3c8605

@bors
Copy link
Collaborator

bors commented Mar 17, 2015

⌛ Testing commit 8ef2fb5 with merge 885ae98...

@bors
Copy link
Collaborator

bors commented Mar 18, 2015

💔 Test failed - auto-win-32-nopt-t

@nrc
Copy link
Member Author

nrc commented Mar 18, 2015

@bors: r=nikomatsakis 46aa621

bors added a commit that referenced this pull request Mar 18, 2015
@bors
Auto merge of #23290 - nrc:pub_priv_mod, r=nikomatsakis
f9a7bc5 
Closes #22261

r? @nikomatsakis 

(+ a new test coming soon...)
@bors
Copy link
Collaborator

bors commented Mar 18, 2015

⌛ Testing commit 46aa621 with merge f9a7bc5...

@bors
Copy link
Collaborator

bors commented Mar 18, 2015

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-nopt-t, auto-win-64-opt

@bors bors merged commit 46aa621 into rust-lang:master Mar 18, 2015
@tomaka tomaka mentioned this pull request Mar 21, 2015
Closed
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers
No reviews
Assignees

@nikomatsakis nikomatsakis

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

private type in public API of non-pub mod is allowed
4 participants
@nrc @bors @nikomatsakis @rust-highfive