From e3be84c6c81c63e11f62105a9befcc39b9a17081 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 16 Sep 2015 19:35:33 +0300 Subject: [PATCH 1/3] libsyntax: forbid visibility modifiers for enum variants fixes #28433 --- src/libsyntax/parse/parser.rs | 7 ++----- .../{issue-3993-2.rs => issue-28433.rs} | 14 +++++--------- .../{useless-priv.rs => useless-pub.rs} | 1 - 3 files changed, 7 insertions(+), 15 deletions(-) rename src/test/compile-fail/{issue-3993-2.rs => issue-28433.rs} (65%) rename src/test/compile-fail/{useless-priv.rs => useless-pub.rs} (91%) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index ff622859cf0b1..4c25714093481 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -5200,13 +5200,10 @@ impl<'a> Parser<'a> { let variant_attrs = self.parse_outer_attributes(); let vlo = self.span.lo; - let vis = try!(self.parse_visibility()); - - let ident; let kind; let mut args = Vec::new(); let mut disr_expr = None; - ident = try!(self.parse_ident()); + let ident = try!(self.parse_ident()); if try!(self.eat(&token::OpenDelim(token::Brace)) ){ // Parse a struct variant. all_nullary = false; @@ -5248,7 +5245,7 @@ impl<'a> Parser<'a> { kind: kind, id: ast::DUMMY_NODE_ID, disr_expr: disr_expr, - vis: vis, + vis: Inherited, }; variants.push(P(spanned(vlo, self.last_span.hi, vr))); diff --git a/src/test/compile-fail/issue-3993-2.rs b/src/test/compile-fail/issue-28433.rs similarity index 65% rename from src/test/compile-fail/issue-3993-2.rs rename to src/test/compile-fail/issue-28433.rs index 9d9e91a141be3..ab0708c28aef6 100644 --- a/src/test/compile-fail/issue-3993-2.rs +++ b/src/test/compile-fail/issue-28433.rs @@ -1,4 +1,4 @@ -// Copyright 2012 The Rust Project Developers. See the COPYRIGHT +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT // file at the top-level directory of this distribution and at // http://rust-lang.org/COPYRIGHT. // @@ -8,16 +8,12 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use zoo::bird::{duck, goose}; - -mod zoo { - pub enum bird { - pub duck, //~ ERROR: unnecessary `pub` visibility - goose - } +enum bird { + pub duck, //~ ERROR: expected identifier, found keyword `pub` + goose } fn main() { - let y = goose; + let y = bird::goose; } diff --git a/src/test/compile-fail/useless-priv.rs b/src/test/compile-fail/useless-pub.rs similarity index 91% rename from src/test/compile-fail/useless-priv.rs rename to src/test/compile-fail/useless-pub.rs index 59964d0df956c..fb6cdf7fa5924 100644 --- a/src/test/compile-fail/useless-priv.rs +++ b/src/test/compile-fail/useless-pub.rs @@ -9,7 +9,6 @@ // except according to those terms. struct A { pub i: isize } -pub enum C { pub Variant } //~ ERROR: unnecessary `pub` pub trait E { fn foo(&self); From f5a99ae7fb43fd44512e2f9294c2c186e776ed36 Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Wed, 16 Sep 2015 20:01:15 +0300 Subject: [PATCH 2/3] Remove Visibility field from enum variants Followup on #28440 --- src/librustc_front/fold.rs | 3 +-- src/librustc_front/hir.rs | 1 - src/librustc_front/lowering.rs | 1 - src/librustc_front/print/pprust.rs | 1 - src/librustc_privacy/lib.rs | 21 ++------------------- src/librustdoc/clean/mod.rs | 2 +- src/librustdoc/doctree.rs | 1 - src/librustdoc/visit_ast.rs | 1 - src/libsyntax/ast.rs | 1 - src/libsyntax/config.rs | 3 +-- src/libsyntax/ext/build.rs | 1 - src/libsyntax/fold.rs | 3 +-- src/libsyntax/parse/parser.rs | 1 - src/libsyntax/print/pprust.rs | 4 +--- 14 files changed, 7 insertions(+), 37 deletions(-) diff --git a/src/librustc_front/fold.rs b/src/librustc_front/fold.rs index 0ff972f830f01..8ee13a408465d 100644 --- a/src/librustc_front/fold.rs +++ b/src/librustc_front/fold.rs @@ -415,7 +415,7 @@ pub fn noop_fold_foreign_mod(ForeignMod {abi, items}: ForeignMod, } pub fn noop_fold_variant(v: P, fld: &mut T) -> P { - v.map(|Spanned {node: Variant_ {id, name, attrs, kind, disr_expr, vis}, span}| Spanned { + v.map(|Spanned {node: Variant_ {id, name, attrs, kind, disr_expr}, span}| Spanned { node: Variant_ { id: fld.new_id(id), name: name, @@ -430,7 +430,6 @@ pub fn noop_fold_variant(v: P, fld: &mut T) -> P { } }, disr_expr: disr_expr.map(|e| fld.fold_expr(e)), - vis: vis, }, span: fld.new_span(span), }) diff --git a/src/librustc_front/hir.rs b/src/librustc_front/hir.rs index f329aa49daabc..8a4cfa28c1d4d 100644 --- a/src/librustc_front/hir.rs +++ b/src/librustc_front/hir.rs @@ -1055,7 +1055,6 @@ pub struct Variant_ { pub id: NodeId, /// Explicit discriminant, eg `Foo = 1` pub disr_expr: Option>, - pub vis: Visibility, } pub type Variant = Spanned; diff --git a/src/librustc_front/lowering.rs b/src/librustc_front/lowering.rs index 38f9ec2c8e6bc..aeaf20cd9d081 100644 --- a/src/librustc_front/lowering.rs +++ b/src/librustc_front/lowering.rs @@ -147,7 +147,6 @@ pub fn lower_variant(v: &Variant) -> P { } }, disr_expr: v.node.disr_expr.as_ref().map(|e| lower_expr(e)), - vis: lower_visibility(v.node.vis), }, span: v.span, }) diff --git a/src/librustc_front/print/pprust.rs b/src/librustc_front/print/pprust.rs index d8d85135dd8f5..4231a7c69b18a 100644 --- a/src/librustc_front/print/pprust.rs +++ b/src/librustc_front/print/pprust.rs @@ -944,7 +944,6 @@ impl<'a> State<'a> { } pub fn print_variant(&mut self, v: &hir::Variant) -> io::Result<()> { - try!(self.print_visibility(v.node.vis)); match v.node.kind { hir::TupleVariantKind(ref args) => { try!(self.print_ident(v.node.name)); diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index 48efd34e21220..0384e7b693245 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -1075,20 +1075,7 @@ impl<'a, 'tcx> SanePrivacyVisitor<'a, 'tcx> { instead"); } - hir::ItemEnum(ref def, _) => { - for v in &def.variants { - match v.node.vis { - hir::Public => { - if item.vis == hir::Public { - span_err!(tcx.sess, v.span, E0448, - "unnecessary `pub` visibility"); - } - } - hir::Inherited => {} - } - } - } - + hir::ItemEnum(..) | hir::ItemTrait(..) | hir::ItemDefaultImpl(..) | hir::ItemConst(..) | hir::ItemStatic(..) | hir::ItemStruct(..) | hir::ItemFn(..) | hir::ItemMod(..) | hir::ItemTy(..) | @@ -1131,14 +1118,10 @@ impl<'a, 'tcx> SanePrivacyVisitor<'a, 'tcx> { check_inherited(tcx, i.span, i.vis); } } - hir::ItemEnum(ref def, _) => { - for v in &def.variants { - check_inherited(tcx, v.span, v.node.vis); - } - } hir::ItemStruct(ref def, _) => check_struct(&**def), + hir::ItemEnum(..) | hir::ItemExternCrate(_) | hir::ItemUse(_) | hir::ItemTrait(..) | hir::ItemDefaultImpl(..) | hir::ItemStatic(..) | hir::ItemConst(..) | diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 8737957c06561..7ed572d7caa7d 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -1850,7 +1850,7 @@ impl Clean for doctree::Variant { name: Some(self.name.clean(cx)), attrs: self.attrs.clean(cx), source: self.whence.clean(cx), - visibility: self.vis.clean(cx), + visibility: None, stability: self.stab.clean(cx), def_id: DefId::local(self.id), inner: VariantItem(Variant { diff --git a/src/librustdoc/doctree.rs b/src/librustdoc/doctree.rs index 1a027a3d14636..e2286ca819a00 100644 --- a/src/librustdoc/doctree.rs +++ b/src/librustdoc/doctree.rs @@ -121,7 +121,6 @@ pub struct Variant { pub attrs: Vec, pub kind: hir::VariantKind, pub id: ast::NodeId, - pub vis: hir::Visibility, pub stab: Option, pub whence: Span, } diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index e3fb13d13906d..1a20a31560bd7 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -109,7 +109,6 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { variants: def.variants.iter().map(|v| Variant { name: v.node.name, attrs: v.node.attrs.clone(), - vis: v.node.vis, stab: self.stability(v.node.id), id: v.node.id, kind: v.node.kind.clone(), diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index bf8c67c7ae181..58d92f5001aae 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -1614,7 +1614,6 @@ pub struct Variant_ { pub id: NodeId, /// Explicit discriminant, eg `Foo = 1` pub disr_expr: Option>, - pub vis: Visibility, } pub type Variant = Spanned; diff --git a/src/libsyntax/config.rs b/src/libsyntax/config.rs index faf0b51c8de0e..889a0d7e440e1 100644 --- a/src/libsyntax/config.rs +++ b/src/libsyntax/config.rs @@ -141,7 +141,7 @@ fn fold_item_underscore(cx: &mut Context, item: ast::Item_) -> ast::Item_ None } else { Some(v.map(|Spanned {node: ast::Variant_ {id, name, attrs, kind, - disr_expr, vis}, span}| { + disr_expr}, span}| { Spanned { node: ast::Variant_ { id: id, @@ -154,7 +154,6 @@ fn fold_item_underscore(cx: &mut Context, item: ast::Item_) -> ast::Item_ } }, disr_expr: disr_expr, - vis: vis }, span: span } diff --git a/src/libsyntax/ext/build.rs b/src/libsyntax/ext/build.rs index 771ac85ef1921..f8beb0e36e2c3 100644 --- a/src/libsyntax/ext/build.rs +++ b/src/libsyntax/ext/build.rs @@ -1013,7 +1013,6 @@ impl<'a> AstBuilder for ExtCtxt<'a> { kind: ast::TupleVariantKind(args), id: ast::DUMMY_NODE_ID, disr_expr: None, - vis: ast::Public }) } diff --git a/src/libsyntax/fold.rs b/src/libsyntax/fold.rs index 0cfddc9857c67..a73cc420eeb04 100644 --- a/src/libsyntax/fold.rs +++ b/src/libsyntax/fold.rs @@ -450,7 +450,7 @@ pub fn noop_fold_foreign_mod(ForeignMod {abi, items}: ForeignMod, } pub fn noop_fold_variant(v: P, fld: &mut T) -> P { - v.map(|Spanned {node: Variant_ {id, name, attrs, kind, disr_expr, vis}, span}| Spanned { + v.map(|Spanned {node: Variant_ {id, name, attrs, kind, disr_expr}, span}| Spanned { node: Variant_ { id: fld.new_id(id), name: name, @@ -465,7 +465,6 @@ pub fn noop_fold_variant(v: P, fld: &mut T) -> P { } }, disr_expr: disr_expr.map(|e| fld.fold_expr(e)), - vis: vis, }, span: fld.new_span(span), }) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 4c25714093481..d4f7509e0c337 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -5245,7 +5245,6 @@ impl<'a> Parser<'a> { kind: kind, id: ast::DUMMY_NODE_ID, disr_expr: disr_expr, - vis: Inherited, }; variants.push(P(spanned(vlo, self.last_span.hi, vr))); diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs index b00ff85051c9a..8d92eaffb9eb6 100644 --- a/src/libsyntax/print/pprust.rs +++ b/src/libsyntax/print/pprust.rs @@ -1507,7 +1507,6 @@ impl<'a> State<'a> { } pub fn print_variant(&mut self, v: &ast::Variant) -> io::Result<()> { - try!(self.print_visibility(v.node.vis)); match v.node.kind { ast::TupleVariantKind(ref args) => { try!(self.print_ident(v.node.name)); @@ -3139,11 +3138,10 @@ mod tests { kind: ast::TupleVariantKind(Vec::new()), id: 0, disr_expr: None, - vis: ast::Public, }); let varstr = variant_to_string(&var); - assert_eq!(varstr, "pub principal_skinner"); + assert_eq!(varstr, "principal_skinner"); } #[test] From a9cb51cf0cf8adc8188097c019daeaf10822d20f Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Thu, 17 Sep 2015 12:47:03 +0300 Subject: [PATCH 3/3] Fix test expectations because of #28439 --- src/test/compile-fail/issue-28433.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/test/compile-fail/issue-28433.rs b/src/test/compile-fail/issue-28433.rs index ab0708c28aef6..3ca2213087d1f 100644 --- a/src/test/compile-fail/issue-28433.rs +++ b/src/test/compile-fail/issue-28433.rs @@ -9,7 +9,9 @@ // except according to those terms. enum bird { - pub duck, //~ ERROR: expected identifier, found keyword `pub` + pub duck, + //~^ ERROR: expected identifier, found keyword `pub` + //~^^ ERROR: expected goose }