From 2ed473487323bb4e5a600a3318e0981981214210 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Tue, 10 Jun 2014 13:54:13 -0700 Subject: [PATCH 01/16] librustc: Fix the issue with labels shadowing variable names by making the leading quote part of the identifier for the purposes of hygiene. This adopts @jbclements' solution to #14539. I'm not sure if this is a breaking change or not. Closes #12512. [breaking-change] --- src/librustc/middle/resolve_lifetime.rs | 8 +- .../middle/typeck/infer/error_reporting.rs | 3 +- src/librustc/util/ppaux.rs | 2 +- src/libsyntax/ext/build.rs | 31 +++++- src/libsyntax/ext/bytes.rs | 14 ++- src/libsyntax/ext/env.rs | 2 +- src/libsyntax/ext/format.rs | 2 +- src/libsyntax/parse/lexer/mod.rs | 39 +++++-- src/libsyntax/parse/parser.rs | 2 +- src/libsyntax/parse/token.rs | 103 +++++++++--------- src/libsyntax/print/pprust.rs | 11 +- src/test/compile-fail/hygienic-label-1.rs | 2 +- src/test/compile-fail/hygienic-label-2.rs | 2 +- src/test/compile-fail/hygienic-label-3.rs | 2 +- src/test/compile-fail/hygienic-label-4.rs | 2 +- src/test/compile-fail/regions-name-static.rs | 2 +- src/test/run-pass/loop-label-shadowing.rs | 19 ++++ 17 files changed, 158 insertions(+), 88 deletions(-) create mode 100644 src/test/run-pass/loop-label-shadowing.rs diff --git a/src/librustc/middle/resolve_lifetime.rs b/src/librustc/middle/resolve_lifetime.rs index f416686efd869..8ff5331cec231 100644 --- a/src/librustc/middle/resolve_lifetime.rs +++ b/src/librustc/middle/resolve_lifetime.rs @@ -165,7 +165,7 @@ impl<'a, 'b> Visitor> for LifetimeContext<'b> { fn visit_lifetime_ref(&mut self, lifetime_ref: &ast::Lifetime, scope: Scope<'a>) { - if lifetime_ref.name == special_idents::statik.name { + if lifetime_ref.name == special_idents::static_lifetime.name { self.insert_lifetime(lifetime_ref, DefStaticRegion); return; } @@ -330,7 +330,7 @@ impl<'a> LifetimeContext<'a> { lifetime_ref: &ast::Lifetime) { self.sess.span_err( lifetime_ref.span, - format!("use of undeclared lifetime name `'{}`", + format!("use of undeclared lifetime name `{}`", token::get_name(lifetime_ref.name)).as_slice()); } @@ -338,7 +338,7 @@ impl<'a> LifetimeContext<'a> { for i in range(0, lifetimes.len()) { let lifetime_i = lifetimes.get(i); - let special_idents = [special_idents::statik]; + let special_idents = [special_idents::static_lifetime]; for lifetime in lifetimes.iter() { if special_idents.iter().any(|&i| i.name == lifetime.name) { self.sess.span_err( @@ -354,7 +354,7 @@ impl<'a> LifetimeContext<'a> { if lifetime_i.name == lifetime_j.name { self.sess.span_err( lifetime_j.span, - format!("lifetime name `'{}` declared twice in \ + format!("lifetime name `{}` declared twice in \ the same scope", token::get_name(lifetime_j.name)).as_slice()); } diff --git a/src/librustc/middle/typeck/infer/error_reporting.rs b/src/librustc/middle/typeck/infer/error_reporting.rs index bcd66ed4d66f2..2e02c798339ea 100644 --- a/src/librustc/middle/typeck/infer/error_reporting.rs +++ b/src/librustc/middle/typeck/infer/error_reporting.rs @@ -1505,7 +1505,8 @@ impl LifeGiver { fn give_lifetime(&self) -> ast::Lifetime { let mut lifetime; loop { - let s = num_to_str(self.counter.get()); + let mut s = String::from_str("'"); + s.push_str(num_to_str(self.counter.get()).as_slice()); if !self.taken.contains(&s) { lifetime = name_to_dummy_lifetime( token::str_to_ident(s.as_slice()).name); diff --git a/src/librustc/util/ppaux.rs b/src/librustc/util/ppaux.rs index 31994d08d2347..80f3508d0cd35 100644 --- a/src/librustc/util/ppaux.rs +++ b/src/librustc/util/ppaux.rs @@ -162,7 +162,7 @@ pub fn bound_region_to_str(cx: &ctxt, match br { BrNamed(_, name) => { - format!("{}'{}{}", prefix, token::get_name(name), space_str) + format!("{}{}{}", prefix, token::get_name(name), space_str) } BrAnon(_) => prefix.to_string(), BrFresh(_) => prefix.to_string(), diff --git a/src/libsyntax/ext/build.rs b/src/libsyntax/ext/build.rs index 148b653b61cb9..4ef7796c454b6 100644 --- a/src/libsyntax/ext/build.rs +++ b/src/libsyntax/ext/build.rs @@ -85,6 +85,7 @@ pub trait AstBuilder { typ: P, ex: Gc) -> Gc; + fn stmt_item(&self, sp: Span, item: Gc) -> Gc; // blocks fn block(&self, span: Span, stmts: Vec>, @@ -239,6 +240,14 @@ pub trait AstBuilder { vi: Vec, items: Vec>) -> Gc; + fn item_static(&self, + span: Span, + name: Ident, + ty: P, + mutbl: ast::Mutability, + expr: Gc) + -> Gc; + fn item_ty_poly(&self, span: Span, name: Ident, @@ -484,11 +493,19 @@ impl<'a> AstBuilder for ExtCtxt<'a> { box(GC) respan(sp, ast::StmtDecl(box(GC) decl, ast::DUMMY_NODE_ID)) } - fn block(&self, span: Span, stmts: Vec>, - expr: Option>) -> P { + fn block(&self, + span: Span, + stmts: Vec>, + expr: Option>) + -> P { self.block_all(span, Vec::new(), stmts, expr) } + fn stmt_item(&self, sp: Span, item: Gc) -> Gc { + let decl = respan(sp, ast::DeclItem(item)); + box(GC) respan(sp, ast::StmtDecl(box(GC) decl, ast::DUMMY_NODE_ID)) + } + fn block_expr(&self, expr: Gc) -> P { self.block_all(expr.span, Vec::new(), Vec::new(), Some(expr)) } @@ -942,6 +959,16 @@ impl<'a> AstBuilder for ExtCtxt<'a> { ) } + fn item_static(&self, + span: Span, + name: Ident, + ty: P, + mutbl: ast::Mutability, + expr: Gc) + -> Gc { + self.item(span, name, Vec::new(), ast::ItemStatic(ty, mutbl, expr)) + } + fn item_ty_poly(&self, span: Span, name: Ident, ty: P, generics: Generics) -> Gc { self.item(span, name, Vec::new(), ast::ItemTy(ty, generics)) diff --git a/src/libsyntax/ext/bytes.rs b/src/libsyntax/ext/bytes.rs index b2088d2bc82f6..b87a25d4a44a2 100644 --- a/src/libsyntax/ext/bytes.rs +++ b/src/libsyntax/ext/bytes.rs @@ -94,6 +94,18 @@ pub fn expand_syntax_ext(cx: &mut ExtCtxt, sp: Span, tts: &[ast::TokenTree]) } let e = cx.expr_vec_slice(sp, bytes); - let e = quote_expr!(cx, { static BYTES: &'static [u8] = $e; BYTES}); + let ty = cx.ty(sp, ast::TyVec(cx.ty_ident(sp, cx.ident_of("u8")))); + let lifetime = cx.lifetime(sp, cx.ident_of("'static").name); + let item = cx.item_static(sp, + cx.ident_of("BYTES"), + cx.ty_rptr(sp, + ty, + Some(lifetime), + ast::MutImmutable), + ast::MutImmutable, + e); + let e = cx.expr_block(cx.block(sp, + vec!(cx.stmt_item(sp, item)), + Some(cx.expr_ident(sp, cx.ident_of("BYTES"))))); MacExpr::new(e) } diff --git a/src/libsyntax/ext/env.rs b/src/libsyntax/ext/env.rs index d1a4d2f3ee3b6..9ef7241ca2484 100644 --- a/src/libsyntax/ext/env.rs +++ b/src/libsyntax/ext/env.rs @@ -43,7 +43,7 @@ pub fn expand_option_env(cx: &mut ExtCtxt, sp: Span, tts: &[ast::TokenTree]) cx.ident_of("str")), Some(cx.lifetime(sp, cx.ident_of( - "static").name)), + "'static").name)), ast::MutImmutable)))) } Some(s) => { diff --git a/src/libsyntax/ext/format.rs b/src/libsyntax/ext/format.rs index d3b73cbe33af6..cfce4b1e0fc5e 100644 --- a/src/libsyntax/ext/format.rs +++ b/src/libsyntax/ext/format.rs @@ -465,7 +465,7 @@ impl<'a, 'b> Context<'a, 'b> { self.ecx.ident_of("rt"), self.ecx.ident_of("Piece")), vec!(self.ecx.lifetime(self.fmtsp, - self.ecx.ident_of("static").name)), + self.ecx.ident_of("'static").name)), Vec::new() ), None); let ty = ast::TyFixedLengthVec( diff --git a/src/libsyntax/parse/lexer/mod.rs b/src/libsyntax/parse/lexer/mod.rs index bb23fe50bd9e2..459cb6d31ed0a 100644 --- a/src/libsyntax/parse/lexer/mod.rs +++ b/src/libsyntax/parse/lexer/mod.rs @@ -757,19 +757,34 @@ impl<'a> StringReader<'a> { while ident_continue(self.curr) { self.bump(); } + + // Include the leading `'` in the real identifier, for macro + // expansion purposes. See #12512 for the gory details of why + // this is necessary. let ident = self.with_str_from(start, |lifetime_name| { - str_to_ident(lifetime_name) + str_to_ident(format!("'{}", lifetime_name).as_slice()) }); - let tok = &token::IDENT(ident, false); - - if token::is_keyword(token::keywords::Self, tok) { - self.err_span(start, self.last_pos, - "invalid lifetime name: 'self \ - is no longer a special lifetime"); - } else if token::is_any_keyword(tok) && - !token::is_keyword(token::keywords::Static, tok) { - self.err_span(start, self.last_pos, - "invalid lifetime name"); + + // Conjure up a "keyword checking ident" to make sure that + // the lifetime name is not a keyword. + let keyword_checking_ident = + self.with_str_from(start, |lifetime_name| { + str_to_ident(lifetime_name) + }); + let keyword_checking_token = + &token::IDENT(keyword_checking_ident, false); + if token::is_keyword(token::keywords::Self, + keyword_checking_token) { + self.err_span(start, + self.last_pos, + "invalid lifetime name: 'self \ + is no longer a special lifetime"); + } else if token::is_any_keyword(keyword_checking_token) && + !token::is_keyword(token::keywords::Static, + keyword_checking_token) { + self.err_span(start, + self.last_pos, + "invalid lifetime name"); } return token::LIFETIME(ident); } @@ -1128,7 +1143,7 @@ mod test { #[test] fn lifetime_name() { assert_eq!(setup(&mk_sh(), "'abc".to_string()).next_token().tok, - token::LIFETIME(token::str_to_ident("abc"))); + token::LIFETIME(token::str_to_ident("'abc"))); } #[test] fn raw_string() { diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 437b06e3df65b..aaedb5709550d 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -3452,7 +3452,7 @@ impl<'a> Parser<'a> { match self.token { token::LIFETIME(lifetime) => { let lifetime_interned_string = token::get_ident(lifetime); - if lifetime_interned_string.equiv(&("static")) { + if lifetime_interned_string.equiv(&("'static")) { result.push(StaticRegionTyParamBound); if allow_any_lifetime && ret_lifetime.is_none() { ret_lifetime = Some(ast::Lifetime { diff --git a/src/libsyntax/parse/token.rs b/src/libsyntax/parse/token.rs index fa70261a7d711..a4a022708d95e 100644 --- a/src/libsyntax/parse/token.rs +++ b/src/libsyntax/parse/token.rs @@ -232,7 +232,7 @@ pub fn to_str(t: &Token) -> String { /* Name components */ IDENT(s, _) => get_ident(s).get().to_string(), LIFETIME(s) => { - (format!("'{}", get_ident(s))).to_string() + (format!("{}", get_ident(s))).to_string() } UNDERSCORE => "_".to_string(), @@ -433,71 +433,72 @@ declare_special_idents_and_keywords! { (0, invalid, ""); (super::SELF_KEYWORD_NAME, self_, "self"); (super::STATIC_KEYWORD_NAME, statik, "static"); + (3, static_lifetime, "'static"); // for matcher NTs - (3, tt, "tt"); - (4, matchers, "matchers"); + (4, tt, "tt"); + (5, matchers, "matchers"); // outside of libsyntax - (5, clownshoe_abi, "__rust_abi"); - (6, opaque, ""); - (7, unnamed_field, ""); - (8, type_self, "Self"); + (6, clownshoe_abi, "__rust_abi"); + (7, opaque, ""); + (8, unnamed_field, ""); + (9, type_self, "Self"); } pub mod keywords { // These ones are variants of the Keyword enum 'strict: - (9, As, "as"); - (10, Break, "break"); - (11, Crate, "crate"); - (12, Else, "else"); - (13, Enum, "enum"); - (14, Extern, "extern"); - (15, False, "false"); - (16, Fn, "fn"); - (17, For, "for"); - (18, If, "if"); - (19, Impl, "impl"); - (20, In, "in"); - (21, Let, "let"); - (22, Loop, "loop"); - (23, Match, "match"); - (24, Mod, "mod"); - (25, Mut, "mut"); - (26, Once, "once"); - (27, Pub, "pub"); - (28, Ref, "ref"); - (29, Return, "return"); + (10, As, "as"); + (11, Break, "break"); + (12, Crate, "crate"); + (13, Else, "else"); + (14, Enum, "enum"); + (15, Extern, "extern"); + (16, False, "false"); + (17, Fn, "fn"); + (18, For, "for"); + (19, If, "if"); + (20, Impl, "impl"); + (21, In, "in"); + (22, Let, "let"); + (23, Loop, "loop"); + (24, Match, "match"); + (25, Mod, "mod"); + (26, Mut, "mut"); + (27, Once, "once"); + (28, Pub, "pub"); + (29, Ref, "ref"); + (30, Return, "return"); // Static and Self are also special idents (prefill de-dupes) (super::STATIC_KEYWORD_NAME, Static, "static"); (super::SELF_KEYWORD_NAME, Self, "self"); - (30, Struct, "struct"); - (31, Super, "super"); - (32, True, "true"); - (33, Trait, "trait"); - (34, Type, "type"); - (35, Unsafe, "unsafe"); - (36, Use, "use"); - (37, Virtual, "virtual"); - (38, While, "while"); - (39, Continue, "continue"); - (40, Proc, "proc"); - (41, Box, "box"); + (31, Struct, "struct"); + (32, Super, "super"); + (33, True, "true"); + (34, Trait, "trait"); + (35, Type, "type"); + (36, Unsafe, "unsafe"); + (37, Use, "use"); + (38, Virtual, "virtual"); + (39, While, "while"); + (40, Continue, "continue"); + (41, Proc, "proc"); + (42, Box, "box"); 'reserved: - (42, Alignof, "alignof"); - (43, Be, "be"); - (44, Const, "const"); - (45, Offsetof, "offsetof"); - (46, Priv, "priv"); - (47, Pure, "pure"); - (48, Sizeof, "sizeof"); - (49, Typeof, "typeof"); - (50, Unsized, "unsized"); - (51, Yield, "yield"); - (52, Do, "do"); + (43, Alignof, "alignof"); + (44, Be, "be"); + (45, Const, "const"); + (46, Offsetof, "offsetof"); + (47, Priv, "priv"); + (48, Pure, "pure"); + (49, Sizeof, "sizeof"); + (50, Typeof, "typeof"); + (51, Unsized, "unsized"); + (52, Yield, "yield"); + (53, Do, "do"); } } diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs index 33b7086d7ae63..77bc967b92d07 100644 --- a/src/libsyntax/print/pprust.rs +++ b/src/libsyntax/print/pprust.rs @@ -9,8 +9,8 @@ // except according to those terms. use abi; -use ast::{P, StaticRegionTyParamBound, OtherRegionTyParamBound, - TraitTyParamBound, UnboxedFnTyParamBound, Required, Provided}; +use ast::{P, StaticRegionTyParamBound, OtherRegionTyParamBound}; +use ast::{TraitTyParamBound, UnboxedFnTyParamBound, Required, Provided}; use ast; use ast_util; use owned_slice::OwnedSlice; @@ -1325,7 +1325,6 @@ impl<'a> State<'a> { } ast::ExprForLoop(ref pat, ref iter, ref blk, opt_ident) => { for ident in opt_ident.iter() { - try!(word(&mut self.s, "'")); try!(self.print_ident(*ident)); try!(self.word_space(":")); } @@ -1339,7 +1338,6 @@ impl<'a> State<'a> { } ast::ExprLoop(ref blk, opt_ident) => { for ident in opt_ident.iter() { - try!(word(&mut self.s, "'")); try!(self.print_ident(*ident)); try!(self.word_space(":")); } @@ -1504,7 +1502,6 @@ impl<'a> State<'a> { try!(word(&mut self.s, "break")); try!(space(&mut self.s)); for ident in opt_ident.iter() { - try!(word(&mut self.s, "'")); try!(self.print_ident(*ident)); try!(space(&mut self.s)); } @@ -1513,7 +1510,6 @@ impl<'a> State<'a> { try!(word(&mut self.s, "continue")); try!(space(&mut self.s)); for ident in opt_ident.iter() { - try!(word(&mut self.s, "'")); try!(self.print_ident(*ident)); try!(space(&mut self.s)) } @@ -1943,7 +1939,7 @@ impl<'a> State<'a> { match *region { Some(ref lt) => { let token = token::get_name(lt.name); - if token.get() != "static" { + if token.get() != "'static" { try!(self.nbsp()); first = false; try!(self.print_lifetime(lt)); @@ -1988,7 +1984,6 @@ impl<'a> State<'a> { pub fn print_lifetime(&mut self, lifetime: &ast::Lifetime) -> IoResult<()> { - try!(word(&mut self.s, "'")); self.print_name(lifetime.name) } diff --git a/src/test/compile-fail/hygienic-label-1.rs b/src/test/compile-fail/hygienic-label-1.rs index 010cde769d755..0e87dc97c2631 100644 --- a/src/test/compile-fail/hygienic-label-1.rs +++ b/src/test/compile-fail/hygienic-label-1.rs @@ -15,5 +15,5 @@ macro_rules! foo { } pub fn main() { - 'x: loop { foo!() } //~ ERROR use of undeclared label `x` + 'x: loop { foo!() } //~ ERROR use of undeclared label `'x` } diff --git a/src/test/compile-fail/hygienic-label-2.rs b/src/test/compile-fail/hygienic-label-2.rs index 78d8fce38d5a6..fe87e32459bb1 100644 --- a/src/test/compile-fail/hygienic-label-2.rs +++ b/src/test/compile-fail/hygienic-label-2.rs @@ -15,5 +15,5 @@ macro_rules! foo { } pub fn main() { - foo!(break 'x); //~ ERROR use of undeclared label `x` + foo!(break 'x); //~ ERROR use of undeclared label `'x` } diff --git a/src/test/compile-fail/hygienic-label-3.rs b/src/test/compile-fail/hygienic-label-3.rs index 439132fa152bb..b5954ac99303b 100644 --- a/src/test/compile-fail/hygienic-label-3.rs +++ b/src/test/compile-fail/hygienic-label-3.rs @@ -16,6 +16,6 @@ macro_rules! foo { pub fn main() { 'x: for _ in range(0,1) { - foo!() //~ ERROR use of undeclared label `x` + foo!() //~ ERROR use of undeclared label `'x` }; } diff --git a/src/test/compile-fail/hygienic-label-4.rs b/src/test/compile-fail/hygienic-label-4.rs index dfda458652704..67fa56b130677 100644 --- a/src/test/compile-fail/hygienic-label-4.rs +++ b/src/test/compile-fail/hygienic-label-4.rs @@ -15,5 +15,5 @@ macro_rules! foo { } pub fn main() { - foo!(break 'x); //~ ERROR use of undeclared label `x` + foo!(break 'x); //~ ERROR use of undeclared label `'x` } diff --git a/src/test/compile-fail/regions-name-static.rs b/src/test/compile-fail/regions-name-static.rs index c1170654dd294..9f50ad3666025 100644 --- a/src/test/compile-fail/regions-name-static.rs +++ b/src/test/compile-fail/regions-name-static.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -struct Foo<'static> { //~ ERROR illegal lifetime parameter name: `static` +struct Foo<'static> { //~ ERROR illegal lifetime parameter name: `'static` x: &'static int } diff --git a/src/test/run-pass/loop-label-shadowing.rs b/src/test/run-pass/loop-label-shadowing.rs new file mode 100644 index 0000000000000..cfe51fe77589f --- /dev/null +++ b/src/test/run-pass/loop-label-shadowing.rs @@ -0,0 +1,19 @@ +// Copyright 2012 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Issue #12512. + +fn main() { + let mut foo = Vec::new(); + 'foo: for i in [1, 2, 3].iter() { + foo.push(i); + } +} + From 298412a6e87be2213bbfc5e6fada9795c405ea13 Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Tue, 3 Jun 2014 12:33:18 -0700 Subject: [PATCH 02/16] Improve error messages for io::fs --- src/libstd/io/fs.rs | 257 +++++++++++++++++++++++++++++++++---------- src/libstd/io/mod.rs | 51 +++++++-- 2 files changed, 244 insertions(+), 64 deletions(-) diff --git a/src/libstd/io/fs.rs b/src/libstd/io/fs.rs index c29c82ab2e9b4..1ac6fdc5ab122 100644 --- a/src/libstd/io/fs.rs +++ b/src/libstd/io/fs.rs @@ -56,9 +56,9 @@ use io::{FilePermission, Write, UnstableFileStat, Open, FileAccess, FileMode}; use io::{IoResult, IoError, FileStat, SeekStyle, Seek, Writer, Reader}; use io::{Read, Truncate, SeekCur, SeekSet, ReadWrite, SeekEnd, Append}; use io; +use io; use iter::Iterator; use kinds::Send; -use libc; use option::{Some, None, Option}; use owned::Box; use path::{Path, GenericPath}; @@ -138,7 +138,7 @@ impl File { Write => rtio::Write, ReadWrite => rtio::ReadWrite, }; - LocalIo::maybe_raise(|io| { + let err = LocalIo::maybe_raise(|io| { io.fs_open(&path.to_c_str(), mode, access).map(|fd| { File { path: path.clone(), @@ -146,7 +146,11 @@ impl File { last_nread: -1 } }) - }).map_err(IoError::from_rtio_error) + }).map_err(IoError::from_rtio_error); + err.update_err("couldn't open file", |e| { + format!("{}; path={}; mode={}; access={}", e, path.display(), + mode_string(mode), access_string(access)) + }) } /// Attempts to open a file in read-only mode. This function is equivalent to @@ -185,6 +189,7 @@ impl File { /// ``` pub fn create(path: &Path) -> IoResult { File::open_mode(path, Truncate, Write) + .update_desc("couldn't create file") } /// Returns the original path which was used to open this file. @@ -196,7 +201,9 @@ impl File { /// device. This will flush any internal buffers necessary to perform this /// operation. pub fn fsync(&mut self) -> IoResult<()> { - self.fd.fsync().map_err(IoError::from_rtio_error) + let err = self.fd.fsync().map_err(IoError::from_rtio_error); + err.update_err("couldn't fsync file", + |e| format!("{}; path={}", e, self.path.display())) } /// This function is similar to `fsync`, except that it may not synchronize @@ -204,7 +211,9 @@ impl File { /// must synchronize content, but don't need the metadata on disk. The goal /// of this method is to reduce disk operations. pub fn datasync(&mut self) -> IoResult<()> { - self.fd.datasync().map_err(IoError::from_rtio_error) + let err = self.fd.datasync().map_err(IoError::from_rtio_error); + err.update_err("couldn't datasync file", + |e| format!("{}; path={}", e, self.path.display())) } /// Either truncates or extends the underlying file, updating the size of @@ -216,7 +225,10 @@ impl File { /// will be extended to `size` and have all of the intermediate data filled /// in with 0s. pub fn truncate(&mut self, size: i64) -> IoResult<()> { - self.fd.truncate(size).map_err(IoError::from_rtio_error) + let err = self.fd.truncate(size).map_err(IoError::from_rtio_error); + err.update_err("couldn't truncate file", |e| { + format!("{}; path={}; size={}", e, self.path.display(), size) + }) } /// Tests whether this stream has reached EOF. @@ -229,10 +241,12 @@ impl File { /// Queries information about the underlying file. pub fn stat(&mut self) -> IoResult { - match self.fd.fstat() { + let err = match self.fd.fstat() { Ok(s) => Ok(from_rtio(s)), Err(e) => Err(IoError::from_rtio_error(e)), - } + }; + err.update_err("couldn't fstat file", + |e| format!("{}; path={}", e, self.path.display())) } } @@ -258,9 +272,11 @@ impl File { /// user lacks permissions to remove the file, or if some other filesystem-level /// error occurs. pub fn unlink(path: &Path) -> IoResult<()> { - LocalIo::maybe_raise(|io| { + let err = LocalIo::maybe_raise(|io| { io.fs_unlink(&path.to_c_str()) - }).map_err(IoError::from_rtio_error) + }).map_err(IoError::from_rtio_error); + err.update_err("couldn't unlink path", + |e| format!("{}; path={}", e, path.display())) } /// Given a path, query the file system to get information about a file, @@ -285,10 +301,12 @@ pub fn unlink(path: &Path) -> IoResult<()> { /// to perform a `stat` call on the given path or if there is no entry in the /// filesystem at the provided path. pub fn stat(path: &Path) -> IoResult { - match LocalIo::maybe_raise(|io| io.fs_stat(&path.to_c_str())) { + let err = match LocalIo::maybe_raise(|io| io.fs_stat(&path.to_c_str())) { Ok(s) => Ok(from_rtio(s)), Err(e) => Err(IoError::from_rtio_error(e)), - } + }; + err.update_err("couldn't stat path", + |e| format!("{}; path={}", e, path.display())) } /// Perform the same operation as the `stat` function, except that this @@ -300,10 +318,12 @@ pub fn stat(path: &Path) -> IoResult { /// /// See `stat` pub fn lstat(path: &Path) -> IoResult { - match LocalIo::maybe_raise(|io| io.fs_lstat(&path.to_c_str())) { + let err = match LocalIo::maybe_raise(|io| io.fs_lstat(&path.to_c_str())) { Ok(s) => Ok(from_rtio(s)), Err(e) => Err(IoError::from_rtio_error(e)), - } + }; + err.update_err("couldn't lstat path", + |e| format!("{}; path={}", e, path.display())) } fn from_rtio(s: rtio::FileStat) -> FileStat { @@ -359,9 +379,12 @@ fn from_rtio(s: rtio::FileStat) -> FileStat { /// permissions to view the contents, or if some other intermittent I/O error /// occurs. pub fn rename(from: &Path, to: &Path) -> IoResult<()> { - LocalIo::maybe_raise(|io| { + let err = LocalIo::maybe_raise(|io| { io.fs_rename(&from.to_c_str(), &to.to_c_str()) - }).map_err(IoError::from_rtio_error) + }).map_err(IoError::from_rtio_error); + err.update_err("couldn't rename path", |e| { + format!("{}; from={}; to={}", e, from.display(), to.display()) + }) } /// Copies the contents of one file to another. This function will also @@ -393,12 +416,17 @@ pub fn rename(from: &Path, to: &Path) -> IoResult<()> { /// ensured to not exist, there is nothing preventing the destination from /// being created and then destroyed by this operation. pub fn copy(from: &Path, to: &Path) -> IoResult<()> { + fn update_err(result: IoResult, from: &Path, to: &Path) -> IoResult { + result.update_err("couldn't copy path", + |e| format!("{}; from={}; to={}", e, from.display(), to.display())) + } + if !from.is_file() { - return Err(IoError { + return update_err(Err(IoError { kind: io::MismatchedFileTypeForOperation, desc: "the source path is not an existing file", - detail: None, - }) + detail: None + }), from, to) } let mut reader = try!(File::open(from)); @@ -409,12 +437,12 @@ pub fn copy(from: &Path, to: &Path) -> IoResult<()> { let amt = match reader.read(buf) { Ok(n) => n, Err(ref e) if e.kind == io::EndOfFile => { break } - Err(e) => return Err(e) + Err(e) => return update_err(Err(e), from, to) }; try!(writer.write(buf.slice_to(amt))); } - chmod(to, try!(from.stat()).perm) + chmod(to, try!(update_err(from.stat(), from, to)).perm) } /// Changes the permission mode bits found on a file or a directory. This @@ -439,33 +467,45 @@ pub fn copy(from: &Path, to: &Path) -> IoResult<()> { /// Some possible error situations are not having the permission to /// change the attributes of a file or the file not existing. pub fn chmod(path: &Path, mode: io::FilePermission) -> IoResult<()> { - LocalIo::maybe_raise(|io| { + let err = LocalIo::maybe_raise(|io| { io.fs_chmod(&path.to_c_str(), mode.bits() as uint) - }).map_err(IoError::from_rtio_error) + }).map_err(IoError::from_rtio_error); + err.update_err("couldn't chmod path", |e| { + format!("{}; path={}; mode={}", e, path.display(), mode) + }) } /// Change the user and group owners of a file at the specified path. pub fn chown(path: &Path, uid: int, gid: int) -> IoResult<()> { - LocalIo::maybe_raise(|io| { + let err = LocalIo::maybe_raise(|io| { io.fs_chown(&path.to_c_str(), uid, gid) - }).map_err(IoError::from_rtio_error) + }).map_err(IoError::from_rtio_error); + err.update_err("couldn't chown path", |e| { + format!("{}; path={}; uid={}; gid={}", e, path.display(), uid, gid) + }) } /// Creates a new hard link on the filesystem. The `dst` path will be a /// link pointing to the `src` path. Note that systems often require these /// two paths to both be located on the same filesystem. pub fn link(src: &Path, dst: &Path) -> IoResult<()> { - LocalIo::maybe_raise(|io| { + let err = LocalIo::maybe_raise(|io| { io.fs_link(&src.to_c_str(), &dst.to_c_str()) - }).map_err(IoError::from_rtio_error) + }).map_err(IoError::from_rtio_error); + err.update_err("couldn't link path", |e| { + format!("{}; src={}; dest={}", e, src.display(), dst.display()) + }) } /// Creates a new symbolic link on the filesystem. The `dst` path will be a /// symlink pointing to the `src` path. pub fn symlink(src: &Path, dst: &Path) -> IoResult<()> { - LocalIo::maybe_raise(|io| { + let err = LocalIo::maybe_raise(|io| { io.fs_symlink(&src.to_c_str(), &dst.to_c_str()) - }).map_err(IoError::from_rtio_error) + }).map_err(IoError::from_rtio_error); + err.update_err("couldn't symlink path", |e| { + format!("{}; src={}; dest={}", e, src.display(), dst.display()) + }) } /// Reads a symlink, returning the file that the symlink points to. @@ -475,9 +515,11 @@ pub fn symlink(src: &Path, dst: &Path) -> IoResult<()> { /// This function will return an error on failure. Failure conditions include /// reading a file that does not exist or reading a file which is not a symlink. pub fn readlink(path: &Path) -> IoResult { - LocalIo::maybe_raise(|io| { + let err = LocalIo::maybe_raise(|io| { Ok(Path::new(try!(io.fs_readlink(&path.to_c_str())))) - }).map_err(IoError::from_rtio_error) + }).map_err(IoError::from_rtio_error); + err.update_err("couldn't resolve symlink for path", + |e| format!("{}; path={}", e, path.display())) } /// Create a new, empty directory at the provided path @@ -498,9 +540,12 @@ pub fn readlink(path: &Path) -> IoResult { /// This call will return an error if the user lacks permissions to make a new /// directory at the provided path, or if the directory already exists. pub fn mkdir(path: &Path, mode: FilePermission) -> IoResult<()> { - LocalIo::maybe_raise(|io| { + let err = LocalIo::maybe_raise(|io| { io.fs_mkdir(&path.to_c_str(), mode.bits() as uint) - }).map_err(IoError::from_rtio_error) + }).map_err(IoError::from_rtio_error); + err.update_err("couldn't create directory", |e| { + format!("{}; path={}; mode={}", e, path.display(), mode) + }) } /// Remove an existing, empty directory @@ -520,9 +565,11 @@ pub fn mkdir(path: &Path, mode: FilePermission) -> IoResult<()> { /// This call will return an error if the user lacks permissions to remove the /// directory at the provided path, or if the directory isn't empty. pub fn rmdir(path: &Path) -> IoResult<()> { - LocalIo::maybe_raise(|io| { + let err = LocalIo::maybe_raise(|io| { io.fs_rmdir(&path.to_c_str()) - }).map_err(IoError::from_rtio_error) + }).map_err(IoError::from_rtio_error); + err.update_err("couldn't remove directory", + |e| format!("{}; path={}", e, path.display())) } /// Retrieve a vector containing all entries within a provided directory @@ -557,11 +604,13 @@ pub fn rmdir(path: &Path) -> IoResult<()> { /// permissions to view the contents or if the `path` points at a non-directory /// file pub fn readdir(path: &Path) -> IoResult> { - LocalIo::maybe_raise(|io| { + let err = LocalIo::maybe_raise(|io| { Ok(try!(io.fs_readdir(&path.to_c_str(), 0)).move_iter().map(|a| { Path::new(a) }).collect()) - }).map_err(IoError::from_rtio_error) + }).map_err(IoError::from_rtio_error); + err.update_err("couldn't read directory", + |e| format!("{}; path={}", e, path.display())) } /// Returns an iterator which will recursively walk the directory structure @@ -569,7 +618,11 @@ pub fn readdir(path: &Path) -> IoResult> { /// perform iteration in some top-down order. The contents of unreadable /// subdirectories are ignored. pub fn walk_dir(path: &Path) -> IoResult { - Ok(Directories { stack: try!(readdir(path)) }) + Ok(Directories { + stack: try!(readdir(path).update_err("couldn't walk directory", + |e| format!("{}; path={}", + e, path.display()))) + }) } /// An iterator which walks over a directory @@ -582,7 +635,12 @@ impl Iterator for Directories { match self.stack.pop() { Some(path) => { if path.is_dir() { - match readdir(&path) { + let result = readdir(&path) + .update_err("couldn't advance Directories iterator", + |e| format!("{}; path={}", + e, path.display())); + + match result { Ok(dirs) => { self.stack.push_all_move(dirs); } Err(..) => {} } @@ -614,7 +672,11 @@ pub fn mkdir_recursive(path: &Path, mode: FilePermission) -> IoResult<()> { for c in comps { curpath.push(c); - match mkdir(&curpath, mode) { + let result = mkdir(&curpath, mode) + .update_err("couldn't recursively mkdir", + |e| format!("{}; path={}", e, path.display())); + + match result { Err(mkdir_err) => { // already exists ? if try!(stat(&curpath)).kind != io::TypeDirectory { @@ -639,8 +701,20 @@ pub fn rmdir_recursive(path: &Path) -> IoResult<()> { let mut rm_stack = Vec::new(); rm_stack.push(path.clone()); + fn rmdir_failed(err: &IoError, path: &Path) -> String { + format!("rmdir_recursive failed; path={}; cause={}", + path.display(), err) + } + + fn update_err(err: IoResult, path: &Path) -> IoResult { + err.update_err("couldn't recursively rmdir", + |e| rmdir_failed(e, path)) + } + while !rm_stack.is_empty() { - let children = try!(readdir(rm_stack.last().unwrap())); + let children = try!(readdir(rm_stack.last().unwrap()) + .update_detail(|e| rmdir_failed(e, path))); + let mut has_child_dir = false; // delete all regular files in the way and push subdirs @@ -648,17 +722,17 @@ pub fn rmdir_recursive(path: &Path) -> IoResult<()> { for child in children.move_iter() { // FIXME(#12795) we should use lstat in all cases let child_type = match cfg!(windows) { - true => try!(stat(&child)).kind, - false => try!(lstat(&child)).kind + true => try!(update_err(stat(&child), path)), + false => try!(update_err(lstat(&child), path)) }; - if child_type == io::TypeDirectory { + if child_type.kind == io::TypeDirectory { rm_stack.push(child); has_child_dir = true; } else { // we can carry on safely if the file is already gone // (eg: deleted by someone else since readdir) - match unlink(&child) { + match update_err(unlink(&child), path) { Ok(()) => (), Err(ref e) if e.kind == io::FileNotFound => (), Err(e) => return Err(e) @@ -668,7 +742,8 @@ pub fn rmdir_recursive(path: &Path) -> IoResult<()> { // if no subdir was found, let's pop and delete if !has_child_dir { - match rmdir(&rm_stack.pop().unwrap()) { + let result = update_err(rmdir(&rm_stack.pop().unwrap()), path); + match result { Ok(()) => (), Err(ref e) if e.kind == io::FileNotFound => (), Err(e) => return Err(e) @@ -685,18 +760,28 @@ pub fn rmdir_recursive(path: &Path) -> IoResult<()> { /// be in milliseconds. // FIXME(#10301) these arguments should not be u64 pub fn change_file_times(path: &Path, atime: u64, mtime: u64) -> IoResult<()> { - LocalIo::maybe_raise(|io| { + let err = LocalIo::maybe_raise(|io| { io.fs_utime(&path.to_c_str(), atime, mtime) - }).map_err(IoError::from_rtio_error) + }).map_err(IoError::from_rtio_error); + err.update_err("couldn't change_file_times", + |e| format!("{}; path={}", e, path.display())) } impl Reader for File { fn read(&mut self, buf: &mut [u8]) -> IoResult { - match self.fd.read(buf) { + fn update_err(result: IoResult, file: &File) -> IoResult { + result.update_err("couldn't read file", + |e| format!("{}; path={}", + e, file.path.display())) + } + + let result: IoResult = update_err(self.fd.read(buf), self); + + match result { Ok(read) => { self.last_nread = read; match read { - 0 => Err(io::standard_error(io::EndOfFile)), + 0 => update_err(Err(standard_error(io::EndOfFile)), self), _ => Ok(read as uint) } }, @@ -707,13 +792,17 @@ impl Reader for File { impl Writer for File { fn write(&mut self, buf: &[u8]) -> IoResult<()> { - self.fd.write(buf).map_err(IoError::from_rtio_error) + let err = self.fd.write(buf).map_err(IoError::from_rtio_error) + err.update_err("couldn't write to file", + |e| format!("{}; path={}", e, self.path.display())) } } impl Seek for File { fn tell(&self) -> IoResult { - self.fd.tell().map_err(IoError::from_rtio_error) + let err = self.fd.tell().map_err(IoError::from_rtio_error); + err.update_err("couldn't retrieve file cursor (`tell`)", + |e| format!("{}; path={}", e, self.path.display())) } fn seek(&mut self, pos: i64, style: SeekStyle) -> IoResult<()> { @@ -722,14 +811,16 @@ impl Seek for File { SeekCur => rtio::SeekCur, SeekEnd => rtio::SeekEnd, }; - match self.fd.seek(pos, style) { + let err = match self.fd.seek(pos, style) { Ok(_) => { // successful seek resets EOF indicator self.last_nread = -1; Ok(()) } Err(e) => Err(IoError::from_rtio_error(e)), - } + }; + err.update_err("couldn't seek in file", + |e| format!("{}; path={}", e, self.path.display())) } } @@ -779,6 +870,22 @@ impl path::Path { } } +fn mode_string(mode: FileMode) -> &'static str { + match mode { + super::Open => "open", + super::Append => "append", + super::Truncate => "truncate" + } +} + +fn access_string(access: FileAccess) -> &'static str { + match access { + super::Read => "read", + super::Write => "write", + super::ReadWrite => "readwrite" + } +} + #[cfg(test)] #[allow(unused_imports)] mod test { @@ -801,6 +908,14 @@ mod test { } ) ) + macro_rules! error( ($e:expr, $s:expr) => ( + match $e { + Ok(val) => fail!("Should have been an error, was {:?}", val), + Err(ref err) => assert!(err.to_str().as_slice().contains($s.as_slice()), + format!("`{}` did not contain `{}`", err, $s)) + } + ) ) + struct TempDir(Path); impl TempDir { @@ -856,13 +971,21 @@ mod test { let tmpdir = tmpdir(); let filename = &tmpdir.join("file_that_does_not_exist.txt"); let result = File::open_mode(filename, Open, Read); - assert!(result.is_err()); + + error!(result, "couldn't open file"); + error!(result, "no such file or directory"); + error!(result, format!("path={}; mode=open; access=read", filename.display())); }) iotest!(fn file_test_iounlinking_invalid_path_should_raise_condition() { let tmpdir = tmpdir(); let filename = &tmpdir.join("file_another_file_that_does_not_exist.txt"); - assert!(unlink(filename).is_err()); + + let result = unlink(filename); + + error!(result, "couldn't unlink path"); + error!(result, "no such file or directory"); + error!(result, format!("path={}", filename.display())); }) iotest!(fn file_test_io_non_positional_read() { @@ -1091,6 +1214,22 @@ mod test { assert!(dir.is_dir()) }) + iotest!(fn recursive_mkdir_failure() { + let tmpdir = tmpdir(); + let dir = tmpdir.join("d1"); + let file = dir.join("f1"); + + check!(mkdir_recursive(&dir, io::UserRWX)); + check!(File::create(&file)); + + let result = mkdir_recursive(&file, io::UserRWX); + + error!(result, "couldn't recursively mkdir"); + error!(result, "couldn't create directory"); + error!(result, "mode=FilePermission { bits: 448 }"); + error!(result, format!("path={}", file.display())); + }) + iotest!(fn recursive_mkdir_slash() { check!(mkdir_recursive(&Path::new("/"), io::UserRWX)); }) @@ -1147,6 +1286,12 @@ mod test { iotest!(fn copy_file_does_not_exist() { let from = Path::new("test/nonexistent-bogus-path"); let to = Path::new("test/other-bogus-path"); + + error!(copy(&from, &to), + format!("couldn't copy path (the source path is not an \ + existing file; from={}; to={})", + from.display(), to.display())); + match copy(&from, &to) { Ok(..) => fail!(), Err(..) => { diff --git a/src/libstd/io/mod.rs b/src/libstd/io/mod.rs index a1e0fa889789f..a7f84899a622e 100644 --- a/src/libstd/io/mod.rs +++ b/src/libstd/io/mod.rs @@ -232,7 +232,7 @@ use owned::Box; use result::{Ok, Err, Result}; use rt::rtio; use slice::{Vector, MutableVector, ImmutableVector}; -use str::{StrSlice, StrAllocating}; +use str::{Str, StrSlice, StrAllocating}; use str; use string::String; use uint; @@ -309,6 +309,7 @@ impl IoError { /// struct is filled with an allocated string describing the error /// in more detail, retrieved from the operating system. pub fn from_errno(errno: uint, detail: bool) -> IoError { + #[cfg(windows)] fn get_err(errno: i32) -> (IoErrorKind, &'static str) { match errno { @@ -388,8 +389,8 @@ impl IoError { IoError { kind: kind, desc: desc, - detail: if detail { - Some(os::error_string(errno)) + detail: if detail && kind == OtherIoError { + Some(os::error_string(errno).as_slice().chars().map(|c| c.to_lowercase()).collect()) } else { None }, @@ -420,10 +421,13 @@ impl IoError { impl fmt::Show for IoError { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - try!(write!(fmt, "{}", self.desc)); - match self.detail { - Some(ref s) => write!(fmt, " ({})", *s), - None => Ok(()) + match *self { + IoError { kind: OtherIoError, desc: "unknown error", detail: Some(ref detail) } => + write!(fmt, "{}", detail), + IoError { detail: None, desc, .. } => + write!(fmt, "{}", desc), + IoError { detail: Some(ref detail), desc, .. } => + write!(fmt, "{} ({})", desc, detail) } } } @@ -484,6 +488,37 @@ pub enum IoErrorKind { NoProgress, } +/// A trait that lets you add a `detail` to an IoError easily +trait UpdateIoError { + /// Returns an IoError with updated description and detail + fn update_err(self, desc: &'static str, detail: |&IoError| -> String) -> Self; + + /// Returns an IoError with updated detail + fn update_detail(self, detail: |&IoError| -> String) -> Self; + + /// Returns an IoError with update description + fn update_desc(self, desc: &'static str) -> Self; +} + +impl UpdateIoError for IoResult { + fn update_err(self, desc: &'static str, detail: |&IoError| -> String) -> IoResult { + self.map_err(|mut e| { + let detail = detail(&e); + e.desc = desc; + e.detail = Some(detail); + e + }) + } + + fn update_detail(self, detail: |&IoError| -> String) -> IoResult { + self.map_err(|mut e| { e.detail = Some(detail(&e)); e }) + } + + fn update_desc(self, desc: &'static str) -> IoResult { + self.map_err(|mut e| { e.desc = desc; e }) + } +} + static NO_PROGRESS_LIMIT: uint = 1000; /// A trait for objects which are byte-oriented streams. Readers are defined by @@ -1577,7 +1612,7 @@ pub fn standard_error(kind: IoErrorKind) -> IoError { ConnectionAborted => "connection aborted", NotConnected => "not connected", BrokenPipe => "broken pipe", - PathAlreadyExists => "file exists", + PathAlreadyExists => "file already exists", PathDoesntExist => "no such file", MismatchedFileTypeForOperation => "mismatched file type", ResourceUnavailable => "resource unavailable", From 03ec8e5cc91b3b6b6ab98ef70aa63a0965c5f6c1 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 11 Jun 2014 10:24:04 -0700 Subject: [PATCH 03/16] std: Rebase better errors on master --- src/libstd/io/fs.rs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/libstd/io/fs.rs b/src/libstd/io/fs.rs index 1ac6fdc5ab122..008be8ffaaef3 100644 --- a/src/libstd/io/fs.rs +++ b/src/libstd/io/fs.rs @@ -52,13 +52,15 @@ fs::unlink(&path); use c_str::ToCStr; use clone::Clone; use collections::Collection; +use io::standard_error; use io::{FilePermission, Write, UnstableFileStat, Open, FileAccess, FileMode}; use io::{IoResult, IoError, FileStat, SeekStyle, Seek, Writer, Reader}; use io::{Read, Truncate, SeekCur, SeekSet, ReadWrite, SeekEnd, Append}; -use io; +use io::UpdateIoError; use io; use iter::Iterator; use kinds::Send; +use libc; use option::{Some, None, Option}; use owned::Box; use path::{Path, GenericPath}; @@ -67,6 +69,7 @@ use result::{Err, Ok}; use rt::rtio::LocalIo; use rt::rtio; use slice::ImmutableVector; +use string::String; use vec::Vec; /// Unconstrained file access type that exposes read and write operations @@ -128,18 +131,18 @@ impl File { pub fn open_mode(path: &Path, mode: FileMode, access: FileAccess) -> IoResult { - let mode = match mode { + let rtio_mode = match mode { Open => rtio::Open, Append => rtio::Append, Truncate => rtio::Truncate, }; - let access = match access { + let rtio_access = match access { Read => rtio::Read, Write => rtio::Write, ReadWrite => rtio::ReadWrite, }; let err = LocalIo::maybe_raise(|io| { - io.fs_open(&path.to_c_str(), mode, access).map(|fd| { + io.fs_open(&path.to_c_str(), rtio_mode, rtio_access).map(|fd| { File { path: path.clone(), fd: fd, @@ -775,7 +778,8 @@ impl Reader for File { e, file.path.display())) } - let result: IoResult = update_err(self.fd.read(buf), self); + let result = update_err(self.fd.read(buf) + .map_err(IoError::from_rtio_error), self); match result { Ok(read) => { @@ -785,14 +789,14 @@ impl Reader for File { _ => Ok(read as uint) } }, - Err(e) => Err(IoError::from_rtio_error(e)), + Err(e) => Err(e) } } } impl Writer for File { fn write(&mut self, buf: &[u8]) -> IoResult<()> { - let err = self.fd.write(buf).map_err(IoError::from_rtio_error) + let err = self.fd.write(buf).map_err(IoError::from_rtio_error); err.update_err("couldn't write to file", |e| format!("{}; path={}", e, self.path.display())) } From 9b9ef442337ee3b9a29449a0792ae2eeb0480d0c Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Wed, 11 Jun 2014 12:14:38 -0700 Subject: [PATCH 04/16] libsyntax: Allow `+` to separate trait bounds from objects. RFC #27. After a snapshot, the old syntax will be removed. This can break some code that looked like `foo as &Trait:Send`. Now you will need to write `foo as (&Trait+Send)`. Closes #12778. [breaking-change] --- src/librustc/middle/typeck/astconv.rs | 1 + .../middle/typeck/infer/error_reporting.rs | 1 + src/librustdoc/clean/mod.rs | 3 +- src/libsyntax/ast.rs | 2 + src/libsyntax/ast_util.rs | 1 + src/libsyntax/fold.rs | 1 + src/libsyntax/parse/parser.rs | 137 +++++++++++------- src/libsyntax/print/pprust.rs | 27 +++- src/libsyntax/visit.rs | 2 +- .../plugin_crate_outlive_expansion_phase.rs | 4 +- src/test/compile-fail/issue-7013.rs | 2 +- src/test/compile-fail/kindck-copy.rs | 8 +- src/test/compile-fail/kindck-send.rs | 10 +- .../compile-fail/trait-bounds-cant-coerce.rs | 6 +- src/test/compile-fail/trait-bounds-sugar.rs | 8 +- src/test/run-fail/fail-macro-any.rs | 2 +- src/test/run-pass/alignment-gep-tup-like-1.rs | 2 +- src/test/run-pass/as-precedence.rs | 18 +++ src/test/run-pass/capturing-logging.rs | 2 +- .../close-over-big-then-small-data.rs | 4 +- src/test/run-pass/trait-bounds-basic.rs | 10 +- src/test/run-pass/trait-bounds-in-arc.rs | 14 +- src/test/run-pass/trait-cast.rs | 6 +- src/test/run-pass/trait-contravariant-self.rs | 6 +- 24 files changed, 176 insertions(+), 101 deletions(-) create mode 100644 src/test/run-pass/as-precedence.rs diff --git a/src/librustc/middle/typeck/astconv.rs b/src/librustc/middle/typeck/astconv.rs index 83c5be238168a..f380004fb081e 100644 --- a/src/librustc/middle/typeck/astconv.rs +++ b/src/librustc/middle/typeck/astconv.rs @@ -724,6 +724,7 @@ pub fn ast_ty_to_ty( .collect(); ty::mk_tup(tcx, flds) } + ast::TyParen(ref typ) => ast_ty_to_ty(this, rscope, &**typ), ast::TyBareFn(ref bf) => { if bf.decl.variadic && bf.abi != abi::C { tcx.sess.span_err(ast_ty.span, diff --git a/src/librustc/middle/typeck/infer/error_reporting.rs b/src/librustc/middle/typeck/infer/error_reporting.rs index 2e02c798339ea..4d396ed4f6e5c 100644 --- a/src/librustc/middle/typeck/infer/error_reporting.rs +++ b/src/librustc/middle/typeck/infer/error_reporting.rs @@ -1140,6 +1140,7 @@ impl<'a> Rebuilder<'a> { } ast::TyTup(new_tys) } + ast::TyParen(ref typ) => ast::TyParen(build_to(*typ, to)), ref other => other.clone() }; box(GC) ast::Ty { id: from.id, node: new_node, span: from.span } diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index dd6cc978ae71c..7e68be09f1d56 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -60,7 +60,7 @@ impl, U> Clean> for VecPerParamSpace { } } -impl, U> Clean for Gc { +impl, U> Clean for Gc { fn clean(&self) -> U { (**self).clean() } @@ -1198,6 +1198,7 @@ impl Clean for ast::Ty { TyClosure(ref c, region) => Closure(box c.clean(), region.clean()), TyProc(ref c) => Proc(box c.clean()), TyBareFn(ref barefn) => BareFunction(box barefn.clean()), + TyParen(ref ty) => ty.clean(), TyBot => Bottom, ref x => fail!("Unimplemented type {:?}", x), } diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index 941078b158b87..b8e08dab722ea 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -784,6 +784,8 @@ pub enum Ty_ { TyUnboxedFn(Gc), TyTup(Vec> ), TyPath(Path, Option>, NodeId), // for #7264; see above + // No-op; kept solely so that we can pretty-print faithfully + TyParen(P), TyTypeof(Gc), // TyInfer means the type should be inferred instead of it having been // specified. This can appear anywhere in a type. diff --git a/src/libsyntax/ast_util.rs b/src/libsyntax/ast_util.rs index fcddbfa9a89b9..8e0a2218ea369 100644 --- a/src/libsyntax/ast_util.rs +++ b/src/libsyntax/ast_util.rs @@ -737,6 +737,7 @@ pub fn get_inner_tys(ty: P) -> Vec> { | ast::TyUniq(ty) | ast::TyFixedLengthVec(ty, _) => vec!(ty), ast::TyTup(ref tys) => tys.clone(), + ast::TyParen(ty) => get_inner_tys(ty), _ => Vec::new() } } diff --git a/src/libsyntax/fold.rs b/src/libsyntax/fold.rs index d8c7ffe4db7ac..d61a79e4e80be 100644 --- a/src/libsyntax/fold.rs +++ b/src/libsyntax/fold.rs @@ -192,6 +192,7 @@ pub trait Folder { }) } TyTup(ref tys) => TyTup(tys.iter().map(|&ty| self.fold_ty(ty)).collect()), + TyParen(ref ty) => TyParen(self.fold_ty(*ty)), TyPath(ref path, ref bounds, id) => { let id = self.new_id(id); TyPath(self.fold_path(path), diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index aaedb5709550d..e1a45e5643d25 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -51,7 +51,7 @@ use ast::{TokenTree, TraitMethod, TraitRef, TTDelim, TTSeq, TTTok}; use ast::{TTNonterminal, TupleVariantKind, Ty, Ty_, TyBot, TyBox}; use ast::{TypeField, TyFixedLengthVec, TyClosure, TyProc, TyBareFn}; use ast::{TyTypeof, TyInfer, TypeMethod}; -use ast::{TyNil, TyParam, TyParamBound, TyPath, TyPtr, TyRptr}; +use ast::{TyNil, TyParam, TyParamBound, TyParen, TyPath, TyPtr, TyRptr}; use ast::{TyTup, TyU32, TyUnboxedFn, TyUniq, TyVec, UnUniq}; use ast::{UnboxedFnTy, UnboxedFnTyParamBound, UnnamedField, UnsafeBlock}; use ast::{UnsafeFn, ViewItem, ViewItem_, ViewItemExternCrate, ViewItemUse}; @@ -105,7 +105,7 @@ pub enum PathParsingMode { /// the type parameters; e.g. `foo::bar::<'a>::Baz::` LifetimeAndTypesWithColons, /// A path with a lifetime and type parameters with bounds before the last - /// set of type parameters only; e.g. `foo::bar<'a>::Baz:X+Y` This + /// set of type parameters only; e.g. `foo::bar<'a>::Baz+X+Y` This /// form does not use extra double colons. LifetimeAndTypesAndBounds, } @@ -1015,7 +1015,14 @@ impl<'a> Parser<'a> { }; let (inputs, variadic) = self.parse_fn_args(false, false); - let (_, bounds) = self.parse_optional_ty_param_bounds(false); + let bounds = { + if self.eat(&token::COLON) { + let (_, bounds) = self.parse_ty_param_bounds(false); + Some(bounds) + } else { + None + } + }; let (ret_style, ret_ty) = self.parse_ret_ty(); let decl = P(FnDecl { inputs: inputs, @@ -1083,7 +1090,14 @@ impl<'a> Parser<'a> { (is_unboxed, inputs) }; - let (region, bounds) = self.parse_optional_ty_param_bounds(true); + let (region, bounds) = { + if self.eat(&token::COLON) { + let (region, bounds) = self.parse_ty_param_bounds(true); + (region, Some(bounds)) + } else { + (None, None) + } + }; let (return_style, output) = self.parse_ret_ty(); let decl = P(FnDecl { @@ -1227,7 +1241,7 @@ impl<'a> Parser<'a> { // parse a possibly mutable type pub fn parse_mt(&mut self) -> MutTy { let mutbl = self.parse_mutability(); - let t = self.parse_ty(false); + let t = self.parse_ty(true); MutTy { ty: t, mutbl: mutbl } } @@ -1238,7 +1252,7 @@ impl<'a> Parser<'a> { let mutbl = self.parse_mutability(); let id = self.parse_ident(); self.expect(&token::COLON); - let ty = self.parse_ty(false); + let ty = self.parse_ty(true); let hi = ty.span.hi; ast::TypeField { ident: id, @@ -1261,7 +1275,7 @@ impl<'a> Parser<'a> { }) ) } else { - (Return, self.parse_ty(false)) + (Return, self.parse_ty(true)) } } else { let pos = self.span.lo; @@ -1276,10 +1290,11 @@ impl<'a> Parser<'a> { } } - // parse a type. - // Useless second parameter for compatibility with quasiquote macros. - // Bleh! - pub fn parse_ty(&mut self, _: bool) -> P { + /// Parse a type. + /// + /// The second parameter specifies whether the `+` binary operator is + /// allowed in the type grammar. + pub fn parse_ty(&mut self, plus_allowed: bool) -> P { maybe_whole!(no_clone self, NtTy); let lo = self.span.lo; @@ -1293,12 +1308,12 @@ impl<'a> Parser<'a> { // (t) is a parenthesized ty // (t,) is the type of a tuple with only one field, // of type t - let mut ts = vec!(self.parse_ty(false)); + let mut ts = vec!(self.parse_ty(true)); let mut one_tuple = false; while self.token == token::COMMA { self.bump(); if self.token != token::RPAREN { - ts.push(self.parse_ty(false)); + ts.push(self.parse_ty(true)); } else { one_tuple = true; @@ -1307,17 +1322,17 @@ impl<'a> Parser<'a> { if ts.len() == 1 && !one_tuple { self.expect(&token::RPAREN); - return *ts.get(0) + TyParen(*ts.get(0)) + } else { + let t = TyTup(ts); + self.expect(&token::RPAREN); + t } - - let t = TyTup(ts); - self.expect(&token::RPAREN); - t } } else if self.token == token::AT { // MANAGED POINTER self.bump(); - TyBox(self.parse_ty(false)) + TyBox(self.parse_ty(plus_allowed)) } else if self.token == token::TILDE { // OWNED POINTER self.bump(); @@ -1326,7 +1341,7 @@ impl<'a> Parser<'a> { self.obsolete(self.last_span, ObsoleteOwnedVector), _ => self.obsolete(self.last_span, ObsoleteOwnedType), }; - TyUniq(self.parse_ty(false)) + TyUniq(self.parse_ty(true)) } else if self.token == token::BINOP(token::STAR) { // STAR POINTER (bare pointer?) self.bump(); @@ -1334,7 +1349,7 @@ impl<'a> Parser<'a> { } else if self.token == token::LBRACKET { // VECTOR self.expect(&token::LBRACKET); - let t = self.parse_ty(false); + let t = self.parse_ty(true); // Parse the `, ..e` in `[ int, ..e ]` // where `e` is a const expression @@ -1377,10 +1392,15 @@ impl<'a> Parser<'a> { } else if self.token == token::MOD_SEP || is_ident_or_path(&self.token) { // NAMED TYPE + let mode = if plus_allowed { + LifetimeAndTypesAndBounds + } else { + LifetimeAndTypesWithoutColons + }; let PathAndBounds { path, bounds - } = self.parse_path(LifetimeAndTypesAndBounds); + } = self.parse_path(mode); TyPath(path, bounds, ast::DUMMY_NODE_ID) } else if self.eat(&token::UNDERSCORE) { // TYPE TO BE INFERRED @@ -1438,7 +1458,7 @@ impl<'a> Parser<'a> { special_idents::invalid) }; - let t = self.parse_ty(false); + let t = self.parse_ty(true); Arg { ty: t, @@ -1456,7 +1476,7 @@ impl<'a> Parser<'a> { pub fn parse_fn_block_arg(&mut self) -> Arg { let pat = self.parse_pat(); let t = if self.eat(&token::COLON) { - self.parse_ty(false) + self.parse_ty(true) } else { P(Ty { id: ast::DUMMY_NODE_ID, @@ -1611,9 +1631,19 @@ impl<'a> Parser<'a> { } } - // Next, parse a colon and bounded type parameters, if applicable. + // Next, parse a plus and bounded type parameters, if applicable. + // + // NOTE(stage0, pcwalton): Remove `token::COLON` after a snapshot. let bounds = if mode == LifetimeAndTypesAndBounds { - let (_, bounds) = self.parse_optional_ty_param_bounds(false); + let bounds = { + if self.eat(&token::BINOP(token::PLUS)) || + self.eat(&token::COLON) { + let (_, bounds) = self.parse_ty_param_bounds(false); + Some(bounds) + } else { + None + } + }; bounds } else { None @@ -2438,7 +2468,7 @@ impl<'a> Parser<'a> { } None => { if as_prec > min_prec && self.eat_keyword(keywords::As) { - let rhs = self.parse_ty(true); + let rhs = self.parse_ty(false); let _as = self.mk_expr(lhs.span.lo, rhs.span.hi, ExprCast(lhs, rhs)); @@ -3067,7 +3097,9 @@ impl<'a> Parser<'a> { node: TyInfer, span: mk_sp(lo, lo), }); - if self.eat(&token::COLON) { ty = self.parse_ty(false); } + if self.eat(&token::COLON) { + ty = self.parse_ty(true); + } let init = self.parse_initializer(); box(GC) ast::Local { ty: ty, @@ -3095,7 +3127,7 @@ impl<'a> Parser<'a> { } let name = self.parse_ident(); self.expect(&token::COLON); - let ty = self.parse_ty(false); + let ty = self.parse_ty(true); spanned(lo, self.last_span.hi, ast::StructField_ { kind: NamedField(name, pr), id: ast::DUMMY_NODE_ID, @@ -3427,7 +3459,7 @@ impl<'a> Parser<'a> { } } - // matches optbounds = ( ( : ( boundseq )? )? ) + // matches bounds = ( boundseq )? // where boundseq = ( bound + boundseq ) | bound // and bound = 'static | ty // Returns "None" if there's no colon (e.g. "T"); @@ -3439,13 +3471,9 @@ impl<'a> Parser<'a> { // AST doesn't support arbitrary lifetimes in bounds on type parameters. In // the future, this flag should be removed, and the return value of this // function should be Option<~[TyParamBound]> - fn parse_optional_ty_param_bounds(&mut self, allow_any_lifetime: bool) - -> (Option, Option>) - { - if !self.eat(&token::COLON) { - return (None, None); - } - + fn parse_ty_param_bounds(&mut self, allow_any_lifetime: bool) + -> (Option, + OwnedSlice) { let mut ret_lifetime = None; let mut result = vec!(); loop { @@ -3489,7 +3517,7 @@ impl<'a> Parser<'a> { } } - return (ret_lifetime, Some(OwnedSlice::from_vec(result))); + return (ret_lifetime, OwnedSlice::from_vec(result)); } // matches typaram = type? IDENT optbounds ( EQ ty )? @@ -3497,13 +3525,20 @@ impl<'a> Parser<'a> { let sized = self.parse_sized(); let span = self.span; let ident = self.parse_ident(); - let (_, opt_bounds) = self.parse_optional_ty_param_bounds(false); + let opt_bounds = { + if self.eat(&token::COLON) { + let (_, bounds) = self.parse_ty_param_bounds(false); + Some(bounds) + } else { + None + } + }; // For typarams we don't care about the difference b/w "" and "". let bounds = opt_bounds.unwrap_or_default(); let default = if self.token == token::EQ { self.bump(); - Some(self.parse_ty(false)) + Some(self.parse_ty(true)) } else { None }; @@ -3548,7 +3583,7 @@ impl<'a> Parser<'a> { Some(token::COMMA), |p| { p.forbid_lifetime(); - p.parse_ty(false) + p.parse_ty(true) } ); (lifetimes, result.into_vec()) @@ -3804,7 +3839,7 @@ impl<'a> Parser<'a> { } }; let output = if self.eat(&token::RARROW) { - self.parse_ty(false) + self.parse_ty(true) } else { P(Ty { id: ast::DUMMY_NODE_ID, @@ -3830,7 +3865,7 @@ impl<'a> Parser<'a> { |p| p.parse_fn_block_arg()); let output = if self.eat(&token::RARROW) { - self.parse_ty(false) + self.parse_ty(true) } else { P(Ty { id: ast::DUMMY_NODE_ID, @@ -3942,7 +3977,7 @@ impl<'a> Parser<'a> { let could_be_trait = self.token != token::LPAREN; // Parse the trait. - let mut ty = self.parse_ty(false); + let mut ty = self.parse_ty(true); // Parse traits, if necessary. let opt_trait = if could_be_trait && self.eat_keyword(keywords::For) { @@ -3965,7 +4000,7 @@ impl<'a> Parser<'a> { } }; - ty = self.parse_ty(false); + ty = self.parse_ty(true); opt_trait_ref } else { None @@ -4008,7 +4043,7 @@ impl<'a> Parser<'a> { let generics = self.parse_generics(); let super_struct = if self.eat(&token::COLON) { - let ty = self.parse_ty(false); + let ty = self.parse_ty(true); match ty.node { TyPath(_, None, _) => { Some(ty) @@ -4051,7 +4086,7 @@ impl<'a> Parser<'a> { let struct_field_ = ast::StructField_ { kind: UnnamedField(p.parse_visibility()), id: ast::DUMMY_NODE_ID, - ty: p.parse_ty(false), + ty: p.parse_ty(true), attrs: attrs, }; spanned(lo, p.span.hi, struct_field_) @@ -4205,7 +4240,7 @@ impl<'a> Parser<'a> { let m = if self.eat_keyword(keywords::Mut) {MutMutable} else {MutImmutable}; let id = self.parse_ident(); self.expect(&token::COLON); - let ty = self.parse_ty(false); + let ty = self.parse_ty(true); self.expect(&token::EQ); let e = self.parse_expr(); self.commit_expr_expecting(e, token::SEMI); @@ -4386,7 +4421,7 @@ impl<'a> Parser<'a> { let ident = self.parse_ident(); self.expect(&token::COLON); - let ty = self.parse_ty(false); + let ty = self.parse_ty(true); let hi = self.span.hi; self.expect(&token::SEMI); box(GC) ast::ForeignItem { @@ -4514,7 +4549,7 @@ impl<'a> Parser<'a> { let ident = self.parse_ident(); let tps = self.parse_generics(); self.expect(&token::EQ); - let ty = self.parse_ty(false); + let ty = self.parse_ty(true); self.expect(&token::SEMI); (ident, ItemTy(ty, tps), None) } @@ -4562,7 +4597,7 @@ impl<'a> Parser<'a> { &token::LPAREN, &token::RPAREN, seq_sep_trailing_disallowed(token::COMMA), - |p| p.parse_ty(false) + |p| p.parse_ty(true) ); for ty in arg_tys.move_iter() { args.push(ast::VariantArg { diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs index 77bc967b92d07..63acdb1a6ca75 100644 --- a/src/libsyntax/print/pprust.rs +++ b/src/libsyntax/print/pprust.rs @@ -495,6 +495,11 @@ impl<'a> State<'a> { } try!(self.pclose()); } + ast::TyParen(ref typ) => { + try!(self.popen()); + try!(self.print_type(&**typ)); + try!(self.pclose()); + } ast::TyBareFn(f) => { let generics = ast::Generics { lifetimes: f.lifetimes.clone(), @@ -1673,7 +1678,7 @@ impl<'a> State<'a> { match *opt_bounds { None => Ok(()), - Some(ref bounds) => self.print_bounds(&None, bounds, true), + Some(ref bounds) => self.print_bounds(&None, bounds, true, true), } } @@ -1932,9 +1937,16 @@ impl<'a> State<'a> { pub fn print_bounds(&mut self, region: &Option, bounds: &OwnedSlice, - print_colon_anyway: bool) -> IoResult<()> { + print_colon_anyway: bool, + print_plus_before_bounds: bool) + -> IoResult<()> { + let separator = if print_plus_before_bounds { + "+" + } else { + ":" + }; if !bounds.is_empty() || region.is_some() { - try!(word(&mut self.s, ":")); + try!(word(&mut self.s, separator)); let mut first = true; match *region { Some(ref lt) => { @@ -1976,7 +1988,7 @@ impl<'a> State<'a> { } Ok(()) } else if print_colon_anyway { - word(&mut self.s, ":") + word(&mut self.s, separator) } else { Ok(()) } @@ -2011,7 +2023,10 @@ impl<'a> State<'a> { try!(s.word_space("type")); } try!(s.print_ident(param.ident)); - try!(s.print_bounds(&None, ¶m.bounds, false)); + try!(s.print_bounds(&None, + ¶m.bounds, + false, + false)); match param.default { Some(ref default) => { try!(space(&mut s.s)); @@ -2214,7 +2229,7 @@ impl<'a> State<'a> { } opt_bounds.as_ref().map(|bounds| { - self.print_bounds(opt_region, bounds, true) + self.print_bounds(opt_region, bounds, true, false) }); try!(self.maybe_print_comment(decl.output.span.lo)); diff --git a/src/libsyntax/visit.rs b/src/libsyntax/visit.rs index 59771a57dfa50..6f0fc217533fc 100644 --- a/src/libsyntax/visit.rs +++ b/src/libsyntax/visit.rs @@ -336,7 +336,7 @@ pub fn skip_ty>(_: &mut V, _: &Ty, _: E) { pub fn walk_ty>(visitor: &mut V, typ: &Ty, env: E) { match typ.node { - TyUniq(ty) | TyVec(ty) | TyBox(ty) => { + TyUniq(ty) | TyVec(ty) | TyBox(ty) | TyParen(ty) => { visitor.visit_ty(&*ty, env) } TyPtr(ref mutable_type) => { diff --git a/src/test/auxiliary/plugin_crate_outlive_expansion_phase.rs b/src/test/auxiliary/plugin_crate_outlive_expansion_phase.rs index 213fdd6b74a29..1592ffb6c6790 100644 --- a/src/test/auxiliary/plugin_crate_outlive_expansion_phase.rs +++ b/src/test/auxiliary/plugin_crate_outlive_expansion_phase.rs @@ -27,7 +27,7 @@ impl Drop for Foo { #[plugin_registrar] pub fn registrar(_: &mut Registry) { - local_data_key!(foo: Box); - foo.replace(Some(box Foo { foo: 10 } as Box)); + local_data_key!(foo: Box); + foo.replace(Some(box Foo { foo: 10 } as Box)); } diff --git a/src/test/compile-fail/issue-7013.rs b/src/test/compile-fail/issue-7013.rs index 1bc4e07655323..3028db00f5871 100644 --- a/src/test/compile-fail/issue-7013.rs +++ b/src/test/compile-fail/issue-7013.rs @@ -31,7 +31,7 @@ struct A { } fn main() { - let a = A {v: box B{v: None} as Box}; + let a = A {v: box B{v: None} as Box}; //~^ ERROR cannot pack type `~B`, which does not fulfill `Send` let v = Rc::new(RefCell::new(a)); let w = v.clone(); diff --git a/src/test/compile-fail/kindck-copy.rs b/src/test/compile-fail/kindck-copy.rs index 5404b32eb2421..651ea6abf08cc 100644 --- a/src/test/compile-fail/kindck-copy.rs +++ b/src/test/compile-fail/kindck-copy.rs @@ -45,15 +45,15 @@ fn test<'a,T,U:Copy>(_: &'a int) { // borrowed object types are generally ok assert_copy::<&'a Dummy>(); - assert_copy::<&'a Dummy:Copy>(); - assert_copy::<&'static Dummy:Copy>(); + assert_copy::<&'a Dummy+Copy>(); + assert_copy::<&'static Dummy+Copy>(); // owned object types are not ok assert_copy::>(); //~ ERROR does not fulfill - assert_copy::>(); //~ ERROR does not fulfill + assert_copy::>(); //~ ERROR does not fulfill // mutable object types are not ok - assert_copy::<&'a mut Dummy:Copy>(); //~ ERROR does not fulfill + assert_copy::<&'a mut Dummy+Copy>(); //~ ERROR does not fulfill // closures are like an `&mut` object assert_copy::<||>(); //~ ERROR does not fulfill diff --git a/src/test/compile-fail/kindck-send.rs b/src/test/compile-fail/kindck-send.rs index aa9d2dda22a96..0414e64f1b7bf 100644 --- a/src/test/compile-fail/kindck-send.rs +++ b/src/test/compile-fail/kindck-send.rs @@ -39,17 +39,17 @@ fn test<'a,T,U:Send>(_: &'a int) { // careful with object types, who knows what they close over... assert_send::<&'static Dummy>(); //~ ERROR does not fulfill `Send` assert_send::<&'a Dummy>(); //~ ERROR does not fulfill `Send` - assert_send::<&'a Dummy:Send>(); //~ ERROR does not fulfill `Send` - assert_send::>(); //~ ERROR does not fulfill `Send` + assert_send::<&'a Dummy+Send>(); //~ ERROR does not fulfill `Send` + assert_send::>(); //~ ERROR does not fulfill `Send` // ...unless they are properly bounded - assert_send::<&'static Dummy:Send>(); - assert_send::>(); + assert_send::<&'static Dummy+Send>(); + assert_send::>(); // but closure and object types can have lifetime bounds which make // them not ok (FIXME #5121) // assert_send::(); // ERROR does not fulfill `Send` - // assert_send::>(); // ERROR does not fulfill `Send` + // assert_send::>(); // ERROR does not fulfill `Send` // unsafe ptrs are ok unless they point at unsendable things assert_send::<*int>(); diff --git a/src/test/compile-fail/trait-bounds-cant-coerce.rs b/src/test/compile-fail/trait-bounds-cant-coerce.rs index 12205ef062da0..3737025da6c12 100644 --- a/src/test/compile-fail/trait-bounds-cant-coerce.rs +++ b/src/test/compile-fail/trait-bounds-cant-coerce.rs @@ -12,14 +12,14 @@ trait Foo { } -fn a(_x: Box) { +fn a(_x: Box) { } -fn c(x: Box) { +fn c(x: Box) { a(x); } -fn d(x: Box) { +fn d(x: Box) { a(x); //~ ERROR found no bounds } diff --git a/src/test/compile-fail/trait-bounds-sugar.rs b/src/test/compile-fail/trait-bounds-sugar.rs index 9447030a7f461..d548098ebe13e 100644 --- a/src/test/compile-fail/trait-bounds-sugar.rs +++ b/src/test/compile-fail/trait-bounds-sugar.rs @@ -13,17 +13,17 @@ trait Foo {} -fn a(_x: Box) { +fn a(_x: Box) { } -fn b(_x: &'static Foo) { // should be same as &'static Foo:'static +fn b(_x: &'static Foo) { // should be same as &'static Foo+'static } -fn c(x: Box) { +fn c(x: Box) { a(x); //~ ERROR expected bounds `Send` } -fn d(x: &'static Foo:Share) { +fn d(x: &'static Foo+Share) { b(x); //~ ERROR expected bounds `'static` } diff --git a/src/test/run-fail/fail-macro-any.rs b/src/test/run-fail/fail-macro-any.rs index 09b507433080e..c56eabbb3b4aa 100644 --- a/src/test/run-fail/fail-macro-any.rs +++ b/src/test/run-fail/fail-macro-any.rs @@ -12,5 +12,5 @@ fn main() { - fail!(box 413 as Box<::std::any::Any:Send>); + fail!(box 413 as Box<::std::any::Any+Send>); } diff --git a/src/test/run-pass/alignment-gep-tup-like-1.rs b/src/test/run-pass/alignment-gep-tup-like-1.rs index 38a3d3c5fffdc..4fa695de5228d 100644 --- a/src/test/run-pass/alignment-gep-tup-like-1.rs +++ b/src/test/run-pass/alignment-gep-tup-like-1.rs @@ -33,7 +33,7 @@ fn f(a: A, b: u16) -> Box:> { box Invoker { a: a, b: b, - } as Box>: + } as (Box>+) } pub fn main() { diff --git a/src/test/run-pass/as-precedence.rs b/src/test/run-pass/as-precedence.rs new file mode 100644 index 0000000000000..0760f13200c8f --- /dev/null +++ b/src/test/run-pass/as-precedence.rs @@ -0,0 +1,18 @@ +// Copyright 2012 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +fn main() { + assert_eq!(3 as uint * 3, 9); + assert_eq!(3 as (uint) * 3, 9); + assert_eq!(3 as (uint) / 3, 1); + assert_eq!(3 as uint + 3, 6); + assert_eq!(3 as (uint) + 3, 6); +} + diff --git a/src/test/run-pass/capturing-logging.rs b/src/test/run-pass/capturing-logging.rs index 3402db7c355a9..23607e16795ed 100644 --- a/src/test/run-pass/capturing-logging.rs +++ b/src/test/run-pass/capturing-logging.rs @@ -41,7 +41,7 @@ fn main() { let (tx, rx) = channel(); let (mut r, w) = (ChanReader::new(rx), ChanWriter::new(tx)); spawn(proc() { - set_logger(box MyWriter(w) as Box); + set_logger(box MyWriter(w) as Box); debug!("debug"); info!("info"); }); diff --git a/src/test/run-pass/close-over-big-then-small-data.rs b/src/test/run-pass/close-over-big-then-small-data.rs index acae61359370b..3d642be082c0a 100644 --- a/src/test/run-pass/close-over-big-then-small-data.rs +++ b/src/test/run-pass/close-over-big-then-small-data.rs @@ -33,11 +33,11 @@ impl Invokable for Invoker { } } -fn f(a: A, b: u16) -> Box:> { +fn f(a: A, b: u16) -> Box+> { box Invoker { a: a, b: b, - } as Box>: + } as (Box>+) } pub fn main() { diff --git a/src/test/run-pass/trait-bounds-basic.rs b/src/test/run-pass/trait-bounds-basic.rs index 68ebf2c6382ca..d1bb0db511b8f 100644 --- a/src/test/run-pass/trait-bounds-basic.rs +++ b/src/test/run-pass/trait-bounds-basic.rs @@ -12,21 +12,21 @@ trait Foo { } -fn a(_x: Box) { +fn a(_x: Box) { } -fn b(_x: Box) { +fn b(_x: Box) { } -fn c(x: Box) { +fn c(x: Box) { a(x); } -fn d(x: Box) { +fn d(x: Box) { b(x); } -fn e(x: Box) { // sugar for Box +fn e(x: Box) { // sugar for Box a(x); } diff --git a/src/test/run-pass/trait-bounds-in-arc.rs b/src/test/run-pass/trait-bounds-in-arc.rs index 4ca67811982d1..18a0e5d471c4a 100644 --- a/src/test/run-pass/trait-bounds-in-arc.rs +++ b/src/test/run-pass/trait-bounds-in-arc.rs @@ -71,10 +71,10 @@ pub fn main() { swim_speed: 998, name: "alec_guinness".to_string(), }; - let arc = Arc::new(vec!(box catte as Box, - box dogge1 as Box, - box fishe as Box, - box dogge2 as Box)); + let arc = Arc::new(vec!(box catte as Box, + box dogge1 as Box, + box fishe as Box, + box dogge2 as Box)); let (tx1, rx1) = channel(); let arc1 = arc.clone(); task::spawn(proc() { check_legs(arc1); tx1.send(()); }); @@ -89,21 +89,21 @@ pub fn main() { rx3.recv(); } -fn check_legs(arc: Arc>>) { +fn check_legs(arc: Arc>>) { let mut legs = 0; for pet in arc.iter() { legs += pet.num_legs(); } assert!(legs == 12); } -fn check_names(arc: Arc>>) { +fn check_names(arc: Arc>>) { for pet in arc.iter() { pet.name(|name| { assert!(name[0] == 'a' as u8 && name[1] == 'l' as u8); }) } } -fn check_pedigree(arc: Arc>>) { +fn check_pedigree(arc: Arc>>) { for pet in arc.iter() { assert!(pet.of_good_pedigree()); } diff --git a/src/test/run-pass/trait-cast.rs b/src/test/run-pass/trait-cast.rs index a9067ef87ba31..0f99998b7a6ed 100644 --- a/src/test/run-pass/trait-cast.rs +++ b/src/test/run-pass/trait-cast.rs @@ -20,7 +20,7 @@ struct Tree(@RefCell); struct TreeR { left: Option, right: Option, - val: Box + val: Box } trait to_str { @@ -57,10 +57,10 @@ fn foo(x: T) -> String { x.to_str_() } pub fn main() { let t1 = Tree(@RefCell::new(TreeR{left: None, right: None, - val: box 1 as Box})); + val: box 1 as Box})); let t2 = Tree(@RefCell::new(TreeR{left: Some(t1), right: Some(t1), - val: box 2 as Box})); + val: box 2 as Box})); let expected = "[2, some([1, none, none]), some([1, none, none])]".to_string(); assert!(t2.to_str_() == expected); diff --git a/src/test/run-pass/trait-contravariant-self.rs b/src/test/run-pass/trait-contravariant-self.rs index 1576c646286a4..d8df5d5600c90 100644 --- a/src/test/run-pass/trait-contravariant-self.rs +++ b/src/test/run-pass/trait-contravariant-self.rs @@ -10,8 +10,8 @@ // This is an interesting test case. We have a trait (Bar) that is // implemented for a `Box` object (note: no bounds). And then we -// have a `Box` object. The impl for `Box` is applicable -// to `Box` because: +// have a `Box` object. The impl for `Box` is applicable +// to `Box` because: // // 1. The trait Bar is contravariant w/r/t Self because `Self` appears // only in argument position. @@ -30,7 +30,7 @@ impl Bar for Box { fn dummy(&self) { } } fn wants_bar(b: &B) { } fn main() { - let x: Box = (box SFoo); + let x: Box = (box SFoo); wants_bar(&x); } From 49fe69047776651114e014a8307c684fcb866efd Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 11 Jun 2014 18:54:14 -0700 Subject: [PATCH 05/16] configure: Don't sync unused submodules If the compiler is built with --{llvm,jemalloc,libuv}-root, then the configure script can skip updating these submodules. Closes #14822 --- configure | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/configure b/configure index fe29e4883166b..e5106f93d3587 100755 --- a/configure +++ b/configure @@ -942,8 +942,25 @@ then msg "git: submodule sync" "${CFG_GIT}" submodule sync + msg "git: submodule init" + "${CFG_GIT}" submodule init + + # Disable submodules that we're not using + if [ ! -z "${CFG_LLVM_ROOT}" ]; then + msg "git: submodule deinit src/llvm" + "${CFG_GIT}" submodule deinit src/llvm + fi + if [ ! -z "${CFG_JEMALLOC_ROOT}" ]; then + msg "git: submodule deinit src/jemalloc" + "${CFG_GIT}" submodule deinit src/jemalloc + fi + if [ ! -z "${CFG_LIBUV_ROOT}" ]; then + msg "git: submodule deinit src/libuv" + "${CFG_GIT}" submodule deinit src/libuv + fi + msg "git: submodule update" - "${CFG_GIT}" submodule update --init + "${CFG_GIT}" submodule update need_ok "git failed" msg "git: submodule foreach sync" @@ -951,7 +968,7 @@ then need_ok "git failed" msg "git: submodule foreach update" - "${CFG_GIT}" submodule update --init --recursive + "${CFG_GIT}" submodule update --recursive need_ok "git failed" # NB: this is just for the sake of getting the submodule SHA1 values From c17af5d7fdd54f727e8cc208a7643082c2f5fe93 Mon Sep 17 00:00:00 2001 From: Renato Riccieri Santos Zannon Date: Thu, 12 Jun 2014 03:36:46 -0300 Subject: [PATCH 06/16] Remove typo on collections::treemap::UnionItems The docstring stated it was a intersection iterator, when in fact it is the union iterator --- src/libcollections/treemap.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcollections/treemap.rs b/src/libcollections/treemap.rs index b59caa9375e90..086b073a9c44b 100644 --- a/src/libcollections/treemap.rs +++ b/src/libcollections/treemap.rs @@ -761,7 +761,7 @@ pub struct IntersectionItems<'a, T> { b: Peekable<&'a T, SetItems<'a, T>>, } -/// Lazy iterator producing elements in the set intersection (in-order) +/// Lazy iterator producing elements in the set union (in-order) pub struct UnionItems<'a, T> { a: Peekable<&'a T, SetItems<'a, T>>, b: Peekable<&'a T, SetItems<'a, T>>, From 42d538e6157cc4f923b4c2b8a3382d6465c90447 Mon Sep 17 00:00:00 2001 From: Jakub Wieczorek Date: Thu, 12 Jun 2014 08:38:30 +0200 Subject: [PATCH 07/16] Fix the unused struct field lint for struct variants Fixes #14837. --- src/librustc/middle/dead.rs | 60 +++++++++++++++++++++----------- src/test/run-pass/issue-14837.rs | 20 +++++++++++ 2 files changed, 59 insertions(+), 21 deletions(-) create mode 100644 src/test/run-pass/issue-14837.rs diff --git a/src/librustc/middle/dead.rs b/src/librustc/middle/dead.rs index 834dc43ee6ed5..aa74614b78cd8 100644 --- a/src/librustc/middle/dead.rs +++ b/src/librustc/middle/dead.rs @@ -55,6 +55,11 @@ struct MarkSymbolVisitor<'a> { live_symbols: Box>, } +#[deriving(Clone)] +struct MarkSymbolVisitorContext { + struct_has_extern_repr: bool +} + impl<'a> MarkSymbolVisitor<'a> { fn new(tcx: &'a ty::ctxt, worklist: Vec) -> MarkSymbolVisitor<'a> { @@ -170,48 +175,61 @@ impl<'a> MarkSymbolVisitor<'a> { } fn visit_node(&mut self, node: &ast_map::Node) { + let ctxt = MarkSymbolVisitorContext { + struct_has_extern_repr: false + }; match *node { ast_map::NodeItem(item) => { match item.node { - ast::ItemStruct(struct_def, _) => { + ast::ItemStruct(..) => { let has_extern_repr = item.attrs.iter().fold(attr::ReprAny, |acc, attr| { attr::find_repr_attr(self.tcx.sess.diagnostic(), attr, acc) }) == attr::ReprExtern; - let live_fields = struct_def.fields.iter().filter(|f| { - has_extern_repr || match f.node.kind { - ast::NamedField(_, ast::Public) => true, - _ => false - } + + visit::walk_item(self, &*item, MarkSymbolVisitorContext { + struct_has_extern_repr: has_extern_repr, + ..(ctxt) }); - self.live_symbols.extend(live_fields.map(|f| f.node.id)); - visit::walk_item(self, &*item, ()); } ast::ItemFn(..) - | ast::ItemTy(..) | ast::ItemEnum(..) + | ast::ItemTy(..) | ast::ItemStatic(..) => { - visit::walk_item(self, &*item, ()); + visit::walk_item(self, &*item, ctxt); } _ => () } } ast_map::NodeTraitMethod(trait_method) => { - visit::walk_trait_method(self, &*trait_method, ()); + visit::walk_trait_method(self, &*trait_method, ctxt); } ast_map::NodeMethod(method) => { - visit::walk_block(self, &*method.body, ()); + visit::walk_block(self, &*method.body, ctxt); } ast_map::NodeForeignItem(foreign_item) => { - visit::walk_foreign_item(self, &*foreign_item, ()); + visit::walk_foreign_item(self, &*foreign_item, ctxt); } _ => () } } } -impl<'a> Visitor<()> for MarkSymbolVisitor<'a> { +impl<'a> Visitor for MarkSymbolVisitor<'a> { + + fn visit_struct_def(&mut self, def: &ast::StructDef, _: ast::Ident, _: &ast::Generics, + _: ast::NodeId, ctxt: MarkSymbolVisitorContext) { + let live_fields = def.fields.iter().filter(|f| { + ctxt.struct_has_extern_repr || match f.node.kind { + ast::NamedField(_, ast::Public) => true, + _ => false + } + }); + self.live_symbols.extend(live_fields.map(|f| f.node.id)); + + visit::walk_struct_def(self, def, ctxt); + } - fn visit_expr(&mut self, expr: &ast::Expr, _: ()) { + fn visit_expr(&mut self, expr: &ast::Expr, ctxt: MarkSymbolVisitorContext) { match expr.node { ast::ExprMethodCall(..) => { self.lookup_and_handle_method(expr.id, expr.span); @@ -222,10 +240,10 @@ impl<'a> Visitor<()> for MarkSymbolVisitor<'a> { _ => () } - visit::walk_expr(self, expr, ()) + visit::walk_expr(self, expr, ctxt); } - fn visit_pat(&mut self, pat: &ast::Pat, _: ()) { + fn visit_pat(&mut self, pat: &ast::Pat, ctxt: MarkSymbolVisitorContext) { match pat.node { ast::PatStruct(_, ref fields, _) => { self.handle_field_pattern_match(pat, fields.as_slice()); @@ -233,15 +251,15 @@ impl<'a> Visitor<()> for MarkSymbolVisitor<'a> { _ => () } - visit::walk_pat(self, pat, ()) + visit::walk_pat(self, pat, ctxt); } - fn visit_path(&mut self, path: &ast::Path, id: ast::NodeId, _: ()) { + fn visit_path(&mut self, path: &ast::Path, id: ast::NodeId, ctxt: MarkSymbolVisitorContext) { self.lookup_and_handle_definition(&id); - visit::walk_path(self, path, ()); + visit::walk_path(self, path, ctxt); } - fn visit_item(&mut self, _: &ast::Item, _: ()) { + fn visit_item(&mut self, _: &ast::Item, _: MarkSymbolVisitorContext) { // Do not recurse into items. These items will be added to the // worklist and recursed into manually if necessary. } diff --git a/src/test/run-pass/issue-14837.rs b/src/test/run-pass/issue-14837.rs new file mode 100644 index 0000000000000..c207980cd6e34 --- /dev/null +++ b/src/test/run-pass/issue-14837.rs @@ -0,0 +1,20 @@ +// Copyright 2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(struct_variant)] +#![deny(warnings)] + +pub enum Foo { + Bar { + pub baz: int + } +} + +fn main() { } From 00e1a6923773500f0d5b9bad64d05eb5d4a24c53 Mon Sep 17 00:00:00 2001 From: P1start Date: Thu, 12 Jun 2014 19:54:03 +1200 Subject: [PATCH 08/16] Clarify `Any` docs The `Any` docs previously did not state that only `'static` types implement it. --- src/libcore/any.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libcore/any.rs b/src/libcore/any.rs index 2a03eacf13cb3..ed743f40a4bcb 100644 --- a/src/libcore/any.rs +++ b/src/libcore/any.rs @@ -8,10 +8,10 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -//! Traits for dynamic typing of any type (through runtime reflection) +//! Traits for dynamic typing of any `'static` type (through runtime reflection) //! //! This module implements the `Any` trait, which enables dynamic typing -//! of any type, through runtime reflection. +//! of any `'static` type through runtime reflection. //! //! `Any` itself can be used to get a `TypeId`, and has more features when used as a trait object. //! As `&Any` (a borrowed trait object), it has the `is` and `as_ref` methods, to test if the @@ -32,8 +32,10 @@ pub enum Void { } // Any trait /////////////////////////////////////////////////////////////////////////////// -/// The `Any` trait is implemented by all types, and can be used as a trait object -/// for dynamic typing +/// The `Any` trait is implemented by all `'static` types, and can be used for dynamic typing +/// +/// Every type with no non-`'static` references implements `Any`, so `Any` can be used as a trait +/// object to emulate the effects dynamic typing. pub trait Any { /// Get the `TypeId` of `self` fn get_type_id(&self) -> TypeId; From b612ae9edea26cb8704363c47a66d583b644ad09 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 12 Jun 2014 10:27:44 -0700 Subject: [PATCH 09/16] rustc: [T, ..N] and [T, ..N+1] are not the same This commit fixes a bug in the calculation of the hash of a type which didn't factor in the length of a constant-sized vector. As a result of this, a type placed into an Any of a fixed length could be peeled out with any other fixed length in a safe manner. --- src/libcore/any.rs | 8 ++++++++ src/librustc/middle/ty.rs | 3 ++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/libcore/any.rs b/src/libcore/any.rs index ed743f40a4bcb..2657cd5348372 100644 --- a/src/libcore/any.rs +++ b/src/libcore/any.rs @@ -263,6 +263,14 @@ mod tests { let s = format!("{}", b); assert_eq!(s.as_slice(), "&Any"); } + + #[test] + fn any_fixed_vec() { + let test = [0u, ..8]; + let test = &test as &Any; + assert!(test.is::<[uint, ..8]>()); + assert!(!test.is::<[uint, ..10]>()); + } } #[cfg(test)] diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index e3d94c73bb4e9..485011e1cb393 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -4514,9 +4514,10 @@ pub fn hash_crate_independent(tcx: &ctxt, t: t, svh: &Svh) -> u64 { ty_uniq(_) => { byte!(10); } - ty_vec(m, Some(_)) => { + ty_vec(m, Some(n)) => { byte!(11); mt(&mut state, m); + n.hash(&mut state); 1u8.hash(&mut state); } ty_vec(m, None) => { From ac7b9ddc545b7f62f00bf8f4d490d31ff4b90d1d Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 12 Jun 2014 11:40:13 -0700 Subject: [PATCH 10/16] Audit usage of NativeMutex Once a native mutex has been used once, it is never allowed to be moved again. This is because some pthreads implementations take pointers inside the mutex itself. This commit adds stern wording around the methods on native mutexes, and fixes one use case in the codebase. The Mutex type in libsync was susceptible to movement, so the inner static mutex is now boxed to ensure that the address of the native mutex is constant. --- src/librustrt/mutex.rs | 78 +++++++++++++++++++++++++++++++++++++++++ src/librustrt/unwind.rs | 2 -- src/libsync/mutex.rs | 11 ++++-- 3 files changed, 87 insertions(+), 4 deletions(-) diff --git a/src/librustrt/mutex.rs b/src/librustrt/mutex.rs index eec19e9d5db86..3fece698a1de4 100644 --- a/src/librustrt/mutex.rs +++ b/src/librustrt/mutex.rs @@ -115,6 +115,18 @@ impl StaticNativeMutex { /// // critical section... /// } // automatically unlocked in `_guard`'s destructor /// ``` + /// + /// # Unsafety + /// + /// This method is unsafe because it will not function correctly if this + /// mutex has been *moved* since it was last used. The mutex can move an + /// arbitrary number of times before its first usage, but once a mutex has + /// been used once it is no longer allowed to move (or otherwise it invokes + /// undefined behavior). + /// + /// Additionally, this type does not take into account any form of + /// scheduling model. This will unconditionally block the *os thread* which + /// is not always desired. pub unsafe fn lock<'a>(&'a self) -> LockGuard<'a> { self.inner.lock(); @@ -123,6 +135,10 @@ impl StaticNativeMutex { /// Attempts to acquire the lock. The value returned is `Some` if /// the attempt succeeded. + /// + /// # Unsafety + /// + /// This method is unsafe for the same reasons as `lock`. pub unsafe fn trylock<'a>(&'a self) -> Option> { if self.inner.trylock() { Some(LockGuard { lock: self }) @@ -135,6 +151,12 @@ impl StaticNativeMutex { /// /// These needs to be paired with a call to `.unlock_noguard`. Prefer using /// `.lock`. + /// + /// # Unsafety + /// + /// This method is unsafe for the same reasons as `lock`. Additionally, this + /// does not guarantee that the mutex will ever be unlocked, and it is + /// undefined to drop an already-locked mutex. pub unsafe fn lock_noguard(&self) { self.inner.lock() } /// Attempts to acquire the lock without creating a @@ -143,12 +165,22 @@ impl StaticNativeMutex { /// /// If `true` is returned, this needs to be paired with a call to /// `.unlock_noguard`. Prefer using `.trylock`. + /// + /// # Unsafety + /// + /// This method is unsafe for the same reasons as `lock_noguard`. pub unsafe fn trylock_noguard(&self) -> bool { self.inner.trylock() } /// Unlocks the lock. This assumes that the current thread already holds the /// lock. + /// + /// # Unsafety + /// + /// This method is unsafe for the same reasons as `lock`. Additionally, it + /// is not guaranteed that this is unlocking a previously locked mutex. It + /// is undefined to unlock an unlocked mutex. pub unsafe fn unlock_noguard(&self) { self.inner.unlock() } /// Block on the internal condition variable. @@ -156,9 +188,19 @@ impl StaticNativeMutex { /// This function assumes that the lock is already held. Prefer /// using `LockGuard.wait` since that guarantees that the lock is /// held. + /// + /// # Unsafety + /// + /// This method is unsafe for the same reasons as `lock`. Additionally, this + /// is unsafe because the mutex may not be currently locked. pub unsafe fn wait_noguard(&self) { self.inner.wait() } /// Signals a thread in `wait` to wake up + /// + /// # Unsafety + /// + /// This method is unsafe for the same reasons as `lock`. Additionally, this + /// is unsafe because the mutex may not be currently locked. pub unsafe fn signal_noguard(&self) { self.inner.signal() } /// This function is especially unsafe because there are no guarantees made @@ -181,6 +223,7 @@ impl NativeMutex { /// already hold the lock. /// /// # Example + /// /// ```rust /// use std::rt::mutex::NativeMutex; /// unsafe { @@ -192,12 +235,22 @@ impl NativeMutex { /// } // automatically unlocked in `_guard`'s destructor /// } /// ``` + /// + /// # Unsafety + /// + /// This method is unsafe due to the same reasons as + /// `StaticNativeMutex::lock`. pub unsafe fn lock<'a>(&'a self) -> LockGuard<'a> { self.inner.lock() } /// Attempts to acquire the lock. The value returned is `Some` if /// the attempt succeeded. + /// + /// # Unsafety + /// + /// This method is unsafe due to the same reasons as + /// `StaticNativeMutex::trylock`. pub unsafe fn trylock<'a>(&'a self) -> Option> { self.inner.trylock() } @@ -206,6 +259,11 @@ impl NativeMutex { /// /// These needs to be paired with a call to `.unlock_noguard`. Prefer using /// `.lock`. + /// + /// # Unsafety + /// + /// This method is unsafe due to the same reasons as + /// `StaticNativeMutex::lock_noguard`. pub unsafe fn lock_noguard(&self) { self.inner.lock_noguard() } /// Attempts to acquire the lock without creating a @@ -214,12 +272,22 @@ impl NativeMutex { /// /// If `true` is returned, this needs to be paired with a call to /// `.unlock_noguard`. Prefer using `.trylock`. + /// + /// # Unsafety + /// + /// This method is unsafe due to the same reasons as + /// `StaticNativeMutex::trylock_noguard`. pub unsafe fn trylock_noguard(&self) -> bool { self.inner.trylock_noguard() } /// Unlocks the lock. This assumes that the current thread already holds the /// lock. + /// + /// # Unsafety + /// + /// This method is unsafe due to the same reasons as + /// `StaticNativeMutex::unlock_noguard`. pub unsafe fn unlock_noguard(&self) { self.inner.unlock_noguard() } /// Block on the internal condition variable. @@ -227,9 +295,19 @@ impl NativeMutex { /// This function assumes that the lock is already held. Prefer /// using `LockGuard.wait` since that guarantees that the lock is /// held. + /// + /// # Unsafety + /// + /// This method is unsafe due to the same reasons as + /// `StaticNativeMutex::wait_noguard`. pub unsafe fn wait_noguard(&self) { self.inner.wait_noguard() } /// Signals a thread in `wait` to wake up + /// + /// # Unsafety + /// + /// This method is unsafe due to the same reasons as + /// `StaticNativeMutex::signal_noguard`. pub unsafe fn signal_noguard(&self) { self.inner.signal_noguard() } } diff --git a/src/librustrt/unwind.rs b/src/librustrt/unwind.rs index f7475db1552f7..aebed5a8829c5 100644 --- a/src/librustrt/unwind.rs +++ b/src/librustrt/unwind.rs @@ -73,7 +73,6 @@ use libc::c_void; use local::Local; use task::{Task, Result}; -use exclusive::Exclusive; use uw = libunwind; @@ -88,7 +87,6 @@ struct Exception { } pub type Callback = fn(msg: &Any:Send, file: &'static str, line: uint); -type Queue = Exclusive>; // Variables used for invoking callbacks when a task starts to unwind. // diff --git a/src/libsync/mutex.rs b/src/libsync/mutex.rs index 6b9ff3cf0527c..ef558d3f9241b 100644 --- a/src/libsync/mutex.rs +++ b/src/libsync/mutex.rs @@ -97,7 +97,14 @@ pub static NATIVE_BLOCKED: uint = 1 << 2; /// drop(guard); // unlock the lock /// ``` pub struct Mutex { - lock: StaticMutex, + // Note that this static mutex is in a *box*, not inlined into the struct + // itself. This is done for memory safety reasons with the usage of a + // StaticNativeMutex inside the static mutex above. Once a native mutex has + // been used once, its address can never change (it can't be moved). This + // mutex type can be safely moved at any time, so to ensure that the native + // mutex is used correctly we box the inner lock to give it a constant + // address. + lock: Box, } #[deriving(PartialEq, Show)] @@ -458,7 +465,7 @@ impl Mutex { /// Creates a new mutex in an unlocked state ready for use. pub fn new() -> Mutex { Mutex { - lock: StaticMutex { + lock: box StaticMutex { state: atomics::AtomicUint::new(0), flavor: Unsafe::new(Unlocked), green_blocker: Unsafe::new(0), From d884cc83b0e44c27d5d01e97a0b0f85582f5d435 Mon Sep 17 00:00:00 2001 From: Daniel Micay Date: Thu, 12 Jun 2014 16:11:21 -0400 Subject: [PATCH 11/16] remove unnecessary PaX detection Rust no longer has support for JIT compilation, so it doesn't currently require a PaX MPROTECT exception. The extended attributes are preferred over modifying the binaries so it's not actually going to work on most systems like this anyway. If JIT compilation ends up being supported again, it should handle this by *always* applying the exception via an extended attribute without performing auto-detection of PaX on the host. The `paxctl` tool is only necessary with the older method involving modifying the ELF binary. --- configure | 52 ---------------------------------------------------- mk/stage0.mk | 4 ---- 2 files changed, 56 deletions(-) diff --git a/configure b/configure index e5106f93d3587..3a306085c77aa 100755 --- a/configure +++ b/configure @@ -416,7 +416,6 @@ opt mingw-cross 0 "cross-compile for win32 using mingw" opt clang 0 "prefer clang to gcc for building the runtime" opt ccache 0 "invoke gcc/clang via ccache to reuse object files between builds" opt local-rust 0 "use an installed rustc rather than downloading a snapshot" -opt pax-flags 0 "apply PaX flags to rustc binaries (required for GRSecurity/PaX-patched kernels)" opt inject-std-version 1 "inject the current compiler version of libstd into programs" opt llvm-static-stdcpp 0 "statically link to libstdc++ for LLVM" opt rpath 1 "build rpaths into rustc itself" @@ -518,12 +517,6 @@ then fi fi -if [ "$CFG_OSTYPE" = "unknown-linux-gnu" ] -then - probe CFG_PAXCTL paxctl /sbin/paxctl - probe CFG_ZCAT zcat -fi - step_msg "looking for target specific programs" probe CFG_ADB adb @@ -546,51 +539,6 @@ then fi fi -if [ "$CFG_OSTYPE" = "unknown-linux-gnu" ] -then - if [ ! -z "$CFG_ENABLE_PAX_FLAGS" -a -z "$CFG_PAXCTL" ] - then - err "enabled PaX markings but no paxctl binary found" - fi - - if [ -z "$CFG_DISABLE_PAX_FLAGS" ] - then - # GRSecurity/PaX detection. This can be very flaky. - GRSEC_DETECTED= - - # /dev/grsec only exists if CONFIG_GRKERNSEC_NO_RBAC is not set. - # /proc/sys/kernel/grsecurity is not available if ÇONFIG_GRKERNSEC_SYSCTL is not set. - if [ -e /dev/grsec -o -d /proc/sys/kernel/grsecurity ] - then - GRSEC_DETECTED=1 - # /proc/config.gz is normally only available to root, and only if CONFIG_IKCONFIG_PROC has been set. - elif [ -r /proc/config.gz -a ! -z "$CFG_ZCAT" ] - then - if "$CFG_ZCAT" /proc/config.gz | grep --quiet "CONFIG_GRKERNSEC=y" - then - GRSEC_DETECTED=1 - fi - # Flaky. - elif grep --quiet grsec /proc/version - then - GRSEC_DETECTED=1 - fi - - if [ ! -z "$GRSEC_DETECTED" ] - then - step_msg "GRSecurity: yes" - if [ ! -z "$CFG_PAXCTL" ] - then - CFG_ENABLE_PAX_FLAGS=1 - else - warn "GRSecurity kernel detected but no paxctl binary found: not setting CFG_ENABLE_PAX_FLAGS" - fi - else - step_msg "GRSecurity: no" - fi - fi -fi - BIN_SUF= if [ "$CFG_OSTYPE" = "pc-mingw32" ] || [ "$CFG_OSTYPE" = "w64-mingw32" ] then diff --git a/mk/stage0.mk b/mk/stage0.mk index 080e733724d46..972cfecea718a 100644 --- a/mk/stage0.mk +++ b/mk/stage0.mk @@ -18,10 +18,6 @@ ifdef CFG_ENABLE_LOCAL_RUST $(Q)$(S)src/etc/local_stage0.sh $(CFG_BUILD) $(CFG_LOCAL_RUST_ROOT) rustlib else $(Q)$(CFG_PYTHON) $(S)src/etc/get-snapshot.py $(CFG_BUILD) $(SNAPSHOT_FILE) -ifdef CFG_ENABLE_PAX_FLAGS - @$(call E, apply PaX flags: $@) - @"$(CFG_PAXCTL)" -cm "$@" -endif endif $(Q)touch $@ From 8c4a10a159a021a586a8a35552cbbe4f60e644a2 Mon Sep 17 00:00:00 2001 From: Luqman Aden Date: Thu, 12 Jun 2014 18:58:37 -0400 Subject: [PATCH 12/16] librustc: Take in account mutability when casting array to raw ptr. --- src/librustc/middle/typeck/check/mod.rs | 6 +++--- src/test/compile-fail/issue-14845.rs | 24 ++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) create mode 100644 src/test/compile-fail/issue-14845.rs diff --git a/src/librustc/middle/typeck/check/mod.rs b/src/librustc/middle/typeck/check/mod.rs index 544990d19a51a..b9933fed296d8 100644 --- a/src/librustc/middle/typeck/check/mod.rs +++ b/src/librustc/middle/typeck/check/mod.rs @@ -3099,9 +3099,9 @@ fn check_expr_with_unifier(fcx: &FnCtxt, /* this cast is only allowed from &[T] to *T or &T to *T. */ match (&ty::get(te).sty, &ty::get(t_1).sty) { - (&ty::ty_rptr(_, mt1), &ty::ty_ptr(mt2)) - if types_compatible(fcx, e.span, - mt1.ty, mt2.ty) => { + (&ty::ty_rptr(_, ty::mt { ty: mt1, mutbl: ast::MutImmutable }), + &ty::ty_ptr(ty::mt { ty: mt2, mutbl: ast::MutImmutable })) + if types_compatible(fcx, e.span, mt1, mt2) => { /* this case is allowed */ } _ => { diff --git a/src/test/compile-fail/issue-14845.rs b/src/test/compile-fail/issue-14845.rs new file mode 100644 index 0000000000000..d87eab7b6dedf --- /dev/null +++ b/src/test/compile-fail/issue-14845.rs @@ -0,0 +1,24 @@ +// Copyright 2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + + +struct X { + a: [u8, ..1] +} + +fn main() { + let x = X { a: [0] }; + let _f = &x.a as *mut u8; + //~^ ERROR mismatched types: expected `*mut u8` but found `&[u8, .. 1]` + + let local = [0u8]; + let _v = &local as *mut u8; + //~^ ERROR mismatched types: expected `*mut u8` but found `&[u8, .. 1]` +} From c9f3f47702602d196de414aa4f10bb2c2148ab9a Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Thu, 12 Jun 2014 14:08:44 -0700 Subject: [PATCH 13/16] librustc: Forbid `transmute` from being called on types whose size is only known post-monomorphization, and report `transmute` errors before the code is generated for that `transmute`. This can break code that looked like: unsafe fn f(x: T) { let y: int = transmute(x); } Change such code to take a type parameter that has the same size as the type being transmuted to. Closes #12898. [breaking-change] --- src/libcore/intrinsics.rs | 14 ++ src/libcore/mem.rs | 25 +-- src/librustc/driver/driver.rs | 3 + src/librustc/lib.rs | 1 + src/librustc/middle/intrinsicck.rs | 155 ++++++++++++++++++ src/librustc/middle/trans/base.rs | 6 + src/librustc/middle/trans/intrinsic.rs | 40 ++++- src/librustc/middle/ty.rs | 20 ++- src/librustc/plugin/load.rs | 6 +- src/librustdoc/plugins.rs | 7 +- src/libstd/dynamic_lib.rs | 2 +- .../compile-fail/transmute-different-sizes.rs | 27 +++ .../compile-fail/transmute-type-parameters.rs | 48 ++++++ 13 files changed, 323 insertions(+), 31 deletions(-) create mode 100644 src/librustc/middle/intrinsicck.rs create mode 100644 src/test/compile-fail/transmute-different-sizes.rs create mode 100644 src/test/compile-fail/transmute-type-parameters.rs diff --git a/src/libcore/intrinsics.rs b/src/libcore/intrinsics.rs index d61416a68e0d0..33f2a49d6d560 100644 --- a/src/libcore/intrinsics.rs +++ b/src/libcore/intrinsics.rs @@ -307,6 +307,20 @@ extern "rust-intrinsic" { /// `forget` is unsafe because the caller is responsible for /// ensuring the argument is deallocated already. pub fn forget(_: T) -> (); + + /// Unsafely transforms a value of one type into a value of another type. + /// + /// Both types must have the same size and alignment, and this guarantee + /// is enforced at compile-time. + /// + /// # Example + /// + /// ```rust + /// use std::mem; + /// + /// let v: &[u8] = unsafe { mem::transmute("L") }; + /// assert!(v == [76u8]); + /// ``` pub fn transmute(e: T) -> U; /// Returns `true` if a type requires drop glue. diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 8933c95350d59..237efcd0096d0 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -17,6 +17,8 @@ use ptr; use intrinsics; use intrinsics::{bswap16, bswap32, bswap64}; +pub use intrinsics::transmute; + /// Returns the size of a type in bytes. #[inline] #[stable] @@ -412,29 +414,6 @@ pub fn drop(_x: T) { } #[stable] pub unsafe fn forget(thing: T) { intrinsics::forget(thing) } -/// Unsafely transforms a value of one type into a value of another type. -/// -/// Both types must have the same size and alignment, and this guarantee is -/// enforced at compile-time. -/// -/// # Example -/// -/// ```rust -/// use std::mem; -/// -/// let v: &[u8] = unsafe { mem::transmute("L") }; -/// assert!(v == [76u8]); -/// ``` -#[inline] -#[unstable = "this function will be modified to reject invocations of it which \ - cannot statically prove that T and U are the same size. For \ - example, this function, as written today, will be rejected in \ - the future because the size of T and U cannot be statically \ - known to be the same"] -pub unsafe fn transmute(thing: T) -> U { - intrinsics::transmute(thing) -} - /// Interprets `src` as `&U`, and then reads `src` without moving the contained /// value. /// diff --git a/src/librustc/driver/driver.rs b/src/librustc/driver/driver.rs index 880c1d6d5104d..2268ce8436587 100644 --- a/src/librustc/driver/driver.rs +++ b/src/librustc/driver/driver.rs @@ -333,6 +333,9 @@ pub fn phase_3_run_analysis_passes(sess: Session, time(time_passes, "privacy checking", maps, |(a, b)| middle::privacy::check_crate(&ty_cx, &exp_map2, a, b, krate)); + time(time_passes, "intrinsic checking", (), |_| + middle::intrinsicck::check_crate(&ty_cx, krate)); + time(time_passes, "effect checking", (), |_| middle::effect::check_crate(&ty_cx, krate)); diff --git a/src/librustc/lib.rs b/src/librustc/lib.rs index f79aaa40d213b..1e39aaa3a5fac 100644 --- a/src/librustc/lib.rs +++ b/src/librustc/lib.rs @@ -85,6 +85,7 @@ pub mod middle { pub mod dependency_format; pub mod weak_lang_items; pub mod save; + pub mod intrinsicck; } pub mod front { diff --git a/src/librustc/middle/intrinsicck.rs b/src/librustc/middle/intrinsicck.rs new file mode 100644 index 0000000000000..93913f8427111 --- /dev/null +++ b/src/librustc/middle/intrinsicck.rs @@ -0,0 +1,155 @@ +// Copyright 2012-2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use metadata::csearch; +use middle::def::DefFn; +use middle::subst::Subst; +use middle::ty::{TransmuteRestriction, ctxt, ty_bare_fn}; +use middle::ty; + +use syntax::abi::RustIntrinsic; +use syntax::ast::DefId; +use syntax::ast; +use syntax::ast_map::NodeForeignItem; +use syntax::codemap::Span; +use syntax::parse::token; +use syntax::visit::Visitor; +use syntax::visit; + +fn type_size_is_affected_by_type_parameters(tcx: &ty::ctxt, typ: ty::t) + -> bool { + let mut result = false; + ty::maybe_walk_ty(typ, |typ| { + match ty::get(typ).sty { + ty::ty_box(_) | ty::ty_uniq(_) | ty::ty_ptr(_) | + ty::ty_rptr(..) | ty::ty_bare_fn(..) | ty::ty_closure(..) => { + false + } + ty::ty_param(_) => { + result = true; + // No need to continue; we now know the result. + false + } + ty::ty_enum(did, ref substs) => { + for enum_variant in (*ty::enum_variants(tcx, did)).iter() { + for argument_type in enum_variant.args.iter() { + let argument_type = argument_type.subst(tcx, substs); + result = result || + type_size_is_affected_by_type_parameters( + tcx, + argument_type); + } + } + + // Don't traverse substitutions. + false + } + ty::ty_struct(did, ref substs) => { + for field in ty::struct_fields(tcx, did, substs).iter() { + result = result || + type_size_is_affected_by_type_parameters(tcx, + field.mt.ty); + } + + // Don't traverse substitutions. + false + } + _ => true, + } + }); + result +} + +struct IntrinsicCheckingVisitor<'a> { + tcx: &'a ctxt, +} + +impl<'a> IntrinsicCheckingVisitor<'a> { + fn def_id_is_transmute(&self, def_id: DefId) -> bool { + if def_id.krate == ast::LOCAL_CRATE { + match self.tcx.map.get(def_id.node) { + NodeForeignItem(ref item) => { + token::get_ident(item.ident) == + token::intern_and_get_ident("transmute") + } + _ => false, + } + } else { + match csearch::get_item_path(self.tcx, def_id).last() { + None => false, + Some(ref last) => { + token::get_name(last.name()) == + token::intern_and_get_ident("transmute") + } + } + } + } + + fn check_transmute(&self, span: Span, from: ty::t, to: ty::t) { + if type_size_is_affected_by_type_parameters(self.tcx, from) { + self.tcx.sess.span_err(span, + "cannot transmute from a type that \ + contains type parameters"); + } + if type_size_is_affected_by_type_parameters(self.tcx, to) { + self.tcx.sess.span_err(span, + "cannot transmute to a type that contains \ + type parameters"); + } + + let restriction = TransmuteRestriction { + span: span, + from: from, + to: to, + }; + self.tcx.transmute_restrictions.borrow_mut().push(restriction); + } +} + +impl<'a> Visitor<()> for IntrinsicCheckingVisitor<'a> { + fn visit_expr(&mut self, expr: &ast::Expr, (): ()) { + match expr.node { + ast::ExprPath(..) => { + match ty::resolve_expr(self.tcx, expr) { + DefFn(did, _) if self.def_id_is_transmute(did) => { + let typ = ty::node_id_to_type(self.tcx, expr.id); + match ty::get(typ).sty { + ty_bare_fn(ref bare_fn_ty) + if bare_fn_ty.abi == RustIntrinsic => { + let from = *bare_fn_ty.sig.inputs.get(0); + let to = bare_fn_ty.sig.output; + self.check_transmute(expr.span, from, to); + } + _ => { + self.tcx + .sess + .span_bug(expr.span, + "transmute wasn't a bare fn?!"); + } + } + } + _ => {} + } + } + _ => {} + } + + visit::walk_expr(self, expr, ()); + } +} + +pub fn check_crate(tcx: &ctxt, krate: &ast::Crate) { + let mut visitor = IntrinsicCheckingVisitor { + tcx: tcx, + }; + + visit::walk_crate(&mut visitor, krate, ()); +} + diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index 3e15d5a1a615a..14c8d5454aac8 100644 --- a/src/librustc/middle/trans/base.rs +++ b/src/librustc/middle/trans/base.rs @@ -59,6 +59,7 @@ use middle::trans::expr; use middle::trans::foreign; use middle::trans::glue; use middle::trans::inline; +use middle::trans::intrinsic; use middle::trans::machine; use middle::trans::machine::{llalign_of_min, llsize_of, llsize_of_real}; use middle::trans::meth; @@ -2329,6 +2330,11 @@ pub fn trans_crate(krate: ast::Crate, let ccx = CrateContext::new(llmod_id.as_slice(), tcx, exp_map2, Sha256::new(), link_meta, reachable); + + // First, verify intrinsics. + intrinsic::check_intrinsics(&ccx); + + // Next, translate the module. { let _icx = push_ctxt("text"); trans_mod(&ccx, &krate.module); diff --git a/src/librustc/middle/trans/intrinsic.rs b/src/librustc/middle/trans/intrinsic.rs index 5f37365e74353..648a2c335e686 100644 --- a/src/librustc/middle/trans/intrinsic.rs +++ b/src/librustc/middle/trans/intrinsic.rs @@ -390,7 +390,7 @@ pub fn trans_intrinsic(ccx: &CrateContext, ast_map::NodeExpr(e) => e.span, _ => fail!("transmute has non-expr arg"), }; - ccx.sess().span_fatal(sp, + ccx.sess().span_bug(sp, format!("transmute called on types with different sizes: \ {} ({} bit{}) to \ {} ({} bit{})", @@ -564,3 +564,41 @@ pub fn trans_intrinsic(ccx: &CrateContext, } fcx.cleanup(); } + +/// Performs late verification that intrinsics are used correctly. At present, +/// the only intrinsic that needs such verification is `transmute`. +pub fn check_intrinsics(ccx: &CrateContext) { + for transmute_restriction in ccx.tcx + .transmute_restrictions + .borrow() + .iter() { + let llfromtype = type_of::sizing_type_of(ccx, + transmute_restriction.from); + let lltotype = type_of::sizing_type_of(ccx, + transmute_restriction.to); + let from_type_size = machine::llbitsize_of_real(ccx, llfromtype); + let to_type_size = machine::llbitsize_of_real(ccx, lltotype); + if from_type_size != to_type_size { + ccx.sess() + .span_err(transmute_restriction.span, + format!("transmute called on types with different sizes: \ + {} ({} bit{}) to {} ({} bit{})", + ty_to_str(ccx.tcx(), transmute_restriction.from), + from_type_size as uint, + if from_type_size == 1 { + "" + } else { + "s" + }, + ty_to_str(ccx.tcx(), transmute_restriction.to), + to_type_size as uint, + if to_type_size == 1 { + "" + } else { + "s" + }).as_slice()); + } + } + ccx.sess().abort_if_errors(); +} + diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index 485011e1cb393..02300fccdae16 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -235,6 +235,17 @@ pub enum AutoRef { AutoBorrowObj(Region, ast::Mutability), } +/// A restriction that certain types must be the same size. The use of +/// `transmute` gives rise to these restrictions. +pub struct TransmuteRestriction { + /// The span from whence the restriction comes. + pub span: Span, + /// The type being transmuted from. + pub from: t, + /// The type being transmuted to. + pub to: t, +} + /// The data structure to keep track of all the information that typechecker /// generates so that so that it can be reused and doesn't have to be redone /// later on. @@ -357,6 +368,11 @@ pub struct ctxt { pub node_lint_levels: RefCell>, + + /// The types that must be asserted to be the same size for `transmute` + /// to be valid. We gather up these restrictions in the intrinsicck pass + /// and check them in trans. + pub transmute_restrictions: RefCell>, } pub enum tbox_flag { @@ -1118,6 +1134,7 @@ pub fn mk_ctxt(s: Session, vtable_map: RefCell::new(FnvHashMap::new()), dependency_formats: RefCell::new(HashMap::new()), node_lint_levels: RefCell::new(HashMap::new()), + transmute_restrictions: RefCell::new(Vec::new()), } } @@ -2711,8 +2728,7 @@ pub fn pat_ty(cx: &ctxt, pat: &ast::Pat) -> t { // // NB (2): This type doesn't provide type parameter substitutions; e.g. if you // ask for the type of "id" in "id(3)", it will return "fn(&int) -> int" -// instead of "fn(t) -> T with T = int". If this isn't what you want, see -// expr_ty_params_and_ty() below. +// instead of "fn(t) -> T with T = int". pub fn expr_ty(cx: &ctxt, expr: &ast::Expr) -> t { return node_id_to_type(cx, expr.id); } diff --git a/src/librustc/plugin/load.rs b/src/librustc/plugin/load.rs index 97ffcf279ae73..7d74b8c729652 100644 --- a/src/librustc/plugin/load.rs +++ b/src/librustc/plugin/load.rs @@ -126,9 +126,11 @@ impl<'a> PluginLoader<'a> { }; unsafe { - let registrar: PluginRegistrarFun = + let registrar = match lib.symbol(symbol.as_slice()) { - Ok(registrar) => registrar, + Ok(registrar) => { + mem::transmute::<*u8,PluginRegistrarFun>(registrar) + } // again fatal if we can't register macros Err(err) => self.sess.span_fatal(vi.span, err.as_slice()) }; diff --git a/src/librustdoc/plugins.rs b/src/librustdoc/plugins.rs index fee1d63a274d5..e5bced8038baf 100644 --- a/src/librustdoc/plugins.rs +++ b/src/librustdoc/plugins.rs @@ -12,6 +12,7 @@ use clean; use dl = std::dynamic_lib; use serialize::json; +use std::mem; use std::string::String; pub type PluginJson = Option<(String, json::Json)>; @@ -45,9 +46,11 @@ impl PluginManager { let x = self.prefix.join(libname(name)); let lib_result = dl::DynamicLibrary::open(Some(&x)); let lib = lib_result.unwrap(); - let plugin = unsafe { lib.symbol("rustdoc_plugin_entrypoint") }.unwrap(); + unsafe { + let plugin = lib.symbol("rustdoc_plugin_entrypoint").unwrap(); + self.callbacks.push(mem::transmute::<*u8,PluginCallback>(plugin)); + } self.dylibs.push(lib); - self.callbacks.push(plugin); } /// Load a normal Rust function as a plugin. diff --git a/src/libstd/dynamic_lib.rs b/src/libstd/dynamic_lib.rs index fa6efc8a4b18a..61f7997071e8b 100644 --- a/src/libstd/dynamic_lib.rs +++ b/src/libstd/dynamic_lib.rs @@ -134,7 +134,7 @@ impl DynamicLibrary { } /// Access the value at the symbol of the dynamic library - pub unsafe fn symbol(&self, symbol: &str) -> Result { + pub unsafe fn symbol(&self, symbol: &str) -> Result<*T, String> { // This function should have a lifetime constraint of 'a on // T but that feature is still unimplemented diff --git a/src/test/compile-fail/transmute-different-sizes.rs b/src/test/compile-fail/transmute-different-sizes.rs new file mode 100644 index 0000000000000..5ea1c496c769f --- /dev/null +++ b/src/test/compile-fail/transmute-different-sizes.rs @@ -0,0 +1,27 @@ +// Copyright 2012 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Tests that `transmute` cannot be called on types of different size. + +use std::mem::transmute; + +unsafe fn f() { + let _: i8 = transmute(16i16); + //~^ ERROR transmute called on types with different sizes +} + +unsafe fn g(x: &T) { + let _: i8 = transmute(x); + //~^ ERROR transmute called on types with different sizes +} + +fn main() {} + + diff --git a/src/test/compile-fail/transmute-type-parameters.rs b/src/test/compile-fail/transmute-type-parameters.rs new file mode 100644 index 0000000000000..53391a0e8947b --- /dev/null +++ b/src/test/compile-fail/transmute-type-parameters.rs @@ -0,0 +1,48 @@ +// Copyright 2012 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Tests that `transmute` cannot be called on type parameters. + +use std::mem::transmute; + +unsafe fn f(x: T) { + let _: int = transmute(x); //~ ERROR cannot transmute +} + +unsafe fn g(x: (T, int)) { + let _: int = transmute(x); //~ ERROR cannot transmute +} + +unsafe fn h(x: [T, ..10]) { + let _: int = transmute(x); //~ ERROR cannot transmute +} + +struct Bad { + f: T, +} + +unsafe fn i(x: Bad) { + let _: int = transmute(x); //~ ERROR cannot transmute +} + +enum Worse { + A(T), + B, +} + +unsafe fn j(x: Worse) { + let _: int = transmute(x); //~ ERROR cannot transmute +} + +unsafe fn k(x: Option) { + let _: int = transmute(x); //~ ERROR cannot transmute +} + +fn main() {} From 9d5ec04d184a5d28e75d74b725ebb7cc21b547af Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Fri, 13 Jun 2014 09:36:26 +1000 Subject: [PATCH 14/16] syntax: fix quote_pat! & unignore a quotation test. --- src/libsyntax/ext/quote.rs | 4 +--- src/test/run-pass-fulldeps/quote-tokens.rs | 8 +++----- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/libsyntax/ext/quote.rs b/src/libsyntax/ext/quote.rs index 185924f704cdd..bc5442a94fb7f 100644 --- a/src/libsyntax/ext/quote.rs +++ b/src/libsyntax/ext/quote.rs @@ -368,9 +368,7 @@ pub fn expand_quote_pat(cx: &mut ExtCtxt, sp: Span, tts: &[ast::TokenTree]) -> Box { - let e_refutable = cx.expr_lit(sp, ast::LitBool(true)); - let expanded = expand_parse_call(cx, sp, "parse_pat", - vec!(e_refutable), tts); + let expanded = expand_parse_call(cx, sp, "parse_pat", vec!(), tts); base::MacExpr::new(expanded) } diff --git a/src/test/run-pass-fulldeps/quote-tokens.rs b/src/test/run-pass-fulldeps/quote-tokens.rs index 7c25246807d57..c41ec0dbd658e 100644 --- a/src/test/run-pass-fulldeps/quote-tokens.rs +++ b/src/test/run-pass-fulldeps/quote-tokens.rs @@ -8,8 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// ignore-test - #![feature(quote)] #![feature(managed_boxes)] @@ -18,11 +16,11 @@ extern crate syntax; use syntax::ext::base::ExtCtxt; fn syntax_extension(cx: &ExtCtxt) { - let e_toks : Vec = quote_tokens!(cx, 1 + 2); - let p_toks : Vec = quote_tokens!(cx, (x, 1 .. 4, *)); + let e_toks : Vec = quote_tokens!(cx, 1 + 2); + let p_toks : Vec = quote_tokens!(cx, (x, 1 .. 4, *)); let a: @syntax::ast::Expr = quote_expr!(cx, 1 + 2); - let _b: Option<@syntax::ast::item> = quote_item!(cx, static foo : int = $e_toks; ); + let _b: Option<@syntax::ast::Item> = quote_item!(cx, static foo : int = $e_toks; ); let _c: @syntax::ast::Pat = quote_pat!(cx, (x, 1 .. 4, *) ); let _d: @syntax::ast::Stmt = quote_stmt!(cx, let x = $a; ); let _e: @syntax::ast::Expr = quote_expr!(cx, match foo { $p_toks => 10 } ); From f907d9772c55d942fb178622b0b2b5a8ca103c11 Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Fri, 13 Jun 2014 09:40:10 +1000 Subject: [PATCH 15/16] syntax: parse outer attributes in `quote_item!` calls. Fixes #14857. --- src/libsyntax/ext/quote.rs | 5 ++--- src/libsyntax/ext/tt/macro_rules.rs | 3 +-- src/libsyntax/parse/mod.rs | 3 +-- src/libsyntax/parse/parser.rs | 5 +++++ src/test/run-pass-fulldeps/quote-tokens.rs | 3 +++ 5 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/libsyntax/ext/quote.rs b/src/libsyntax/ext/quote.rs index bc5442a94fb7f..4f1e2ab356e1e 100644 --- a/src/libsyntax/ext/quote.rs +++ b/src/libsyntax/ext/quote.rs @@ -358,9 +358,8 @@ pub fn expand_quote_item(cx: &mut ExtCtxt, sp: Span, tts: &[ast::TokenTree]) -> Box { - let e_attrs = cx.expr_vec_ng(sp); - let expanded = expand_parse_call(cx, sp, "parse_item", - vec!(e_attrs), tts); + let expanded = expand_parse_call(cx, sp, "parse_item_with_outer_attributes", + vec!(), tts); base::MacExpr::new(expanded) } diff --git a/src/libsyntax/ext/tt/macro_rules.rs b/src/libsyntax/ext/tt/macro_rules.rs index c4990255719f3..72c578b87699c 100644 --- a/src/libsyntax/ext/tt/macro_rules.rs +++ b/src/libsyntax/ext/tt/macro_rules.rs @@ -73,8 +73,7 @@ impl<'a> MacResult for ParserAnyMacro<'a> { let mut ret = SmallVector::zero(); loop { let mut parser = self.parser.borrow_mut(); - let attrs = parser.parse_outer_attributes(); - match parser.parse_item(attrs) { + match parser.parse_item_with_outer_attributes() { Some(item) => ret.push(item), None => break } diff --git a/src/libsyntax/parse/mod.rs b/src/libsyntax/parse/mod.rs index 88746d145b6fc..1ebcbc8a7d102 100644 --- a/src/libsyntax/parse/mod.rs +++ b/src/libsyntax/parse/mod.rs @@ -117,8 +117,7 @@ pub fn parse_item_from_source_str(name: String, sess: &ParseSess) -> Option> { let mut p = new_parser_from_source_str(sess, cfg, name, source); - let attrs = p.parse_outer_attributes(); - maybe_aborted(p.parse_item(attrs),p) + maybe_aborted(p.parse_item_with_outer_attributes(),p) } pub fn parse_meta_from_source_str(name: String, diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index e1a45e5643d25..d11d303059fa1 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -4965,6 +4965,11 @@ impl<'a> Parser<'a> { return IoviNone(attrs); } + pub fn parse_item_with_outer_attributes(&mut self) -> Option> { + let attrs = self.parse_outer_attributes(); + self.parse_item(attrs) + } + pub fn parse_item(&mut self, attrs: Vec ) -> Option> { match self.parse_item_or_view_item(attrs, true) { IoviNone(_) => None, diff --git a/src/test/run-pass-fulldeps/quote-tokens.rs b/src/test/run-pass-fulldeps/quote-tokens.rs index c41ec0dbd658e..96f5fca5a2ded 100644 --- a/src/test/run-pass-fulldeps/quote-tokens.rs +++ b/src/test/run-pass-fulldeps/quote-tokens.rs @@ -28,6 +28,9 @@ fn syntax_extension(cx: &ExtCtxt) { let _f: @syntax::ast::Expr = quote_expr!(cx, ()); let _g: @syntax::ast::Expr = quote_expr!(cx, true); let _h: @syntax::ast::Expr = quote_expr!(cx, 'a'); + + let i: Option<@syntax::ast::Item> = quote_item!(cx, #[deriving(Eq)] struct Foo; ); + assert!(i.is_some()); } fn main() { From b7af25060a1b0451cb06085ba5893980bc4e5333 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 12 Jun 2014 21:34:32 -0700 Subject: [PATCH 16/16] Rolling up PRs in the queue Closes #14797 (librustc: Fix the issue with labels shadowing variable names by making) Closes #14823 (Improve error messages for io::fs) Closes #14827 (libsyntax: Allow `+` to separate trait bounds from objects.) Closes #14834 (configure: Don't sync unused submodules) Closes #14838 (Remove typo on collections::treemap::UnionItems) Closes #14839 (Fix the unused struct field lint for struct variants) Closes #14840 (Clarify `Any` docs) Closes #14846 (rustc: [T, ..N] and [T, ..N+1] are not the same) Closes #14847 (Audit usage of NativeMutex) Closes #14850 (remove unnecessary PaX detection) Closes #14856 (librustc: Take in account mutability when casting array to raw ptr.) Closes #14859 (librustc: Forbid `transmute` from being called on types whose size is) Closes #14860 (Fix `quote_pat!` & parse outer attributes in `quote_item!`) --- src/libnative/io/c_win32.rs | 25 +++++++++++-------- src/libstd/dynamic_lib.rs | 3 ++- src/libstd/io/fs.rs | 8 ++++-- src/libstd/rt/backtrace.rs | 8 +++--- src/test/compile-fail/issue-14845.rs | 2 +- .../compile-fail/transmute-different-sizes.rs | 2 ++ src/test/pretty/path-type-bounds.rs | 6 ++--- src/test/run-pass-fulldeps/quote-tokens.rs | 3 +++ src/test/run-pass/issue-14837.rs | 2 +- 9 files changed, 37 insertions(+), 22 deletions(-) diff --git a/src/libnative/io/c_win32.rs b/src/libnative/io/c_win32.rs index e855b8bd4f264..1b6525c6e3877 100644 --- a/src/libnative/io/c_win32.rs +++ b/src/libnative/io/c_win32.rs @@ -77,15 +77,20 @@ pub mod compat { fn GetProcAddress(hModule: HMODULE, lpProcName: LPCSTR) -> LPVOID; } - // store_func() is idempotent, so using relaxed ordering for the atomics should be enough. - // This way, calling a function in this compatibility layer (after it's loaded) shouldn't - // be any slower than a regular DLL call. - unsafe fn store_func(ptr: *mut T, module: &str, symbol: &str, fallback: T) { + // store_func() is idempotent, so using relaxed ordering for the atomics + // should be enough. This way, calling a function in this compatibility + // layer (after it's loaded) shouldn't be any slower than a regular DLL + // call. + unsafe fn store_func(ptr: *mut uint, module: &str, symbol: &str, fallback: uint) { let module = module.to_utf16().append_one(0); symbol.with_c_str(|symbol| { let handle = GetModuleHandleW(module.as_ptr()); - let func: Option = transmute(GetProcAddress(handle, symbol)); - atomic_store_relaxed(ptr, func.unwrap_or(fallback)) + let func: uint = transmute(GetProcAddress(handle, symbol)); + atomic_store_relaxed(ptr, if func == 0 { + fallback + } else { + func + }) }) } @@ -109,10 +114,10 @@ pub mod compat { extern "system" fn thunk($($argname: $argtype),*) -> $rettype { unsafe { - ::io::c::compat::store_func(&mut ptr, - stringify!($module), - stringify!($symbol), - fallback); + ::io::c::compat::store_func(&mut ptr as *mut _ as *mut uint, + stringify!($module), + stringify!($symbol), + fallback as uint); ::std::intrinsics::atomic_load_relaxed(&ptr)($($argname),*) } } diff --git a/src/libstd/dynamic_lib.rs b/src/libstd/dynamic_lib.rs index 61f7997071e8b..76dbfa8c29ec3 100644 --- a/src/libstd/dynamic_lib.rs +++ b/src/libstd/dynamic_lib.rs @@ -158,6 +158,7 @@ mod test { use super::*; use prelude::*; use libc; + use mem; #[test] #[ignore(cfg(windows))] // FIXME #8818 @@ -174,7 +175,7 @@ mod test { let cosine: extern fn(libc::c_double) -> libc::c_double = unsafe { match libm.symbol("cos") { Err(error) => fail!("Could not load function cos: {}", error), - Ok(cosine) => cosine + Ok(cosine) => mem::transmute::<*u8, _>(cosine) } }; diff --git a/src/libstd/io/fs.rs b/src/libstd/io/fs.rs index 008be8ffaaef3..10dfec0f56609 100644 --- a/src/libstd/io/fs.rs +++ b/src/libstd/io/fs.rs @@ -977,7 +977,9 @@ mod test { let result = File::open_mode(filename, Open, Read); error!(result, "couldn't open file"); - error!(result, "no such file or directory"); + if cfg!(unix) { + error!(result, "no such file or directory"); + } error!(result, format!("path={}; mode=open; access=read", filename.display())); }) @@ -988,7 +990,9 @@ mod test { let result = unlink(filename); error!(result, "couldn't unlink path"); - error!(result, "no such file or directory"); + if cfg!(unix) { + error!(result, "no such file or directory"); + } error!(result, format!("path={}", filename.display())); }) diff --git a/src/libstd/rt/backtrace.rs b/src/libstd/rt/backtrace.rs index a1372b51d472a..e2a963c5a878d 100644 --- a/src/libstd/rt/backtrace.rs +++ b/src/libstd/rt/backtrace.rs @@ -873,12 +873,12 @@ mod imp { Err(..) => return Ok(()), }; - macro_rules! sym( ($e:expr, $t:ident) => ( - match unsafe { lib.symbol::<$t>($e) } { - Ok(f) => f, + macro_rules! sym( ($e:expr, $t:ident) => (unsafe { + match lib.symbol($e) { + Ok(f) => mem::transmute::<*u8, $t>(f), Err(..) => return Ok(()) } - ) ) + }) ) // Fetch the symbols necessary from dbghelp.dll let SymFromAddr = sym!("SymFromAddr", SymFromAddrFn); diff --git a/src/test/compile-fail/issue-14845.rs b/src/test/compile-fail/issue-14845.rs index d87eab7b6dedf..90366d09e2dea 100644 --- a/src/test/compile-fail/issue-14845.rs +++ b/src/test/compile-fail/issue-14845.rs @@ -17,7 +17,7 @@ fn main() { let x = X { a: [0] }; let _f = &x.a as *mut u8; //~^ ERROR mismatched types: expected `*mut u8` but found `&[u8, .. 1]` - + let local = [0u8]; let _v = &local as *mut u8; //~^ ERROR mismatched types: expected `*mut u8` but found `&[u8, .. 1]` diff --git a/src/test/compile-fail/transmute-different-sizes.rs b/src/test/compile-fail/transmute-different-sizes.rs index 5ea1c496c769f..abdfe983e3a8f 100644 --- a/src/test/compile-fail/transmute-different-sizes.rs +++ b/src/test/compile-fail/transmute-different-sizes.rs @@ -10,6 +10,8 @@ // Tests that `transmute` cannot be called on types of different size. +#![allow(warnings)] + use std::mem::transmute; unsafe fn f() { diff --git a/src/test/pretty/path-type-bounds.rs b/src/test/pretty/path-type-bounds.rs index 0fdbad67f160d..f90937c34a6d6 100644 --- a/src/test/pretty/path-type-bounds.rs +++ b/src/test/pretty/path-type-bounds.rs @@ -14,11 +14,11 @@ trait Tr { } impl Tr for int { } -fn foo(x: Box) -> Box { x } +fn foo(x: Box) -> Box { x } fn main() { - let x: Box; + let x: Box; - box() 1 as Box; + box() 1 as Box; } diff --git a/src/test/run-pass-fulldeps/quote-tokens.rs b/src/test/run-pass-fulldeps/quote-tokens.rs index 96f5fca5a2ded..4b2e29135ad2e 100644 --- a/src/test/run-pass-fulldeps/quote-tokens.rs +++ b/src/test/run-pass-fulldeps/quote-tokens.rs @@ -8,6 +8,9 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// ignore-android +// ignore-pretty: does not work well with `--test` + #![feature(quote)] #![feature(managed_boxes)] diff --git a/src/test/run-pass/issue-14837.rs b/src/test/run-pass/issue-14837.rs index c207980cd6e34..7876fe86d4724 100644 --- a/src/test/run-pass/issue-14837.rs +++ b/src/test/run-pass/issue-14837.rs @@ -9,8 +9,8 @@ // except according to those terms. #![feature(struct_variant)] -#![deny(warnings)] +#[deny(dead_code)] pub enum Foo { Bar { pub baz: int