Skip to content

Correct span in privacy error #19953

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

Merged
merged 2 commits into from
Dec 21, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/librustc/middle/privacy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1261,13 +1261,13 @@ impl<'a, 'tcx> VisiblePrivateTypesVisitor<'a, 'tcx> {
}

fn check_ty_param_bound(&self,
span: Span,
ty_param_bound: &ast::TyParamBound) {
if let ast::TraitTyParamBound(ref trait_ref) = *ty_param_bound {
if !self.tcx.sess.features.borrow().visible_private_types &&
self.path_is_private_type(trait_ref.trait_ref.ref_id) {
let span = trait_ref.trait_ref.path.span;
self.tcx.sess.span_err(span,
"private type in exported type \
"private trait in exported type \
parameter bound");
}
}
Expand Down Expand Up @@ -1311,7 +1311,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for VisiblePrivateTypesVisitor<'a, 'tcx> {
}

for bound in bounds.iter() {
self.check_ty_param_bound(item.span, bound)
self.check_ty_param_bound(bound)
}
}

Expand Down Expand Up @@ -1449,14 +1449,14 @@ impl<'a, 'tcx, 'v> Visitor<'v> for VisiblePrivateTypesVisitor<'a, 'tcx> {
fn visit_generics(&mut self, generics: &ast::Generics) {
for ty_param in generics.ty_params.iter() {
for bound in ty_param.bounds.iter() {
self.check_ty_param_bound(ty_param.span, bound)
self.check_ty_param_bound(bound)
}
}
for predicate in generics.where_clause.predicates.iter() {
match predicate {
&ast::WherePredicate::BoundPredicate(ref bound_pred) => {
for bound in bound_pred.bounds.iter() {
self.check_ty_param_bound(bound_pred.span, bound)
self.check_ty_param_bound(bound)
}
}
&ast::WherePredicate::EqPredicate(ref eq_pred) => {
Expand Down
53 changes: 46 additions & 7 deletions src/test/compile-fail/visible-private-types-generics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,56 @@

trait Foo {}

pub fn f<T:Foo>() {} //~ ERROR private type in exported type
pub fn f<
T
: Foo //~ ERROR private trait in exported type parameter bound
>() {}

pub fn g<T>() where T: Foo {} //~ ERROR private type in exported type
pub fn g<T>() where
T
: Foo //~ ERROR private trait in exported type parameter bound
{}

pub struct H<T:Foo> { //~ ERROR private type in exported type
x: T,
pub struct S;
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
Member 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
Member 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
Member 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
Member 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.


impl S {
pub fn f<
T
: Foo //~ ERROR private trait in exported type parameter bound
>() {}

pub fn g<T>() where
T
: Foo //~ ERROR private trait in exported type parameter bound
{}
}

pub struct I<T> where T: Foo { //~ ERROR private type in exported type
x: T,
pub struct S1<
T
: Foo //~ ERROR private trait in exported type parameter bound
> {
x: T
}

fn main() {}
pub struct S2<T> where
T
: Foo //~ ERROR private trait in exported type parameter bound
{
x: T
}

pub enum E1<
T
: Foo //~ ERROR private trait in exported type parameter bound
> {
V1(T)
}

pub enum E2<T> where
T
: Foo //~ ERROR private trait in exported type parameter bound
{
V2(T)
}

fn main() {}
3 changes: 1 addition & 2 deletions src/test/compile-fail/visible-private-types-supertrait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

trait Foo {}

pub trait Bar : Foo {} //~ ERROR private type in exported type
pub trait Bar : Foo {} //~ ERROR private trait in exported type

fn main() {}