Skip to content

Conversation

@sanxiyn
Copy link
Contributor

@sanxiyn sanxiyn commented Dec 17, 2014

Fix #19062.

Copy link
Member

Choose a reason for hiding this comment

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

These two tests seem to have been deleted, was that intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. They are redundant.

Copy link
Member

Choose a reason for hiding this comment

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

Just making sure, but you've verified that there are other tests which are ensuring that struct where clauses are checked for privacy?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, but bounds in functions and structs (also methods, enums, etc.) are checked by the same code. I don't think separate tests are necessary. (Or, if they are necessary, tests for methods, enums, etc. are also necessary.)

Copy link
Member

Choose a reason for hiding this comment

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

In the past the visitation code has forgotten to visit new constructs like where clauses, so could we please keep these tests? We surely can't be hurt by having a few extra tests!

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 did not delete tests for where clauses (on functions). Both <T: B> and where T: B are tested. What I deleted is tests for structs.

Copy link
Member

Choose a reason for hiding this comment

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

Can... we please keep these? The visitation code has to specifically walk where clauses for structs and it can possibly be different than functions, and it's not really hurting us to keep them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It can possibly be different, but it isn't. Look, deleted tests are not decreasing any code coverage, and I checked that. But whatever. I will add tests. Thanks.

@sanxiyn
Copy link
Contributor Author

sanxiyn commented Dec 19, 2014

Added tests.

bors added a commit that referenced this pull request Dec 21, 2014
@bors bors merged commit 2800695 into rust-lang:master Dec 21, 2014
@sanxiyn sanxiyn deleted the privacy-span branch December 22, 2014 02:52
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.

Private type error has poor span and inaccurate phrasing for private traits used as bounds

3 participants