Skip to content

Commit a380df8

Browse files
nikomatsakisnikomatsakis
committed
Fix subtle error in caching during kind computation that could cause linear
values to be copied. Rewrite kind computation so that instead of directly computing the kind it computes what kinds of values are present in the type, and then derive kinds based on that. I find this easier to think about. Fixes #4821.
1 parent 6647a34 commit a380df8

31 files changed

+588
-555
lines changed

‎src/libcargo/cargo.rc‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,7 @@ pub fn parse_source(name: ~str, j: &json::Json) -> @Source {
457457
}
458458

459459
match *j {
460-
json::Object(j) => {
460+
json::Object(ref j) => {
461461
let mut url = match j.find(&~"url") {
462462
Some(&json::String(u)) => copy u,
463463
_ => die!(~"needed 'url' field in source")
@@ -563,7 +563,7 @@ pub fn load_one_source_package(src: @Source, p: &json::Object) {
563563

564564
let mut tags = ~[];
565565
match p.find(&~"tags") {
566-
Some(&json::List(js)) => {
566+
Some(&json::List(ref js)) => {
567567
for js.each |j| {
568568
match *j {
569569
json::String(ref j) => tags.grow(1u, j),
@@ -635,11 +635,11 @@ pub fn load_source_packages(c: &Cargo, src: @Source) {
635635
if !os::path_exists(&pkgfile) { return; }
636636
let pkgstr = io::read_whole_file_str(&pkgfile);
637637
match json::from_str(pkgstr.get()) {
638-
Ok(json::List(js)) => {
638+
Ok(json::List(ref js)) => {
639639
for js.each |j| {
640640
match *j {
641-
json::Object(p) => {
642-
load_one_source_package(src, p);
641+
json::Object(ref p) => {
642+
load_one_source_package(src, *p);
643643
}
644644
_ => {
645645
warn(~"malformed source json: " + src.name +

‎src/libcore/str.rs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,16 @@ pub pure fn connect(v: &[~str], sep: &str) -> ~str {
220220
s
221221
}
222222

223+
/// Concatenate a vector of strings, placing a given separator between each
224+
pub pure fn connect_slices(v: &[&str], sep: &str) -> ~str {
225+
let mut s = ~"", first = true;
226+
for vec::each(v) |ss| {
227+
if first { first = false; } else { unsafe { push_str(&mut s, sep); } }
228+
unsafe { push_str(&mut s, *ss) };
229+
}
230+
s
231+
}
232+
223233
/// Given a string, make a new string with repeated copies of it
224234
pub pure fn repeat(ss: &str, nn: uint) -> ~str {
225235
let mut acc = ~"";
@@ -2667,6 +2677,17 @@ mod tests {
26672677
t(~[~"hi"], ~" ", ~"hi");
26682678
}
26692679

2680+
#[test]
2681+
fn test_connect_slices() {
2682+
fn t(v: &[&str], sep: &str, s: &str) {
2683+
assert connect_slices(v, sep) == s.to_str();
2684+
}
2685+
t(["you", "know", "I'm", "no", "good"],
2686+
" ", "you know I'm no good");
2687+
t([], " ", "");
2688+
t(["hi"], " ", "hi");
2689+
}
2690+
26702691
#[test]
26712692
fn test_repeat() {
26722693
assert repeat(~"x", 4) == ~"xxxx";

‎src/librustc/middle/kind.rs‎

Lines changed: 67 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ use middle::freevars;
1515
use middle::lint::{non_implicitly_copyable_typarams, implicit_copies};
1616
use middle::liveness;
1717
use middle::pat_util;
18-
use middle::ty::{Kind, kind_copyable, kind_noncopyable, kind_const};
1918
use middle::ty;
2019
use middle::typeck;
2120
use middle;
@@ -61,26 +60,6 @@ use syntax::{visit, ast_util};
6160

6261
pub const try_adding: &str = "Try adding a move";
6362

64-
pub fn kind_to_str(k: Kind) -> ~str {
65-
let mut kinds = ~[];
66-
67-
if ty::kind_lteq(kind_const(), k) {
68-
kinds.push(~"const");
69-
}
70-
71-
if ty::kind_can_be_copied(k) {
72-
kinds.push(~"copy");
73-
}
74-
75-
if ty::kind_can_be_sent(k) {
76-
kinds.push(~"owned");
77-
} else if ty::kind_is_durable(k) {
78-
kinds.push(~"&static");
79-
}
80-
81-
str::connect(kinds, ~" ")
82-
}
83-
8463
pub type rval_map = HashMap<node_id, ()>;
8564

8665
pub type ctx = {
@@ -119,11 +98,11 @@ type check_fn = fn@(ctx, @freevar_entry);
11998
// closure.
12099
fn with_appropriate_checker(cx: ctx, id: node_id, b: fn(check_fn)) {
121100
fn check_for_uniq(cx: ctx, fv: @freevar_entry) {
122-
// all captured data must be sendable, regardless of whether it is
123-
// moved in or copied in. Note that send implies owned.
101+
// all captured data must be owned, regardless of whether it is
102+
// moved in or copied in.
124103
let id = ast_util::def_id_of_def(fv.def).node;
125104
let var_t = ty::node_id_to_type(cx.tcx, id);
126-
if !check_send(cx, var_t, fv.span) { return; }
105+
if !check_owned(cx, var_t, fv.span) { return; }
127106

128107
// check that only immutable variables are implicitly copied in
129108
check_imm_free_var(cx, fv.def, fv.span);
@@ -281,30 +260,54 @@ fn check_ty(aty: @Ty, cx: ctx, v: visit::vt<ctx>) {
281260
visit::visit_ty(aty, cx, v);
282261
}
283262

284-
pub fn check_bounds(cx: ctx, id: node_id, sp: span,
285-
ty: ty::t, bounds: ty::param_bounds) {
286-
let kind = ty::type_kind(cx.tcx, ty);
287-
let p_kind = ty::param_bounds_to_kind(bounds);
288-
if !ty::kind_lteq(p_kind, kind) {
289-
// If the only reason the kind check fails is because the
290-
// argument type isn't implicitly copyable, consult the warning
291-
// settings to figure out what to do.
292-
let implicit = ty::kind_implicitly_copyable() - ty::kind_copyable();
293-
if ty::kind_lteq(p_kind, kind | implicit) {
294-
cx.tcx.sess.span_lint(
295-
non_implicitly_copyable_typarams,
296-
id, cx.current_item, sp,
297-
~"instantiating copy type parameter with a \
298-
not implicitly copyable type");
299-
} else {
300-
cx.tcx.sess.span_err(
301-
sp,
302-
~"instantiating a type parameter with an incompatible type " +
303-
~"(needs `" + kind_to_str(p_kind) +
304-
~"`, got `" + kind_to_str(kind) +
305-
~"`, missing `" + kind_to_str(p_kind - kind) + ~"`)");
263+
pub fn check_bounds(cx: ctx,
264+
_type_parameter_id: node_id,
265+
sp: span,
266+
ty: ty::t,
267+
bounds: ty::param_bounds)
268+
{
269+
let kind = ty::type_contents(cx.tcx, ty);
270+
let mut missing = ~[];
271+
for bounds.each |bound| {
272+
match *bound {
273+
ty::bound_trait(_) => {
274+
/* Not our job, checking in typeck */
275+
}
276+
277+
ty::bound_copy => {
278+
if !kind.is_copy(cx.tcx) {
279+
missing.push("Copy");
280+
}
281+
}
282+
283+
ty::bound_durable => {
284+
if !kind.is_durable(cx.tcx) {
285+
missing.push("&static");
286+
}
287+
}
288+
289+
ty::bound_owned => {
290+
if !kind.is_owned(cx.tcx) {
291+
missing.push("Owned");
292+
}
293+
}
294+
295+
ty::bound_const => {
296+
if !kind.is_const(cx.tcx) {
297+
missing.push("Const");
298+
}
299+
}
306300
}
307301
}
302+
303+
if !missing.is_empty() {
304+
cx.tcx.sess.span_err(
305+
sp,
306+
fmt!("instantiating a type parameter with an incompatible type \
307+
`%s`, which does not fulfill `%s`",
308+
ty_to_str(cx.tcx, ty),
309+
str::connect_slices(missing, " ")));
310+
}
308311
}
309312

310313
fn is_nullary_variant(cx: ctx, ex: @expr) -> bool {
@@ -342,16 +345,22 @@ fn check_imm_free_var(cx: ctx, def: def, sp: span) {
342345
}
343346

344347
fn check_copy(cx: ctx, ty: ty::t, sp: span, reason: &str) {
345-
let k = ty::type_kind(cx.tcx, ty);
346-
if !ty::kind_can_be_copied(k) {
347-
cx.tcx.sess.span_err(sp, ~"copying a noncopyable value");
348+
debug!("type_contents(%s)=%s",
349+
ty_to_str(cx.tcx, ty),
350+
ty::type_contents(cx.tcx, ty).to_str());
351+
if !ty::type_is_copyable(cx.tcx, ty) {
352+
cx.tcx.sess.span_err(
353+
sp, fmt!("copying a value of non-copyable type `%s`",
354+
ty_to_str(cx.tcx, ty)));
348355
cx.tcx.sess.span_note(sp, fmt!("%s", reason));
349356
}
350357
}
351358

352-
pub fn check_send(cx: ctx, ty: ty::t, sp: span) -> bool {
353-
if !ty::kind_can_be_sent(ty::type_kind(cx.tcx, ty)) {
354-
cx.tcx.sess.span_err(sp, ~"not a sendable value");
359+
pub fn check_owned(cx: ctx, ty: ty::t, sp: span) -> bool {
360+
if !ty::type_is_owned(cx.tcx, ty) {
361+
cx.tcx.sess.span_err(
362+
sp, fmt!("value has non-owned type `%s`",
363+
ty_to_str(cx.tcx, ty)));
355364
false
356365
} else {
357366
true
@@ -360,7 +369,7 @@ pub fn check_send(cx: ctx, ty: ty::t, sp: span) -> bool {
360369

361370
// note: also used from middle::typeck::regionck!
362371
pub fn check_durable(tcx: ty::ctxt, ty: ty::t, sp: span) -> bool {
363-
if !ty::kind_is_durable(ty::type_kind(tcx, ty)) {
372+
if !ty::type_is_durable(tcx, ty) {
364373
match ty::get(ty).sty {
365374
ty::ty_param(*) => {
366375
tcx.sess.span_err(sp, ~"value may contain borrowed \
@@ -403,8 +412,8 @@ pub fn check_durable(tcx: ty::ctxt, ty: ty::t, sp: span) -> bool {
403412
pub fn check_cast_for_escaping_regions(
404413
cx: ctx,
405414
source: @expr,
406-
target: @expr) {
407-
415+
target: @expr)
416+
{
408417
// Determine what type we are casting to; if it is not an trait, then no
409418
// worries.
410419
let target_ty = ty::expr_ty(cx.tcx, target);
@@ -450,13 +459,9 @@ pub fn check_kind_bounds_of_cast(cx: ctx, source: @expr, target: @expr) {
450459
match ty::get(target_ty).sty {
451460
ty::ty_trait(_, _, ty::vstore_uniq) => {
452461
let source_ty = ty::expr_ty(cx.tcx, source);
453-
let source_kind = ty::type_kind(cx.tcx, source_ty);
454-
if !ty::kind_can_be_copied(source_kind) {
455-
cx.tcx.sess.span_err(target.span,
456-
~"uniquely-owned trait objects must be copyable");
457-
}
458-
if !ty::kind_can_be_sent(source_kind) {
459-
cx.tcx.sess.span_err(target.span,
462+
if !ty::type_is_owned(cx.tcx, source_ty) {
463+
cx.tcx.sess.span_err(
464+
target.span,
460465
~"uniquely-owned trait objects must be sendable");
461466
}
462467
}

‎src/librustc/middle/lint.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -925,8 +925,8 @@ fn check_fn_deprecated_modes(tcx: ty::ctxt, fn_ty: ty::t, decl: ast::fn_decl,
925925

926926
ast::infer(_) => {
927927
if tcx.legacy_modes {
928-
let kind = ty::type_kind(tcx, arg_ty.ty);
929-
if !ty::kind_is_safe_for_default_mode(kind) {
928+
let kind = ty::type_contents(tcx, arg_ty.ty);
929+
if !kind.is_safe_for_default_mode(tcx) {
930930
tcx.sess.span_lint(
931931
deprecated_mode, id, id,
932932
span,

‎src/librustc/middle/moves.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,7 @@ impl VisitContext {
374374
* not implicitly copyable.
375375
*/
376376

377-
let result = if ty::type_implicitly_moves(self.tcx, ty) {
377+
let result = if ty::type_moves_by_default(self.tcx, ty) {
378378
MoveInWhole
379379
} else {
380380
Read
@@ -495,7 +495,7 @@ impl VisitContext {
495495
// moves-by-default:
496496
let consume_with = with_fields.any(|tf| {
497497
!fields.any(|f| f.node.ident == tf.ident) &&
498-
ty::type_implicitly_moves(self.tcx, tf.mt.ty)
498+
ty::type_moves_by_default(self.tcx, tf.mt.ty)
499499
});
500500

501501
if consume_with {
@@ -830,7 +830,7 @@ impl VisitContext {
830830
let fvar_ty = ty::node_id_to_type(self.tcx, fvar_def_id);
831831
debug!("fvar_def_id=%? fvar_ty=%s",
832832
fvar_def_id, ppaux::ty_to_str(self.tcx, fvar_ty));
833-
let mode = if ty::type_implicitly_moves(self.tcx, fvar_ty) {
833+
let mode = if ty::type_moves_by_default(self.tcx, fvar_ty) {
834834
CapMove
835835
} else {
836836
CapCopy

0 commit comments

Comments
 (0)