From da78718d20214abbb050cfce4b607236ae9df1c4 Mon Sep 17 00:00:00 2001 From: York Xiang Date: Sat, 18 Apr 2015 09:18:46 +0800 Subject: [PATCH 1/8] Fix #20616 Conflicts: src/libsyntax/parse/parser.rs --- src/librustc_typeck/check/method/suggest.rs | 2 +- src/libsyntax/parse/parser.rs | 35 +++++++++++++- src/libsyntax/parse/token.rs | 8 ++++ src/test/compile-fail/issue-20616-1.rs | 46 ++++++++++++++++++ src/test/compile-fail/issue-20616-2.rs | 46 ++++++++++++++++++ src/test/compile-fail/issue-20616-3.rs | 46 ++++++++++++++++++ src/test/compile-fail/issue-20616-4.rs | 46 ++++++++++++++++++ src/test/compile-fail/issue-20616-5.rs | 46 ++++++++++++++++++ src/test/compile-fail/issue-20616-6.rs | 46 ++++++++++++++++++ src/test/compile-fail/issue-20616-7.rs | 46 ++++++++++++++++++ src/test/compile-fail/issue-20616-8.rs | 46 ++++++++++++++++++ src/test/compile-fail/issue-20616-9.rs | 46 ++++++++++++++++++ src/test/run-pass/issue-20616.rs | 52 +++++++++++++++++++++ 13 files changed, 508 insertions(+), 3 deletions(-) create mode 100644 src/test/compile-fail/issue-20616-1.rs create mode 100644 src/test/compile-fail/issue-20616-2.rs create mode 100644 src/test/compile-fail/issue-20616-3.rs create mode 100644 src/test/compile-fail/issue-20616-4.rs create mode 100644 src/test/compile-fail/issue-20616-5.rs create mode 100644 src/test/compile-fail/issue-20616-6.rs create mode 100644 src/test/compile-fail/issue-20616-7.rs create mode 100644 src/test/compile-fail/issue-20616-8.rs create mode 100644 src/test/compile-fail/issue-20616-9.rs create mode 100644 src/test/run-pass/issue-20616.rs diff --git a/src/librustc_typeck/check/method/suggest.rs b/src/librustc_typeck/check/method/suggest.rs index a4175fe8325b2..17658675ee280 100644 --- a/src/librustc_typeck/check/method/suggest.rs +++ b/src/librustc_typeck/check/method/suggest.rs @@ -364,7 +364,7 @@ pub fn all_traits<'a>(ccx: &'a CrateCtxt) -> AllTraits<'a> { } pub struct AllTraits<'a> { - borrow: cell::Ref<'a Option>, + borrow: cell::Ref<'a, Option>, idx: usize } diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 0515d1ae945bd..a78347a13dc87 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -896,8 +896,10 @@ impl<'a> Parser<'a> { pub fn bump(&mut self) -> PResult<()> { self.last_span = self.span; // Stash token for error recovery (sometimes; clone is not necessarily cheap). - self.last_token = if self.token.is_ident() || self.token.is_path() { - Some(box self.token.clone()) + self.last_token = if self.token.is_ident() || + self.token.is_path() || + self.token == token::Comma { + Some(Box::new(self.token.clone())) } else { None }; @@ -3796,8 +3798,37 @@ impl<'a> Parser<'a> { fn parse_generic_values_after_lt(&mut self) -> PResult<(Vec, Vec>, Vec>)> { + let span_lo = self.span.lo; let lifetimes = try!(self.parse_lifetimes(token::Comma)); + let missing_comma = !lifetimes.is_empty() && + !self.token.is_like_gt() && + self.last_token + .as_ref().map_or(true, + |x| &**x != &token::Comma); + + if missing_comma { + + let msg = format!("expected `,` or `>` after lifetime \ + name, found `{}`", + self.this_token_to_string()); + self.span_err(self.span, &msg); + + let span_hi = self.span.hi; + let span_hi = if self.parse_ty_nopanic().is_ok() { + self.span.hi + } else { + span_hi + }; + + let msg = format!("did you mean a single argument type &'a Type, \ + or did you mean the comma-separated arguments \ + 'a, Type?"); + self.span_note(mk_sp(span_lo, span_hi), &msg); + + self.abort_if_errors() + } + // First parse types. let (types, returned) = try!(self.parse_seq_to_gt_or_return( Some(token::Comma), diff --git a/src/libsyntax/parse/token.rs b/src/libsyntax/parse/token.rs index 2bb74944ce91a..9ec66f19dfe0a 100644 --- a/src/libsyntax/parse/token.rs +++ b/src/libsyntax/parse/token.rs @@ -173,6 +173,14 @@ pub enum Token { } impl Token { + /// Returns `true` if the token starts with '>'. + pub fn is_like_gt(&self) -> bool { + match *self { + BinOp(Shr) | BinOpEq(Shr) | Gt | Ge => true, + _ => false, + } + } + /// Returns `true` if the token can appear at the start of an expression. pub fn can_begin_expr(&self) -> bool { match *self { diff --git a/src/test/compile-fail/issue-20616-1.rs b/src/test/compile-fail/issue-20616-1.rs new file mode 100644 index 0000000000000..5b4f9942d28b6 --- /dev/null +++ b/src/test/compile-fail/issue-20616-1.rs @@ -0,0 +1,46 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// 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. + +// We need all these 9 issue-20616-N.rs files +// becase we can only catch one parsing error at a time + + + +type Type_1_<'a, T> = &'a T; + + +type Type_1<'a T> = &'a T; //~ error: expected `,` or `>` after lifetime name, found `T` + + +//type Type_2 = Type_1_<'static ()>; // error: expected `,` or `>` after lifetime name, found `(` + + +//type Type_3 = Box; // error: expected type, found `,` + + +//type Type_4 = Type_1_<'static,, T>; // error: expected type, found `,` + + +type Type_5_<'a> = Type_1_<'a, ()>; + + +//type Type_5<'a> = Type_1_<'a, (),,>; // error: expected type, found `,` + + +//type Type_6 = Type_5_<'a,,>; // error: expected type, found `,` + + +//type Type_7 = Box<(),,>; // error: expected type, found `,` + + +//type Type_8<'a,,> = &'a (); // error: expected ident, found `,` + + +//type Type_9 = Box; // error: expected ident, found `,` diff --git a/src/test/compile-fail/issue-20616-2.rs b/src/test/compile-fail/issue-20616-2.rs new file mode 100644 index 0000000000000..65305ff3ac82f --- /dev/null +++ b/src/test/compile-fail/issue-20616-2.rs @@ -0,0 +1,46 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// 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. + +// We need all these 9 issue-20616-N.rs files +// becase we can only catch one parsing error at a time + + + +type Type_1_<'a, T> = &'a T; + + +//type Type_1<'a T> = &'a T; // error: expected `,` or `>` after lifetime name, found `T` + + +type Type_2 = Type_1_<'static ()>; //~ error: expected `,` or `>` after lifetime name, found `(` + + +//type Type_3 = Box; // error: expected type, found `,` + + +//type Type_4 = Type_1_<'static,, T>; // error: expected type, found `,` + + +type Type_5_<'a> = Type_1_<'a, ()>; + + +//type Type_5<'a> = Type_1_<'a, (),,>; // error: expected type, found `,` + + +//type Type_6 = Type_5_<'a,,>; // error: expected type, found `,` + + +//type Type_7 = Box<(),,>; // error: expected type, found `,` + + +//type Type_8<'a,,> = &'a (); // error: expected ident, found `,` + + +//type Type_9 = Box; // error: expected ident, found `,` diff --git a/src/test/compile-fail/issue-20616-3.rs b/src/test/compile-fail/issue-20616-3.rs new file mode 100644 index 0000000000000..101f81019d97c --- /dev/null +++ b/src/test/compile-fail/issue-20616-3.rs @@ -0,0 +1,46 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// 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. + +// We need all these 9 issue-20616-N.rs files +// becase we can only catch one parsing error at a time + + + +type Type_1_<'a, T> = &'a T; + + +//type Type_1<'a T> = &'a T; // error: expected `,` or `>` after lifetime name, found `T` + + +//type Type_2 = Type_1_<'static ()>; // error: expected `,` or `>` after lifetime name, found `(` + + +type Type_3 = Box; //~ error: expected type, found `,` + + +//type Type_4 = Type_1_<'static,, T>; // error: expected type, found `,` + + +type Type_5_<'a> = Type_1_<'a, ()>; + + +//type Type_5<'a> = Type_1_<'a, (),,>; // error: expected type, found `,` + + +//type Type_6 = Type_5_<'a,,>; // error: expected type, found `,` + + +//type Type_7 = Box<(),,>; // error: expected type, found `,` + + +//type Type_8<'a,,> = &'a (); // error: expected ident, found `,` + + +//type Type_9 = Box; // error: expected ident, found `,` diff --git a/src/test/compile-fail/issue-20616-4.rs b/src/test/compile-fail/issue-20616-4.rs new file mode 100644 index 0000000000000..6450e9ed68cb7 --- /dev/null +++ b/src/test/compile-fail/issue-20616-4.rs @@ -0,0 +1,46 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// 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. + +// We need all these 9 issue-20616-N.rs files +// becase we can only catch one parsing error at a time + + + +type Type_1_<'a, T> = &'a T; + + +//type Type_1<'a T> = &'a T; // error: expected `,` or `>` after lifetime name, found `T` + + +//type Type_2 = Type_1_<'static ()>; // error: expected `,` or `>` after lifetime name, found `(` + + +//type Type_3 = Box; // error: expected type, found `,` + + +type Type_4 = Type_1_<'static,, T>; //~ error: expected type, found `,` + + +type Type_5_<'a> = Type_1_<'a, ()>; + + +//type Type_5<'a> = Type_1_<'a, (),,>; // error: expected type, found `,` + + +//type Type_6 = Type_5_<'a,,>; // error: expected type, found `,` + + +//type Type_7 = Box<(),,>; // error: expected type, found `,` + + +//type Type_8<'a,,> = &'a (); // error: expected ident, found `,` + + +//type Type_9 = Box; // error: expected ident, found `,` diff --git a/src/test/compile-fail/issue-20616-5.rs b/src/test/compile-fail/issue-20616-5.rs new file mode 100644 index 0000000000000..d1840427ad8b2 --- /dev/null +++ b/src/test/compile-fail/issue-20616-5.rs @@ -0,0 +1,46 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// 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. + +// We need all these 9 issue-20616-N.rs files +// becase we can only catch one parsing error at a time + + + +type Type_1_<'a, T> = &'a T; + + +//type Type_1<'a T> = &'a T; // error: expected `,` or `>` after lifetime name, found `T` + + +//type Type_2 = Type_1_<'static ()>; // error: expected `,` or `>` after lifetime name, found `(` + + +//type Type_3 = Box; // error: expected type, found `,` + + +//type Type_4 = Type_1_<'static,, T>; // error: expected type, found `,` + + +type Type_5_<'a> = Type_1_<'a, ()>; + + +type Type_5<'a> = Type_1_<'a, (),,>; //~ error: expected type, found `,` + + +//type Type_6 = Type_5_<'a,,>; // error: expected type, found `,` + + +//type Type_7 = Box<(),,>; // error: expected type, found `,` + + +//type Type_8<'a,,> = &'a (); // error: expected ident, found `,` + + +//type Type_9 = Box; // error: expected ident, found `,` diff --git a/src/test/compile-fail/issue-20616-6.rs b/src/test/compile-fail/issue-20616-6.rs new file mode 100644 index 0000000000000..b0b5bc653f6d0 --- /dev/null +++ b/src/test/compile-fail/issue-20616-6.rs @@ -0,0 +1,46 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// 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. + +// We need all these 9 issue-20616-N.rs files +// becase we can only catch one parsing error at a time + + + +type Type_1_<'a, T> = &'a T; + + +//type Type_1<'a T> = &'a T; // error: expected `,` or `>` after lifetime name, found `T` + + +//type Type_2 = Type_1_<'static ()>; // error: expected `,` or `>` after lifetime name, found `(` + + +//type Type_3 = Box; // error: expected type, found `,` + + +//type Type_4 = Type_1_<'static,, T>; // error: expected type, found `,` + + +type Type_5_<'a> = Type_1_<'a, ()>; + + +//type Type_5<'a> = Type_1_<'a, (),,>; // error: expected type, found `,` + + +type Type_6 = Type_5_<'a,,>; //~ error: expected type, found `,` + + +//type Type_7 = Box<(),,>; // error: expected type, found `,` + + +//type Type_8<'a,,> = &'a (); // error: expected ident, found `,` + + +//type Type_9 = Box; // error: expected ident, found `,` diff --git a/src/test/compile-fail/issue-20616-7.rs b/src/test/compile-fail/issue-20616-7.rs new file mode 100644 index 0000000000000..0958f8b4ed22c --- /dev/null +++ b/src/test/compile-fail/issue-20616-7.rs @@ -0,0 +1,46 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// 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. + +// We need all these 9 issue-20616-N.rs files +// becase we can only catch one parsing error at a time + + + +type Type_1_<'a, T> = &'a T; + + +//type Type_1<'a T> = &'a T; // error: expected `,` or `>` after lifetime name, found `T` + + +//type Type_2 = Type_1_<'static ()>; // error: expected `,` or `>` after lifetime name, found `(` + + +//type Type_3 = Box; // error: expected type, found `,` + + +//type Type_4 = Type_1_<'static,, T>; // error: expected type, found `,` + + +type Type_5_<'a> = Type_1_<'a, ()>; + + +//type Type_5<'a> = Type_1_<'a, (),,>; // error: expected type, found `,` + + +//type Type_6 = Type_5_<'a,,>; // error: expected type, found `,` + + +type Type_7 = Box<(),,>; //~ error: expected type, found `,` + + +//type Type_8<'a,,> = &'a (); // error: expected ident, found `,` + + +//type Type_9 = Box; // error: expected ident, found `,` diff --git a/src/test/compile-fail/issue-20616-8.rs b/src/test/compile-fail/issue-20616-8.rs new file mode 100644 index 0000000000000..07ced7a97ba96 --- /dev/null +++ b/src/test/compile-fail/issue-20616-8.rs @@ -0,0 +1,46 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// 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. + +// We need all these 9 issue-20616-N.rs files +// becase we can only catch one parsing error at a time + + + +type Type_1_<'a, T> = &'a T; + + +//type Type_1<'a T> = &'a T; // error: expected `,` or `>` after lifetime name, found `T` + + +//type Type_2 = Type_1_<'static ()>; // error: expected `,` or `>` after lifetime name, found `(` + + +//type Type_3 = Box; // error: expected type, found `,` + + +//type Type_4 = Type_1_<'static,, T>; // error: expected type, found `,` + + +type Type_5_<'a> = Type_1_<'a, ()>; + + +//type Type_5<'a> = Type_1_<'a, (),,>; // error: expected type, found `,` + + +//type Type_6 = Type_5_<'a,,>; // error: expected type, found `,` + + +//type Type_7 = Box<(),,>; // error: expected type, found `,` + + +type Type_8<'a,,> = &'a (); //~ error: expected ident, found `,` + + +//type Type_9 = Box; // error: expected ident, found `,` diff --git a/src/test/compile-fail/issue-20616-9.rs b/src/test/compile-fail/issue-20616-9.rs new file mode 100644 index 0000000000000..7847dea69ef59 --- /dev/null +++ b/src/test/compile-fail/issue-20616-9.rs @@ -0,0 +1,46 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// 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. + +// We need all these 9 issue-20616-N.rs files +// becase we can only catch one parsing error at a time + + + +type Type_1_<'a, T> = &'a T; + + +//type Type_1<'a T> = &'a T; // error: expected `,` or `>` after lifetime name, found `T` + + +//type Type_2 = Type_1_<'static ()>; // error: expected `,` or `>` after lifetime name, found `(` + + +//type Type_3 = Box; // error: expected type, found `,` + + +//type Type_4 = Type_1_<'static,, T>; // error: expected type, found `,` + + +type Type_5_<'a> = Type_1_<'a, ()>; + + +//type Type_5<'a> = Type_1_<'a, (),,>; // error: expected type, found `,` + + +//type Type_6 = Type_5_<'a,,>; // error: expected type, found `,` + + +//type Type_7 = Box<(),,>; // error: expected type, found `,` + + +//type Type_8<'a,,> = &'a (); // error: expected ident, found `,` + + +type Type_9 = Box; //~ error: expected ident, found `,` diff --git a/src/test/run-pass/issue-20616.rs b/src/test/run-pass/issue-20616.rs new file mode 100644 index 0000000000000..e6b256f7e8413 --- /dev/null +++ b/src/test/run-pass/issue-20616.rs @@ -0,0 +1,52 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// 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. + +type MyType<'a, T> = &'a T; + +// combine lifetime bounds and type arguments in usual way +type TypeA<'a> = MyType<'a, ()>; + +// ensure token `>>` works fine +type TypeB = Box>; +type TypeB_ = Box>; + +// trailing comma when combine lifetime bounds and type arguments +type TypeC<'a> = MyType<'a, (),>; + +// normal lifetime bounds +type TypeD = TypeA<'static>; + +// trailing comma on lifetime bounds +type TypeE = TypeA<'static,>; + +// normal type arugment +type TypeF = Box; + +// type argument with trailing comma +type TypeG = Box; + +// trailing comma on liftime defs +type TypeH<'a,> = &'a (); + +// trailing comma on type argument +type TypeI = T; + +static STATIC: () = (); + +fn main() { + + // ensure token `>=` works fine + let _: TypeA<'static>= &STATIC; + let _: TypeA<'static,>= &STATIC; + + // ensure token `>>=` works fine + let _: Box>= Box::new(&STATIC); + let _: Box>= Box::new(&STATIC); +} From 37277a8d3329253e5784eaced9a77d1294199a6b Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 27 Apr 2015 13:44:20 -0700 Subject: [PATCH 2/8] std: Fix inheriting standard handles on windows Currently if a standard I/O handle is set to inherited on Windows, no action is taken and the slot in the process information description is set to `INVALID_HANDLE_VALUE`. Due to our passing of `STARTF_USESTDHANDLES`, however, this means that the handle is actually set to nothing and if a child tries to print it will generate an error. This commit fixes this behavior by explicitly creating stdio handles to be placed in these slots by duplicating the current process's I/O handles. This is presumably what previously happened silently by using a file-descriptor-based implementation instead of a `HANDLE`-centric implementation. Along the way this cleans up a lot of code in `Process::spawn` for Windows by ensuring destructors are always run, using more RAII, and limiting the scope of `unsafe` wherever possible. --- src/libstd/sys/windows/fs2.rs | 8 +- src/libstd/sys/windows/handle.rs | 13 ++ src/libstd/sys/windows/process2.rs | 278 +++++++++++------------------ src/libstd/sys/windows/stdio.rs | 16 +- 4 files changed, 138 insertions(+), 177 deletions(-) diff --git a/src/libstd/sys/windows/fs2.rs b/src/libstd/sys/windows/fs2.rs index e66c356b7c56b..16635fb9ae9b3 100644 --- a/src/libstd/sys/windows/fs2.rs +++ b/src/libstd/sys/windows/fs2.rs @@ -55,6 +55,7 @@ pub struct OpenOptions { share_mode: Option, creation_disposition: Option, flags_and_attributes: Option, + security_attributes: usize, // *mut T doesn't have a Default impl } #[derive(Clone, PartialEq, Eq, Debug)] @@ -134,6 +135,9 @@ impl OpenOptions { pub fn share_mode(&mut self, val: i32) { self.share_mode = Some(val as libc::DWORD); } + pub fn security_attributes(&mut self, attrs: libc::LPSECURITY_ATTRIBUTES) { + self.security_attributes = attrs as usize; + } fn get_desired_access(&self) -> libc::DWORD { self.desired_access.unwrap_or({ @@ -185,7 +189,7 @@ impl File { libc::CreateFileW(path.as_ptr(), opts.get_desired_access(), opts.get_share_mode(), - ptr::null_mut(), + opts.security_attributes as *mut _, opts.get_creation_disposition(), opts.get_flags_and_attributes(), ptr::null_mut()) @@ -262,6 +266,8 @@ impl File { } pub fn handle(&self) -> &Handle { &self.handle } + + pub fn into_handle(self) -> Handle { self.handle } } impl FromInner for File { diff --git a/src/libstd/sys/windows/handle.rs b/src/libstd/sys/windows/handle.rs index c3a30aae9e0e8..9481e180ce578 100644 --- a/src/libstd/sys/windows/handle.rs +++ b/src/libstd/sys/windows/handle.rs @@ -12,6 +12,7 @@ use prelude::v1::*; use io::ErrorKind; use io; +use libc::funcs::extra::kernel32::{GetCurrentProcess, DuplicateHandle}; use libc::{self, HANDLE}; use mem; use ptr; @@ -65,6 +66,18 @@ impl Handle { })); Ok(amt as usize) } + + pub fn duplicate(&self, access: libc::DWORD, inherit: bool, + options: libc::DWORD) -> io::Result { + let mut ret = 0 as libc::HANDLE; + try!(cvt(unsafe { + let cur_proc = GetCurrentProcess(); + DuplicateHandle(cur_proc, self.0, cur_proc, &mut ret, + access, inherit as libc::BOOL, + options) + })); + Ok(Handle::new(ret)) + } } impl Drop for Handle { diff --git a/src/libstd/sys/windows/process2.rs b/src/libstd/sys/windows/process2.rs index 5ddcf3d1ea299..470e5891ee9d6 100644 --- a/src/libstd/sys/windows/process2.rs +++ b/src/libstd/sys/windows/process2.rs @@ -13,17 +13,23 @@ use prelude::v1::*; use ascii::*; use collections::HashMap; use collections; +use env::split_paths; use env; use ffi::{OsString, OsStr}; use fmt; use fs; use io::{self, Error}; use libc::{self, c_void}; +use mem; use os::windows::ffi::OsStrExt; +use path::Path; use ptr; use sync::{StaticMutex, MUTEX_INIT}; +use sys::c; +use sys::fs2::{OpenOptions, File}; use sys::handle::Handle; use sys::pipe2::AnonPipe; +use sys::stdio; use sys::{self, cvt}; use sys_common::{AsInner, FromInner}; @@ -90,18 +96,12 @@ impl Command { // Processes //////////////////////////////////////////////////////////////////////////////// -// `CreateProcess` is racy! -// http://support.microsoft.com/kb/315939 -static CREATE_PROCESS_LOCK: StaticMutex = MUTEX_INIT; - /// A value representing a child process. /// /// The lifetime of this value is linked to the lifetime of the actual /// process - the Process destructor calls self.finish() which waits /// for the process to terminate. pub struct Process { - /// A HANDLE to the process, which will prevent the pid being - /// re-used until the handle is closed. handle: Handle, } @@ -112,32 +112,17 @@ pub enum Stdio { } impl Process { - #[allow(deprecated)] pub fn spawn(cfg: &Command, - in_fd: Stdio, - out_fd: Stdio, - err_fd: Stdio) -> io::Result + in_handle: Stdio, + out_handle: Stdio, + err_handle: Stdio) -> io::Result { - use libc::types::os::arch::extra::{DWORD, HANDLE, STARTUPINFO}; - use libc::consts::os::extra::{ - TRUE, FALSE, - STARTF_USESTDHANDLES, - INVALID_HANDLE_VALUE, - DUPLICATE_SAME_ACCESS - }; - use libc::funcs::extra::kernel32::{ - GetCurrentProcess, - DuplicateHandle, - CloseHandle, - CreateProcessW - }; - - use env::split_paths; - use mem; - use iter::Iterator; - - // To have the spawning semantics of unix/windows stay the same, we need to - // read the *child's* PATH if one is provided. See #15149 for more details. + use libc::{TRUE, STARTF_USESTDHANDLES}; + use libc::{DWORD, STARTUPINFO, CreateProcessW}; + + // To have the spawning semantics of unix/windows stay the same, we need + // to read the *child's* PATH if one is provided. See #15149 for more + // details. let program = cfg.env.as_ref().and_then(|env| { for (key, v) in env { if OsStr::new("PATH") != &**key { continue } @@ -156,118 +141,51 @@ impl Process { None }); - unsafe { - let mut si = zeroed_startupinfo(); - si.cb = mem::size_of::() as DWORD; - si.dwFlags = STARTF_USESTDHANDLES; - - let cur_proc = GetCurrentProcess(); - - let set_fd = |fd: &Stdio, slot: &mut HANDLE, - is_stdin: bool| { - match *fd { - Stdio::Inherit => {} - - // Similarly to unix, we don't actually leave holes for the - // stdio file descriptors, but rather open up /dev/null - // equivalents. These equivalents are drawn from libuv's - // windows process spawning. - Stdio::None => { - let access = if is_stdin { - libc::FILE_GENERIC_READ - } else { - libc::FILE_GENERIC_WRITE | libc::FILE_READ_ATTRIBUTES - }; - let size = mem::size_of::(); - let mut sa = libc::SECURITY_ATTRIBUTES { - nLength: size as libc::DWORD, - lpSecurityDescriptor: ptr::null_mut(), - bInheritHandle: 1, - }; - let mut filename: Vec = "NUL".utf16_units().collect(); - filename.push(0); - *slot = libc::CreateFileW(filename.as_ptr(), - access, - libc::FILE_SHARE_READ | - libc::FILE_SHARE_WRITE, - &mut sa, - libc::OPEN_EXISTING, - 0, - ptr::null_mut()); - if *slot == INVALID_HANDLE_VALUE { - return Err(Error::last_os_error()) - } - } - Stdio::Piped(ref pipe) => { - let orig = pipe.handle().raw(); - if DuplicateHandle(cur_proc, orig, cur_proc, slot, - 0, TRUE, DUPLICATE_SAME_ACCESS) == FALSE { - return Err(Error::last_os_error()) - } - } - } - Ok(()) - }; - - try!(set_fd(&in_fd, &mut si.hStdInput, true)); - try!(set_fd(&out_fd, &mut si.hStdOutput, false)); - try!(set_fd(&err_fd, &mut si.hStdError, false)); + let mut si = zeroed_startupinfo(); + si.cb = mem::size_of::() as DWORD; + si.dwFlags = STARTF_USESTDHANDLES; - let mut cmd_str = make_command_line(program.as_ref().unwrap_or(&cfg.program), - &cfg.args); - cmd_str.push(0); // add null terminator + let stdin = try!(in_handle.to_handle(c::STD_INPUT_HANDLE)); + let stdout = try!(out_handle.to_handle(c::STD_OUTPUT_HANDLE)); + let stderr = try!(err_handle.to_handle(c::STD_ERROR_HANDLE)); - let mut pi = zeroed_process_information(); - let mut create_err = None; - - // stolen from the libuv code. - let mut flags = libc::CREATE_UNICODE_ENVIRONMENT; - if cfg.detach { - flags |= libc::DETACHED_PROCESS | libc::CREATE_NEW_PROCESS_GROUP; - } + si.hStdInput = stdin.raw(); + si.hStdOutput = stdout.raw(); + si.hStdError = stderr.raw(); - with_envp(cfg.env.as_ref(), |envp| { - with_dirp(cfg.cwd.as_ref(), |dirp| { - let _lock = CREATE_PROCESS_LOCK.lock().unwrap(); - let created = CreateProcessW(ptr::null(), - cmd_str.as_mut_ptr(), - ptr::null_mut(), - ptr::null_mut(), - TRUE, - flags, envp, dirp, - &mut si, &mut pi); - if created == FALSE { - create_err = Some(Error::last_os_error()); - } - }) - }); + let program = program.as_ref().unwrap_or(&cfg.program); + let mut cmd_str = make_command_line(program, &cfg.args); + cmd_str.push(0); // add null terminator - if !in_fd.inherited() { - assert!(CloseHandle(si.hStdInput) != 0); - } - if !out_fd.inherited() { - assert!(CloseHandle(si.hStdOutput) != 0); - } - if !err_fd.inherited() { - assert!(CloseHandle(si.hStdError) != 0); - } + // stolen from the libuv code. + let mut flags = libc::CREATE_UNICODE_ENVIRONMENT; + if cfg.detach { + flags |= libc::DETACHED_PROCESS | libc::CREATE_NEW_PROCESS_GROUP; + } - match create_err { - Some(err) => return Err(err), - None => {} - } + let (envp, _data) = make_envp(cfg.env.as_ref()); + let (dirp, _data) = make_dirp(cfg.cwd.as_ref()); + let mut pi = zeroed_process_information(); + try!(unsafe { + // `CreateProcess` is racy! + // http://support.microsoft.com/kb/315939 + static CREATE_PROCESS_LOCK: StaticMutex = MUTEX_INIT; + let _lock = CREATE_PROCESS_LOCK.lock(); + + cvt(CreateProcessW(ptr::null(), + cmd_str.as_mut_ptr(), + ptr::null_mut(), + ptr::null_mut(), + TRUE, flags, envp, dirp, + &mut si, &mut pi)) + }); - // We close the thread handle because we don't care about keeping the - // thread id valid, and we aren't keeping the thread handle around to be - // able to close it later. We don't close the process handle however - // because std::we want the process id to stay valid at least until the - // calling code closes the process handle. - assert!(CloseHandle(pi.hThread) != 0); + // We close the thread handle because we don't care about keeping + // the thread id valid, and we aren't keeping the thread handle + // around to be able to close it later. + drop(Handle::new(pi.hThread)); - Ok(Process { - handle: Handle::new(pi.hProcess) - }) - } + Ok(Process { handle: Handle::new(pi.hProcess) }) } pub unsafe fn kill(&self) -> io::Result<()> { @@ -276,45 +194,25 @@ impl Process { } pub fn wait(&self) -> io::Result { - use libc::consts::os::extra::{ - FALSE, - STILL_ACTIVE, - INFINITE, - WAIT_OBJECT_0, - }; - use libc::funcs::extra::kernel32::{ - GetExitCodeProcess, - WaitForSingleObject, - }; + use libc::{STILL_ACTIVE, INFINITE, WAIT_OBJECT_0}; + use libc::{GetExitCodeProcess, WaitForSingleObject}; unsafe { loop { let mut status = 0; - if GetExitCodeProcess(self.handle.raw(), &mut status) == FALSE { - let err = Err(Error::last_os_error()); - return err; - } + try!(cvt(GetExitCodeProcess(self.handle.raw(), &mut status))); if status != STILL_ACTIVE { return Ok(ExitStatus(status as i32)); } match WaitForSingleObject(self.handle.raw(), INFINITE) { WAIT_OBJECT_0 => {} - _ => { - let err = Err(Error::last_os_error()); - return err - } + _ => return Err(Error::last_os_error()), } } } } } -impl Stdio { - fn inherited(&self) -> bool { - match *self { Stdio::Inherit => true, _ => false } - } -} - #[derive(PartialEq, Eq, Clone, Copy, Debug)] pub struct ExitStatus(i32); @@ -416,9 +314,8 @@ fn make_command_line(prog: &OsStr, args: &[OsString]) -> Vec { } } -fn with_envp(env: Option<&collections::HashMap>, cb: F) -> T - where F: FnOnce(*mut c_void) -> T, -{ +fn make_envp(env: Option<&collections::HashMap>) + -> (*mut c_void, Vec) { // On Windows we pass an "environment block" which is not a char**, but // rather a concatenation of null-terminated k=v\0 sequences, with a final // \0 to terminate. @@ -433,22 +330,57 @@ fn with_envp(env: Option<&collections::HashMap>, cb: F blk.push(0); } blk.push(0); - cb(blk.as_mut_ptr() as *mut c_void) + (blk.as_mut_ptr() as *mut c_void, blk) } - _ => cb(ptr::null_mut()) + _ => (ptr::null_mut(), Vec::new()) } } -fn with_dirp(d: Option<&OsString>, cb: F) -> T where - F: FnOnce(*const u16) -> T, -{ +fn make_dirp(d: Option<&OsString>) -> (*const u16, Vec) { match d { - Some(dir) => { - let mut dir_str: Vec = dir.encode_wide().collect(); - dir_str.push(0); - cb(dir_str.as_ptr()) - }, - None => cb(ptr::null()) + Some(dir) => { + let mut dir_str: Vec = dir.encode_wide().collect(); + dir_str.push(0); + (dir_str.as_ptr(), dir_str) + }, + None => (ptr::null(), Vec::new()) + } +} + +impl Stdio { + fn to_handle(&self, stdio_id: libc::DWORD) -> io::Result { + use libc::DUPLICATE_SAME_ACCESS; + + match *self { + Stdio::Inherit => { + stdio::get(stdio_id).and_then(|io| { + io.handle().duplicate(0, true, DUPLICATE_SAME_ACCESS) + }) + } + Stdio::Piped(ref pipe) => { + pipe.handle().duplicate(0, true, DUPLICATE_SAME_ACCESS) + } + + // Similarly to unix, we don't actually leave holes for the + // stdio file descriptors, but rather open up /dev/null + // equivalents. These equivalents are drawn from libuv's + // windows process spawning. + Stdio::None => { + let size = mem::size_of::(); + let mut sa = libc::SECURITY_ATTRIBUTES { + nLength: size as libc::DWORD, + lpSecurityDescriptor: ptr::null_mut(), + bInheritHandle: 1, + }; + let mut opts = OpenOptions::new(); + opts.read(stdio_id == c::STD_INPUT_HANDLE); + opts.write(stdio_id != c::STD_INPUT_HANDLE); + opts.security_attributes(&mut sa); + File::open(Path::new("NUL"), &opts).map(|file| { + file.into_handle() + }) + } + } } } diff --git a/src/libstd/sys/windows/stdio.rs b/src/libstd/sys/windows/stdio.rs index 91f6f328ff6e0..03547165f5d87 100644 --- a/src/libstd/sys/windows/stdio.rs +++ b/src/libstd/sys/windows/stdio.rs @@ -21,9 +21,9 @@ use sys::c; use sys::cvt; use sys::handle::Handle; -struct NoClose(Option); +pub struct NoClose(Option); -enum Output { +pub enum Output { Console(NoClose), Pipe(NoClose), } @@ -35,7 +35,7 @@ pub struct Stdin { pub struct Stdout(Output); pub struct Stderr(Output); -fn get(handle: libc::DWORD) -> io::Result { +pub fn get(handle: libc::DWORD) -> io::Result { let handle = unsafe { c::GetStdHandle(handle) }; if handle == libc::INVALID_HANDLE_VALUE { Err(io::Error::last_os_error()) @@ -159,6 +159,16 @@ impl Drop for NoClose { } } +impl Output { + pub fn handle(&self) -> &Handle { + let nc = match *self { + Output::Console(ref c) => c, + Output::Pipe(ref c) => c, + }; + nc.0.as_ref().unwrap() + } +} + fn invalid_encoding() -> io::Error { io::Error::new(io::ErrorKind::InvalidInput, "text was not valid unicode") } From dc81472679fa54002b6cabaedc1dfa5146453a3d Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Tue, 28 Apr 2015 17:47:16 +0200 Subject: [PATCH 3/8] Fix #24895. [breaking-change] What does this break? Basically, code that implements `Drop` and is using `T:Copy` for one of its type parameters and is relying on the Drop Check rule not applying to it. Here is an example: ```rust #![allow(dead_code,unused_variables,unused_assignments)] struct D(T); impl Drop for D { fn drop(&mut self) { } } trait UserT { fn c(&self) { } } impl UserT for T { } struct E(T); impl Drop for E { fn drop(&mut self) { } } // This one will start breaking. fn foo() { let (d2, d1); d1 = D(34); d2 = D(&d1); } #[cfg(this_one_does_and_should_always_break)] fn bar() { let (e2, e1); e1 = E(34); e2 = E(&e1); } fn main() { foo(); } ``` --- src/librustc_typeck/check/dropck.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_typeck/check/dropck.rs b/src/librustc_typeck/check/dropck.rs index 2f7e0073e1751..008ba1c6bf83e 100644 --- a/src/librustc_typeck/check/dropck.rs +++ b/src/librustc_typeck/check/dropck.rs @@ -464,9 +464,9 @@ fn iterate_over_potentially_unsafe_regions_in_type<'a, 'tcx>( ty::Predicate::Trait(ty::Binder(ref t_pred)) => { let def_id = t_pred.trait_ref.def_id; match rcx.tcx().lang_items.to_builtin_kind(def_id) { + // Issue 24895: deliberately do not include `BoundCopy` here. Some(ty::BoundSend) | Some(ty::BoundSized) | - Some(ty::BoundCopy) | Some(ty::BoundSync) => false, _ => true, } From 403bb254b53f923f90b0136ce9cfdbcc8a6cd9b6 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Tue, 28 Apr 2015 17:51:08 +0200 Subject: [PATCH 4/8] regression test for Issue 24895. --- .../issue-24895-copy-clone-dropck.rs | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 src/test/compile-fail/issue-24895-copy-clone-dropck.rs diff --git a/src/test/compile-fail/issue-24895-copy-clone-dropck.rs b/src/test/compile-fail/issue-24895-copy-clone-dropck.rs new file mode 100644 index 0000000000000..2883511736920 --- /dev/null +++ b/src/test/compile-fail/issue-24895-copy-clone-dropck.rs @@ -0,0 +1,38 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// 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. + +// Check that one cannot subvert Drop Check rule via a user-defined +// Clone implementation. + +#![allow(unused_variables, unused_assignments)] + +struct D(T, &'static str); + +#[derive(Copy)] +struct S<'a>(&'a D, &'static str); +impl<'a> Clone for S<'a> { + fn clone(&self) -> S<'a> { + println!("cloning `S(_, {})` and thus accessing: {}", self.1, (self.0).0); + S(self.0, self.1) + } +} + +impl Drop for D { + fn drop(&mut self) { + println!("calling Drop for {}", self.1); + let _call = self.0.clone(); + } +} + +fn main() { + let (d2, d1); + d1 = D(34, "d1"); + d2 = D(S(&d1, "inner"), "d2"); //~ ERROR `d1` does not live long enough +} From e45a9190d72e612a89a8cbc12c3fae0679dd97c7 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 29 Apr 2015 10:21:29 +0200 Subject: [PATCH 5/8] Fix zero-normalization of the pos of a `MultiByteChar`. Fix #24687 --- src/librustc/metadata/creader.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/metadata/creader.rs b/src/librustc/metadata/creader.rs index 802c5815398f2..241d8fd00252c 100644 --- a/src/librustc/metadata/creader.rs +++ b/src/librustc/metadata/creader.rs @@ -664,7 +664,7 @@ fn import_codemap(local_codemap: &codemap::CodeMap, .into_inner() .map_in_place(|mbc| codemap::MultiByteChar { - pos: mbc.pos + start_pos, + pos: mbc.pos - start_pos, bytes: mbc.bytes }); From a4043756a5242892544fead270f825c05fffebbb Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 29 Apr 2015 10:21:47 +0200 Subject: [PATCH 6/8] Regression test for issue 24687. use visible characters for the multibyte character filler. --- src/test/auxiliary/issue24687_lib.rs | 20 ++++++++++ .../auxiliary/issue24687_mbcs_in_comments.rs | 37 +++++++++++++++++++ .../run-pass/issue24687-embed-debuginfo.rs | 21 +++++++++++ 3 files changed, 78 insertions(+) create mode 100644 src/test/auxiliary/issue24687_lib.rs create mode 100644 src/test/auxiliary/issue24687_mbcs_in_comments.rs create mode 100644 src/test/run-pass/issue24687-embed-debuginfo.rs diff --git a/src/test/auxiliary/issue24687_lib.rs b/src/test/auxiliary/issue24687_lib.rs new file mode 100644 index 0000000000000..f1624fd2e58b0 --- /dev/null +++ b/src/test/auxiliary/issue24687_lib.rs @@ -0,0 +1,20 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// 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. + +#![crate_type="lib"] + +// This is a file that pulls in a separate file as a submodule, where +// that separate file has many multi-byte characters, to try to +// encourage the compiler to trip on them. + +mod issue24687_mbcs_in_comments; + +pub use issue24687_mbcs_in_comments::D; + diff --git a/src/test/auxiliary/issue24687_mbcs_in_comments.rs b/src/test/auxiliary/issue24687_mbcs_in_comments.rs new file mode 100644 index 0000000000000..8dc243aed7e1e --- /dev/null +++ b/src/test/auxiliary/issue24687_mbcs_in_comments.rs @@ -0,0 +1,37 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// 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 std::fmt; + +// This ia file with many multi-byte characters, to try to encourage +// the compiler to trip on them. The Drop implementation below will +// need to have its source location embedded into the debug info for +// the output file. + +// αααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααα +// ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ +// γγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγγ +// δδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδδ +// εεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεεε + +// ζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζζ +// ηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηηη +// θθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθθ +// ιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιιι +// κκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκκ + +pub struct D(pub X); + +impl Drop for D { + fn drop(&mut self) { + // λλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλλ + println!("Dropping D({:?})", self.0); + } +} diff --git a/src/test/run-pass/issue24687-embed-debuginfo.rs b/src/test/run-pass/issue24687-embed-debuginfo.rs new file mode 100644 index 0000000000000..ad30d53f1a65f --- /dev/null +++ b/src/test/run-pass/issue24687-embed-debuginfo.rs @@ -0,0 +1,21 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// 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. + +// aux-build:issue24687_lib.rs + +extern crate issue24687_lib as d; + +fn main() { + // Create a d, which has a destructor whose body will be trans'ed + // into the generated code here, and thus the local debuginfo will + // need references into the original source locations from + // `importer` above. + let _d = d::D("Hi"); +} From 344b79c1552262b305f403234382f345662313af Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 29 Apr 2015 11:37:19 +0200 Subject: [PATCH 7/8] lint for mixing `#[repr(C)]` with an impl of `Drop`. --- src/librustc_lint/builtin.rs | 54 ++++++++++++++++++++++++++++++++++++ src/librustc_lint/lib.rs | 1 + 2 files changed, 55 insertions(+) diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index bdc3fdcfc1415..176ed9d97ff9a 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -2107,3 +2107,57 @@ impl LintPass for UnstableFeatures { } } } + +/// Lints for attempts to impl Drop on types that have `#[repr(C)]` +/// attribute (see issue #24585). +#[derive(Copy, Clone)] +pub struct DropWithReprExtern; + +declare_lint! { + DROP_WITH_REPR_EXTERN, + Warn, + "use of #[repr(C)] on a type that implements Drop" +} + +impl LintPass for DropWithReprExtern { + fn get_lints(&self) -> LintArray { + lint_array!(DROP_WITH_REPR_EXTERN) + } + fn check_crate(&mut self, ctx: &Context, _: &ast::Crate) { + for dtor_did in ctx.tcx.destructors.borrow().iter() { + let (drop_impl_did, dtor_self_type) = + if dtor_did.krate == ast::LOCAL_CRATE { + let impl_did = ctx.tcx.map.get_parent_did(dtor_did.node); + let ty = ty::lookup_item_type(ctx.tcx, impl_did).ty; + (impl_did, ty) + } else { + continue; + }; + + match dtor_self_type.sty { + ty::ty_enum(self_type_did, _) | + ty::ty_struct(self_type_did, _) | + ty::ty_closure(self_type_did, _) => { + let hints = ty::lookup_repr_hints(ctx.tcx, self_type_did); + if hints.iter().any(|attr| *attr == attr::ReprExtern) && + ty::ty_dtor(ctx.tcx, self_type_did).has_drop_flag() { + let drop_impl_span = ctx.tcx.map.def_id_span(drop_impl_did, + codemap::DUMMY_SP); + let self_defn_span = ctx.tcx.map.def_id_span(self_type_did, + codemap::DUMMY_SP); + ctx.span_lint(DROP_WITH_REPR_EXTERN, + drop_impl_span, + "implementing Drop adds hidden state to types, \ + possibly conflicting with `#[repr(C)]`"); + // FIXME #19668: could be span_lint_note instead of manual guard. + if ctx.current_level(DROP_WITH_REPR_EXTERN) != Level::Allow { + ctx.sess().span_note(self_defn_span, + "the `#[repr(C)]` attribute is attached here"); + } + } + } + _ => {} + } + } + } +} diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index 34f7436d0cd5d..0b13382f15483 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -109,6 +109,7 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) { UnconditionalRecursion, InvalidNoMangleItems, PluginAsLibrary, + DropWithReprExtern, ); add_builtin_with_new!(sess, From 76a9a6f6dd71b145b44464e056d74fc18e08d011 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 29 Apr 2015 12:08:53 +0200 Subject: [PATCH 8/8] tests for lint that warns about mixing #[repr(C)] with Drop. THis includes tests for struct and enum. (I suspect the closure case is actually unreachable, but i see no harm in including it.) --- .../lint-no-drop-on-repr-extern.rs | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 src/test/compile-fail/lint-no-drop-on-repr-extern.rs diff --git a/src/test/compile-fail/lint-no-drop-on-repr-extern.rs b/src/test/compile-fail/lint-no-drop-on-repr-extern.rs new file mode 100644 index 0000000000000..2df57b08f283c --- /dev/null +++ b/src/test/compile-fail/lint-no-drop-on-repr-extern.rs @@ -0,0 +1,59 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// 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. + +// Check we reject structs that mix a `Drop` impl with `#[repr(C)]`. +// +// As a special case, also check that we do not warn on such structs +// if they also are declared with `#[unsafe_no_drop_flag]` + +#![feature(unsafe_no_drop_flag)] +#![deny(drop_with_repr_extern)] + +#[repr(C)] struct As { x: Box } +#[repr(C)] enum Ae { Ae(Box), _None } + +struct Bs { x: Box } +enum Be { Be(Box), _None } + +#[repr(C)] struct Cs { x: Box } +//~^ NOTE the `#[repr(C)]` attribute is attached here + +impl Drop for Cs { fn drop(&mut self) { } } +//~^ ERROR implementing Drop adds hidden state to types, possibly conflicting with `#[repr(C)]` + +#[repr(C)] enum Ce { Ce(Box), _None } +//~^ NOTE the `#[repr(C)]` attribute is attached here + +impl Drop for Ce { fn drop(&mut self) { } } +//~^ ERROR implementing Drop adds hidden state to types, possibly conflicting with `#[repr(C)]` + +#[unsafe_no_drop_flag] +#[repr(C)] struct Ds { x: Box } + +impl Drop for Ds { fn drop(&mut self) { } } + +#[unsafe_no_drop_flag] +#[repr(C)] enum De { De(Box), _None } + +impl Drop for De { fn drop(&mut self) { } } + +fn main() { + let a = As { x: Box::new(3) }; + let b = Bs { x: Box::new(3) }; + let c = Cs { x: Box::new(3) }; + let d = Ds { x: Box::new(3) }; + + println!("{:?}", (*a.x, *b.x, *c.x, *d.x)); + + let _a = Ae::Ae(Box::new(3)); + let _b = Be::Be(Box::new(3)); + let _c = Ce::Ce(Box::new(3)); + let _d = De::De(Box::new(3)); +}