Skip to content

Fix subtle error in caching during kind computation that could cause lin... #4848

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Fix subtle error in caching during kind computation that could cause lin... #4848

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/libcargo/cargo.rc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ pub fn parse_source(name: ~str, j: &json::Json) -> @Source {
}

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

let mut tags = ~[];
match p.find(&~"tags") {
Some(&json::List(js)) => {
Some(&json::List(ref js)) => {
for js.each |j| {
match *j {
json::String(ref j) => tags.grow(1u, j),
Expand Down Expand Up @@ -635,11 +635,11 @@ pub fn load_source_packages(c: &Cargo, src: @Source) {
if !os::path_exists(&pkgfile) { return; }
let pkgstr = io::read_whole_file_str(&pkgfile);
match json::from_str(pkgstr.get()) {
Ok(json::List(js)) => {
Ok(json::List(ref js)) => {
for js.each |j| {
match *j {
json::Object(p) => {
load_one_source_package(src, p);
json::Object(ref p) => {
load_one_source_package(src, *p);
}
_ => {
warn(~"malformed source json: " + src.name +
Expand Down
21 changes: 21 additions & 0 deletions src/libcore/str.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,16 @@ pub pure fn connect(v: &[~str], sep: &str) -> ~str {
s
}

/// Concatenate a vector of strings, placing a given separator between each
pub pure fn connect_slices(v: &[&str], sep: &str) -> ~str {
let mut s = ~"", first = true;
for vec::each(v) |ss| {
if first { first = false; } else { unsafe { push_str(&mut s, sep); } }
unsafe { push_str(&mut s, *ss) };
}
s
}

/// Given a string, make a new string with repeated copies of it
pub pure fn repeat(ss: &str, nn: uint) -> ~str {
let mut acc = ~"";
Expand Down Expand Up @@ -2667,6 +2677,17 @@ mod tests {
t(~[~"hi"], ~" ", ~"hi");
}

#[test]
fn test_connect_slices() {
fn t(v: &[&str], sep: &str, s: &str) {
assert connect_slices(v, sep) == s.to_str();
}
t(["you", "know", "I'm", "no", "good"],
" ", "you know I'm no good");
t([], " ", "");
t(["hi"], " ", "hi");
}

#[test]
fn test_repeat() {
assert repeat(~"x", 4) == ~"xxxx";
Expand Down
129 changes: 67 additions & 62 deletions src/librustc/middle/kind.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ use middle::freevars;
use middle::lint::{non_implicitly_copyable_typarams, implicit_copies};
use middle::liveness;
use middle::pat_util;
use middle::ty::{Kind, kind_copyable, kind_noncopyable, kind_const};
use middle::ty;
use middle::typeck;
use middle;
Expand Down Expand Up @@ -61,26 +60,6 @@ use syntax::{visit, ast_util};

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

pub fn kind_to_str(k: Kind) -> ~str {
let mut kinds = ~[];

if ty::kind_lteq(kind_const(), k) {
kinds.push(~"const");
}

if ty::kind_can_be_copied(k) {
kinds.push(~"copy");
}

if ty::kind_can_be_sent(k) {
kinds.push(~"owned");
} else if ty::kind_is_durable(k) {
kinds.push(~"&static");
}

str::connect(kinds, ~" ")
}

pub type rval_map = HashMap<node_id, ()>;

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

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

pub fn check_bounds(cx: ctx, id: node_id, sp: span,
ty: ty::t, bounds: ty::param_bounds) {
let kind = ty::type_kind(cx.tcx, ty);
let p_kind = ty::param_bounds_to_kind(bounds);
if !ty::kind_lteq(p_kind, kind) {
// If the only reason the kind check fails is because the
// argument type isn't implicitly copyable, consult the warning
// settings to figure out what to do.
let implicit = ty::kind_implicitly_copyable() - ty::kind_copyable();
if ty::kind_lteq(p_kind, kind | implicit) {
cx.tcx.sess.span_lint(
non_implicitly_copyable_typarams,
id, cx.current_item, sp,
~"instantiating copy type parameter with a \
not implicitly copyable type");
} else {
cx.tcx.sess.span_err(
sp,
~"instantiating a type parameter with an incompatible type " +
~"(needs `" + kind_to_str(p_kind) +
~"`, got `" + kind_to_str(kind) +
~"`, missing `" + kind_to_str(p_kind - kind) + ~"`)");
pub fn check_bounds(cx: ctx,
_type_parameter_id: node_id,
sp: span,
ty: ty::t,
bounds: ty::param_bounds)
{
let kind = ty::type_contents(cx.tcx, ty);
let mut missing = ~[];
for bounds.each |bound| {
match *bound {
ty::bound_trait(_) => {
/* Not our job, checking in typeck */
}

ty::bound_copy => {
if !kind.is_copy(cx.tcx) {
missing.push("Copy");
}
}

ty::bound_durable => {
if !kind.is_durable(cx.tcx) {
missing.push("&static");
}
}

ty::bound_owned => {
if !kind.is_owned(cx.tcx) {
missing.push("Owned");
}
}

ty::bound_const => {
if !kind.is_const(cx.tcx) {
missing.push("Const");
}
}
}
}

if !missing.is_empty() {
cx.tcx.sess.span_err(
sp,
fmt!("instantiating a type parameter with an incompatible type \
`%s`, which does not fulfill `%s`",
ty_to_str(cx.tcx, ty),
str::connect_slices(missing, " ")));
}
}

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

fn check_copy(cx: ctx, ty: ty::t, sp: span, reason: &str) {
let k = ty::type_kind(cx.tcx, ty);
if !ty::kind_can_be_copied(k) {
cx.tcx.sess.span_err(sp, ~"copying a noncopyable value");
debug!("type_contents(%s)=%s",
ty_to_str(cx.tcx, ty),
ty::type_contents(cx.tcx, ty).to_str());
if !ty::type_is_copyable(cx.tcx, ty) {
cx.tcx.sess.span_err(
sp, fmt!("copying a value of non-copyable type `%s`",
ty_to_str(cx.tcx, ty)));
cx.tcx.sess.span_note(sp, fmt!("%s", reason));
}
}

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

// note: also used from middle::typeck::regionck!
pub fn check_durable(tcx: ty::ctxt, ty: ty::t, sp: span) -> bool {
if !ty::kind_is_durable(ty::type_kind(tcx, ty)) {
if !ty::type_is_durable(tcx, ty) {
match ty::get(ty).sty {
ty::ty_param(*) => {
tcx.sess.span_err(sp, ~"value may contain borrowed \
Expand Down Expand Up @@ -403,8 +412,8 @@ pub fn check_durable(tcx: ty::ctxt, ty: ty::t, sp: span) -> bool {
pub fn check_cast_for_escaping_regions(
cx: ctx,
source: @expr,
target: @expr) {

target: @expr)
{
// Determine what type we are casting to; if it is not an trait, then no
// worries.
let target_ty = ty::expr_ty(cx.tcx, target);
Expand Down Expand Up @@ -450,13 +459,9 @@ pub fn check_kind_bounds_of_cast(cx: ctx, source: @expr, target: @expr) {
match ty::get(target_ty).sty {
ty::ty_trait(_, _, ty::vstore_uniq) => {
let source_ty = ty::expr_ty(cx.tcx, source);
let source_kind = ty::type_kind(cx.tcx, source_ty);
if !ty::kind_can_be_copied(source_kind) {
cx.tcx.sess.span_err(target.span,
~"uniquely-owned trait objects must be copyable");
}
if !ty::kind_can_be_sent(source_kind) {
cx.tcx.sess.span_err(target.span,
if !ty::type_is_owned(cx.tcx, source_ty) {
cx.tcx.sess.span_err(
target.span,
~"uniquely-owned trait objects must be sendable");
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/middle/lint.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -925,8 +925,8 @@ fn check_fn_deprecated_modes(tcx: ty::ctxt, fn_ty: ty::t, decl: ast::fn_decl,

ast::infer(_) => {
if tcx.legacy_modes {
let kind = ty::type_kind(tcx, arg_ty.ty);
if !ty::kind_is_safe_for_default_mode(kind) {
let kind = ty::type_contents(tcx, arg_ty.ty);
if !kind.is_safe_for_default_mode(tcx) {
tcx.sess.span_lint(
deprecated_mode, id, id,
span,
Expand Down
6 changes: 3 additions & 3 deletions src/librustc/middle/moves.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ impl VisitContext {
* not implicitly copyable.
*/

let result = if ty::type_implicitly_moves(self.tcx, ty) {
let result = if ty::type_moves_by_default(self.tcx, ty) {
MoveInWhole
} else {
Read
Expand Down Expand Up @@ -495,7 +495,7 @@ impl VisitContext {
// moves-by-default:
let consume_with = with_fields.any(|tf| {
!fields.any(|f| f.node.ident == tf.ident) &&
ty::type_implicitly_moves(self.tcx, tf.mt.ty)
ty::type_moves_by_default(self.tcx, tf.mt.ty)
});

if consume_with {
Expand Down Expand Up @@ -830,7 +830,7 @@ impl VisitContext {
let fvar_ty = ty::node_id_to_type(self.tcx, fvar_def_id);
debug!("fvar_def_id=%? fvar_ty=%s",
fvar_def_id, ppaux::ty_to_str(self.tcx, fvar_ty));
let mode = if ty::type_implicitly_moves(self.tcx, fvar_ty) {
let mode = if ty::type_moves_by_default(self.tcx, fvar_ty) {
CapMove
} else {
CapCopy
Expand Down
Loading