RFC: Forbid `pub` and `priv` in positions where they do not change the default behavior

[bstrie]

Right now structs are private-by-default and struct fields are public-by-default. And yet this is legal:

priv struct Foo {
    pub bar: int
}

fn main() {}

The pub and priv declarations there are meaningless. Allowing them in those positions will increase confusion over what the defaults actually are. I propose that this be forbidden.

[thestinger]

I think this would make the rules much easier to understand, so definitely a +1 from me. It's just a removal of redundancy from the valid syntax and could even be considered a lint error/warning if backwards compatibility is a concern.

[AndresOsinski]

I've hit this while trying out some code with multiple nested modules and it would be great to have this implemented.

[catamorphism]

Nominating for milestone 2, backwards-compatible

[pnkfelix]

As long as its a lint I can toggle, sounds fine to me.

[bstrie]

@pnkfelix I don't think the plan is to make it a lint, rather to modify the grammar directly. IMO allowing the qualifiers in those positions is unwarranted.

[graydon]

inside #8385

[graydon]

close when landed.

[alexcrichton]

Closed by e99eff1

[bstrie]

Reopening because I'm not certain that pub is now forbidden in all the proper places.

[bstrie]

Test cases might also make sense here, to help us ensure that we have the rules properly implemented.

[alexcrichton]

Here's the specific cases that I can think of which need to be fixed:

struct A { pub i: int } // public field in private struct is useless
struct A { priv i: int } // priv should not be allowed on struct fields at all
pub enum A { pub Variant } // variant is already public
enum A { priv Variant } // variant is already private

pub trait A { pub fn foo(); } // function is already public
trait A { pub fn foo(); } // no effect

These should all be verified with a test case if they're already disallowed because I don't think this exists in the test suite. There may be others as well.