From 9a88364525a4660dbd6f6a371b3b25199f5bbe4a Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 7 Nov 2019 11:26:36 +0100 Subject: [PATCH 01/19] syntactically allow visibility on trait item & enum variant --- src/librustc_parse/parser/diagnostics.rs | 20 -------- src/librustc_parse/parser/item.rs | 18 ++++--- src/librustc_parse/parser/mod.rs | 8 ++- src/librustc_passes/ast_validation.rs | 6 +++ src/libsyntax/ast.rs | 50 +++++++++++-------- src/libsyntax/mut_visit.rs | 46 +++++++++-------- src/libsyntax/print/pprust/tests.rs | 1 + src/libsyntax/visit.rs | 2 + src/libsyntax_expand/build.rs | 4 +- src/libsyntax_expand/mbe/macro_parser.rs | 4 +- src/libsyntax_expand/placeholders.rs | 3 +- src/test/ui/ast-json/ast-json-output.stdout | 2 +- src/test/ui/issues/issue-28433.stderr | 9 ++-- src/test/ui/issues/issue-60075.rs | 2 +- src/test/ui/issues/issue-60075.stderr | 2 +- src/test/ui/parser/issue-32446.stderr | 4 +- .../issue-65041-empty-vis-matcher-in-enum.rs | 28 +++++++++++ .../issue-65041-empty-vis-matcher-in-trait.rs | 28 +++++++++++ .../ui/parser/macro/trait-non-item-macros.rs | 2 +- .../parser/macro/trait-non-item-macros.stderr | 4 +- .../missing-close-brace-in-trait.rs | 4 +- .../missing-close-brace-in-trait.stderr | 31 +++++++----- .../ui/parser/trait-pub-assoc-const.stderr | 5 +- src/test/ui/parser/trait-pub-assoc-ty.stderr | 5 +- src/test/ui/parser/trait-pub-method.stderr | 5 +- 25 files changed, 184 insertions(+), 109 deletions(-) create mode 100644 src/test/ui/parser/issue-65041-empty-vis-matcher-in-enum.rs create mode 100644 src/test/ui/parser/issue-65041-empty-vis-matcher-in-trait.rs diff --git a/src/librustc_parse/parser/diagnostics.rs b/src/librustc_parse/parser/diagnostics.rs index 38eae008537d7..1b465f326c081 100644 --- a/src/librustc_parse/parser/diagnostics.rs +++ b/src/librustc_parse/parser/diagnostics.rs @@ -1187,26 +1187,6 @@ impl<'a> Parser<'a> { } } - /// Recovers from `pub` keyword in places where it seems _reasonable_ but isn't valid. - pub(super) fn eat_bad_pub(&mut self) { - // When `unclosed_delims` is populated, it means that the code being parsed is already - // quite malformed, which might mean that, for example, a pub struct definition could be - // parsed as being a trait item, which is invalid and this error would trigger - // unconditionally, resulting in misleading diagnostics. Because of this, we only attempt - // this nice to have recovery for code that is otherwise well formed. - if self.token.is_keyword(kw::Pub) && self.unclosed_delims.is_empty() { - match self.parse_visibility(false) { - Ok(vis) => { - self.diagnostic() - .struct_span_err(vis.span, "unnecessary visibility qualifier") - .span_label(vis.span, "`pub` not permitted here") - .emit(); - } - Err(mut err) => err.emit(), - } - } - } - /// Eats tokens until we can be relatively sure we reached the end of the /// statement. This is something of a best-effort heuristic. /// diff --git a/src/librustc_parse/parser/item.rs b/src/librustc_parse/parser/item.rs index 3e21436d3136e..f183b6c8fbc6d 100644 --- a/src/librustc_parse/parser/item.rs +++ b/src/librustc_parse/parser/item.rs @@ -1,4 +1,4 @@ -use super::{Parser, PathStyle}; +use super::{Parser, PathStyle, FollowedByType}; use super::diagnostics::{Error, dummy_arg, ConsumeClosingDelim}; use crate::maybe_whole; @@ -93,7 +93,7 @@ impl<'a> Parser<'a> { let lo = self.token.span; - let vis = self.parse_visibility(false)?; + let vis = self.parse_visibility(FollowedByType::No)?; if self.eat_keyword(kw::Use) { // USE ITEM @@ -706,7 +706,7 @@ impl<'a> Parser<'a> { mut attrs: Vec, ) -> PResult<'a, ImplItem> { let lo = self.token.span; - let vis = self.parse_visibility(false)?; + let vis = self.parse_visibility(FollowedByType::No)?; let defaultness = self.parse_defaultness(); let (name, kind, generics) = if let Some(type_) = self.eat_type() { let (name, alias, generics) = type_?; @@ -896,7 +896,7 @@ impl<'a> Parser<'a> { mut attrs: Vec, ) -> PResult<'a, TraitItem> { let lo = self.token.span; - self.eat_bad_pub(); + let vis = self.parse_visibility(FollowedByType::No)?; let (name, kind, generics) = if self.eat_keyword(kw::Type) { self.parse_trait_item_assoc_ty()? } else if self.is_const_item() { @@ -912,6 +912,7 @@ impl<'a> Parser<'a> { id: DUMMY_NODE_ID, ident: name, attrs, + vis, generics, kind, span: lo.to(self.prev_span), @@ -1142,7 +1143,7 @@ impl<'a> Parser<'a> { let attrs = self.parse_outer_attributes()?; let lo = self.token.span; - let visibility = self.parse_visibility(false)?; + let visibility = self.parse_visibility(FollowedByType::No)?; // FOREIGN STATIC ITEM // Treat `const` as `static` for error recovery, but don't add it to expected tokens. @@ -1370,7 +1371,7 @@ impl<'a> Parser<'a> { let variant_attrs = self.parse_outer_attributes()?; let vlo = self.token.span; - self.eat_bad_pub(); + let vis = self.parse_visibility(FollowedByType::No)?; let ident = self.parse_ident()?; let struct_def = if self.check(&token::OpenDelim(token::Brace)) { @@ -1397,6 +1398,7 @@ impl<'a> Parser<'a> { let vr = ast::Variant { ident, + vis, id: DUMMY_NODE_ID, attrs: variant_attrs, data: struct_def, @@ -1550,7 +1552,7 @@ impl<'a> Parser<'a> { self.parse_paren_comma_seq(|p| { let attrs = p.parse_outer_attributes()?; let lo = p.token.span; - let vis = p.parse_visibility(true)?; + let vis = p.parse_visibility(FollowedByType::Yes)?; let ty = p.parse_ty()?; Ok(StructField { span: lo.to(ty.span), @@ -1568,7 +1570,7 @@ impl<'a> Parser<'a> { fn parse_struct_decl_field(&mut self) -> PResult<'a, StructField> { let attrs = self.parse_outer_attributes()?; let lo = self.token.span; - let vis = self.parse_visibility(false)?; + let vis = self.parse_visibility(FollowedByType::No)?; self.parse_single_struct_field(lo, vis, attrs) } diff --git a/src/librustc_parse/parser/mod.rs b/src/librustc_parse/parser/mod.rs index a491d91e20fb2..29536146a9b47 100644 --- a/src/librustc_parse/parser/mod.rs +++ b/src/librustc_parse/parser/mod.rs @@ -346,6 +346,8 @@ impl SeqSep { } } +pub enum FollowedByType { Yes, No } + impl<'a> Parser<'a> { pub fn new( sess: &'a ParseSess, @@ -1116,7 +1118,7 @@ impl<'a> Parser<'a> { /// If the following element can't be a tuple (i.e., it's a function definition), then /// it's not a tuple struct field), and the contents within the parentheses isn't valid, /// so emit a proper diagnostic. - pub fn parse_visibility(&mut self, can_take_tuple: bool) -> PResult<'a, Visibility> { + pub fn parse_visibility(&mut self, fbt: FollowedByType) -> PResult<'a, Visibility> { maybe_whole!(self, NtVis, |x| x); self.expected_tokens.push(TokenType::Keyword(kw::Crate)); @@ -1171,7 +1173,9 @@ impl<'a> Parser<'a> { id: ast::DUMMY_NODE_ID, }; return Ok(respan(lo.to(self.prev_span), vis)); - } else if !can_take_tuple { // Provide this diagnostic if this is not a tuple struct. + } else if let FollowedByType::No = fbt { + // Provide this diagnostic if a type cannot follow; + // in particular, if this is not a tuple struct. self.recover_incorrect_vis_restriction()?; // Emit diagnostic, but continue with public visibility. } diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs index ec5572914d8ee..e43212a8f29e1 100644 --- a/src/librustc_passes/ast_validation.rs +++ b/src/librustc_passes/ast_validation.rs @@ -524,6 +524,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { } ItemKind::Enum(ref def, _) => { for variant in &def.variants { + self.invalid_visibility(&variant.vis, None); for field in variant.data.fields() { self.invalid_visibility(&field.vis, None); } @@ -754,6 +755,11 @@ impl<'a> Visitor<'a> for AstValidator<'a> { } visit::walk_impl_item(self, ii); } + + fn visit_trait_item(&mut self, ti: &'a TraitItem) { + self.invalid_visibility(&ti.vis, None); + visit::walk_trait_item(self, ti); + } } pub fn check_crate(session: &Session, krate: &Crate, lints: &mut lint::LintBuffer) -> bool { diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index 170d089d4bb8d..550520790b524 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -961,12 +961,12 @@ pub struct Arm { /// Access of a named (e.g., `obj.foo`) or unnamed (e.g., `obj.0`) struct field. #[derive(Clone, RustcEncodable, RustcDecodable, Debug)] pub struct Field { + pub attrs: ThinVec, + pub id: NodeId, + pub span: Span, pub ident: Ident, pub expr: P, - pub span: Span, pub is_shorthand: bool, - pub attrs: ThinVec, - pub id: NodeId, pub is_placeholder: bool, } @@ -1515,12 +1515,14 @@ pub struct FnSig { /// signature) or provided (meaning it has a default implementation). #[derive(Clone, RustcEncodable, RustcDecodable, Debug)] pub struct TraitItem { + pub attrs: Vec, pub id: NodeId, + pub span: Span, + pub vis: Visibility, pub ident: Ident, - pub attrs: Vec, + pub generics: Generics, pub kind: TraitItemKind, - pub span: Span, /// See `Item::tokens` for what this is. pub tokens: Option, } @@ -1536,14 +1538,15 @@ pub enum TraitItemKind { /// Represents anything within an `impl` block. #[derive(Clone, RustcEncodable, RustcDecodable, Debug)] pub struct ImplItem { + pub attrs: Vec, pub id: NodeId, - pub ident: Ident, + pub span: Span, pub vis: Visibility, + pub ident: Ident, + pub defaultness: Defaultness, - pub attrs: Vec, pub generics: Generics, pub kind: ImplItemKind, - pub span: Span, /// See `Item::tokens` for what this is. pub tokens: Option, } @@ -2101,22 +2104,24 @@ pub struct GlobalAsm { pub struct EnumDef { pub variants: Vec, } - /// Enum variant. #[derive(Clone, RustcEncodable, RustcDecodable, Debug)] pub struct Variant { - /// Name of the variant. - pub ident: Ident, /// Attributes of the variant. pub attrs: Vec, /// Id of the variant (not the constructor, see `VariantData::ctor_id()`). pub id: NodeId, + /// Span + pub span: Span, + /// The visibility of the variant. Syntactically accepted but not semantically. + pub vis: Visibility, + /// Name of the variant. + pub ident: Ident, + /// Fields and constructor id of the variant. pub data: VariantData, /// Explicit discriminant, e.g., `Foo = 1`. pub disr_expr: Option, - /// Span - pub span: Span, /// Is a macro placeholder pub is_placeholder: bool, } @@ -2295,12 +2300,13 @@ impl VisibilityKind { /// E.g., `bar: usize` as in `struct Foo { bar: usize }`. #[derive(Clone, RustcEncodable, RustcDecodable, Debug)] pub struct StructField { + pub attrs: Vec, + pub id: NodeId, pub span: Span, - pub ident: Option, pub vis: Visibility, - pub id: NodeId, + pub ident: Option, + pub ty: P, - pub attrs: Vec, pub is_placeholder: bool, } @@ -2344,12 +2350,13 @@ impl VariantData { /// The name might be a dummy name in case of anonymous items. #[derive(Clone, RustcEncodable, RustcDecodable, Debug)] pub struct Item { - pub ident: Ident, pub attrs: Vec, pub id: NodeId, - pub kind: ItemKind, - pub vis: Visibility, pub span: Span, + pub vis: Visibility, + pub ident: Ident, + + pub kind: ItemKind, /// Original tokens this item was parsed from. This isn't necessarily /// available for all items, although over time more and more items should @@ -2518,12 +2525,13 @@ impl ItemKind { #[derive(Clone, RustcEncodable, RustcDecodable, Debug)] pub struct ForeignItem { - pub ident: Ident, pub attrs: Vec, - pub kind: ForeignItemKind, pub id: NodeId, pub span: Span, pub vis: Visibility, + pub ident: Ident, + + pub kind: ForeignItemKind, } /// An item within an `extern` block. diff --git a/src/libsyntax/mut_visit.rs b/src/libsyntax/mut_visit.rs index 376323a83eacc..d79571ccf262e 100644 --- a/src/libsyntax/mut_visit.rs +++ b/src/libsyntax/mut_visit.rs @@ -472,16 +472,17 @@ pub fn noop_visit_foreign_mod(foreign_mod: &mut ForeignMod, vis: items.flat_map_in_place(|item| vis.flat_map_foreign_item(item)); } -pub fn noop_flat_map_variant(mut variant: Variant, vis: &mut T) +pub fn noop_flat_map_variant(mut variant: Variant, visitor: &mut T) -> SmallVec<[Variant; 1]> { - let Variant { ident, attrs, id, data, disr_expr, span, is_placeholder: _ } = &mut variant; - vis.visit_ident(ident); - visit_attrs(attrs, vis); - vis.visit_id(id); - vis.visit_variant_data(data); - visit_opt(disr_expr, |disr_expr| vis.visit_anon_const(disr_expr)); - vis.visit_span(span); + let Variant { ident, vis, attrs, id, data, disr_expr, span, is_placeholder: _ } = &mut variant; + visitor.visit_ident(ident); + visitor.visit_vis(vis); + visit_attrs(attrs, visitor); + visitor.visit_id(id); + visitor.visit_variant_data(data); + visit_opt(disr_expr, |disr_expr| visitor.visit_anon_const(disr_expr)); + visitor.visit_span(span); smallvec![variant] } @@ -920,32 +921,33 @@ pub fn noop_visit_item_kind(kind: &mut ItemKind, vis: &mut T) { } } -pub fn noop_flat_map_trait_item(mut item: TraitItem, vis: &mut T) +pub fn noop_flat_map_trait_item(mut item: TraitItem, visitor: &mut T) -> SmallVec<[TraitItem; 1]> { - let TraitItem { id, ident, attrs, generics, kind, span, tokens: _ } = &mut item; - vis.visit_id(id); - vis.visit_ident(ident); - visit_attrs(attrs, vis); - vis.visit_generics(generics); + let TraitItem { id, ident, vis, attrs, generics, kind, span, tokens: _ } = &mut item; + visitor.visit_id(id); + visitor.visit_ident(ident); + visitor.visit_vis(vis); + visit_attrs(attrs, visitor); + visitor.visit_generics(generics); match kind { TraitItemKind::Const(ty, default) => { - vis.visit_ty(ty); - visit_opt(default, |default| vis.visit_expr(default)); + visitor.visit_ty(ty); + visit_opt(default, |default| visitor.visit_expr(default)); } TraitItemKind::Method(sig, body) => { - visit_fn_sig(sig, vis); - visit_opt(body, |body| vis.visit_block(body)); + visit_fn_sig(sig, visitor); + visit_opt(body, |body| visitor.visit_block(body)); } TraitItemKind::Type(bounds, default) => { - visit_bounds(bounds, vis); - visit_opt(default, |default| vis.visit_ty(default)); + visit_bounds(bounds, visitor); + visit_opt(default, |default| visitor.visit_ty(default)); } TraitItemKind::Macro(mac) => { - vis.visit_mac(mac); + visitor.visit_mac(mac); } } - vis.visit_span(span); + visitor.visit_span(span); smallvec![item] } diff --git a/src/libsyntax/print/pprust/tests.rs b/src/libsyntax/print/pprust/tests.rs index 2c6dd0fb1c6dc..d7725acb5d450 100644 --- a/src/libsyntax/print/pprust/tests.rs +++ b/src/libsyntax/print/pprust/tests.rs @@ -50,6 +50,7 @@ fn test_variant_to_string() { let var = ast::Variant { ident, + vis: source_map::respan(syntax_pos::DUMMY_SP, ast::VisibilityKind::Inherited), attrs: Vec::new(), id: ast::DUMMY_NODE_ID, data: ast::VariantData::Unit(ast::DUMMY_NODE_ID), diff --git a/src/libsyntax/visit.rs b/src/libsyntax/visit.rs index ea2dc357e6ebf..1fad6af0f5b39 100644 --- a/src/libsyntax/visit.rs +++ b/src/libsyntax/visit.rs @@ -314,6 +314,7 @@ pub fn walk_variant<'a, V: Visitor<'a>>(visitor: &mut V, variant: &'a Variant) where V: Visitor<'a>, { visitor.visit_ident(variant.ident); + visitor.visit_vis(&variant.vis); visitor.visit_variant_data(&variant.data); walk_list!(visitor, visit_anon_const, &variant.disr_expr); walk_list!(visitor, visit_attribute, &variant.attrs); @@ -585,6 +586,7 @@ pub fn walk_fn<'a, V>(visitor: &mut V, kind: FnKind<'a>, declaration: &'a FnDecl } pub fn walk_trait_item<'a, V: Visitor<'a>>(visitor: &mut V, trait_item: &'a TraitItem) { + visitor.visit_vis(&trait_item.vis); visitor.visit_ident(trait_item.ident); walk_list!(visitor, visit_attribute, &trait_item.attrs); visitor.visit_generics(&trait_item.generics); diff --git a/src/libsyntax_expand/build.rs b/src/libsyntax_expand/build.rs index 105ffe3ee8a9f..7fe37f377a8d0 100644 --- a/src/libsyntax_expand/build.rs +++ b/src/libsyntax_expand/build.rs @@ -582,12 +582,13 @@ impl<'a> ExtCtxt<'a> { } pub fn variant(&self, span: Span, ident: Ident, tys: Vec> ) -> ast::Variant { + let vis_span = span.shrink_to_lo(); let fields: Vec<_> = tys.into_iter().map(|ty| { ast::StructField { span: ty.span, ty, ident: None, - vis: respan(span.shrink_to_lo(), ast::VisibilityKind::Inherited), + vis: respan(vis_span, ast::VisibilityKind::Inherited), attrs: Vec::new(), id: ast::DUMMY_NODE_ID, is_placeholder: false, @@ -606,6 +607,7 @@ impl<'a> ExtCtxt<'a> { disr_expr: None, id: ast::DUMMY_NODE_ID, ident, + vis: respan(vis_span, ast::VisibilityKind::Inherited), span, is_placeholder: false, } diff --git a/src/libsyntax_expand/mbe/macro_parser.rs b/src/libsyntax_expand/mbe/macro_parser.rs index 0dcb2c39fdc4d..bf7960f90660a 100644 --- a/src/libsyntax_expand/mbe/macro_parser.rs +++ b/src/libsyntax_expand/mbe/macro_parser.rs @@ -77,7 +77,7 @@ use TokenTreeOrTokenTreeSlice::*; use crate::mbe::{self, TokenTree}; use rustc_parse::Directory; -use rustc_parse::parser::{Parser, PathStyle}; +use rustc_parse::parser::{Parser, PathStyle, FollowedByType}; use syntax::ast::{Ident, Name}; use syntax::print::pprust; use syntax::sess::ParseSess; @@ -933,7 +933,7 @@ fn parse_nt_inner<'a>(p: &mut Parser<'a>, sp: Span, name: Symbol) -> PResult<'a, } sym::path => token::NtPath(p.parse_path(PathStyle::Type)?), sym::meta => token::NtMeta(p.parse_attr_item()?), - sym::vis => token::NtVis(p.parse_visibility(true)?), + sym::vis => token::NtVis(p.parse_visibility(FollowedByType::Yes)?), sym::lifetime => if p.check_lifetime() { token::NtLifetime(p.expect_lifetime().ident) } else { diff --git a/src/libsyntax_expand/placeholders.rs b/src/libsyntax_expand/placeholders.rs index e595888dae7d1..36a097000767b 100644 --- a/src/libsyntax_expand/placeholders.rs +++ b/src/libsyntax_expand/placeholders.rs @@ -53,7 +53,7 @@ pub fn placeholder(kind: AstFragmentKind, id: ast::NodeId) -> AstFragment { tokens: None, })]), AstFragmentKind::TraitItems => AstFragment::TraitItems(smallvec![ast::TraitItem { - id, span, ident, attrs, generics, + id, span, ident, vis, attrs, generics, kind: ast::TraitItemKind::Macro(mac_placeholder()), tokens: None, }]), @@ -150,6 +150,7 @@ pub fn placeholder(kind: AstFragmentKind, id: ast::NodeId) -> AstFragment { id, ident, span, + vis, is_placeholder: true, } ]) diff --git a/src/test/ui/ast-json/ast-json-output.stdout b/src/test/ui/ast-json/ast-json-output.stdout index 64c9f69311582..b299fc558410d 100644 --- a/src/test/ui/ast-json/ast-json-output.stdout +++ b/src/test/ui/ast-json/ast-json-output.stdout @@ -1 +1 @@ -{"module":{"inner":{"lo":0,"hi":0},"items":[{"ident":{"name":"core","span":{"lo":0,"hi":0}},"attrs":[],"id":0,"kind":{"variant":"ExternCrate","fields":[null]},"vis":{"node":"Inherited","span":{"lo":0,"hi":0}},"span":{"lo":0,"hi":0},"tokens":{"_field0":[[{"variant":"Token","fields":[{"kind":{"variant":"Ident","fields":["extern",false]},"span":{"lo":0,"hi":0}}]},"NonJoint"],[{"variant":"Token","fields":[{"kind":{"variant":"Ident","fields":["crate",false]},"span":{"lo":0,"hi":0}}]},"NonJoint"],[{"variant":"Token","fields":[{"kind":{"variant":"Ident","fields":["core",false]},"span":{"lo":0,"hi":0}}]},"NonJoint"],[{"variant":"Token","fields":[{"kind":"Semi","span":{"lo":0,"hi":0}}]},"NonJoint"]]}}],"inline":true},"attrs":[],"span":{"lo":0,"hi":0}} +{"module":{"inner":{"lo":0,"hi":0},"items":[{"attrs":[],"id":0,"span":{"lo":0,"hi":0},"vis":{"node":"Inherited","span":{"lo":0,"hi":0}},"ident":{"name":"core","span":{"lo":0,"hi":0}},"kind":{"variant":"ExternCrate","fields":[null]},"tokens":{"_field0":[[{"variant":"Token","fields":[{"kind":{"variant":"Ident","fields":["extern",false]},"span":{"lo":0,"hi":0}}]},"NonJoint"],[{"variant":"Token","fields":[{"kind":{"variant":"Ident","fields":["crate",false]},"span":{"lo":0,"hi":0}}]},"NonJoint"],[{"variant":"Token","fields":[{"kind":{"variant":"Ident","fields":["core",false]},"span":{"lo":0,"hi":0}}]},"NonJoint"],[{"variant":"Token","fields":[{"kind":"Semi","span":{"lo":0,"hi":0}}]},"NonJoint"]]}}],"inline":true},"attrs":[],"span":{"lo":0,"hi":0}} diff --git a/src/test/ui/issues/issue-28433.stderr b/src/test/ui/issues/issue-28433.stderr index 851bc5dfbdd9e..9f5f6333602f5 100644 --- a/src/test/ui/issues/issue-28433.stderr +++ b/src/test/ui/issues/issue-28433.stderr @@ -1,14 +1,15 @@ -error: unnecessary visibility qualifier +error[E0449]: unnecessary visibility qualifier --> $DIR/issue-28433.rs:2:5 | LL | pub Duck, - | ^^^ `pub` not permitted here + | ^^^ `pub` not permitted here because it's implied -error: unnecessary visibility qualifier +error[E0449]: unnecessary visibility qualifier --> $DIR/issue-28433.rs:5:5 | LL | pub(crate) Dove - | ^^^^^^^^^^ `pub` not permitted here + | ^^^^^^^^^^ error: aborting due to 2 previous errors +For more information about this error, try `rustc --explain E0449`. diff --git a/src/test/ui/issues/issue-60075.rs b/src/test/ui/issues/issue-60075.rs index 1f53a92793236..7d3fc83786e9d 100644 --- a/src/test/ui/issues/issue-60075.rs +++ b/src/test/ui/issues/issue-60075.rs @@ -4,7 +4,7 @@ trait T { fn qux() -> Option { let _ = if true { }); -//~^ ERROR expected one of `async`, `const`, `extern`, `fn`, `type`, `unsafe`, or `}`, found `;` +//~^ ERROR expected one of `async` //~| ERROR expected one of `.`, `;`, `?`, `else`, or an operator, found `}` //~| ERROR expected identifier, found `;` Some(4) diff --git a/src/test/ui/issues/issue-60075.stderr b/src/test/ui/issues/issue-60075.stderr index 39e3ad7b6b4fd..e0b15130c337d 100644 --- a/src/test/ui/issues/issue-60075.stderr +++ b/src/test/ui/issues/issue-60075.stderr @@ -4,7 +4,7 @@ error: expected one of `.`, `;`, `?`, `else`, or an operator, found `}` LL | }); | ^ expected one of `.`, `;`, `?`, `else`, or an operator -error: expected one of `async`, `const`, `extern`, `fn`, `type`, `unsafe`, or `}`, found `;` +error: expected one of `async`, `const`, `crate`, `extern`, `fn`, `pub`, `type`, `unsafe`, or `}`, found `;` --> $DIR/issue-60075.rs:6:11 | LL | fn qux() -> Option { diff --git a/src/test/ui/parser/issue-32446.stderr b/src/test/ui/parser/issue-32446.stderr index ab37dd7c39dbe..70256a59231fb 100644 --- a/src/test/ui/parser/issue-32446.stderr +++ b/src/test/ui/parser/issue-32446.stderr @@ -1,8 +1,8 @@ -error: expected one of `async`, `const`, `extern`, `fn`, `type`, `unsafe`, or `}`, found `...` +error: expected one of `async`, `const`, `crate`, `extern`, `fn`, `pub`, `type`, `unsafe`, or `}`, found `...` --> $DIR/issue-32446.rs:4:11 | LL | trait T { ... } - | ^^^ expected one of 7 possible tokens + | ^^^ expected one of 9 possible tokens error: aborting due to previous error diff --git a/src/test/ui/parser/issue-65041-empty-vis-matcher-in-enum.rs b/src/test/ui/parser/issue-65041-empty-vis-matcher-in-enum.rs new file mode 100644 index 0000000000000..ef89e31d8429d --- /dev/null +++ b/src/test/ui/parser/issue-65041-empty-vis-matcher-in-enum.rs @@ -0,0 +1,28 @@ +// check-pass + +// Here we check that a `:vis` macro matcher subsititued for the empty visibility +// (`VisibilityKind::Inherited`) is accepted when used before an enum variant. + +fn main() {} + +macro_rules! mac_variant { + ($vis:vis MARKER) => { + enum Enum { + $vis Unit, + + $vis Tuple(u8, u16), + + $vis Struct { f: u8 }, + } + } +} + +mac_variant!(MARKER); + +// We also accept visibilities on variants syntactically but not semantically. +#[cfg(FALSE)] +enum E { + pub U, + pub(crate) T(u8), + pub(super) T { f: String } +} diff --git a/src/test/ui/parser/issue-65041-empty-vis-matcher-in-trait.rs b/src/test/ui/parser/issue-65041-empty-vis-matcher-in-trait.rs new file mode 100644 index 0000000000000..b08767b210b06 --- /dev/null +++ b/src/test/ui/parser/issue-65041-empty-vis-matcher-in-trait.rs @@ -0,0 +1,28 @@ +// check-pass + +// Here we check that a `:vis` macro matcher subsititued for the empty visibility +// (`VisibilityKind::Inherited`) is accepted when used before an item in a trait. + +fn main() {} + +macro_rules! mac_in_trait { + ($vis:vis MARKER) => { + $vis fn beta() {} + + $vis const GAMMA: u8; + + $vis type Delta; + } +} + +trait Alpha { + mac_in_trait!(MARKER); +} + +// We also accept visibilities on items in traits syntactically but not semantically. +#[cfg(FALSE)] +trait Foo { + pub fn bar(); + pub(crate) type baz; + pub(super) const QUUX: u8; +} diff --git a/src/test/ui/parser/macro/trait-non-item-macros.rs b/src/test/ui/parser/macro/trait-non-item-macros.rs index 958c90b7c0a16..5021886bf9881 100644 --- a/src/test/ui/parser/macro/trait-non-item-macros.rs +++ b/src/test/ui/parser/macro/trait-non-item-macros.rs @@ -1,6 +1,6 @@ macro_rules! bah { ($a:expr) => ($a) - //~^ ERROR expected one of `async`, `const`, `extern`, `fn`, `type`, or `unsafe`, found `2` + //~^ ERROR expected one of `async` } trait bar { diff --git a/src/test/ui/parser/macro/trait-non-item-macros.stderr b/src/test/ui/parser/macro/trait-non-item-macros.stderr index dd97a3afa99fe..0a433ab278e43 100644 --- a/src/test/ui/parser/macro/trait-non-item-macros.stderr +++ b/src/test/ui/parser/macro/trait-non-item-macros.stderr @@ -1,8 +1,8 @@ -error: expected one of `async`, `const`, `extern`, `fn`, `type`, or `unsafe`, found `2` +error: expected one of `async`, `const`, `crate`, `extern`, `fn`, `pub`, `type`, or `unsafe`, found `2` --> $DIR/trait-non-item-macros.rs:2:19 | LL | ($a:expr) => ($a) - | ^^ expected one of `async`, `const`, `extern`, `fn`, `type`, or `unsafe` + | ^^ expected one of 8 possible tokens ... LL | bah!(2); | -------- in this macro invocation diff --git a/src/test/ui/parser/mismatched-braces/missing-close-brace-in-trait.rs b/src/test/ui/parser/mismatched-braces/missing-close-brace-in-trait.rs index 7954719541127..9f3d78d584d44 100644 --- a/src/test/ui/parser/mismatched-braces/missing-close-brace-in-trait.rs +++ b/src/test/ui/parser/mismatched-braces/missing-close-brace-in-trait.rs @@ -1,7 +1,9 @@ trait T { +//~^ ERROR `main` function not found in crate `missing_close_brace_in_trait` fn foo(&self); -pub(crate) struct Bar(); //~ ERROR expected one of +pub(crate) struct Bar(); +//~^ ERROR expected one of impl T for Bar { fn foo(&self) {} diff --git a/src/test/ui/parser/mismatched-braces/missing-close-brace-in-trait.stderr b/src/test/ui/parser/mismatched-braces/missing-close-brace-in-trait.stderr index 1bd8e445fadd3..cbaf9315e8540 100644 --- a/src/test/ui/parser/mismatched-braces/missing-close-brace-in-trait.stderr +++ b/src/test/ui/parser/mismatched-braces/missing-close-brace-in-trait.stderr @@ -1,5 +1,5 @@ error: this file contains an un-closed delimiter - --> $DIR/missing-close-brace-in-trait.rs:10:66 + --> $DIR/missing-close-brace-in-trait.rs:12:66 | LL | trait T { | - un-closed delimiter @@ -7,19 +7,24 @@ LL | trait T { LL | fn main() {} | ^ -error: expected one of `async`, `const`, `extern`, `fn`, `type`, `unsafe`, or `}`, found keyword `pub` - --> $DIR/missing-close-brace-in-trait.rs:4:1 +error: expected one of `async`, `const`, `extern`, `fn`, `type`, or `unsafe`, found keyword `struct` + --> $DIR/missing-close-brace-in-trait.rs:5:12 | -LL | trait T { - | - unclosed delimiter -LL | fn foo(&self); - | - - | | - | expected one of 7 possible tokens - | help: `}` may belong here -LL | LL | pub(crate) struct Bar(); - | ^^^ unexpected token + | ^^^^^^ expected one of `async`, `const`, `extern`, `fn`, `type`, or `unsafe` + +error[E0601]: `main` function not found in crate `missing_close_brace_in_trait` + --> $DIR/missing-close-brace-in-trait.rs:1:1 + | +LL | / trait T { +LL | | +LL | | fn foo(&self); +LL | | +... | +LL | | +LL | | fn main() {} + | |_________________________________________________________________^ consider adding a `main` function to `$DIR/missing-close-brace-in-trait.rs` -error: aborting due to 2 previous errors +error: aborting due to 3 previous errors +For more information about this error, try `rustc --explain E0601`. diff --git a/src/test/ui/parser/trait-pub-assoc-const.stderr b/src/test/ui/parser/trait-pub-assoc-const.stderr index 817692cc82c86..efd09a0364ee0 100644 --- a/src/test/ui/parser/trait-pub-assoc-const.stderr +++ b/src/test/ui/parser/trait-pub-assoc-const.stderr @@ -1,8 +1,9 @@ -error: unnecessary visibility qualifier +error[E0449]: unnecessary visibility qualifier --> $DIR/trait-pub-assoc-const.rs:2:5 | LL | pub const Foo: u32; - | ^^^ `pub` not permitted here + | ^^^ `pub` not permitted here because it's implied error: aborting due to previous error +For more information about this error, try `rustc --explain E0449`. diff --git a/src/test/ui/parser/trait-pub-assoc-ty.stderr b/src/test/ui/parser/trait-pub-assoc-ty.stderr index 400be6af22a82..e76373f5c5f8f 100644 --- a/src/test/ui/parser/trait-pub-assoc-ty.stderr +++ b/src/test/ui/parser/trait-pub-assoc-ty.stderr @@ -1,8 +1,9 @@ -error: unnecessary visibility qualifier +error[E0449]: unnecessary visibility qualifier --> $DIR/trait-pub-assoc-ty.rs:2:5 | LL | pub type Foo; - | ^^^ `pub` not permitted here + | ^^^ `pub` not permitted here because it's implied error: aborting due to previous error +For more information about this error, try `rustc --explain E0449`. diff --git a/src/test/ui/parser/trait-pub-method.stderr b/src/test/ui/parser/trait-pub-method.stderr index b3617a4aa9b01..0e3fe027cb5f8 100644 --- a/src/test/ui/parser/trait-pub-method.stderr +++ b/src/test/ui/parser/trait-pub-method.stderr @@ -1,8 +1,9 @@ -error: unnecessary visibility qualifier +error[E0449]: unnecessary visibility qualifier --> $DIR/trait-pub-method.rs:2:5 | LL | pub fn foo(); - | ^^^ `pub` not permitted here + | ^^^ `pub` not permitted here because it's implied error: aborting due to previous error +For more information about this error, try `rustc --explain E0449`. From 7ec20dd31ddf10e9d8b932a27cca17a056406e07 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Wed, 20 Nov 2019 08:27:42 -0500 Subject: [PATCH 02/19] Remove pretty printing of specific nodes in AST The ability to print a specific item as identified by NodeId or path seems not particularly useful, and certainly carries quite a bit of complexity with it. --- src/librustc/session/config.rs | 117 ++---------------- src/librustc_driver/lib.rs | 5 +- src/librustc_driver/pretty.rs | 65 ++-------- src/librustc_interface/passes.rs | 2 +- .../pretty-print-path-suffix/Makefile | 9 -- .../pretty-print-path-suffix/foo.pp | 5 - .../pretty-print-path-suffix/foo_method.pp | 7 -- .../pretty-print-path-suffix/input.rs | 18 --- .../pretty-print-path-suffix/nest_foo.pp | 4 - 9 files changed, 17 insertions(+), 215 deletions(-) delete mode 100644 src/test/run-make-fulldeps/pretty-print-path-suffix/Makefile delete mode 100644 src/test/run-make-fulldeps/pretty-print-path-suffix/foo.pp delete mode 100644 src/test/run-make-fulldeps/pretty-print-path-suffix/foo_method.pp delete mode 100644 src/test/run-make-fulldeps/pretty-print-path-suffix/input.rs delete mode 100644 src/test/run-make-fulldeps/pretty-print-path-suffix/nest_foo.pp diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 4474b008c7949..772a81276fb9a 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -1,13 +1,10 @@ //! Contains infrastructure for configuring the compiler, including parsing //! command-line options. -// ignore-tidy-filelength - use crate::lint; use crate::middle::cstore; use crate::session::{early_error, early_warn, Session}; use crate::session::search_paths::SearchPath; -use crate::hir::map as hir_map; use rustc_data_structures::fx::FxHashSet; @@ -444,7 +441,7 @@ top_level_options!( // by the compiler. json_artifact_notifications: bool [TRACKED], - pretty: Option<(PpMode, Option)> [UNTRACKED], + pretty: Option [UNTRACKED], } ); @@ -2560,7 +2557,7 @@ fn parse_pretty( matches: &getopts::Matches, debugging_opts: &DebuggingOptions, efmt: ErrorOutputType, -) -> Option<(PpMode, Option)> { +) -> Option { let pretty = if debugging_opts.unstable_options { matches.opt_default("pretty", "normal").map(|a| { // stable pretty-print variants only @@ -2583,13 +2580,10 @@ fn parse_pretty( efmt: ErrorOutputType, name: &str, extended: bool, - ) -> (PpMode, Option) { + ) -> PpMode { use PpMode::*; use PpSourceMode::*; - let mut split = name.splitn(2, '='); - let first = split.next().unwrap(); - let opt_second = split.next(); - let first = match (first, extended) { + let first = match (name, extended) { ("normal", _) => PpmSource(PpmNormal), ("identified", _) => PpmSource(PpmIdentified), ("everybody_loops", true) => PpmSource(PpmEveryBodyLoops), @@ -2617,8 +2611,7 @@ fn parse_pretty( } } }; - let opt_second = opt_second.and_then(|s| s.parse::().ok()); - (first, opt_second) + first } } @@ -2750,13 +2743,13 @@ pub enum PpMode { } impl PpMode { - pub fn needs_ast_map(&self, opt_uii: &Option) -> bool { + pub fn needs_ast_map(&self) -> bool { use PpMode::*; use PpSourceMode::*; match *self { PpmSource(PpmNormal) | PpmSource(PpmEveryBodyLoops) | - PpmSource(PpmIdentified) => opt_uii.is_some(), + PpmSource(PpmIdentified) => false, PpmSource(PpmExpanded) | PpmSource(PpmExpandedIdentified) | @@ -2778,102 +2771,6 @@ impl PpMode { } } -#[derive(Clone, Debug)] -pub enum UserIdentifiedItem { - ItemViaNode(ast::NodeId), - ItemViaPath(Vec), -} - -impl FromStr for UserIdentifiedItem { - type Err = (); - fn from_str(s: &str) -> Result { - use UserIdentifiedItem::*; - Ok(s.parse() - .map(ast::NodeId::from_u32) - .map(ItemViaNode) - .unwrap_or_else(|_| ItemViaPath(s.split("::").map(|s| s.to_string()).collect()))) - } -} - -pub enum NodesMatchingUII<'a> { - NodesMatchingDirect(std::option::IntoIter), - NodesMatchingSuffix(Box + 'a>), -} - -impl<'a> Iterator for NodesMatchingUII<'a> { - type Item = ast::NodeId; - - fn next(&mut self) -> Option { - use NodesMatchingUII::*; - match self { - &mut NodesMatchingDirect(ref mut iter) => iter.next(), - &mut NodesMatchingSuffix(ref mut iter) => iter.next(), - } - } - - fn size_hint(&self) -> (usize, Option) { - use NodesMatchingUII::*; - match self { - &NodesMatchingDirect(ref iter) => iter.size_hint(), - &NodesMatchingSuffix(ref iter) => iter.size_hint(), - } - } -} - -impl UserIdentifiedItem { - pub fn reconstructed_input(&self) -> String { - use UserIdentifiedItem::*; - match *self { - ItemViaNode(node_id) => node_id.to_string(), - ItemViaPath(ref parts) => parts.join("::"), - } - } - - pub fn all_matching_node_ids<'a, 'hir>(&'a self, - map: &'a hir_map::Map<'hir>) - -> NodesMatchingUII<'a> { - use UserIdentifiedItem::*; - use NodesMatchingUII::*; - match *self { - ItemViaNode(node_id) => NodesMatchingDirect(Some(node_id).into_iter()), - ItemViaPath(ref parts) => { - NodesMatchingSuffix(Box::new(map.nodes_matching_suffix(&parts))) - } - } - } - - pub fn to_one_node_id(self, - user_option: &str, - sess: &Session, - map: &hir_map::Map<'_>) - -> ast::NodeId { - let fail_because = |is_wrong_because| -> ast::NodeId { - let message = format!("{} needs NodeId (int) or unique path suffix (b::c::d); got \ - {}, which {}", - user_option, - self.reconstructed_input(), - is_wrong_because); - sess.fatal(&message) - }; - - let mut saw_node = ast::DUMMY_NODE_ID; - let mut seen = 0; - for node in self.all_matching_node_ids(map) { - saw_node = node; - seen += 1; - if seen > 1 { - fail_because("does not resolve uniquely"); - } - } - if seen == 0 { - fail_because("does not resolve to any item"); - } - - assert!(seen == 1); - return saw_node; - } -} - /// Command-line arguments passed to the compiler have to be incorporated with /// the dependency tracking system for incremental compilation. This module /// provides some utilities to make this more convenient. diff --git a/src/librustc_driver/lib.rs b/src/librustc_driver/lib.rs index ef638464adce9..9b0909918decc 100644 --- a/src/librustc_driver/lib.rs +++ b/src/librustc_driver/lib.rs @@ -292,8 +292,8 @@ pub fn run_compiler( compiler.parse()?; - if let Some((ppm, opt_uii)) = &sess.opts.pretty { - if ppm.needs_ast_map(&opt_uii) { + if let Some(ppm) = &sess.opts.pretty { + if ppm.needs_ast_map() { compiler.global_ctxt()?.peek_mut().enter(|tcx| { let expanded_crate = compiler.expansion()?.take().0; pretty::print_after_hir_lowering( @@ -301,7 +301,6 @@ pub fn run_compiler( compiler.input(), &expanded_crate, *ppm, - opt_uii.clone(), compiler.output_file().as_ref().map(|p| &**p), ); Ok(()) diff --git a/src/librustc_driver/pretty.rs b/src/librustc_driver/pretty.rs index 23253dc4dadec..e202ae4949902 100644 --- a/src/librustc_driver/pretty.rs +++ b/src/librustc_driver/pretty.rs @@ -5,7 +5,7 @@ use rustc::hir::map as hir_map; use rustc::hir::print as pprust_hir; use rustc::hir::def_id::LOCAL_CRATE; use rustc::session::Session; -use rustc::session::config::{PpMode, PpSourceMode, UserIdentifiedItem, Input}; +use rustc::session::config::{PpMode, PpSourceMode, Input}; use rustc::ty::{self, TyCtxt}; use rustc::util::common::ErrorReported; use rustc_mir::util::{write_mir_pretty, write_mir_graphviz}; @@ -19,7 +19,6 @@ use std::fs::File; use std::io::Write; use std::path::Path; -pub use self::UserIdentifiedItem::*; pub use self::PpSourceMode::*; pub use self::PpMode::*; use crate::abort_on_err; @@ -448,14 +447,12 @@ pub fn print_after_hir_lowering<'tcx>( input: &Input, krate: &ast::Crate, ppm: PpMode, - opt_uii: Option, ofile: Option<&Path>, ) { if ppm.needs_analysis() { abort_on_err(print_with_analysis( tcx, ppm, - opt_uii, ofile ), tcx.sess); return; @@ -465,8 +462,8 @@ pub fn print_after_hir_lowering<'tcx>( let mut out = String::new(); - match (ppm, opt_uii) { - (PpmSource(s), _) => { + match ppm { + PpmSource(s) => { // Silently ignores an identified node. let out = &mut out; let src = src.clone(); @@ -483,7 +480,7 @@ pub fn print_after_hir_lowering<'tcx>( }) } - (PpmHir(s), None) => { + PpmHir(s) => { let out = &mut out; let src = src.clone(); call_with_pp_support_hir(&s, tcx, move |annotation, krate| { @@ -498,7 +495,7 @@ pub fn print_after_hir_lowering<'tcx>( }) } - (PpmHirTree(s), None) => { + PpmHirTree(s) => { let out = &mut out; call_with_pp_support_hir(&s, tcx, move |_annotation, krate| { debug!("pretty printing source code {:?}", s); @@ -506,44 +503,6 @@ pub fn print_after_hir_lowering<'tcx>( }); } - (PpmHir(s), Some(uii)) => { - let out = &mut out; - let src = src.clone(); - call_with_pp_support_hir(&s, tcx, move |annotation, _| { - debug!("pretty printing source code {:?}", s); - let sess = annotation.sess(); - let hir_map = annotation.hir_map().expect("-Z unpretty missing HIR map"); - let mut pp_state = pprust_hir::State::new_from_input(sess.source_map(), - &sess.parse_sess, - src_name, - src, - annotation.pp_ann()); - for node_id in uii.all_matching_node_ids(hir_map) { - let hir_id = tcx.hir().node_to_hir_id(node_id); - let node = hir_map.get(hir_id); - pp_state.print_node(node); - pp_state.s.space(); - let path = annotation.node_path(hir_id) - .expect("-Z unpretty missing node paths"); - pp_state.synth_comment(path); - pp_state.s.hardbreak(); - } - *out = pp_state.s.eof(); - }) - } - - (PpmHirTree(s), Some(uii)) => { - let out = &mut out; - call_with_pp_support_hir(&s, tcx, move |_annotation, _krate| { - debug!("pretty printing source code {:?}", s); - for node_id in uii.all_matching_node_ids(tcx.hir()) { - let hir_id = tcx.hir().node_to_hir_id(node_id); - let node = tcx.hir().get(hir_id); - out.push_str(&format!("{:#?}", node)); - } - }) - } - _ => unreachable!(), } @@ -557,27 +516,17 @@ pub fn print_after_hir_lowering<'tcx>( fn print_with_analysis( tcx: TyCtxt<'_>, ppm: PpMode, - uii: Option, ofile: Option<&Path>, ) -> Result<(), ErrorReported> { - let nodeid = if let Some(uii) = uii { - debug!("pretty printing for {:?}", uii); - Some(uii.to_one_node_id("-Z unpretty", tcx.sess, tcx.hir())) - } else { - debug!("pretty printing for whole crate"); - None - }; - let mut out = Vec::new(); tcx.analysis(LOCAL_CRATE)?; match ppm { PpmMir | PpmMirCFG => { - let def_id = nodeid.map(|nid| tcx.hir().local_def_id_from_node_id(nid)); match ppm { - PpmMir => write_mir_pretty(tcx, def_id, &mut out), - PpmMirCFG => write_mir_graphviz(tcx, def_id, &mut out), + PpmMir => write_mir_pretty(tcx, None, &mut out), + PpmMirCFG => write_mir_graphviz(tcx, None, &mut out), _ => unreachable!(), } } diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index 86d58bfe8bdac..4bb82bfa96821 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -388,7 +388,7 @@ fn configure_and_expand_inner<'a>( // If we're actually rustdoc then there's no need to actually compile // anything, so switch everything to just looping let mut should_loop = sess.opts.actually_rustdoc; - if let Some((PpMode::PpmSource(PpSourceMode::PpmEveryBodyLoops), _)) = sess.opts.pretty { + if let Some(PpMode::PpmSource(PpSourceMode::PpmEveryBodyLoops)) = sess.opts.pretty { should_loop |= true; } if should_loop { diff --git a/src/test/run-make-fulldeps/pretty-print-path-suffix/Makefile b/src/test/run-make-fulldeps/pretty-print-path-suffix/Makefile deleted file mode 100644 index 899457fc7486e..0000000000000 --- a/src/test/run-make-fulldeps/pretty-print-path-suffix/Makefile +++ /dev/null @@ -1,9 +0,0 @@ --include ../tools.mk - -all: - $(RUSTC) -o $(TMPDIR)/foo.out -Z unpretty=hir=foo input.rs - $(RUSTC) -o $(TMPDIR)/nest_foo.out -Z unpretty=hir=nest::foo input.rs - $(RUSTC) -o $(TMPDIR)/foo_method.out -Z unpretty=hir=foo_method input.rs - diff -u $(TMPDIR)/foo.out foo.pp - diff -u $(TMPDIR)/nest_foo.out nest_foo.pp - diff -u $(TMPDIR)/foo_method.out foo_method.pp diff --git a/src/test/run-make-fulldeps/pretty-print-path-suffix/foo.pp b/src/test/run-make-fulldeps/pretty-print-path-suffix/foo.pp deleted file mode 100644 index fa754af956005..0000000000000 --- a/src/test/run-make-fulldeps/pretty-print-path-suffix/foo.pp +++ /dev/null @@ -1,5 +0,0 @@ - -pub fn foo() -> i32 { 45 } /* foo */ - - -pub fn foo() -> &'static str { "i am a foo." } /* nest::foo */ diff --git a/src/test/run-make-fulldeps/pretty-print-path-suffix/foo_method.pp b/src/test/run-make-fulldeps/pretty-print-path-suffix/foo_method.pp deleted file mode 100644 index 2408c3a208fdd..0000000000000 --- a/src/test/run-make-fulldeps/pretty-print-path-suffix/foo_method.pp +++ /dev/null @@ -1,7 +0,0 @@ - - - - -fn foo_method(self: &Self) - -> &'static str { return "i am very similar to foo."; } /* -nest::{{impl}}::foo_method */ diff --git a/src/test/run-make-fulldeps/pretty-print-path-suffix/input.rs b/src/test/run-make-fulldeps/pretty-print-path-suffix/input.rs deleted file mode 100644 index d075c46d8b003..0000000000000 --- a/src/test/run-make-fulldeps/pretty-print-path-suffix/input.rs +++ /dev/null @@ -1,18 +0,0 @@ -#![crate_type="lib"] - -pub fn -foo() -> i32 -{ 45 } - -pub fn bar() -> &'static str { "i am not a foo." } - -pub mod nest { - pub fn foo() -> &'static str { "i am a foo." } - - struct S; - impl S { - fn foo_method(&self) -> &'static str { - return "i am very similar to foo."; - } - } -} diff --git a/src/test/run-make-fulldeps/pretty-print-path-suffix/nest_foo.pp b/src/test/run-make-fulldeps/pretty-print-path-suffix/nest_foo.pp deleted file mode 100644 index 0be392976da17..0000000000000 --- a/src/test/run-make-fulldeps/pretty-print-path-suffix/nest_foo.pp +++ /dev/null @@ -1,4 +0,0 @@ - - - -pub fn foo() -> &'static str { "i am a foo." } /* nest::foo */ From a1d04cc1d87d990c12da5c816dee412704b38d7f Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Thu, 24 Oct 2019 17:20:18 +0200 Subject: [PATCH 03/19] Remove statics from HAIR by lowering them to a pointer constant --- src/librustc_mir/build/expr/as_place.rs | 7 ------ src/librustc_mir/build/expr/as_rvalue.rs | 1 - src/librustc_mir/build/expr/category.rs | 1 - src/librustc_mir/build/expr/into.rs | 1 - src/librustc_mir/hair/cx/expr.rs | 28 ++++++++++++++++++++++-- src/librustc_mir/hair/mod.rs | 3 --- 6 files changed, 26 insertions(+), 15 deletions(-) diff --git a/src/librustc_mir/build/expr/as_place.rs b/src/librustc_mir/build/expr/as_place.rs index d3e013acc9e3a..aed4759322cb9 100644 --- a/src/librustc_mir/build/expr/as_place.rs +++ b/src/librustc_mir/build/expr/as_place.rs @@ -197,13 +197,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { }; block.and(place_builder) } - ExprKind::StaticRef { id } => block.and(PlaceBuilder::from( - PlaceBase::Static(Box::new(Static { - ty: expr.ty, - kind: StaticKind::Static, - def_id: id, - })) - )), ExprKind::PlaceTypeAscription { source, user_ty } => { let source = this.hir.mirror(source); diff --git a/src/librustc_mir/build/expr/as_rvalue.rs b/src/librustc_mir/build/expr/as_rvalue.rs index f9b77a4b5dd2a..3bbd8093d3bae 100644 --- a/src/librustc_mir/build/expr/as_rvalue.rs +++ b/src/librustc_mir/build/expr/as_rvalue.rs @@ -288,7 +288,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { | ExprKind::Continue { .. } | ExprKind::Return { .. } | ExprKind::InlineAsm { .. } - | ExprKind::StaticRef { .. } | ExprKind::PlaceTypeAscription { .. } | ExprKind::ValueTypeAscription { .. } => { // these do not have corresponding `Rvalue` variants, diff --git a/src/librustc_mir/build/expr/category.rs b/src/librustc_mir/build/expr/category.rs index ae5289986e77c..e7b68acc2ef97 100644 --- a/src/librustc_mir/build/expr/category.rs +++ b/src/librustc_mir/build/expr/category.rs @@ -40,7 +40,6 @@ impl Category { | ExprKind::Index { .. } | ExprKind::SelfRef | ExprKind::VarRef { .. } - | ExprKind::StaticRef { .. } | ExprKind::PlaceTypeAscription { .. } | ExprKind::ValueTypeAscription { .. } => Some(Category::Place), diff --git a/src/librustc_mir/build/expr/into.rs b/src/librustc_mir/build/expr/into.rs index 404ca3204e6c0..1a19878a1f18e 100644 --- a/src/librustc_mir/build/expr/into.rs +++ b/src/librustc_mir/build/expr/into.rs @@ -384,7 +384,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Avoid creating a temporary ExprKind::VarRef { .. } | ExprKind::SelfRef | - ExprKind::StaticRef { .. } | ExprKind::PlaceTypeAscription { .. } | ExprKind::ValueTypeAscription { .. } => { debug_assert!(Category::of(&expr.kind) == Some(Category::Place)); diff --git a/src/librustc_mir/hair/cx/expr.rs b/src/librustc_mir/hair/cx/expr.rs index f25e4b0ae8639..4bae4bf9052d6 100644 --- a/src/librustc_mir/hair/cx/expr.rs +++ b/src/librustc_mir/hair/cx/expr.rs @@ -5,7 +5,7 @@ use crate::hair::cx::to_ref::ToRef; use crate::hair::util::UserAnnotatedTyHelpers; use rustc_index::vec::Idx; use rustc::hir::def::{CtorOf, Res, DefKind, CtorKind}; -use rustc::mir::interpret::{GlobalId, ErrorHandled}; +use rustc::mir::interpret::{GlobalId, ErrorHandled, ConstValue, Scalar}; use rustc::ty::{self, AdtKind, Ty}; use rustc::ty::adjustment::{Adjustment, Adjust, AutoBorrow, AutoBorrowMutability, PointerCast}; use rustc::ty::subst::{InternalSubsts, SubstsRef}; @@ -961,7 +961,31 @@ fn convert_path_expr<'a, 'tcx>( } } - Res::Def(DefKind::Static, id) => ExprKind::StaticRef { id }, + // We encode uses of statics as a `*&STATIC` where the `&STATIC` part is + // a constant reference (or constant raw pointer for `static mut`) in MIR + Res::Def(DefKind::Static, id) => { + let ty = cx.tcx.type_of(id); + let ty = if cx.tcx.is_mutable_static(id) { + cx.tcx.mk_mut_ptr(ty) + } else if cx.tcx.is_foreign_item(id) { + cx.tcx.mk_imm_ptr(ty) + } else { + cx.tcx.mk_imm_ref(cx.tcx.lifetimes.re_static, ty) + }; + let ptr = cx.tcx.alloc_map.lock().create_static_alloc(id); + let temp_lifetime = cx.region_scope_tree.temporary_scope(expr.hir_id.local_id); + ExprKind::Deref { arg: Expr { + ty, + temp_lifetime, + span: expr.span, + kind: ExprKind::Literal { + literal: cx.tcx.mk_const(ty::Const { + ty, val: ConstValue::Scalar(Scalar::Ptr(ptr.into())), + }), + user_ty: None, + } + }.to_ref() } + }, Res::Local(var_hir_id) => convert_var(cx, expr, var_hir_id), diff --git a/src/librustc_mir/hair/mod.rs b/src/librustc_mir/hair/mod.rs index 78e3a17d76632..28794859c564d 100644 --- a/src/librustc_mir/hair/mod.rs +++ b/src/librustc_mir/hair/mod.rs @@ -208,9 +208,6 @@ pub enum ExprKind<'tcx> { }, /// first argument, used for self in a closure SelfRef, - StaticRef { - id: DefId, - }, Borrow { borrow_kind: BorrowKind, arg: ExprRef<'tcx>, From ae9677c82f592a7d6d25a5e3451f1f9cda55e754 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Mon, 11 Nov 2019 12:15:38 +0100 Subject: [PATCH 04/19] Readjust const qualification to detect statics again --- src/librustc/mir/mod.rs | 20 ++++++++++++++++++- .../transform/check_consts/qualifs.rs | 18 ++++++----------- .../transform/check_consts/validation.rs | 15 +++++++++++--- .../transform/qualify_min_const_fn.rs | 13 +++++------- .../consts/const-fn-not-safe-for-const.stderr | 4 ++-- .../consts/min_const_fn/min_const_fn.stderr | 4 ++-- 6 files changed, 46 insertions(+), 28 deletions(-) diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index b7d0f538db5dc..dd27a59200446 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -7,7 +7,7 @@ use crate::hir::def::{CtorKind, Namespace}; use crate::hir::def_id::DefId; use crate::hir; -use crate::mir::interpret::{PanicInfo, Scalar}; +use crate::mir::interpret::{ConstValue, GlobalAlloc, PanicInfo, Scalar}; use crate::mir::visit::MirVisitable; use crate::ty::adjustment::PointerCast; use crate::ty::fold::{TypeFoldable, TypeFolder, TypeVisitor}; @@ -2341,6 +2341,24 @@ pub struct Constant<'tcx> { pub literal: &'tcx ty::Const<'tcx>, } +impl Constant<'tcx> { + pub fn check_static_ptr(&self, tcx: TyCtxt<'_>) -> Option { + match self.literal.val { + ConstValue::Scalar(Scalar::Ptr(ptr)) => match tcx.alloc_map.lock().get(ptr.alloc_id) { + Some(GlobalAlloc::Static(def_id)) => Some(def_id), + Some(_) => None, + None => { + tcx.sess.delay_span_bug( + DUMMY_SP, "MIR cannot contain dangling const pointers", + ); + None + }, + }, + _ => None, + } + } +} + /// A collection of projections into user types. /// /// They are projections because a binding can occur a part of a diff --git a/src/librustc_mir/transform/check_consts/qualifs.rs b/src/librustc_mir/transform/check_consts/qualifs.rs index aad14299c1d94..9ed1ca740b8e7 100644 --- a/src/librustc_mir/transform/check_consts/qualifs.rs +++ b/src/librustc_mir/transform/check_consts/qualifs.rs @@ -2,6 +2,7 @@ use rustc::mir::*; use rustc::ty::{self, Ty}; +use rustc::hir::def_id::DefId; use syntax_pos::DUMMY_SP; use super::{ConstKind, Item as ConstCx}; @@ -32,7 +33,7 @@ pub trait Qualif { /// of the type. fn in_any_value_of_ty(_cx: &ConstCx<'_, 'tcx>, _ty: Ty<'tcx>) -> bool; - fn in_static(_cx: &ConstCx<'_, 'tcx>, _static: &Static<'tcx>) -> bool { + fn in_static(_cx: &ConstCx<'_, 'tcx>, _def_id: DefId) -> bool { // FIXME(eddyb) should we do anything here for value properties? false } @@ -86,18 +87,9 @@ pub trait Qualif { projection: [], } => per_local(*local), PlaceRef { - base: PlaceBase::Static(box Static { - kind: StaticKind::Promoted(..), - .. - }), + base: PlaceBase::Static(_), projection: [], } => bug!("qualifying already promoted MIR"), - PlaceRef { - base: PlaceBase::Static(static_), - projection: [], - } => { - Self::in_static(cx, static_) - }, PlaceRef { base: _, projection: [.., _], @@ -115,7 +107,9 @@ pub trait Qualif { Operand::Move(ref place) => Self::in_place(cx, per_local, place.as_ref()), Operand::Constant(ref constant) => { - if let ty::ConstKind::Unevaluated(def_id, _) = constant.literal.val { + if let Some(static_) = constant.check_static_ptr(cx.tcx) { + Self::in_static(cx, static_) + } else if let ty::ConstKind::Unevaluated(def_id, _) = constant.literal.val { // Don't peek inside trait associated constants. if cx.tcx.trait_of_item(def_id).is_some() { Self::in_any_value_of_ty(cx, constant.literal.ty) diff --git a/src/librustc_mir/transform/check_consts/validation.rs b/src/librustc_mir/transform/check_consts/validation.rs index 21e7c9ce565f0..61de6b185e32d 100644 --- a/src/librustc_mir/transform/check_consts/validation.rs +++ b/src/librustc_mir/transform/check_consts/validation.rs @@ -408,12 +408,21 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> { match place_base { PlaceBase::Local(_) => {} - PlaceBase::Static(box Static{ kind: StaticKind::Promoted(_, _), .. }) => { + PlaceBase::Static(_) => { bug!("Promotion must be run after const validation"); } + } + } - PlaceBase::Static(box Static{ kind: StaticKind::Static, def_id, .. }) => { - let is_thread_local = self.tcx.has_attr(*def_id, sym::thread_local); + fn visit_operand( + &mut self, + op: &Operand<'tcx>, + location: Location, + ) { + self.super_operand(op, location); + if let Operand::Constant(c) = op { + if let Some(def_id) = c.check_static_ptr(self.tcx) { + let is_thread_local = self.tcx.has_attr(def_id, sym::thread_local); if is_thread_local { self.check_op(ops::ThreadLocalAccess); } else if self.const_kind() != ConstKind::Static || !context.is_mutating_use() { diff --git a/src/librustc_mir/transform/qualify_min_const_fn.rs b/src/librustc_mir/transform/qualify_min_const_fn.rs index 83bde5ed34eae..65fc7cd20439d 100644 --- a/src/librustc_mir/transform/qualify_min_const_fn.rs +++ b/src/librustc_mir/transform/qualify_min_const_fn.rs @@ -249,7 +249,10 @@ fn check_operand( Operand::Move(place) | Operand::Copy(place) => { check_place(tcx, place, span, def_id, body) } - Operand::Constant(_) => Ok(()), + Operand::Constant(c) => match c.check_static_ptr(tcx) { + Some(_) => Err((span, "cannot access `static` items in const fn".into())), + None => Ok(()), + }, } } @@ -285,13 +288,7 @@ fn check_place( } } - match place.base { - PlaceBase::Static(box Static { kind: StaticKind::Static, .. }) => { - Err((span, "cannot access `static` items in const fn".into())) - } - PlaceBase::Local(_) - | PlaceBase::Static(box Static { kind: StaticKind::Promoted(_, _), .. }) => Ok(()), - } + Ok(()) } /// Returns whether `allow_internal_unstable(..., , ...)` is present. diff --git a/src/test/ui/consts/const-fn-not-safe-for-const.stderr b/src/test/ui/consts/const-fn-not-safe-for-const.stderr index ba5d58a51d2dd..2d4175ea8eb7d 100644 --- a/src/test/ui/consts/const-fn-not-safe-for-const.stderr +++ b/src/test/ui/consts/const-fn-not-safe-for-const.stderr @@ -11,10 +11,10 @@ LL | Y | ^ error[E0013]: constant functions cannot refer to statics, use a constant instead - --> $DIR/const-fn-not-safe-for-const.rs:25:5 + --> $DIR/const-fn-not-safe-for-const.rs:25:6 | LL | &Y - | ^^ + | ^ error: aborting due to 3 previous errors diff --git a/src/test/ui/consts/min_const_fn/min_const_fn.stderr b/src/test/ui/consts/min_const_fn/min_const_fn.stderr index 5ce21e378cd1e..cb1663ed22f95 100644 --- a/src/test/ui/consts/min_const_fn/min_const_fn.stderr +++ b/src/test/ui/consts/min_const_fn/min_const_fn.stderr @@ -116,10 +116,10 @@ LL | const fn foo25() -> u32 { BAR } = help: add `#![feature(const_fn)]` to the crate attributes to enable error[E0723]: cannot access `static` items in const fn - --> $DIR/min_const_fn.rs:91:36 + --> $DIR/min_const_fn.rs:91:37 | LL | const fn foo26() -> &'static u32 { &BAR } - | ^^^^ + | ^^^ | = note: for more information, see issue https://github.com/rust-lang/rust/issues/57563 = help: add `#![feature(const_fn)]` to the crate attributes to enable From 36006955e7c8e120f7ae9651ae23a3cd98a715e8 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Mon, 11 Nov 2019 12:49:21 +0100 Subject: [PATCH 05/19] Simplify pattern --- src/librustc_codegen_ssa/mir/constant.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_codegen_ssa/mir/constant.rs b/src/librustc_codegen_ssa/mir/constant.rs index 181787e398546..1cc091d962063 100644 --- a/src/librustc_codegen_ssa/mir/constant.rs +++ b/src/librustc_codegen_ssa/mir/constant.rs @@ -14,8 +14,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { constant: &mir::Constant<'tcx>, ) -> Result<&'tcx ty::Const<'tcx>, ErrorHandled> { match constant.literal.val { - ty::ConstKind::Unevaluated(def_id, ref substs) => { - let substs = self.monomorphize(substs); + ty::ConstKind::Unevaluated(def_id, substs) => { + let substs = self.monomorphize(&substs); let instance = ty::Instance::resolve( self.cx.tcx(), ty::ParamEnv::reveal_all(), def_id, substs, ).unwrap(); From 47a3294a1cc2deab2f55795a6ef9b7d4ce51adf9 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Mon, 11 Nov 2019 12:53:31 +0100 Subject: [PATCH 06/19] Readjust constant evaluation for operands --- src/librustc_codegen_ssa/mir/constant.rs | 23 +++++++++++++++++++++++ src/librustc_codegen_ssa/mir/operand.rs | 3 +-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/librustc_codegen_ssa/mir/constant.rs b/src/librustc_codegen_ssa/mir/constant.rs index 1cc091d962063..41d2baaab8035 100644 --- a/src/librustc_codegen_ssa/mir/constant.rs +++ b/src/librustc_codegen_ssa/mir/constant.rs @@ -5,10 +5,33 @@ use rustc::ty::{self, Ty}; use rustc::ty::layout::{self, HasTyCtxt}; use syntax::source_map::Span; use crate::traits::*; +use crate::mir::operand::OperandRef; use super::FunctionCx; impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { + pub fn eval_mir_constant_to_operand( + &mut self, + bx: &mut Bx, + constant: &mir::Constant<'tcx>, + ) -> Result, ErrorHandled> { + match constant.literal.val { + mir::interpret::ConstValue::Unevaluated(def_id, substs) + if self.cx.tcx().is_static(def_id) => { + assert!(substs.is_empty(), "we don't support generic statics yet"); + let static_ = bx.get_static(def_id); + // we treat operands referring to statics as if they were `&STATIC` instead + let ptr_ty = self.cx.tcx().mk_mut_ptr(self.monomorphize(&constant.literal.ty)); + let layout = bx.layout_of(ptr_ty); + Ok(OperandRef::from_immediate_or_packed_pair(bx, static_, layout)) + } + _ => { + let val = self.eval_mir_constant(constant)?; + Ok(OperandRef::from_const(bx, val)) + } + } + } + pub fn eval_mir_constant( &mut self, constant: &mir::Constant<'tcx>, diff --git a/src/librustc_codegen_ssa/mir/operand.rs b/src/librustc_codegen_ssa/mir/operand.rs index 78d09f834c68c..de0edf82d1793 100644 --- a/src/librustc_codegen_ssa/mir/operand.rs +++ b/src/librustc_codegen_ssa/mir/operand.rs @@ -465,8 +465,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { } mir::Operand::Constant(ref constant) => { - self.eval_mir_constant(constant) - .map(|c| OperandRef::from_const(bx, c)) + self.eval_mir_constant_to_operand(bx, constant) .unwrap_or_else(|err| { match err { // errored or at least linted From c6d97dfd835b5d9a740249570e008eb94f1a9e85 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Mon, 18 Nov 2019 22:15:33 +0000 Subject: [PATCH 07/19] Fix rebase --- src/librustc/mir/mod.rs | 6 +++--- src/librustc_codegen_ssa/mir/constant.rs | 2 +- src/librustc_mir/hair/cx/expr.rs | 10 ++++------ src/librustc_mir/transform/check_consts/validation.rs | 2 +- 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index dd27a59200446..c0cd74ecf3a45 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -7,7 +7,7 @@ use crate::hir::def::{CtorKind, Namespace}; use crate::hir::def_id::DefId; use crate::hir; -use crate::mir::interpret::{ConstValue, GlobalAlloc, PanicInfo, Scalar}; +use crate::mir::interpret::{GlobalAlloc, PanicInfo, Scalar}; use crate::mir::visit::MirVisitable; use crate::ty::adjustment::PointerCast; use crate::ty::fold::{TypeFoldable, TypeFolder, TypeVisitor}; @@ -2343,8 +2343,8 @@ pub struct Constant<'tcx> { impl Constant<'tcx> { pub fn check_static_ptr(&self, tcx: TyCtxt<'_>) -> Option { - match self.literal.val { - ConstValue::Scalar(Scalar::Ptr(ptr)) => match tcx.alloc_map.lock().get(ptr.alloc_id) { + match self.literal.val.try_to_scalar() { + Some(Scalar::Ptr(ptr)) => match tcx.alloc_map.lock().get(ptr.alloc_id) { Some(GlobalAlloc::Static(def_id)) => Some(def_id), Some(_) => None, None => { diff --git a/src/librustc_codegen_ssa/mir/constant.rs b/src/librustc_codegen_ssa/mir/constant.rs index 41d2baaab8035..27891be6b82c5 100644 --- a/src/librustc_codegen_ssa/mir/constant.rs +++ b/src/librustc_codegen_ssa/mir/constant.rs @@ -16,7 +16,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { constant: &mir::Constant<'tcx>, ) -> Result, ErrorHandled> { match constant.literal.val { - mir::interpret::ConstValue::Unevaluated(def_id, substs) + ty::ConstKind::Unevaluated(def_id, substs) if self.cx.tcx().is_static(def_id) => { assert!(substs.is_empty(), "we don't support generic statics yet"); let static_ = bx.get_static(def_id); diff --git a/src/librustc_mir/hair/cx/expr.rs b/src/librustc_mir/hair/cx/expr.rs index 4bae4bf9052d6..de50755616bb0 100644 --- a/src/librustc_mir/hair/cx/expr.rs +++ b/src/librustc_mir/hair/cx/expr.rs @@ -5,7 +5,7 @@ use crate::hair::cx::to_ref::ToRef; use crate::hair::util::UserAnnotatedTyHelpers; use rustc_index::vec::Idx; use rustc::hir::def::{CtorOf, Res, DefKind, CtorKind}; -use rustc::mir::interpret::{GlobalId, ErrorHandled, ConstValue, Scalar}; +use rustc::mir::interpret::{GlobalId, ErrorHandled, Scalar}; use rustc::ty::{self, AdtKind, Ty}; use rustc::ty::adjustment::{Adjustment, Adjust, AutoBorrow, AutoBorrowMutability, PointerCast}; use rustc::ty::subst::{InternalSubsts, SubstsRef}; @@ -978,11 +978,9 @@ fn convert_path_expr<'a, 'tcx>( ty, temp_lifetime, span: expr.span, - kind: ExprKind::Literal { - literal: cx.tcx.mk_const(ty::Const { - ty, val: ConstValue::Scalar(Scalar::Ptr(ptr.into())), - }), - user_ty: None, + kind: ExprKind::StaticRef { + literal: ty::Const::from_scalar(cx.tcx, Scalar::Ptr(ptr.into()), ty), + def_id: id, } }.to_ref() } }, diff --git a/src/librustc_mir/transform/check_consts/validation.rs b/src/librustc_mir/transform/check_consts/validation.rs index 61de6b185e32d..772f27fb7e14f 100644 --- a/src/librustc_mir/transform/check_consts/validation.rs +++ b/src/librustc_mir/transform/check_consts/validation.rs @@ -425,7 +425,7 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> { let is_thread_local = self.tcx.has_attr(def_id, sym::thread_local); if is_thread_local { self.check_op(ops::ThreadLocalAccess); - } else if self.const_kind() != ConstKind::Static || !context.is_mutating_use() { + } else { self.check_op(ops::StaticAccess); } } From 9abc34ed9d34873066a186ac5551d5aad9e783b6 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Mon, 18 Nov 2019 23:04:06 +0000 Subject: [PATCH 08/19] Track pointers to statics in MIR --- src/librustc/mir/mod.rs | 74 ++++++++--- src/librustc/mir/visit.rs | 2 +- src/librustc_mir/borrow_check/borrow_set.rs | 4 +- .../borrow_check/conflict_errors.rs | 35 +++-- .../borrow_check/error_reporting.rs | 78 ++++------- src/librustc_mir/borrow_check/mod.rs | 35 +++-- src/librustc_mir/borrow_check/move_errors.rs | 27 ++-- .../borrow_check/mutability_errors.rs | 124 ++++++++---------- .../borrow_check/nll/type_check/mod.rs | 4 +- src/librustc_mir/borrow_check/place_ext.rs | 63 +++++---- src/librustc_mir/build/expr/as_constant.rs | 7 + src/librustc_mir/build/expr/as_place.rs | 1 + src/librustc_mir/build/expr/as_rvalue.rs | 1 + src/librustc_mir/build/expr/as_temp.rs | 6 + src/librustc_mir/build/expr/category.rs | 3 +- src/librustc_mir/build/expr/into.rs | 3 +- src/librustc_mir/build/matches/mod.rs | 28 ++-- src/librustc_mir/build/mod.rs | 22 ++-- src/librustc_mir/dataflow/impls/borrows.rs | 4 +- src/librustc_mir/hair/mod.rs | 5 + src/librustc_mir/shim.rs | 2 +- .../transform/check_consts/validation.rs | 36 +++-- src/librustc_mir/transform/check_unsafety.rs | 43 +++--- src/librustc_mir/transform/generator.rs | 6 +- src/librustc_mir/transform/promote_consts.rs | 45 +++---- 25 files changed, 359 insertions(+), 299 deletions(-) diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index c0cd74ecf3a45..2928a8ad9bcc5 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -292,7 +292,7 @@ impl<'tcx> Body<'tcx> { pub fn temps_iter<'a>(&'a self) -> impl Iterator + 'a { (self.arg_count + 1..self.local_decls.len()).filter_map(move |index| { let local = Local::new(index); - if self.local_decls[local].is_user_variable.is_some() { + if self.local_decls[local].is_user_variable() { None } else { Some(local) @@ -305,7 +305,7 @@ impl<'tcx> Body<'tcx> { pub fn vars_iter<'a>(&'a self) -> impl Iterator + 'a { (self.arg_count + 1..self.local_decls.len()).filter_map(move |index| { let local = Local::new(index); - if self.local_decls[local].is_user_variable.is_some() { + if self.local_decls[local].is_user_variable() { Some(local) } else { None @@ -319,7 +319,7 @@ impl<'tcx> Body<'tcx> { (self.arg_count + 1..self.local_decls.len()).filter_map(move |index| { let local = Local::new(index); let decl = &self.local_decls[local]; - if decl.is_user_variable.is_some() && decl.mutability == Mutability::Mut { + if decl.is_user_variable() && decl.mutability == Mutability::Mut { Some(local) } else { None @@ -333,7 +333,7 @@ impl<'tcx> Body<'tcx> { (1..self.local_decls.len()).filter_map(move |index| { let local = Local::new(index); let decl = &self.local_decls[local]; - if (decl.is_user_variable.is_some() || index < self.arg_count + 1) + if (decl.is_user_variable() || index < self.arg_count + 1) && decl.mutability == Mutability::Mut { Some(local) @@ -696,7 +696,8 @@ pub struct LocalDecl<'tcx> { /// therefore it need not be visible across crates. pnkfelix /// currently hypothesized we *need* to wrap this in a /// `ClearCrossCrate` as long as it carries as `HirId`. - pub is_user_variable: Option>>, + // FIXME(matthewjasper) Don't store in this in `Body` + pub local_info: LocalInfo<'tcx>, /// `true` if this is an internal local. /// @@ -721,6 +722,7 @@ pub struct LocalDecl<'tcx> { /// then it is a temporary created for evaluation of some /// subexpression of some block's tail expression (with no /// intervening statement context). + // FIXME(matthewjasper) Don't store in this in `Body` pub is_block_tail: Option, /// The type of this local. @@ -730,6 +732,7 @@ pub struct LocalDecl<'tcx> { /// e.g., via `let x: T`, then we carry that type here. The MIR /// borrow checker needs this information since it can affect /// region inference. + // FIXME(matthewjasper) Don't store in this in `Body` pub user_ty: UserTypeProjections, /// The name of the local, used in debuginfo and pretty-printing. @@ -824,6 +827,17 @@ pub struct LocalDecl<'tcx> { pub visibility_scope: SourceScope, } +/// Extra information about a local that's used for diagnostics. +#[derive(Clone, Debug, RustcEncodable, RustcDecodable, HashStable, TypeFoldable)] +pub enum LocalInfo<'tcx> { + /// A user-defined local variable or function parameter + User(ClearCrossCrate>), + /// A temporary created that references the static with the given `DefId`. + StaticRef { def_id: DefId, is_thread_local: bool }, + /// Any other temporary, the return place, or an anonymous function parameter. + Other, +} + impl<'tcx> LocalDecl<'tcx> { /// Returns `true` only if local is a binding that can itself be /// made mutable via the addition of the `mut` keyword, namely @@ -832,15 +846,17 @@ impl<'tcx> LocalDecl<'tcx> { /// - `let x = ...`, /// - or `match ... { C(x) => ... }` pub fn can_be_made_mutable(&self) -> bool { - match self.is_user_variable { - Some(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm { + match self.local_info { + LocalInfo::User(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm { binding_mode: ty::BindingMode::BindByValue(_), opt_ty_info: _, opt_match_place: _, pat_span: _, }))) => true, - Some(ClearCrossCrate::Set(BindingForm::ImplicitSelf(ImplicitSelfKind::Imm))) => true, + LocalInfo::User( + ClearCrossCrate::Set(BindingForm::ImplicitSelf(ImplicitSelfKind::Imm)), + ) => true, _ => false, } @@ -850,16 +866,26 @@ impl<'tcx> LocalDecl<'tcx> { /// `ref mut ident` binding. (Such bindings cannot be made into /// mutable bindings, but the inverse does not necessarily hold). pub fn is_nonref_binding(&self) -> bool { - match self.is_user_variable { - Some(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm { + match self.local_info { + LocalInfo::User(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm { binding_mode: ty::BindingMode::BindByValue(_), opt_ty_info: _, opt_match_place: _, pat_span: _, }))) => true, - Some(ClearCrossCrate::Set(BindingForm::ImplicitSelf(_))) => true, + LocalInfo::User(ClearCrossCrate::Set(BindingForm::ImplicitSelf(_))) => true, + + _ => false, + } + } + /// Returns `true` if this variable is a named variable or function + /// parameter declared by the user. + #[inline] + pub fn is_user_variable(&self) -> bool { + match self.local_info { + LocalInfo::User(_) => true, _ => false, } } @@ -868,8 +894,26 @@ impl<'tcx> LocalDecl<'tcx> { /// expression that is used to access said variable for the guard of the /// match arm. pub fn is_ref_for_guard(&self) -> bool { - match self.is_user_variable { - Some(ClearCrossCrate::Set(BindingForm::RefForGuard)) => true, + match self.local_info { + LocalInfo::User(ClearCrossCrate::Set(BindingForm::RefForGuard)) => true, + _ => false, + } + } + + /// Returns `Some` if this is a reference to a static item that is used to + /// access that static + pub fn is_ref_to_static(&self) -> bool { + match self.local_info { + LocalInfo::StaticRef { .. } => true, + _ => false, + } + } + + /// Returns `Some` if this is a reference to a static item that is used to + /// access that static + pub fn is_ref_to_thread_local(&self) -> bool { + match self.local_info { + LocalInfo::StaticRef { is_thread_local, .. } => is_thread_local, _ => false, } } @@ -918,7 +962,7 @@ impl<'tcx> LocalDecl<'tcx> { source_info: SourceInfo { span, scope: OUTERMOST_SOURCE_SCOPE }, visibility_scope: OUTERMOST_SOURCE_SCOPE, internal, - is_user_variable: None, + local_info: LocalInfo::Other, is_block_tail: None, } } @@ -937,7 +981,7 @@ impl<'tcx> LocalDecl<'tcx> { internal: false, is_block_tail: None, name: None, // FIXME maybe we do want some name here? - is_user_variable: None, + local_info: LocalInfo::Other, } } } diff --git a/src/librustc/mir/visit.rs b/src/librustc/mir/visit.rs index 6a41b843e5794..fc0e77aab43a4 100644 --- a/src/librustc/mir/visit.rs +++ b/src/librustc/mir/visit.rs @@ -691,7 +691,7 @@ macro_rules! make_mir_visitor { source_info, visibility_scope, internal: _, - is_user_variable: _, + local_info: _, is_block_tail: _, } = local_decl; diff --git a/src/librustc_mir/borrow_check/borrow_set.rs b/src/librustc_mir/borrow_check/borrow_set.rs index 98641031c1787..943234319906a 100644 --- a/src/librustc_mir/borrow_check/borrow_set.rs +++ b/src/librustc_mir/borrow_check/borrow_set.rs @@ -189,8 +189,8 @@ impl<'a, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'tcx> { location: mir::Location, ) { if let mir::Rvalue::Ref(region, kind, ref borrowed_place) = *rvalue { - if borrowed_place.ignore_borrow( - self.tcx, self.body, &self.locals_state_at_exit) { + if borrowed_place.ignore_borrow(self.tcx, self.body, &self.locals_state_at_exit) { + debug!("ignoring_borrow of {:?}", borrowed_place); return; } diff --git a/src/librustc_mir/borrow_check/conflict_errors.rs b/src/librustc_mir/borrow_check/conflict_errors.rs index ebc25138a0619..3595312f3f41b 100644 --- a/src/librustc_mir/borrow_check/conflict_errors.rs +++ b/src/librustc_mir/borrow_check/conflict_errors.rs @@ -3,8 +3,8 @@ use rustc::hir::def_id::DefId; use rustc::hir::{AsyncGeneratorKind, GeneratorKind}; use rustc::mir::{ self, AggregateKind, BindingForm, BorrowKind, ClearCrossCrate, ConstraintCategory, - FakeReadCause, Local, LocalDecl, LocalKind, Location, Operand, Place, PlaceBase, PlaceRef, - ProjectionElem, Rvalue, Statement, StatementKind, TerminatorKind, VarBindingForm, + FakeReadCause, Local, LocalDecl, LocalInfo, LocalKind, Location, Operand, Place, PlaceBase, + PlaceRef, ProjectionElem, Rvalue, Statement, StatementKind, TerminatorKind, VarBindingForm, }; use rustc::ty::{self, Ty}; use rustc_data_structures::fx::FxHashSet; @@ -744,6 +744,17 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { projection: root_place_projection, }, borrow_span)); + if let PlaceBase::Local(local) = borrow.borrowed_place.base { + if self.body.local_decls[local].is_ref_to_thread_local() { + let err = self.report_thread_local_value_does_not_live_long_enough( + drop_span, + borrow_span, + ); + err.buffer(&mut self.errors_buffer); + return; + } + }; + if let StorageDeadOrDrop::Destructor(dropped_ty) = self.classify_drop_access_kind(borrow.borrowed_place.as_ref()) { @@ -770,9 +781,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { explanation ); let err = match (place_desc, explanation) { - (Some(_), _) if self.is_place_thread_local(root_place) => { - self.report_thread_local_value_does_not_live_long_enough(drop_span, borrow_span) - } // If the outlives constraint comes from inside the closure, // for example: // @@ -1509,19 +1517,22 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // place being assigned later. let (place_description, assigned_span) = match local_decl { Some(LocalDecl { - is_user_variable: Some(ClearCrossCrate::Clear), + local_info: LocalInfo::User(ClearCrossCrate::Clear), .. }) | Some(LocalDecl { - is_user_variable: - Some(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm { - opt_match_place: None, - .. - }))), + local_info: LocalInfo::User(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm { + opt_match_place: None, + .. + }))), + .. + }) + | Some(LocalDecl { + local_info: LocalInfo::StaticRef { .. }, .. }) | Some(LocalDecl { - is_user_variable: None, + local_info: LocalInfo::Other, .. }) | None => (self.describe_place(place.as_ref()), assigned_span), diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 4036e9db33b34..3835503b0ef35 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -3,7 +3,7 @@ use rustc::hir::def::Namespace; use rustc::hir::def_id::DefId; use rustc::hir::GeneratorKind; use rustc::mir::{ - AggregateKind, Constant, Field, Local, LocalKind, Location, Operand, + AggregateKind, Constant, Field, Local, LocalInfo, LocalKind, Location, Operand, Place, PlaceBase, PlaceRef, ProjectionElem, Rvalue, Statement, StatementKind, Static, StaticKind, Terminator, TerminatorKind, }; @@ -12,7 +12,6 @@ use rustc::ty::layout::VariantIdx; use rustc::ty::print::Print; use rustc_errors::DiagnosticBuilder; use syntax_pos::Span; -use syntax::symbol::sym; use super::borrow_set::BorrowData; use super::MirBorrowckCtxt; @@ -178,6 +177,31 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } => { buf.push_str(&self.infcx.tcx.item_name(*def_id).to_string()); } + PlaceRef { + base: &PlaceBase::Local(local), + projection: [ProjectionElem::Deref] + } if self.body.local_decls[local].is_ref_for_guard() => { + self.append_place_to_string( + PlaceRef { + base: &PlaceBase::Local(local), + projection: &[], + }, + buf, + autoderef, + &including_downcast, + )?; + }, + PlaceRef { + base: &PlaceBase::Local(local), + projection: [ProjectionElem::Deref] + } if self.body.local_decls[local].is_ref_to_static() => { + let local_info = &self.body.local_decls[local].local_info; + if let LocalInfo::StaticRef { def_id, .. } = *local_info { + buf.push_str(&self.infcx.tcx.item_name(def_id).as_str()); + } else { + unreachable!(); + } + }, PlaceRef { base, projection: [proj_base @ .., elem], @@ -208,32 +232,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { )?; } else { match (proj_base, base) { - ([], PlaceBase::Local(local)) => { - if self.body.local_decls[*local].is_ref_for_guard() { - self.append_place_to_string( - PlaceRef { - base, - projection: proj_base, - }, - buf, - autoderef, - &including_downcast, - )?; - } else { - // FIXME deduplicate this and the _ => body below - buf.push_str(&"*"); - self.append_place_to_string( - PlaceRef { - base, - projection: proj_base, - }, - buf, - autoderef, - &including_downcast, - )?; - } - } - _ => { buf.push_str(&"*"); self.append_place_to_string( @@ -440,30 +438,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } - /// Checks if a place is a thread-local static. - pub fn is_place_thread_local(&self, place_ref: PlaceRef<'cx, 'tcx>) -> bool { - if let PlaceRef { - base: PlaceBase::Static(box Static { - kind: StaticKind::Static, - def_id, - .. - }), - projection: [], - } = place_ref { - let attrs = self.infcx.tcx.get_attrs(*def_id); - let is_thread_local = attrs.iter().any(|attr| attr.check_name(sym::thread_local)); - - debug!( - "is_place_thread_local: attrs={:?} is_thread_local={:?}", - attrs, is_thread_local - ); - is_thread_local - } else { - debug!("is_place_thread_local: no"); - false - } - } - /// Add a note that a type does not implement `Copy` pub(super) fn note_type_does_not_implement_copy( &self, diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index b8b4a1053e5b0..8e34eb6f413d8 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -308,7 +308,7 @@ fn do_mir_borrowck<'a, 'tcx>( // would have a chance of erroneously adding non-user-defined mutable vars // to the set. let temporary_used_locals: FxHashSet = mbcx.used_mut.iter() - .filter(|&local| mbcx.body.local_decls[*local].is_user_variable.is_none()) + .filter(|&local| !mbcx.body.local_decls[*local].is_user_variable()) .cloned() .collect(); // For the remaining unused locals that are marked as mutable, we avoid linting any that @@ -1287,7 +1287,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { match *operand { Operand::Move(ref place) | Operand::Copy(ref place) => { match place.as_local() { - Some(local) if self.body.local_decls[local].is_user_variable.is_none() => { + Some(local) if !self.body.local_decls[local].is_user_variable() => { if self.body.local_decls[local].ty.is_mutable_ptr() { // The variable will be marked as mutable by the borrow. return; @@ -1399,7 +1399,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ) { debug!("check_for_invalidation_at_exit({:?})", borrow); let place = &borrow.borrowed_place; - let root_place = self.prefixes(place.as_ref(), PrefixSet::All).last().unwrap(); + let deref = [ProjectionElem::Deref]; + let mut root_place = PlaceRef { base: &place.base, projection: &[] }; // FIXME(nll-rfc#40): do more precise destructor tracking here. For now // we just know that all locals are dropped at function exit (otherwise @@ -1407,26 +1408,20 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // // FIXME: allow thread-locals to borrow other thread locals? - assert!(root_place.projection.is_empty()); let (might_be_alive, will_be_dropped) = match root_place.base { - PlaceBase::Static(box Static { - kind: StaticKind::Promoted(..), - .. - }) => { + PlaceBase::Static(_) => { (true, false) } - PlaceBase::Static(box Static { - kind: StaticKind::Static, - .. - }) => { - // Thread-locals might be dropped after the function exits, but - // "true" statics will never be. - (true, self.is_place_thread_local(root_place)) - } - PlaceBase::Local(_) => { - // Locals are always dropped at function exit, and if they - // have a destructor it would've been called already. - (false, self.locals_are_invalidated_at_exit) + PlaceBase::Local(local) => { + if self.body.local_decls[*local].is_ref_to_thread_local() { + // Thread-locals might be dropped after the function exits + // We have to dereference the outer reference because + // borrows don't conflict behind shared references. + root_place.projection = &deref; + (true, true) + } else { + (false, self.locals_are_invalidated_at_exit) + } } }; diff --git a/src/librustc_mir/borrow_check/move_errors.rs b/src/librustc_mir/borrow_check/move_errors.rs index d9e958d945001..b1f63d729ba9b 100644 --- a/src/librustc_mir/borrow_check/move_errors.rs +++ b/src/librustc_mir/borrow_check/move_errors.rs @@ -104,13 +104,14 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { // // opt_match_place is None for let [mut] x = ... statements, // whether or not the right-hand side is a place expression - if let Some(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm { - opt_match_place: Some((ref opt_match_place, match_span)), - binding_mode: _, - opt_ty_info: _, - pat_span: _, - }))) = local_decl.is_user_variable - { + if let LocalInfo::User(ClearCrossCrate::Set(BindingForm::Var( + VarBindingForm { + opt_match_place: Some((ref opt_match_place, match_span)), + binding_mode: _, + opt_ty_info: _, + pat_span: _, + }, + ))) = local_decl.local_info { let stmt_source_info = self.body.source_info(location); self.append_binding_error( grouped_errors, @@ -242,7 +243,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { ( match kind { IllegalMoveOriginKind::Static => { - self.report_cannot_move_from_static(original_path, span) + unreachable!(); } IllegalMoveOriginKind::BorrowedContent { target_place } => { self.report_cannot_move_from_borrowed_content( @@ -272,12 +273,12 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { place: &Place<'tcx>, span: Span ) -> DiagnosticBuilder<'a> { - let description = if place.projection.is_empty() { + let description = if place.projection.len() == 1 { format!("static item `{}`", self.describe_place(place.as_ref()).unwrap()) } else { let base_static = PlaceRef { base: &place.base, - projection: &place.projection[..1], + projection: &[ProjectionElem::Deref], }; format!( @@ -327,6 +328,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { "variables bound in patterns cannot be moved from \ until after the end of the pattern guard"); return err; + } else if decl.is_ref_to_static() { + return self.report_cannot_move_from_static(move_place, span); } } @@ -508,12 +511,12 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { let mut suggestions: Vec<(Span, &str, String)> = Vec::new(); for local in binds_to { let bind_to = &self.body.local_decls[*local]; - if let Some( + if let LocalInfo::User( ClearCrossCrate::Set(BindingForm::Var(VarBindingForm { pat_span, .. })) - ) = bind_to.is_user_variable { + ) = bind_to.local_info { if let Ok(pat_snippet) = self.infcx.tcx.sess.source_map().span_to_snippet(pat_span) { if pat_snippet.starts_with('&') { diff --git a/src/librustc_mir/borrow_check/mutability_errors.rs b/src/librustc_mir/borrow_check/mutability_errors.rs index 11e89de810e5b..404684c07a09c 100644 --- a/src/librustc_mir/borrow_check/mutability_errors.rs +++ b/src/librustc_mir/borrow_check/mutability_errors.rs @@ -1,9 +1,7 @@ use rustc::hir; use rustc::hir::Node; -use rustc::mir::{self, BindingForm, ClearCrossCrate, Local, Location, Body}; -use rustc::mir::{ - Mutability, Place, PlaceRef, PlaceBase, ProjectionElem, Static, StaticKind -}; +use rustc::mir::{self, Body, ClearCrossCrate, Local, LocalInfo, Location}; +use rustc::mir::{Mutability, Place, PlaceRef, PlaceBase, ProjectionElem}; use rustc::ty::{self, Ty, TyCtxt}; use rustc_index::vec::Idx; use syntax_pos::Span; @@ -76,6 +74,31 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { } } + PlaceRef { + base: &PlaceBase::Local(local), + projection: [ProjectionElem::Deref], + } if self.body.local_decls[local].is_ref_for_guard() => { + item_msg = format!("`{}`", access_place_desc.unwrap()); + reason = ", as it is immutable for the pattern guard".to_string(); + } + PlaceRef { + base: &PlaceBase::Local(local), + projection: [ProjectionElem::Deref], + } if self.body.local_decls[local].is_ref_to_static() => { + if access_place.projection.len() == 1 { + item_msg = format!("immutable static item `{}`", access_place_desc.unwrap()); + reason = String::new(); + } else { + item_msg = format!("`{}`", access_place_desc.unwrap()); + let local_info = &self.body.local_decls[local].local_info; + if let LocalInfo::StaticRef { def_id, .. } = *local_info { + let static_name = &self.infcx.tcx.item_name(def_id); + reason = format!(", as `{}` is an immutable static item", static_name); + } else { + bug!("is_ref_to_static return true, but not ref to static?"); + } + } + } PlaceRef { base: _, projection: [proj_base @ .., ProjectionElem::Deref], @@ -101,15 +124,6 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { } else { ", as `Fn` closures cannot mutate their captured variables".to_string() } - } else if { - if let (PlaceBase::Local(local), []) = (&the_place_err.base, proj_base) { - self.body.local_decls[*local].is_ref_for_guard() - } else { - false - } - } { - item_msg = format!("`{}`", access_place_desc.unwrap()); - reason = ", as it is immutable for the pattern guard".to_string(); } else { let source = self.borrowed_content_source(PlaceRef { base: the_place_err.base, @@ -133,37 +147,10 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { } PlaceRef { - base: - PlaceBase::Static(box Static { - kind: StaticKind::Promoted(..), - .. - }), - projection: [], - } => unreachable!(), - - PlaceRef { - base: - PlaceBase::Static(box Static { - kind: StaticKind::Static, - def_id, - .. - }), - projection: [], - } => { - if let PlaceRef { - base: &PlaceBase::Static(_), - projection: &[], - } = access_place.as_ref() { - item_msg = format!("immutable static item `{}`", access_place_desc.unwrap()); - reason = String::new(); - } else { - item_msg = format!("`{}`", access_place_desc.unwrap()); - let static_name = &self.infcx.tcx.item_name(*def_id); - reason = format!(", as `{}` is an immutable static item", static_name); - } + base: PlaceBase::Static(_), + .. } - - PlaceRef { + | PlaceRef { base: _, projection: [.., ProjectionElem::Index(_)], } @@ -257,15 +244,15 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { projection: [], } if { self.body.local_decls.get(*local).map(|local_decl| { - if let ClearCrossCrate::Set( + if let LocalInfo::User(ClearCrossCrate::Set( mir::BindingForm::ImplicitSelf(kind) - ) = local_decl.is_user_variable.as_ref().unwrap() { + )) = local_decl.local_info { // Check if the user variable is a `&mut self` and we can therefore // suggest removing the `&mut`. // // Deliberately fall into this case for all implicit self types, // so that we don't fall in to the next case with them. - *kind == mir::ImplicitSelfKind::MutRef + kind == mir::ImplicitSelfKind::MutRef } else if Some(kw::SelfLower) == local_decl.name { // Otherwise, check if the name is the self kewyord - in which case // we have an explicit self. Do the same thing in this case and check @@ -360,16 +347,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { PlaceRef { base: PlaceBase::Local(local), projection: [ProjectionElem::Deref], - } if { - if let Some(ClearCrossCrate::Set(BindingForm::RefForGuard)) = - self.body.local_decls[*local].is_user_variable - { - true - } else { - false - } - } => - { + } if self.body.local_decls[*local].is_ref_for_guard() => { err.span_label(span, format!("cannot {ACT}", ACT = act)); err.note( "variables bound in patterns are immutable until the end of the pattern guard", @@ -384,38 +362,42 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { PlaceRef { base: PlaceBase::Local(local), projection: [ProjectionElem::Deref], - } if self.body.local_decls[*local].is_user_variable.is_some() => + } if self.body.local_decls[*local].is_user_variable() => { let local_decl = &self.body.local_decls[*local]; - let suggestion = match local_decl.is_user_variable.as_ref().unwrap() { - ClearCrossCrate::Set(mir::BindingForm::ImplicitSelf(_)) => { + let suggestion = match local_decl.local_info { + LocalInfo::User(ClearCrossCrate::Set(mir::BindingForm::ImplicitSelf(_))) => { Some(suggest_ampmut_self(self.infcx.tcx, local_decl)) } - ClearCrossCrate::Set(mir::BindingForm::Var(mir::VarBindingForm { - binding_mode: ty::BindingMode::BindByValue(_), - opt_ty_info, - .. - })) => Some(suggest_ampmut( + LocalInfo::User(ClearCrossCrate::Set(mir::BindingForm::Var( + mir::VarBindingForm { + binding_mode: ty::BindingMode::BindByValue(_), + opt_ty_info, + .. + }, + ))) => Some(suggest_ampmut( self.infcx.tcx, self.body, *local, local_decl, - *opt_ty_info, + opt_ty_info, )), - ClearCrossCrate::Set(mir::BindingForm::Var(mir::VarBindingForm { - binding_mode: ty::BindingMode::BindByReference(_), - .. - })) => { + LocalInfo::User(ClearCrossCrate::Set(mir::BindingForm::Var( + mir::VarBindingForm { + binding_mode: ty::BindingMode::BindByReference(_), + .. + }, + ))) => { let pattern_span = local_decl.source_info.span; suggest_ref_mut(self.infcx.tcx, pattern_span) .map(|replacement| (pattern_span, replacement)) } - ClearCrossCrate::Set(mir::BindingForm::RefForGuard) => unreachable!(), + LocalInfo::User(ClearCrossCrate::Clear) => bug!("saw cleared local state"), - ClearCrossCrate::Clear => bug!("saw cleared local state"), + _ => unreachable!(), }; let (pointer_sigil, pointer_desc) = if local_decl.ty.is_region_ptr() { diff --git a/src/librustc_mir/borrow_check/nll/type_check/mod.rs b/src/librustc_mir/borrow_check/nll/type_check/mod.rs index 9f20a24a183c9..99bcfa9bc2599 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/mod.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/mod.rs @@ -1387,7 +1387,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { } else { ConstraintCategory::Return }, - Some(l) if !body.local_decls[l].is_user_variable.is_some() => { + Some(l) if !body.local_decls[l].is_user_variable() => { ConstraintCategory::Boring } _ => ConstraintCategory::Assignment, @@ -1693,7 +1693,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { ConstraintCategory::Return } } - Some(l) if !body.local_decls[l].is_user_variable.is_some() => { + Some(l) if !body.local_decls[l].is_user_variable() => { ConstraintCategory::Boring } _ => ConstraintCategory::Assignment, diff --git a/src/librustc_mir/borrow_check/place_ext.rs b/src/librustc_mir/borrow_check/place_ext.rs index f0d2927ba45e7..c62de2af55f44 100644 --- a/src/librustc_mir/borrow_check/place_ext.rs +++ b/src/librustc_mir/borrow_check/place_ext.rs @@ -1,6 +1,6 @@ use rustc::hir; use rustc::mir::ProjectionElem; -use rustc::mir::{Body, Place, PlaceBase, Mutability, Static, StaticKind}; +use rustc::mir::{Body, Place, PlaceBase, Mutability}; use rustc::ty::{self, TyCtxt}; use crate::borrow_check::borrow_set::LocalsStateAtExit; @@ -25,7 +25,7 @@ impl<'tcx> PlaceExt<'tcx> for Place<'tcx> { body: &Body<'tcx>, locals_state_at_exit: &LocalsStateAtExit, ) -> bool { - let ignore = match self.base { + let local = match self.base { // If a local variable is immutable, then we only need to track borrows to guard // against two kinds of errors: // * The variable being dropped while still borrowed (e.g., because the fn returns @@ -34,22 +34,22 @@ impl<'tcx> PlaceExt<'tcx> for Place<'tcx> { // // In particular, the variable cannot be mutated -- the "access checks" will fail -- // so we don't have to worry about mutation while borrowed. - PlaceBase::Local(index) => { + PlaceBase::Local(local) => { match locals_state_at_exit { - LocalsStateAtExit::AllAreInvalidated => false, + LocalsStateAtExit::AllAreInvalidated => local, LocalsStateAtExit::SomeAreInvalidated { has_storage_dead_or_moved } => { - let ignore = !has_storage_dead_or_moved.contains(index) && - body.local_decls[index].mutability == Mutability::Not; - debug!("ignore_borrow: local {:?} => {:?}", index, ignore); - ignore + let ignore = !has_storage_dead_or_moved.contains(local) && + body.local_decls[local].mutability == Mutability::Not; + debug!("ignore_borrow: local {:?} => {:?}", local, ignore); + if ignore { + return true; + } else { + local + } } } } - PlaceBase::Static(box Static{ kind: StaticKind::Promoted(_, _), .. }) => - false, - PlaceBase::Static(box Static{ kind: StaticKind::Static, def_id, .. }) => { - tcx.is_mutable_static(def_id) - } + PlaceBase::Static(_) => return true, }; for (i, elem) in self.projection.iter().enumerate() { @@ -57,22 +57,33 @@ impl<'tcx> PlaceExt<'tcx> for Place<'tcx> { if *elem == ProjectionElem::Deref { let ty = Place::ty_from(&self.base, proj_base, body, tcx).ty; - if let ty::RawPtr(..) | ty::Ref(_, _, hir::Mutability::Immutable) = ty.kind { - // For both derefs of raw pointers and `&T` - // references, the original path is `Copy` and - // therefore not significant. In particular, - // there is nothing the user can do to the - // original path that would invalidate the - // newly created reference -- and if there - // were, then the user could have copied the - // original path into a new variable and - // borrowed *that* one, leaving the original - // path unborrowed. - return true; + match ty.kind { + ty::Ref(_, _, hir::Mutability::Immutable) if i == 0 => { + // For references to thread-local statics, we do need + // to track the borrow. + if body.local_decls[local].is_ref_to_thread_local() { + continue; + } + return true; + } + ty::RawPtr(..) | ty::Ref(_, _, hir::Mutability::Immutable) => { + // For both derefs of raw pointers and `&T` + // references, the original path is `Copy` and + // therefore not significant. In particular, + // there is nothing the user can do to the + // original path that would invalidate the + // newly created reference -- and if there + // were, then the user could have copied the + // original path into a new variable and + // borrowed *that* one, leaving the original + // path unborrowed. + return true; + } + _ => {} } } } - ignore + false } } diff --git a/src/librustc_mir/build/expr/as_constant.rs b/src/librustc_mir/build/expr/as_constant.rs index 39bdc871d83c6..6db7ec65096ec 100644 --- a/src/librustc_mir/build/expr/as_constant.rs +++ b/src/librustc_mir/build/expr/as_constant.rs @@ -45,6 +45,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { literal, } }, + ExprKind::StaticRef { literal, .. } => { + Constant { + span, + user_ty: None, + literal, + } + } _ => span_bug!(span, "expression is not a valid constant {:?}", kind), } } diff --git a/src/librustc_mir/build/expr/as_place.rs b/src/librustc_mir/build/expr/as_place.rs index aed4759322cb9..f66f1cb73666a 100644 --- a/src/librustc_mir/build/expr/as_place.rs +++ b/src/librustc_mir/build/expr/as_place.rs @@ -285,6 +285,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { | ExprKind::Continue { .. } | ExprKind::Return { .. } | ExprKind::Literal { .. } + | ExprKind::StaticRef { .. } | ExprKind::InlineAsm { .. } | ExprKind::Yield { .. } | ExprKind::Call { .. } => { diff --git a/src/librustc_mir/build/expr/as_rvalue.rs b/src/librustc_mir/build/expr/as_rvalue.rs index 3bbd8093d3bae..37eb0cc9d961e 100644 --- a/src/librustc_mir/build/expr/as_rvalue.rs +++ b/src/librustc_mir/build/expr/as_rvalue.rs @@ -270,6 +270,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { resume.and(this.unit_rvalue()) } ExprKind::Literal { .. } + | ExprKind::StaticRef { .. } | ExprKind::Block { .. } | ExprKind::Match { .. } | ExprKind::NeverToAny { .. } diff --git a/src/librustc_mir/build/expr/as_temp.rs b/src/librustc_mir/build/expr/as_temp.rs index 18332ed68f8bd..864b449c29c9e 100644 --- a/src/librustc_mir/build/expr/as_temp.rs +++ b/src/librustc_mir/build/expr/as_temp.rs @@ -6,6 +6,7 @@ use crate::hair::*; use rustc::hir; use rustc::middle::region; use rustc::mir::*; +use syntax_pos::symbol::sym; impl<'a, 'tcx> Builder<'a, 'tcx> { /// Compile `expr` into a fresh temporary. This is used when building @@ -63,6 +64,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { if let Some(tail_info) = this.block_context.currently_in_block_tail() { local_decl = local_decl.block_tail(tail_info); } + if let ExprKind::StaticRef { def_id, .. } = expr.kind { + let attrs = this.hir.tcx().get_attrs(def_id); + let is_thread_local = attrs.iter().any(|attr| attr.check_name(sym::thread_local)); + local_decl.local_info = LocalInfo::StaticRef {def_id, is_thread_local }; + } this.local_decls.push(local_decl) }; let temp_place = &Place::from(temp); diff --git a/src/librustc_mir/build/expr/category.rs b/src/librustc_mir/build/expr/category.rs index e7b68acc2ef97..270a1a6447435 100644 --- a/src/librustc_mir/build/expr/category.rs +++ b/src/librustc_mir/build/expr/category.rs @@ -65,7 +65,8 @@ impl Category { | ExprKind::Yield { .. } | ExprKind::InlineAsm { .. } => Some(Category::Rvalue(RvalueFunc::AsRvalue)), - ExprKind::Literal { .. } => Some(Category::Constant), + ExprKind::Literal { .. } + | ExprKind::StaticRef { .. } => Some(Category::Constant), ExprKind::Loop { .. } | ExprKind::Block { .. } diff --git a/src/librustc_mir/build/expr/into.rs b/src/librustc_mir/build/expr/into.rs index 1a19878a1f18e..e991181189f41 100644 --- a/src/librustc_mir/build/expr/into.rs +++ b/src/librustc_mir/build/expr/into.rs @@ -231,7 +231,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { source_info, visibility_scope: source_info.scope, internal: true, - is_user_variable: None, + local_info: LocalInfo::Other, is_block_tail: None, }); let ptr_temp = Place::from(ptr_temp); @@ -425,6 +425,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { | ExprKind::Tuple { .. } | ExprKind::Closure { .. } | ExprKind::Literal { .. } + | ExprKind::StaticRef { .. } | ExprKind::Yield { .. } => { debug_assert!(match Category::of(&expr.kind).unwrap() { // should be handled above diff --git a/src/librustc_mir/build/matches/mod.rs b/src/librustc_mir/build/matches/mod.rs index 667b37bbd80c8..ada547aa39c9e 100644 --- a/src/librustc_mir/build/matches/mod.rs +++ b/src/librustc_mir/build/matches/mod.rs @@ -458,10 +458,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { for binding in &candidate.bindings { let local = self.var_local_id(binding.var_id, OutsideGuard); - if let Some(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm { + if let LocalInfo::User(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm { opt_match_place: Some((ref mut match_place, _)), .. - }))) = self.local_decls[local].is_user_variable + }))) = self.local_decls[local].local_info { *match_place = Some(initializer.clone()); } else { @@ -1734,16 +1734,18 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { visibility_scope, internal: false, is_block_tail: None, - is_user_variable: Some(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm { - binding_mode, - // hypothetically, `visit_bindings` could try to unzip - // an outermost hir::Ty as we descend, matching up - // idents in pat; but complex w/ unclear UI payoff. - // Instead, just abandon providing diagnostic info. - opt_ty_info: None, - opt_match_place, - pat_span, - }))), + local_info: LocalInfo::User(ClearCrossCrate::Set(BindingForm::Var( + VarBindingForm { + binding_mode, + // hypothetically, `visit_bindings` could try to unzip + // an outermost hir::Ty as we descend, matching up + // idents in pat; but complex w/ unclear UI payoff. + // Instead, just abandon providing diagnostic info. + opt_ty_info: None, + opt_match_place, + pat_span, + }, + ))), }; let for_arm_body = self.local_decls.push(local); let locals = if has_guard.0 { @@ -1758,7 +1760,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { visibility_scope, internal: false, is_block_tail: None, - is_user_variable: Some(ClearCrossCrate::Set(BindingForm::RefForGuard)), + local_info: LocalInfo::User(ClearCrossCrate::Set(BindingForm::RefForGuard)), }); LocalsForNode::ForGuard { ref_for_guard, diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index fb605bb2b557b..6b458cc244c9e 100644 --- a/src/librustc_mir/build/mod.rs +++ b/src/librustc_mir/build/mod.rs @@ -820,7 +820,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { visibility_scope: source_info.scope, name, internal: false, - is_user_variable: None, + local_info: LocalInfo::Other, is_block_tail: None, }); } @@ -855,17 +855,21 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } => { self.local_decls[local].mutability = mutability; self.local_decls[local].source_info.scope = self.source_scope; - self.local_decls[local].is_user_variable = + self.local_decls[local].local_info = if let Some(kind) = self_binding { - Some(ClearCrossCrate::Set(BindingForm::ImplicitSelf(*kind))) + LocalInfo::User(ClearCrossCrate::Set( + BindingForm::ImplicitSelf(*kind), + )) } else { let binding_mode = ty::BindingMode::BindByValue(mutability.into()); - Some(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm { - binding_mode, - opt_ty_info, - opt_match_place: Some((Some(place.clone()), span)), - pat_span: span, - }))) + LocalInfo::User(ClearCrossCrate::Set(BindingForm::Var( + VarBindingForm { + binding_mode, + opt_ty_info, + opt_match_place: Some((Some(place.clone()), span)), + pat_span: span, + }, + ))) }; self.var_indices.insert(var, LocalsForNode::One(local)); } diff --git a/src/librustc_mir/dataflow/impls/borrows.rs b/src/librustc_mir/dataflow/impls/borrows.rs index 5e64144df2cd1..402e5aeacbf24 100644 --- a/src/librustc_mir/dataflow/impls/borrows.rs +++ b/src/librustc_mir/dataflow/impls/borrows.rs @@ -209,7 +209,9 @@ impl<'a, 'tcx> Borrows<'a, 'tcx> { // local must conflict. This is purely an optimization so we don't have to call // `places_conflict` for every borrow. if place.projection.is_empty() { - trans.kill_all(other_borrows_of_local); + if !self.body.local_decls[local].is_ref_to_static() { + trans.kill_all(other_borrows_of_local); + } return; } diff --git a/src/librustc_mir/hair/mod.rs b/src/librustc_mir/hair/mod.rs index 28794859c564d..f47c92cbd542d 100644 --- a/src/librustc_mir/hair/mod.rs +++ b/src/librustc_mir/hair/mod.rs @@ -264,6 +264,11 @@ pub enum ExprKind<'tcx> { literal: &'tcx Const<'tcx>, user_ty: Option>>, }, + /// A literal containing the address of a `static` + StaticRef { + literal: &'tcx Const<'tcx>, + def_id: DefId, + }, InlineAsm { asm: &'tcx hir::InlineAsmInner, outputs: Vec>, diff --git a/src/librustc_mir/shim.rs b/src/librustc_mir/shim.rs index 2913d6e59eb3f..17f5e3d4e47a9 100644 --- a/src/librustc_mir/shim.rs +++ b/src/librustc_mir/shim.rs @@ -152,7 +152,7 @@ fn temp_decl(mutability: Mutability, ty: Ty<'_>, span: Span) -> LocalDecl<'_> { source_info, visibility_scope: source_info.scope, internal: false, - is_user_variable: None, + local_info: LocalInfo::Other, is_block_tail: None, } } diff --git a/src/librustc_mir/transform/check_consts/validation.rs b/src/librustc_mir/transform/check_consts/validation.rs index 772f27fb7e14f..bee37f69a5ec5 100644 --- a/src/librustc_mir/transform/check_consts/validation.rs +++ b/src/librustc_mir/transform/check_consts/validation.rs @@ -1,6 +1,6 @@ //! The `Visitor` responsible for actually checking a `mir::Body` for invalid operations. -use rustc::hir::HirId; +use rustc::hir::{HirId, def_id::DefId}; use rustc::middle::lang_items; use rustc::mir::visit::{PlaceContext, Visitor, MutatingUseContext, NonMutatingUseContext}; use rustc::mir::*; @@ -288,6 +288,15 @@ impl Validator<'a, 'mir, 'tcx> { let span = self.span; self.check_op_spanned(op, span) } + + fn check_static(&mut self, def_id: DefId, span: Span) -> CheckOpResult { + let is_thread_local = self.tcx.has_attr(def_id, sym::thread_local); + if is_thread_local { + self.check_op_spanned(ops::ThreadLocalAccess, span) + } else { + self.check_op_spanned(ops::StaticAccess, span) + } + } } impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> { @@ -422,12 +431,7 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> { self.super_operand(op, location); if let Operand::Constant(c) = op { if let Some(def_id) = c.check_static_ptr(self.tcx) { - let is_thread_local = self.tcx.has_attr(def_id, sym::thread_local); - if is_thread_local { - self.check_op(ops::ThreadLocalAccess); - } else { - self.check_op(ops::StaticAccess); - } + self.check_static(def_id, self.span); } } } @@ -506,14 +510,24 @@ impl Visitor<'tcx> for Validator<'_, 'mir, 'tcx> { match elem { ProjectionElem::Deref => { - if context.is_mutating_use() { - self.check_op(ops::MutDeref); - } - let base_ty = Place::ty_from(place_base, proj_base, self.body, self.tcx).ty; if let ty::RawPtr(_) = base_ty.kind { + if proj_base.is_empty() { + if let (PlaceBase::Local(local), []) = (place_base, proj_base) { + let decl = &self.body.local_decls[*local]; + if let LocalInfo::StaticRef { def_id, .. } = decl.local_info { + let span = decl.source_info.span; + self.check_static(def_id, span); + return; + } + } + } self.check_op(ops::RawPtrDeref); } + + if context.is_mutating_use() { + self.check_op(ops::MutDeref); + } } ProjectionElem::ConstantIndex {..} | diff --git a/src/librustc_mir/transform/check_unsafety.rs b/src/librustc_mir/transform/check_unsafety.rs index 9374109c82e41..b7cc4e9fcf66c 100644 --- a/src/librustc_mir/transform/check_unsafety.rs +++ b/src/librustc_mir/transform/check_unsafety.rs @@ -206,25 +206,10 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> { // Locals are safe. } PlaceBase::Static(box Static { kind: StaticKind::Promoted(_, _), .. }) => { - bug!("unsafety checking should happen before promotion") + bug!("unsafety checking should happen before promotion"); } - PlaceBase::Static(box Static { kind: StaticKind::Static, def_id, .. }) => { - if self.tcx.is_mutable_static(def_id) { - self.require_unsafe( - "use of mutable static", - "mutable statics can be mutated by multiple threads: aliasing \ - violations or data races will cause undefined behavior", - UnsafetyViolationKind::General, - ); - } else if self.tcx.is_foreign_item(def_id) { - self.require_unsafe( - "use of extern static", - "extern statics are not controlled by the Rust type system: \ - invalid data, aliasing violations or data races will cause \ - undefined behavior", - UnsafetyViolationKind::General, - ); - } + PlaceBase::Static(box Static { kind: StaticKind::Static, .. }) => { + bug!("StaticKind::Static should not exist"); } } @@ -264,11 +249,31 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> { } let old_source_info = self.source_info; if let (PlaceBase::Local(local), []) = (&place.base, proj_base) { - if self.body.local_decls[*local].internal { + let decl = &self.body.local_decls[*local]; + if decl.internal { // Internal locals are used in the `move_val_init` desugaring. // We want to check unsafety against the source info of the // desugaring, rather than the source info of the RHS. self.source_info = self.body.local_decls[*local].source_info; + } else if let LocalInfo::StaticRef { def_id, .. } = decl.local_info { + if self.tcx.is_mutable_static(def_id) { + self.require_unsafe( + "use of mutable static", + "mutable statics can be mutated by multiple threads: aliasing \ + violations or data races will cause undefined behavior", + UnsafetyViolationKind::General, + ); + return; + } else if self.tcx.is_foreign_item(def_id) { + self.require_unsafe( + "use of extern static", + "extern statics are not controlled by the Rust type system: \ + invalid data, aliasing violations or data races will cause \ + undefined behavior", + UnsafetyViolationKind::General, + ); + return; + } } } let base_ty = Place::ty_from(&place.base, proj_base, self.body, self.tcx).ty; diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index 37c239001a505..524b6b087908c 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -432,7 +432,7 @@ fn replace_result_variable<'tcx>( visibility_scope: source_info.scope, internal: false, is_block_tail: None, - is_user_variable: None, + local_info: LocalInfo::Other }; let new_ret_local = Local::new(body.local_decls.len()); body.local_decls.push(new_ret); @@ -967,7 +967,7 @@ fn create_generator_drop_shim<'tcx>( visibility_scope: source_info.scope, internal: false, is_block_tail: None, - is_user_variable: None, + local_info: LocalInfo::Other }; make_generator_state_argument_indirect(tcx, def_id, &mut body); @@ -985,7 +985,7 @@ fn create_generator_drop_shim<'tcx>( visibility_scope: source_info.scope, internal: false, is_block_tail: None, - is_user_variable: None, + local_info: LocalInfo::Other }; if tcx.sess.opts.debugging_opts.mir_emit_retag { // Alias tracking must know we changed the type diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs index c79d382a37480..86ecfbb4fbea5 100644 --- a/src/librustc_mir/transform/promote_consts.rs +++ b/src/librustc_mir/transform/promote_consts.rs @@ -512,34 +512,9 @@ impl<'tcx> Validator<'_, 'tcx> { projection: [], } => self.validate_local(*local), PlaceRef { - base: PlaceBase::Static(box Static { - kind: StaticKind::Promoted { .. }, - .. - }), + base: PlaceBase::Static(_), projection: [], } => bug!("qualifying already promoted MIR"), - PlaceRef { - base: PlaceBase::Static(box Static { - kind: StaticKind::Static, - def_id, - .. - }), - projection: [], - } => { - // Only allow statics (not consts) to refer to other statics. - // FIXME(eddyb) does this matter at all for promotion? - let is_static = self.const_kind.map_or(false, |k| k.is_static()); - if !is_static { - return Err(Unpromotable); - } - - let is_thread_local = self.tcx.has_attr(*def_id, sym::thread_local); - if is_thread_local { - return Err(Unpromotable); - } - - Ok(()) - } PlaceRef { base: _, projection: [proj_base @ .., elem], @@ -584,7 +559,23 @@ impl<'tcx> Validator<'_, 'tcx> { // The qualifs for a constant (e.g. `HasMutInterior`) are checked in // `validate_rvalue` upon access. - Operand::Constant(_) => Ok(()), + Operand::Constant(c) => { + if let Some(def_id) = c.check_static_ptr(self.tcx) { + // Only allow statics (not consts) to refer to other statics. + // FIXME(eddyb) does this matter at all for promotion? + let is_static = self.const_kind.map_or(false, |k| k.is_static()); + if !is_static { + return Err(Unpromotable); + } + + let is_thread_local = self.tcx.has_attr(def_id, sym::thread_local); + if is_thread_local { + return Err(Unpromotable); + } + } + + Ok(()) + }, } } From 025630d189c66a83b6f334e715cec6690cb85adf Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Tue, 19 Nov 2019 23:16:58 +0000 Subject: [PATCH 09/19] Bless remaining test output --- .../const_prop/read_immutable_static.rs | 12 ++++--- src/test/ui/consts/projection_qualif.stderr | 10 +++--- .../issue-17718-const-bad-values.stderr | 10 +++--- .../ui/issues/issue-17718-references.stderr | 4 +-- src/test/ui/issues/issue-18118-2.stderr | 4 +-- src/test/ui/thread-local-in-ctfe.rs | 2 -- src/test/ui/thread-local-in-ctfe.stderr | 31 +++++-------------- 7 files changed, 28 insertions(+), 45 deletions(-) diff --git a/src/test/mir-opt/const_prop/read_immutable_static.rs b/src/test/mir-opt/const_prop/read_immutable_static.rs index c2902dbd7c129..d14ec0397166f 100644 --- a/src/test/mir-opt/const_prop/read_immutable_static.rs +++ b/src/test/mir-opt/const_prop/read_immutable_static.rs @@ -10,10 +10,12 @@ fn main() { // START rustc.main.ConstProp.before.mir // bb0: { // ... -// _2 = (FOO: u8); +// _3 = const Scalar(AllocId(0).0x0) : &u8; +// _2 = (*_3); // ... -// _3 = (FOO: u8); -// _1 = Add(move _2, move _3); +// _5 = const Scalar(AllocId(0).0x0) : &u8; +// _4 = (*_5); +// _1 = Add(move _2, move _4); // ... // } // END rustc.main.ConstProp.before.mir @@ -22,8 +24,8 @@ fn main() { // ... // _2 = const 2u8; // ... -// _3 = const 2u8; -// _1 = Add(move _2, move _3); +// _4 = const 2u8; +// _1 = Add(move _2, move _4); // ... // } // END rustc.main.ConstProp.after.mir diff --git a/src/test/ui/consts/projection_qualif.stderr b/src/test/ui/consts/projection_qualif.stderr index 0c09f7ec52fa2..0efb6bfd10a17 100644 --- a/src/test/ui/consts/projection_qualif.stderr +++ b/src/test/ui/consts/projection_qualif.stderr @@ -4,20 +4,20 @@ error[E0017]: references in constants may only refer to immutable values LL | let b: *mut u32 = &mut a; | ^^^^^^ constants require immutable values -error[E0019]: constant contains unimplemented expression type +error[E0658]: dereferencing raw pointers in constants is unstable --> $DIR/projection_qualif.rs:7:18 | LL | unsafe { *b = 5; } | ^^^^^^ + | + = note: for more information, see https://github.com/rust-lang/rust/issues/51911 + = help: add `#![feature(const_raw_ptr_deref)]` to the crate attributes to enable -error[E0658]: dereferencing raw pointers in constants is unstable +error[E0019]: constant contains unimplemented expression type --> $DIR/projection_qualif.rs:7:18 | LL | unsafe { *b = 5; } | ^^^^^^ - | - = note: for more information, see https://github.com/rust-lang/rust/issues/51911 - = help: add `#![feature(const_raw_ptr_deref)]` to the crate attributes to enable error: aborting due to 3 previous errors diff --git a/src/test/ui/issues/issue-17718-const-bad-values.stderr b/src/test/ui/issues/issue-17718-const-bad-values.stderr index 7a49e89a1af70..8d875d37d85f9 100644 --- a/src/test/ui/issues/issue-17718-const-bad-values.stderr +++ b/src/test/ui/issues/issue-17718-const-bad-values.stderr @@ -4,17 +4,17 @@ error[E0017]: references in constants may only refer to immutable values LL | const C1: &'static mut [usize] = &mut []; | ^^^^^^^ constants require immutable values -error[E0017]: references in constants may only refer to immutable values - --> $DIR/issue-17718-const-bad-values.rs:5:41 +error[E0013]: constants cannot refer to statics, use a constant instead + --> $DIR/issue-17718-const-bad-values.rs:5:46 | LL | const C2: &'static mut usize = unsafe { &mut S }; - | ^^^^^^ constants require immutable values + | ^ -error[E0013]: constants cannot refer to statics, use a constant instead +error[E0017]: references in constants may only refer to immutable values --> $DIR/issue-17718-const-bad-values.rs:5:41 | LL | const C2: &'static mut usize = unsafe { &mut S }; - | ^^^^^^ + | ^^^^^^ constants require immutable values error: aborting due to 3 previous errors diff --git a/src/test/ui/issues/issue-17718-references.stderr b/src/test/ui/issues/issue-17718-references.stderr index 15c3e67f7a1a6..27aad9c03cebe 100644 --- a/src/test/ui/issues/issue-17718-references.stderr +++ b/src/test/ui/issues/issue-17718-references.stderr @@ -1,8 +1,8 @@ error[E0013]: constants cannot refer to statics, use a constant instead - --> $DIR/issue-17718-references.rs:9:28 + --> $DIR/issue-17718-references.rs:9:29 | LL | const T2: &'static usize = &S; - | ^^ + | ^ error[E0013]: constants cannot refer to statics, use a constant instead --> $DIR/issue-17718-references.rs:14:19 diff --git a/src/test/ui/issues/issue-18118-2.stderr b/src/test/ui/issues/issue-18118-2.stderr index 4e848c261be33..d58822f16eb35 100644 --- a/src/test/ui/issues/issue-18118-2.stderr +++ b/src/test/ui/issues/issue-18118-2.stderr @@ -1,8 +1,8 @@ error[E0013]: constants cannot refer to statics, use a constant instead - --> $DIR/issue-18118-2.rs:4:9 + --> $DIR/issue-18118-2.rs:4:10 | LL | &p - | ^^ + | ^ error: aborting due to previous error diff --git a/src/test/ui/thread-local-in-ctfe.rs b/src/test/ui/thread-local-in-ctfe.rs index 722c3883fdda4..313d39de36b6a 100644 --- a/src/test/ui/thread-local-in-ctfe.rs +++ b/src/test/ui/thread-local-in-ctfe.rs @@ -8,14 +8,12 @@ static B: u32 = A; static C: &u32 = &A; //~^ ERROR thread-local statics cannot be accessed at compile-time -//~| ERROR thread-local variable borrowed past end of function const D: u32 = A; //~^ ERROR thread-local statics cannot be accessed at compile-time const E: &u32 = &A; //~^ ERROR thread-local statics cannot be accessed at compile-time -//~| ERROR thread-local variable borrowed past end of function const fn f() -> u32 { A diff --git a/src/test/ui/thread-local-in-ctfe.stderr b/src/test/ui/thread-local-in-ctfe.stderr index 2983ac3f60cf2..9890597b7bd5b 100644 --- a/src/test/ui/thread-local-in-ctfe.stderr +++ b/src/test/ui/thread-local-in-ctfe.stderr @@ -5,45 +5,28 @@ LL | static B: u32 = A; | ^ error[E0625]: thread-local statics cannot be accessed at compile-time - --> $DIR/thread-local-in-ctfe.rs:9:18 + --> $DIR/thread-local-in-ctfe.rs:9:19 | LL | static C: &u32 = &A; - | ^^ - -error[E0712]: thread-local variable borrowed past end of function - --> $DIR/thread-local-in-ctfe.rs:9:18 - | -LL | static C: &u32 = &A; - | ^^- end of enclosing function is here - | | - | thread-local variables cannot be borrowed beyond the end of the function + | ^ error[E0625]: thread-local statics cannot be accessed at compile-time - --> $DIR/thread-local-in-ctfe.rs:13:16 + --> $DIR/thread-local-in-ctfe.rs:12:16 | LL | const D: u32 = A; | ^ error[E0625]: thread-local statics cannot be accessed at compile-time - --> $DIR/thread-local-in-ctfe.rs:16:17 - | -LL | const E: &u32 = &A; - | ^^ - -error[E0712]: thread-local variable borrowed past end of function - --> $DIR/thread-local-in-ctfe.rs:16:17 + --> $DIR/thread-local-in-ctfe.rs:15:18 | LL | const E: &u32 = &A; - | ^^- end of enclosing function is here - | | - | thread-local variables cannot be borrowed beyond the end of the function + | ^ error[E0625]: thread-local statics cannot be accessed at compile-time - --> $DIR/thread-local-in-ctfe.rs:21:5 + --> $DIR/thread-local-in-ctfe.rs:19:5 | LL | A | ^ -error: aborting due to 7 previous errors +error: aborting due to 5 previous errors -For more information about this error, try `rustc --explain E0712`. From bccc59a8e6e25c2b63308e77f853c6f23782e631 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Thu, 21 Nov 2019 21:20:47 +0000 Subject: [PATCH 10/19] Address review comments --- src/librustc/mir/mod.rs | 11 ++++------- src/librustc_mir/build/expr/as_temp.rs | 3 +-- src/librustc_mir/hair/mod.rs | 5 ++++- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index 2928a8ad9bcc5..67681dd2da44a 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -689,13 +689,6 @@ pub struct LocalDecl<'tcx> { /// Temporaries and the return place are always mutable. pub mutability: Mutability, - /// `Some(binding_mode)` if this corresponds to a user-declared local variable. - /// - /// This is solely used for local diagnostics when generating - /// warnings/errors when compiling the current crate, and - /// therefore it need not be visible across crates. pnkfelix - /// currently hypothesized we *need* to wrap this in a - /// `ClearCrossCrate` as long as it carries as `HirId`. // FIXME(matthewjasper) Don't store in this in `Body` pub local_info: LocalInfo<'tcx>, @@ -831,6 +824,10 @@ pub struct LocalDecl<'tcx> { #[derive(Clone, Debug, RustcEncodable, RustcDecodable, HashStable, TypeFoldable)] pub enum LocalInfo<'tcx> { /// A user-defined local variable or function parameter + /// + /// The `BindingForm` is solely used for local diagnostics when generating + /// warnings/errors when compiling the current crate, and therefore it need + /// not be visible across crates. User(ClearCrossCrate>), /// A temporary created that references the static with the given `DefId`. StaticRef { def_id: DefId, is_thread_local: bool }, diff --git a/src/librustc_mir/build/expr/as_temp.rs b/src/librustc_mir/build/expr/as_temp.rs index 864b449c29c9e..4dad9ab498f63 100644 --- a/src/librustc_mir/build/expr/as_temp.rs +++ b/src/librustc_mir/build/expr/as_temp.rs @@ -65,8 +65,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { local_decl = local_decl.block_tail(tail_info); } if let ExprKind::StaticRef { def_id, .. } = expr.kind { - let attrs = this.hir.tcx().get_attrs(def_id); - let is_thread_local = attrs.iter().any(|attr| attr.check_name(sym::thread_local)); + let is_thread_local = this.hir.tcx().has_attr(def_id, sym::thread_local); local_decl.local_info = LocalInfo::StaticRef {def_id, is_thread_local }; } this.local_decls.push(local_decl) diff --git a/src/librustc_mir/hair/mod.rs b/src/librustc_mir/hair/mod.rs index f47c92cbd542d..47644d9ba8372 100644 --- a/src/librustc_mir/hair/mod.rs +++ b/src/librustc_mir/hair/mod.rs @@ -264,7 +264,10 @@ pub enum ExprKind<'tcx> { literal: &'tcx Const<'tcx>, user_ty: Option>>, }, - /// A literal containing the address of a `static` + /// A literal containing the address of a `static`. + /// + /// This is only distinguished from `Literal` so that we can register some + /// info for diagnostics. StaticRef { literal: &'tcx Const<'tcx>, def_id: DefId, From 5028fd8ab9bda648840bb48e03e618f027cc8c85 Mon Sep 17 00:00:00 2001 From: Robert Bamler Date: Tue, 19 Nov 2019 19:57:03 -0800 Subject: [PATCH 11/19] Document pitfall with `impl PartialEq for A` Fixes #66476 by turning the violating example into an explicit counterexample. --- src/libcore/cmp.rs | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/src/libcore/cmp.rs b/src/libcore/cmp.rs index 1ac51291b93d7..1fb3e89a42f9a 100644 --- a/src/libcore/cmp.rs +++ b/src/libcore/cmp.rs @@ -135,10 +135,15 @@ use self::Ordering::*; /// By changing `impl PartialEq for Book` to `impl PartialEq for Book`, /// we allow `BookFormat`s to be compared with `Book`s. /// -/// You can also combine these implementations to let the `==` operator work with -/// two different types: -/// -/// ``` +/// A comparison like the one above, which ignores some fields of the struct, +/// can be dangerous. It can easily lead to an unintended violation of the +/// requirements for a partial equivalence relation. For example, if we kept +/// the above implementation of `PartialEq` for `BookFormat` and added an +/// implementation of `PartialEq` for `Book` (either via a `#[derive]` or +/// via the manual implementation from the first example) then the result would +/// violate transitivity: +/// +/// ```should_panic /// #[derive(PartialEq)] /// enum BookFormat { /// Paperback, @@ -146,6 +151,7 @@ use self::Ordering::*; /// Ebook, /// } /// +/// #[derive(PartialEq)] /// struct Book { /// isbn: i32, /// format: BookFormat, @@ -163,18 +169,16 @@ use self::Ordering::*; /// } /// } /// -/// impl PartialEq for Book { -/// fn eq(&self, other: &Book) -> bool { -/// self.isbn == other.isbn -/// } -/// } +/// fn main() { +/// let b1 = Book { isbn: 1, format: BookFormat::Paperback }; +/// let b2 = Book { isbn: 2, format: BookFormat::Paperback }; /// -/// let b1 = Book { isbn: 3, format: BookFormat::Paperback }; -/// let b2 = Book { isbn: 3, format: BookFormat::Ebook }; +/// assert!(b1 == BookFormat::Paperback); +/// assert!(BookFormat::Paperback == b2); /// -/// assert!(b1 == BookFormat::Paperback); -/// assert!(BookFormat::Ebook != b1); -/// assert!(b1 == b2); +/// // The following should hold by transitivity but doesn't. +/// assert!(b1 == b2); // <-- PANICS +/// } /// ``` /// /// # Examples From da5539cf7c55c46fda21a5fed8e003785b964a0e Mon Sep 17 00:00:00 2001 From: Guanqun Lu Date: Fri, 22 Nov 2019 15:33:49 +0800 Subject: [PATCH 12/19] follow the convention in this file to use third-person singular verbs --- src/libcore/iter/traits/iterator.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcore/iter/traits/iterator.rs b/src/libcore/iter/traits/iterator.rs index b7a35568e3fc5..61e8b07511a67 100644 --- a/src/libcore/iter/traits/iterator.rs +++ b/src/libcore/iter/traits/iterator.rs @@ -1255,7 +1255,7 @@ pub trait Iterator { Fuse::new(self) } - /// Do something with each element of an iterator, passing the value on. + /// Does something with each element of an iterator, passing the value on. /// /// When using iterators, you'll often chain several of them together. /// While working on such code, you might want to check out what's @@ -1548,7 +1548,7 @@ pub trait Iterator { (left, right) } - /// Reorder the elements of this iterator *in-place* according to the given predicate, + /// Reorders the elements of this iterator *in-place* according to the given predicate, /// such that all those that return `true` precede all those that return `false`. /// Returns the number of `true` elements found. /// From 9ff91ab2d3151078053bc3966f7163d048209d38 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 22 Nov 2019 18:11:28 +0100 Subject: [PATCH 13/19] fix reoccuring typo: dereferencable -> dereferenceable --- src/librustc/mir/tcx.rs | 2 +- src/librustc_mir/interpret/memory.rs | 6 +++--- src/librustc_mir/interpret/validity.rs | 4 ++-- src/librustc_typeck/check/pat.rs | 6 +++--- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/librustc/mir/tcx.rs b/src/librustc/mir/tcx.rs index ccf7d8fbf8668..a66a49f103f68 100644 --- a/src/librustc/mir/tcx.rs +++ b/src/librustc/mir/tcx.rs @@ -81,7 +81,7 @@ impl<'tcx> PlaceTy<'tcx> { let ty = self.ty .builtin_deref(true) .unwrap_or_else(|| { - bug!("deref projection of non-dereferencable ty {:?}", self) + bug!("deref projection of non-dereferenceable ty {:?}", self) }) .ty; PlaceTy::from_ty(ty) diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index e929b0855834e..842ef915ad226 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -47,7 +47,7 @@ impl MayLeak for MemoryKind { #[derive(Debug, Copy, Clone)] pub enum AllocCheck { /// Allocation must be live and not a function pointer. - Dereferencable, + Dereferenceable, /// Allocations needs to be live, but may be a function pointer. Live, /// Allocation may be dead. @@ -365,7 +365,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { } Err(ptr) => { let (allocation_size, alloc_align) = - self.get_size_and_align(ptr.alloc_id, AllocCheck::Dereferencable)?; + self.get_size_and_align(ptr.alloc_id, AllocCheck::Dereferenceable)?; // Test bounds. This also ensures non-NULL. // It is sufficient to check this for the end pointer. The addition // checks for overflow. @@ -569,7 +569,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { // # Function pointers // (both global from `alloc_map` and local from `extra_fn_ptr_map`) if let Ok(_) = self.get_fn_alloc(id) { - return if let AllocCheck::Dereferencable = liveness { + return if let AllocCheck::Dereferenceable = liveness { // The caller requested no function pointers. throw_unsup!(DerefFunctionPointer) } else { diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index d698b2e8d8f80..e358df2f213ba 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -286,7 +286,7 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, 'tcx, M "non-integer slice length in wide pointer", self.path); // We do not check that `len * elem_size <= isize::MAX`: // that is only required for references, and there it falls out of the - // "dereferencable" check performed by Stacked Borrows. + // "dereferenceable" check performed by Stacked Borrows. } ty::Foreign(..) => { // Unsized, but not wide. @@ -404,7 +404,7 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> if place.layout.is_unsized() { self.check_wide_ptr_meta(place.meta, place.layout)?; } - // Make sure this is dereferencable and all. + // Make sure this is dereferenceable and all. let (size, align) = self.ecx.size_and_align_of(place.meta, place.layout)? // for the purpose of validity, consider foreign types to have // alignment and size determined by the layout (size will be 0, diff --git a/src/librustc_typeck/check/pat.rs b/src/librustc_typeck/check/pat.rs index bba30ebbbe7bb..9dd3bc624a51a 100644 --- a/src/librustc_typeck/check/pat.rs +++ b/src/librustc_typeck/check/pat.rs @@ -538,7 +538,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } - pub fn check_dereferencable(&self, span: Span, expected: Ty<'tcx>, inner: &Pat) -> bool { + pub fn check_dereferenceable(&self, span: Span, expected: Ty<'tcx>, inner: &Pat) -> bool { if let PatKind::Binding(..) = inner.kind { if let Some(mt) = self.shallow_resolve(expected).builtin_deref(true) { if let ty::Dynamic(..) = mt.ty.kind { @@ -1075,7 +1075,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { discrim_span: Option, ) -> Ty<'tcx> { let tcx = self.tcx; - let (box_ty, inner_ty) = if self.check_dereferencable(span, expected, &inner) { + let (box_ty, inner_ty) = if self.check_dereferenceable(span, expected, &inner) { // Here, `demand::subtype` is good enough, but I don't // think any errors can be introduced by using `demand::eqtype`. let inner_ty = self.next_ty_var(TypeVariableOrigin { @@ -1103,7 +1103,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ) -> Ty<'tcx> { let tcx = self.tcx; let expected = self.shallow_resolve(expected); - let (rptr_ty, inner_ty) = if self.check_dereferencable(pat.span, expected, &inner) { + let (rptr_ty, inner_ty) = if self.check_dereferenceable(pat.span, expected, &inner) { // `demand::subtype` would be good enough, but using `eqtype` turns // out to be equally general. See (note_1) for details. From ea62c2e5b31cec7f540784cc1fad84b7c0006e3f Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 22 Nov 2019 13:23:33 +0100 Subject: [PATCH 14/19] Improve E0015 long error explanation --- src/librustc_error_codes/error_codes/E0015.md | 36 ++++++++++++++----- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/src/librustc_error_codes/error_codes/E0015.md b/src/librustc_error_codes/error_codes/E0015.md index 2bf59983fa93a..361cb425809d7 100644 --- a/src/librustc_error_codes/error_codes/E0015.md +++ b/src/librustc_error_codes/error_codes/E0015.md @@ -1,14 +1,32 @@ -The only functions that can be called in static or constant expressions are -`const` functions, and struct/enum constructors. `const` functions are only -available on a nightly compiler. Rust currently does not support more general -compile-time function execution. +A constant item was initialized with something that is not a constant expression. + +Erroneous code example: + +```compile_fail,E0015 +fn create_some() -> Option { + Some(1) +} +const FOO: Option = create_some(); // error! ``` -const FOO: Option = Some(1); // enum constructor -struct Bar {x: u8} -const BAR: Bar = Bar {x: 1}; // struct constructor + +The only functions that can be called in static or constant expressions are +`const` functions, and struct/enum constructors. + +To fix this error, you can declare `create_some` as a constant function: + ``` +const fn create_some() -> Option { // declared as a const function + Some(1) +} -See [RFC 911] for more details on the design of `const fn`s. +const FOO: Option = create_some(); // ok! -[RFC 911]: https://github.com/rust-lang/rfcs/blob/master/text/0911-const-fn.md +// These are also working: +struct Bar { + x: u8, +} + +const OTHER_FOO: Option = Some(1); +const BAR: Bar = Bar {x: 1}; +``` From 60d9c2c239a401e88c118e6df34584adef4c4ebb Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 22 Nov 2019 13:25:48 +0100 Subject: [PATCH 15/19] Improve E0023 long error explanation --- src/librustc_error_codes/error_codes/E0023.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/librustc_error_codes/error_codes/E0023.md b/src/librustc_error_codes/error_codes/E0023.md index 23a9d22a60d82..c1d85705da3a9 100644 --- a/src/librustc_error_codes/error_codes/E0023.md +++ b/src/librustc_error_codes/error_codes/E0023.md @@ -2,11 +2,18 @@ A pattern attempted to extract an incorrect number of fields from a variant. Erroneous code example: -``` +```compile_fail,E0023 enum Fruit { Apple(String, String), Pear(u32), } + +let x = Fruit::Apple(String::new(), String::new()); + +match x { + Fruit::Apple(a) => {}, // error! + _ => {} +} ``` A pattern used to match against an enum variant must provide a sub-pattern for From f798804cd9612daa324c93d911eae00fa1775597 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 22 Nov 2019 13:31:47 +0100 Subject: [PATCH 16/19] Improve E0057 long error explanation --- src/librustc_error_codes/error_codes/E0057.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/librustc_error_codes/error_codes/E0057.md b/src/librustc_error_codes/error_codes/E0057.md index e11c07f835ad7..bb5e4b48d2deb 100644 --- a/src/librustc_error_codes/error_codes/E0057.md +++ b/src/librustc_error_codes/error_codes/E0057.md @@ -1,8 +1,6 @@ -When invoking closures or other implementations of the function traits `Fn`, -`FnMut` or `FnOnce` using call notation, the number of parameters passed to the -function must match its definition. +An invalid number of arguments was given when calling a closure. -An example using a closure: +Erroneous code example: ```compile_fail,E0057 let f = |x| x * 3; @@ -11,6 +9,10 @@ let b = f(4); // this works! let c = f(2, 3); // invalid, too many parameters ``` +When invoking closures or other implementations of the function traits `Fn`, +`FnMut` or `FnOnce` using call notation, the number of parameters passed to the +function must match its definition. + A generic function must be treated similarly: ``` From 9bb2e3cd3419b14f68b29f214bfa7776409c5403 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 22 Nov 2019 13:34:52 +0100 Subject: [PATCH 17/19] Improve E0061 long error explanation --- src/librustc_error_codes/error_codes/E0061.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/librustc_error_codes/error_codes/E0061.md b/src/librustc_error_codes/error_codes/E0061.md index 3386dff2c8513..143251c13b069 100644 --- a/src/librustc_error_codes/error_codes/E0061.md +++ b/src/librustc_error_codes/error_codes/E0061.md @@ -1,3 +1,13 @@ +An invalid number of arguments was passed when calling a function. + +Erroneous code example: + +```compile_fail,E0061 +fn f(u: i32) {} + +f(); // error! +``` + The number of arguments passed to a function must match the number of arguments specified in the function signature. From a8de11cdd5c20a75d3cef4c7a322d29539b7bda2 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 22 Nov 2019 13:35:08 +0100 Subject: [PATCH 18/19] small error code explanations improvements --- src/librustc_error_codes/error_codes/E0033.md | 2 +- src/librustc_error_codes/error_codes/E0038.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_error_codes/error_codes/E0033.md b/src/librustc_error_codes/error_codes/E0033.md index c49cedf2d5869..735a2d1f3fe85 100644 --- a/src/librustc_error_codes/error_codes/E0033.md +++ b/src/librustc_error_codes/error_codes/E0033.md @@ -24,4 +24,4 @@ dereferencing the pointer. You can read more about trait objects in the [Trait Objects] section of the Reference. -[Trait Objects]: https://doc.rust-lang.org/reference/types.html#trait-objects \ No newline at end of file +[Trait Objects]: https://doc.rust-lang.org/reference/types.html#trait-objects diff --git a/src/librustc_error_codes/error_codes/E0038.md b/src/librustc_error_codes/error_codes/E0038.md index 21b5eb47480e6..25e380b02e647 100644 --- a/src/librustc_error_codes/error_codes/E0038.md +++ b/src/librustc_error_codes/error_codes/E0038.md @@ -62,7 +62,7 @@ cause this problem.) In such a case, the compiler cannot predict the return type of `foo()` in a situation like the following: -```compile_fail +```compile_fail,E0038 trait Trait { fn foo(&self) -> Self; } @@ -183,7 +183,7 @@ fn call_foo(thing: Box) { We don't just need to create a table of all implementations of all methods of `Trait`, we need to create such a table, for each different type fed to -`foo()`. In this case this turns out to be (10 types implementing `Trait`)*(3 +`foo()`. In this case this turns out to be (10 types implementing `Trait`)\*(3 types being fed to `foo()`) = 30 implementations! With real world traits these numbers can grow drastically. From 94b7ea97bf1c817a94237fa789c841eb0e66b5c0 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 22 Nov 2019 19:13:51 +0100 Subject: [PATCH 19/19] resolve: more declarative fresh_binding --- src/librustc_resolve/late.rs | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/src/librustc_resolve/late.rs b/src/librustc_resolve/late.rs index f48df7faea25a..666c482c68046 100644 --- a/src/librustc_resolve/late.rs +++ b/src/librustc_resolve/late.rs @@ -69,6 +69,7 @@ impl PatternSource { /// Denotes whether the context for the set of already bound bindings is a `Product` /// or `Or` context. This is used in e.g., `fresh_binding` and `resolve_pattern_inner`. /// See those functions for more information. +#[derive(PartialEq)] enum PatBoundCtx { /// A product pattern context, e.g., `Variant(a, b)`. Product, @@ -1417,21 +1418,12 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> { // later passes make about or-patterns.) let ident = ident.modern_and_legacy(); - // Walk outwards the stack of products / or-patterns and - // find out if the identifier has been bound in any of these. - let mut already_bound_and = false; - let mut already_bound_or = false; - for (is_sum, set) in bindings.iter_mut().rev() { - match (is_sum, set.get(&ident).cloned()) { - // Already bound in a product pattern, e.g. `(a, a)` which is not allowed. - (PatBoundCtx::Product, Some(..)) => already_bound_and = true, - // Already bound in an or-pattern, e.g. `V1(a) | V2(a)`. - // This is *required* for consistency which is checked later. - (PatBoundCtx::Or, Some(..)) => already_bound_or = true, - // Not already bound here. - _ => {} - } - } + let mut bound_iter = bindings.iter().filter(|(_, set)| set.contains(&ident)); + // Already bound in a product pattern? e.g. `(a, a)` which is not allowed. + let already_bound_and = bound_iter.clone().any(|(ctx, _)| *ctx == PatBoundCtx::Product); + // Already bound in an or-pattern? e.g. `V1(a) | V2(a)`. + // This is *required* for consistency which is checked later. + let already_bound_or = bound_iter.any(|(ctx, _)| *ctx == PatBoundCtx::Or); if already_bound_and { // Overlap in a product pattern somewhere; report an error.