Skip to content

Commit de51bc0

Browse files
committed
auto merge of #11979 : FlaPer87/rust/static, r=nikomatsakis
This pull request partially addresses the 2 issues listed before. As part of the work required for this PR, `NonCopyable` was completely removed. This PR also replaces the content of `type_is_pod` with `TypeContents::is_pod`, although `type_is_content` is currently not being used anywhere. I kept it for consistency with the other functions that exist in this module. cc #10834 cc #10577 Proposed static restrictions ===================== Taken from [this](#11979 (comment)) comment. I expect some code that, at a high-level, works like this: - For each *mutable* static item, check that the **type**: - cannot own any value whose type has a dtor - cannot own any values whose type is an owned pointer - For each *immutable* static item, check that the **value**: - does not contain any ~ or box expressions (including ~[1, 2, 3] sort of things, for now) - does not contain a struct literal or call to an enum variant / struct constructor where - the type of the struct/enum is freeze - the type of the struct/enum has a dtor
2 parents 68a92c5 + 59a04f5 commit de51bc0

18 files changed

+458
-93
lines changed

src/librustc/driver/driver.rs

+3
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,9 @@ pub fn phase_3_run_analysis_passes(sess: Session,
301301
// passes are timed inside typeck
302302
let (method_map, vtable_map) = typeck::check_crate(ty_cx, trait_map, krate);
303303

304+
time(time_passes, "check static items", (), |_|
305+
middle::check_static::check_crate(ty_cx, krate));
306+
304307
// These next two const passes can probably be merged
305308
time(time_passes, "const marking", (), |_|
306309
middle::const_eval::process_crate(krate, ty_cx));

src/librustc/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ pub mod middle {
6969
pub mod check_loop;
7070
pub mod check_match;
7171
pub mod check_const;
72+
pub mod check_static;
7273
pub mod lint;
7374
pub mod borrowck;
7475
pub mod dataflow;

src/librustc/middle/borrowck/gather_loans/gather_moves.rs

+1-14
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ fn check_is_legal_to_move_from(bccx: &BorrowckCtxt,
104104
mc::cat_deref(_, _, mc::BorrowedPtr(..)) |
105105
mc::cat_deref(_, _, mc::GcPtr) |
106106
mc::cat_deref(_, _, mc::UnsafePtr(..)) |
107-
mc::cat_upvar(..) |
107+
mc::cat_upvar(..) | mc::cat_static_item |
108108
mc::cat_copied_upvar(mc::CopiedUpvar { onceness: ast::Many, .. }) => {
109109
bccx.span_err(
110110
cmt0.span,
@@ -120,19 +120,6 @@ fn check_is_legal_to_move_from(bccx: &BorrowckCtxt,
120120
true
121121
}
122122

123-
// It seems strange to allow a move out of a static item,
124-
// but what happens in practice is that you have a
125-
// reference to a constant with a type that should be
126-
// moved, like `None::<~int>`. The type of this constant
127-
// is technically `Option<~int>`, which moves, but we know
128-
// that the content of static items will never actually
129-
// contain allocated pointers, so we can just memcpy it.
130-
// Since static items can never have allocated memory,
131-
// this is ok. For now anyhow.
132-
mc::cat_static_item => {
133-
true
134-
}
135-
136123
mc::cat_rvalue(..) |
137124
mc::cat_local(..) |
138125
mc::cat_arg(..) => {

src/librustc/middle/check_static.rs

+159
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// Verifies that the types and values of static items
12+
// are safe. The rules enforced by this module are:
13+
//
14+
// - For each *mutable* static item, it checks that its **type**:
15+
// - doesn't have a destructor
16+
// - doesn't own an owned pointer
17+
//
18+
// - For each *immutable* static item, it checks that its **value**:
19+
// - doesn't own owned, managed pointers
20+
// - doesn't contain a struct literal or a call to an enum variant / struct constructor where
21+
// - the type of the struct/enum is not freeze
22+
// - the type of the struct/enum has a dtor
23+
24+
use middle::ty;
25+
26+
use syntax::ast;
27+
use syntax::codemap::Span;
28+
use syntax::visit::Visitor;
29+
use syntax::visit;
30+
use syntax::print::pprust;
31+
32+
33+
fn safe_type_for_static_mut(cx: ty::ctxt, e: &ast::Expr) -> Option<~str> {
34+
let node_ty = ty::node_id_to_type(cx, e.id);
35+
let tcontents = ty::type_contents(cx, node_ty);
36+
debug!("safe_type_for_static_mut(dtor={}, managed={}, owned={})",
37+
tcontents.has_dtor(), tcontents.owns_managed(), tcontents.owns_owned())
38+
39+
let suffix = if tcontents.has_dtor() {
40+
"destructors"
41+
} else if tcontents.owns_managed() {
42+
"managed pointers"
43+
} else if tcontents.owns_owned() {
44+
"owned pointers"
45+
} else {
46+
return None;
47+
};
48+
49+
Some(format!("mutable static items are not allowed to have {}", suffix))
50+
}
51+
52+
struct CheckStaticVisitor {
53+
tcx: ty::ctxt,
54+
}
55+
56+
pub fn check_crate(tcx: ty::ctxt, krate: &ast::Crate) {
57+
visit::walk_crate(&mut CheckStaticVisitor { tcx: tcx }, krate, false)
58+
}
59+
60+
impl CheckStaticVisitor {
61+
62+
fn report_error(&self, span: Span, result: Option<~str>) -> bool {
63+
match result {
64+
None => { false }
65+
Some(msg) => {
66+
self.tcx.sess.span_err(span, msg);
67+
true
68+
}
69+
}
70+
}
71+
}
72+
73+
impl Visitor<bool> for CheckStaticVisitor {
74+
75+
fn visit_item(&mut self, i: &ast::Item, _is_const: bool) {
76+
debug!("visit_item(item={})", pprust::item_to_str(i));
77+
match i.node {
78+
ast::ItemStatic(_, mutability, expr) => {
79+
match mutability {
80+
ast::MutImmutable => {
81+
self.visit_expr(expr, true);
82+
}
83+
ast::MutMutable => {
84+
self.report_error(expr.span, safe_type_for_static_mut(self.tcx, expr));
85+
}
86+
}
87+
}
88+
_ => { visit::walk_item(self, i, false) }
89+
}
90+
}
91+
92+
/// This method is used to enforce the constraints on
93+
/// immutable static items. It walks through the *value*
94+
/// of the item walking down the expression and evaluating
95+
/// every nested expression. if the expression is not part
96+
/// of a static item, this method does nothing but walking
97+
/// down through it.
98+
fn visit_expr(&mut self, e: &ast::Expr, is_const: bool) {
99+
debug!("visit_expr(expr={})", pprust::expr_to_str(e));
100+
101+
if !is_const {
102+
return visit::walk_expr(self, e, is_const);
103+
}
104+
105+
match e.node {
106+
ast::ExprField(..) | ast::ExprVec(..) |
107+
ast::ExprBlock(..) | ast::ExprTup(..) |
108+
ast::ExprVstore(_, ast::ExprVstoreSlice) => {
109+
visit::walk_expr(self, e, is_const);
110+
}
111+
ast::ExprUnary(ast::UnBox, _) => {
112+
self.tcx.sess.span_err(e.span,
113+
"static items are not allowed to have managed pointers");
114+
}
115+
ast::ExprBox(..) |
116+
ast::ExprUnary(ast::UnUniq, _) |
117+
ast::ExprVstore(_, ast::ExprVstoreUniq) => {
118+
self.tcx.sess.span_err(e.span,
119+
"static items are not allowed to have owned pointers");
120+
}
121+
ast::ExprProc(..) => {
122+
self.report_error(e.span,
123+
Some(~"immutable static items must be `Freeze`"));
124+
return;
125+
}
126+
ast::ExprAddrOf(mutability, _) => {
127+
match mutability {
128+
ast::MutMutable => {
129+
self.report_error(e.span,
130+
Some(~"immutable static items must be `Freeze`"));
131+
return;
132+
}
133+
_ => {}
134+
}
135+
}
136+
_ => {
137+
let node_ty = ty::node_id_to_type(self.tcx, e.id);
138+
139+
match ty::get(node_ty).sty {
140+
ty::ty_struct(did, _) |
141+
ty::ty_enum(did, _) => {
142+
if ty::has_dtor(self.tcx, did) {
143+
self.report_error(e.span,
144+
Some(~"static items are not allowed to have destructors"));
145+
return;
146+
}
147+
if Some(did) == self.tcx.lang_items.no_freeze_bound() {
148+
self.report_error(e.span,
149+
Some(~"immutable static items must be `Freeze`"));
150+
return;
151+
}
152+
}
153+
_ => {}
154+
}
155+
visit::walk_expr(self, e, is_const);
156+
}
157+
}
158+
}
159+
}

src/librustc/middle/ty.rs

+12-55
Original file line numberDiff line numberDiff line change
@@ -1950,6 +1950,10 @@ impl TypeContents {
19501950
self.intersects(TC::OwnsManaged)
19511951
}
19521952

1953+
pub fn owns_owned(&self) -> bool {
1954+
self.intersects(TC::OwnsOwned)
1955+
}
1956+
19531957
pub fn is_freezable(&self, _: ctxt) -> bool {
19541958
!self.intersects(TC::Nonfreezable)
19551959
}
@@ -2012,6 +2016,10 @@ impl TypeContents {
20122016
pub fn inverse(&self) -> TypeContents {
20132017
TypeContents { bits: !self.bits }
20142018
}
2019+
2020+
pub fn has_dtor(&self) -> bool {
2021+
self.intersects(TC::OwnsDtor)
2022+
}
20152023
}
20162024

20172025
impl ops::BitOr<TypeContents,TypeContents> for TypeContents {
@@ -2038,6 +2046,10 @@ impl fmt::Show for TypeContents {
20382046
}
20392047
}
20402048

2049+
pub fn type_has_dtor(cx: ctxt, t: ty::t) -> bool {
2050+
type_contents(cx, t).has_dtor()
2051+
}
2052+
20412053
pub fn type_is_static(cx: ctxt, t: ty::t) -> bool {
20422054
type_contents(cx, t).is_static(cx)
20432055
}
@@ -2624,61 +2636,6 @@ pub fn type_is_machine(ty: t) -> bool {
26242636
}
26252637
}
26262638

2627-
// Whether a type is Plain Old Data -- meaning it does not contain pointers
2628-
// that the cycle collector might care about.
2629-
pub fn type_is_pod(cx: ctxt, ty: t) -> bool {
2630-
let mut result = true;
2631-
match get(ty).sty {
2632-
// Scalar types
2633-
ty_nil | ty_bot | ty_bool | ty_char | ty_int(_) | ty_float(_) | ty_uint(_) |
2634-
ty_ptr(_) | ty_bare_fn(_) => result = true,
2635-
// Boxed types
2636-
ty_box(_) | ty_uniq(_) | ty_closure(_) |
2637-
ty_str(vstore_uniq) |
2638-
ty_vec(_, vstore_uniq) |
2639-
ty_trait(_, _, _, _, _) | ty_rptr(_,_) => result = false,
2640-
// Structural types
2641-
ty_enum(did, ref substs) => {
2642-
let variants = enum_variants(cx, did);
2643-
for variant in (*variants).iter() {
2644-
// FIXME(pcwalton): This is an inefficient way to do this. Don't
2645-
// synthesize a tuple!
2646-
//
2647-
// Perform any type parameter substitutions.
2648-
let tup_ty = mk_tup(cx, variant.args.clone());
2649-
let tup_ty = subst(cx, substs, tup_ty);
2650-
if !type_is_pod(cx, tup_ty) { result = false; }
2651-
}
2652-
}
2653-
ty_tup(ref elts) => {
2654-
for elt in elts.iter() { if !type_is_pod(cx, *elt) { result = false; } }
2655-
}
2656-
ty_str(vstore_fixed(_)) => result = true,
2657-
ty_vec(ref mt, vstore_fixed(_)) | ty_unboxed_vec(ref mt) => {
2658-
result = type_is_pod(cx, mt.ty);
2659-
}
2660-
ty_param(_) => result = false,
2661-
ty_struct(did, ref substs) => {
2662-
let fields = lookup_struct_fields(cx, did);
2663-
result = fields.iter().all(|f| {
2664-
let fty = ty::lookup_item_type(cx, f.id);
2665-
let sty = subst(cx, substs, fty.ty);
2666-
type_is_pod(cx, sty)
2667-
});
2668-
}
2669-
2670-
ty_str(vstore_slice(..)) | ty_vec(_, vstore_slice(..)) => {
2671-
result = false;
2672-
}
2673-
2674-
ty_infer(..) | ty_self(..) | ty_err => {
2675-
cx.sess.bug("non concrete type in type_is_pod");
2676-
}
2677-
}
2678-
2679-
return result;
2680-
}
2681-
26822639
pub fn type_is_enum(ty: t) -> bool {
26832640
match get(ty).sty {
26842641
ty_enum(_, _) => return true,

src/librustc/middle/typeck/coherence.rs

+1
Original file line numberDiff line numberDiff line change
@@ -725,6 +725,7 @@ impl CoherenceChecker {
725725

726726
let self_type = self.get_self_type_for_implementation(*impl_info);
727727
match ty::get(self_type.ty).sty {
728+
ty::ty_enum(type_def_id, _) |
728729
ty::ty_struct(type_def_id, _) => {
729730
let mut destructor_for_type = tcx.destructor_for_type
730731
.borrow_mut();
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// Ensure that moves out of static items is forbidden
12+
13+
use std::kinds::marker;
14+
15+
struct Foo {
16+
foo: int,
17+
nopod: marker::NoPod
18+
}
19+
20+
static BAR: Foo = Foo{foo: 5, nopod: marker::NoPod};
21+
22+
23+
fn test(f: Foo) {
24+
let _f = Foo{foo: 4, ..f};
25+
}
26+
27+
fn main() {
28+
test(BAR); //~ ERROR cannot move out of static item
29+
}

0 commit comments

Comments
 (0)