Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

privacy: Add publically-reexported foreign item to exported item set #15848

Closed
wants to merge 1 commit into from

Conversation

ktt3ja
Copy link
Contributor

@ktt3ja ktt3ja commented Jul 21, 2014

Closes #15740

@@ -326,7 +326,8 @@ impl<'a> Visitor<()> for EmbargoVisitor<'a> {
}

fn visit_foreign_item(&mut self, a: &ast::ForeignItem, _: ()) {
if self.prev_exported && a.vis == ast::Public {
if (self.prev_exported && a.vis == ast::Public)
|| self.reexports.contains(&a.id) {
Copy link
Contributor

Choose a reason for hiding this comment

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

For readability's sake, this should be a single line. The current style rules allow for 99 characters in a line, and this is only 91 characters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I personally strive for 80 characters per line, hence the newline. Anyway, commit updated.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks. I know @alexcrichton also preferred to limit himself to 80 characters when appropriate, but we have recently officially removed the 80/100 split and settled on 99 (although that 99 is not set in stone yet). I wouldn't have said anything except that the line continuation here ended up indented only 1 level, which matches the indentation of the body. If it didn't fit on one line, I would have instead suggested indenting it a second level.

@lilyball
Copy link
Contributor

Offhand, LGTM. I haven't looked at the privacy code before, but this appears to match the logic used in visit_item(). r=me with the line continuation removed if nobody else has any issues with this.

bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 21, 2024
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.

dead_code fails to consider reexports of foreign items
2 participants