Skip to content

allow constant evaluation of function calls #26848

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.

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

Merged
merged 3 commits into from
Oct 27, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion src/librustc/middle/check_const.rs
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for CheckCrateVisitor<'a, 'tcx> {
ty::TyUint(_) | ty::TyInt(_) if div_or_rem => {
if !self.qualif.intersects(ConstQualif::NOT_CONST) {
match const_eval::eval_const_expr_partial(
self.tcx, ex, ExprTypeChecked) {
self.tcx, ex, ExprTypeChecked, None) {
Ok(_) => {}
Err(msg) => {
self.tcx.sess.add_lint(::lint::builtin::CONST_ERR, ex.id,
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ fn check_for_bindings_named_the_same_as_variants(cx: &MatchCheckCtxt, pat: &Pat)
fn check_for_static_nan(cx: &MatchCheckCtxt, pat: &Pat) {
front_util::walk_pat(pat, |p| {
if let hir::PatLit(ref expr) = p.node {
match eval_const_expr_partial(cx.tcx, &**expr, ExprTypeChecked) {
match eval_const_expr_partial(cx.tcx, &**expr, ExprTypeChecked, None) {
Ok(ConstVal::Float(f)) if f.is_nan() => {
span_warn!(cx.tcx.sess, p.span, E0003,
"unmatchable NaN in pattern, \
Expand Down
113 changes: 95 additions & 18 deletions src/librustc/middle/const_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,14 @@ use front::map::blocks::FnLikeNode;
use metadata::csearch;
use metadata::inline::InlinedItem;
use middle::{astencode, def, infer, subst, traits};
use middle::def_id::{DefId};
use middle::def_id::DefId;
use middle::pat_util::def_to_path;
use middle::ty::{self, Ty};
use middle::astconv_util::ast_ty_to_prim_ty;
use util::num::ToPrimitive;
use util::nodemap::NodeMap;

use syntax::ast;
use syntax::{ast, abi};
use rustc_front::hir::Expr;
use rustc_front::hir;
use rustc_front::visit::FnKind;
Expand Down Expand Up @@ -253,14 +254,14 @@ pub enum ConstVal {
Bool(bool),
Struct(ast::NodeId),
Tuple(ast::NodeId),
Function(DefId),
}

/// Note that equality for `ConstVal` means that the it is the same
/// constant, not that the rust values are equal. In particular, `NaN
/// == NaN` (at least if it's the same NaN; distinct encodings for NaN
/// are considering unequal).
impl PartialEq for ConstVal {
#[stable(feature = "rust1", since = "1.0.0")]
fn eq(&self, other: &ConstVal) -> bool {
match (self, other) {
(&Float(a), &Float(b)) => unsafe{transmute::<_,u64>(a) == transmute::<_,u64>(b)},
Expand All @@ -271,6 +272,7 @@ impl PartialEq for ConstVal {
(&Bool(a), &Bool(b)) => a == b,
(&Struct(a), &Struct(b)) => a == b,
(&Tuple(a), &Tuple(b)) => a == b,
(&Function(a), &Function(b)) => a == b,
_ => false,
}
}
Expand All @@ -288,6 +290,7 @@ impl ConstVal {
Bool(_) => "boolean",
Struct(_) => "struct",
Tuple(_) => "tuple",
Function(_) => "function definition",
}
}
}
Expand Down Expand Up @@ -350,12 +353,13 @@ pub fn const_expr_to_pat(tcx: &ty::ctxt, expr: &Expr, span: Span) -> P<hir::Pat>
}

pub fn eval_const_expr(tcx: &ty::ctxt, e: &Expr) -> ConstVal {
match eval_const_expr_partial(tcx, e, ExprTypeChecked) {
match eval_const_expr_partial(tcx, e, ExprTypeChecked, None) {
Ok(r) => r,
Err(s) => tcx.sess.span_fatal(s.span, &s.description())
}
}

pub type FnArgMap<'a> = Option<&'a NodeMap<ConstVal>>;

#[derive(Clone)]
pub struct ConstEvalErr {
Expand Down Expand Up @@ -739,7 +743,8 @@ pub_fn_checked_op!{ const_uint_checked_shr_via_int(a: u64, b: i64,.. UintTy) {
/// computing the length of an array. (See also the FIXME above EvalHint.)
pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>,
e: &Expr,
ty_hint: EvalHint<'tcx>) -> EvalResult {
ty_hint: EvalHint<'tcx>,
fn_args: FnArgMap) -> EvalResult {
fn fromb(b: bool) -> ConstVal { Int(b as i64) }

// Try to compute the type of the expression based on the EvalHint.
Expand Down Expand Up @@ -776,7 +781,7 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>,

let result = match e.node {
hir::ExprUnary(hir::UnNeg, ref inner) => {
match try!(eval_const_expr_partial(tcx, &**inner, ty_hint)) {
match try!(eval_const_expr_partial(tcx, &**inner, ty_hint, fn_args)) {
Float(f) => Float(-f),
Int(n) => try!(const_int_checked_neg(n, e, expr_int_type)),
Uint(i) => {
Expand All @@ -786,7 +791,7 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>,
}
}
hir::ExprUnary(hir::UnNot, ref inner) => {
match try!(eval_const_expr_partial(tcx, &**inner, ty_hint)) {
match try!(eval_const_expr_partial(tcx, &**inner, ty_hint, fn_args)) {
Int(i) => Int(!i),
Uint(i) => const_uint_not(i, expr_uint_type),
Bool(b) => Bool(!b),
Expand All @@ -804,8 +809,8 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>,
}
_ => ty_hint
};
match (try!(eval_const_expr_partial(tcx, &**a, ty_hint)),
try!(eval_const_expr_partial(tcx, &**b, b_ty))) {
match (try!(eval_const_expr_partial(tcx, &**a, ty_hint, fn_args)),
try!(eval_const_expr_partial(tcx, &**b, b_ty, fn_args))) {
(Float(a), Float(b)) => {
match op.node {
hir::BiAdd => Float(a + b),
Expand Down Expand Up @@ -912,7 +917,7 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>,
}
};

let val = try!(eval_const_expr_partial(tcx, &**base, base_hint));
let val = try!(eval_const_expr_partial(tcx, &**base, base_hint, fn_args));
match cast_const(tcx, val, ety) {
Ok(val) => val,
Err(kind) => return Err(ConstEvalErr { span: e.span, kind: kind }),
Expand Down Expand Up @@ -990,6 +995,16 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>,
Some(def::DefStruct(_)) => {
return Ok(ConstVal::Struct(e.id))
}
Some(def::DefLocal(_, id)) => {
debug!("DefLocal({:?}): {:?}", id, fn_args);
if let Some(val) = fn_args.and_then(|args| args.get(&id)) {
return Ok(val.clone());
} else {
(None, None)
}
},
Some(def::DefFn(id, _)) => return Ok(Function(id)),
// FIXME: implement const methods?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should at least check that the function is const fn - check_const runs long after type collection - and type collection is what triggers the evaluation of array length expressions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that is unfortunate... but for all practical purposes irrelevant, since the function is simply interpreted as const fn and if it hits a non-const value anywhere const_eval bails out anyway. I'd rather keep const eval for all functions, so regular statements calling non-const-fn-but-could-be-const-fn functions with const arguments can also be const evaluated

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does your PR do if it attempts to eval a call to a non const fn whose body is more complex than a single expression?

From my initial reading of the PR, it seems like it is assuming that it is always given a const fn and thus can assume that the block.expr.as_ref().unwrap() is all it needs to look at in the body, in particular it is not (I think) double-checking that the block has no statements...


Update: oh wait, let me double check, but now I'm thinking that all such functions should have already been marked as NOT_CONST from the check_const run.

Update 2: But wait, as eddyb already said, check_const runs long after type collection ... so it still seems like you could mistakenly think you are evaluating a non-const fn correctly but in fact should have flagged the fn as non-const on-the-fly...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea, I misunderstood. I will force const-fn here. This should also un-break all the unit tests :D

_ => (None, None)
};
let const_expr = match const_expr {
Expand All @@ -1007,14 +1022,76 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>,
} else {
ty_hint
};
try!(eval_const_expr_partial(tcx, const_expr, item_hint))
try!(eval_const_expr_partial(tcx, const_expr, item_hint, fn_args))
}
hir::ExprCall(ref callee, ref args) => {
let sub_ty_hint = if let ExprTypeChecked = ty_hint {
ExprTypeChecked
} else {
UncheckedExprNoHint // we cannot reason about UncheckedExprHint here
};
let (
decl,
unsafety,
abi,
block,
constness,
) = match try!(eval_const_expr_partial(tcx, callee, sub_ty_hint, fn_args)) {
Function(did) => if did.is_local() {
match tcx.map.find(did.index.as_u32()) {
Some(ast_map::NodeItem(it)) => match it.node {
hir::ItemFn(
ref decl,
unsafety,
constness,
abi,
_, // ducktype generics? types are funky in const_eval
ref block,
) => (decl, unsafety, abi, block, constness),
_ => signal!(e, NonConstPath),
},
_ => signal!(e, NonConstPath),
}
} else {
signal!(e, NonConstPath)
},
_ => signal!(e, NonConstPath),
};
if let ExprTypeChecked = ty_hint {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eddyb's comment is addressed here

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you for pointing this out.

// no need to check for constness... either check_const
// already forbids this or we const eval over whatever
// we want
} else {
// we don't know much about the function, so we force it to be a const fn
// so compilation will fail later in case the const fn's body is not const
assert_eq!(constness, hir::Constness::Const)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm getting a ICE here with the current nightly on playpen:

thread 'rustc' panicked at 'assertion failed: `(left == right)` (left: `NotConst`, right: `Const`)', ../src/librustc/middle/const_eval.rs:106

in this simple test program:

fn f(x: usize) -> usize {
    x
}

const Y: usize = f(2);

fn main() {
    let z = [0; X];
}

Curiously if you move the const Y... line to the top, then it at least doesn't ICE, although I'd expect to get E0015 as an error, rather than E0307 (you can then get the E0015 by removing the let z = ... line)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't the right place to file a bug report; try https://github.com/rust-lang/rust/issues/new . (You can link to the PR from there if you think it's useful.)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just been playing on my phone on playpen on the bus, and won't have a laptop for at least a week, so was just hoping to put this somewhere that someone would spot it, and they can triage it (otherwise I'll just forget)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed #29587

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks

On Wed, 4 Nov 2015 at 20:46 eefriedman [email protected] wrote:

In src/librustc/middle/const_eval.rs
#26848 (comment):

  •                  },
    
  •                  _ => signal!(e, NonConstPath),
    
  •              }
    
  •          } else {
    
  •              signal!(e, NonConstPath)
    
  •          },
    
  •          _ => signal!(e, NonConstPath),
    
  •      };
    
  •      if let ExprTypeChecked = ty_hint {
    
  •          // no need to check for constness... either check_const
    
  •          // already forbids this or we const eval over whatever
    
  •          // we want
    
  •      } else {
    
  •          // we don't know much about the function, so we force it to be a const fn
    
  •          // so compilation will fail later in case the const fn's body is not const
    
  •          assert_eq!(constness, hir::Constness::Const)
    

Filed #29587 #29587


Reply to this email directly or view it on GitHub
https://github.com/rust-lang/rust/pull/26848/files#r43936838.

}
assert_eq!(decl.inputs.len(), args.len());
assert_eq!(unsafety, hir::Unsafety::Normal);
assert_eq!(abi, abi::Abi::Rust);

let mut call_args = NodeMap();
for (arg, arg_expr) in decl.inputs.iter().zip(args.iter()) {
let arg_val = try!(eval_const_expr_partial(
tcx,
arg_expr,
sub_ty_hint,
fn_args
));
debug!("const call arg: {:?}", arg);
let old = call_args.insert(arg.pat.id, arg_val);
assert!(old.is_none());
}
let result = block.expr.as_ref().unwrap();
debug!("const call({:?})", call_args);
try!(eval_const_expr_partial(tcx, &**result, ty_hint, Some(&call_args)))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this ty_hint is just forwarded, as the type of the call expression is the same as the type of the functions body-expression

},
hir::ExprLit(ref lit) => {
lit_to_const(&**lit, ety)
}
hir::ExprBlock(ref block) => {
match block.expr {
Some(ref expr) => try!(eval_const_expr_partial(tcx, &**expr, ty_hint)),
Some(ref expr) => try!(eval_const_expr_partial(tcx, &**expr, ty_hint, fn_args)),
None => Int(0)
}
}
Expand All @@ -1026,11 +1103,11 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>,
} else {
UncheckedExprNoHint
};
if let Ok(c) = eval_const_expr_partial(tcx, base, base_hint) {
if let Ok(c) = eval_const_expr_partial(tcx, base, base_hint, fn_args) {
if let Tuple(tup_id) = c {
if let hir::ExprTup(ref fields) = tcx.map.expect_expr(tup_id).node {
if index.node < fields.len() {
return eval_const_expr_partial(tcx, &fields[index.node], base_hint)
return eval_const_expr_partial(tcx, &fields[index.node], base_hint, fn_args)
} else {
signal!(e, TupleIndexOutOfBounds);
}
Expand All @@ -1051,14 +1128,14 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>,
} else {
UncheckedExprNoHint
};
if let Ok(c) = eval_const_expr_partial(tcx, base, base_hint) {
if let Ok(c) = eval_const_expr_partial(tcx, base, base_hint, fn_args) {
if let Struct(struct_id) = c {
if let hir::ExprStruct(_, ref fields, _) = tcx.map.expect_expr(struct_id).node {
// Check that the given field exists and evaluate it
// if the idents are compared run-pass/issue-19244 fails
if let Some(f) = fields.iter().find(|f| f.name.node
== field_name.node) {
return eval_const_expr_partial(tcx, &*f.expr, base_hint)
return eval_const_expr_partial(tcx, &*f.expr, base_hint, fn_args)
} else {
signal!(e, MissingStructField);
}
Expand Down Expand Up @@ -1237,14 +1314,14 @@ pub fn compare_const_vals(a: &ConstVal, b: &ConstVal) -> Option<Ordering> {
pub fn compare_lit_exprs<'tcx>(tcx: &ty::ctxt<'tcx>,
a: &Expr,
b: &Expr) -> Option<Ordering> {
let a = match eval_const_expr_partial(tcx, a, ExprTypeChecked) {
let a = match eval_const_expr_partial(tcx, a, ExprTypeChecked, None) {
Ok(a) => a,
Err(e) => {
tcx.sess.span_err(a.span, &e.description());
return None;
}
};
let b = match eval_const_expr_partial(tcx, b, ExprTypeChecked) {
let b = match eval_const_expr_partial(tcx, b, ExprTypeChecked, None) {
Ok(b) => b,
Err(e) => {
tcx.sess.span_err(b.span, &e.description());
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ impl<'tcx> ty::ctxt<'tcx> {
/// Returns the repeat count for a repeating vector expression.
pub fn eval_repeat_count(&self, count_expr: &hir::Expr) -> usize {
let hint = UncheckedExprHint(self.types.usize);
match const_eval::eval_const_expr_partial(self, count_expr, hint) {
match const_eval::eval_const_expr_partial(self, count_expr, hint, None) {
Ok(val) => {
let found = match val {
ConstVal::Uint(count) => return count as usize,
Expand Down
3 changes: 1 addition & 2 deletions src/librustc_lint/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ impl LateLintPass for TypeLimits {
if let ast::LitInt(shift, _) = lit.node { shift >= bits }
else { false }
} else {
match eval_const_expr_partial(cx.tcx, &r, ExprTypeChecked) {
match eval_const_expr_partial(cx.tcx, &r, ExprTypeChecked, None) {
Ok(ConstVal::Int(shift)) => { shift as u64 >= bits },
Ok(ConstVal::Uint(shift)) => { shift >= bits },
_ => { false }
Expand Down Expand Up @@ -674,4 +674,3 @@ impl LateLintPass for ImproperCTypes {
}
}
}

3 changes: 2 additions & 1 deletion src/librustc_mir/tcx/pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,8 @@ impl<'tcx> Mirror<'tcx> for PatNode<'tcx> {
let opt_value =
const_eval::eval_const_expr_partial(
cx.tcx, const_expr,
const_eval::EvalHint::ExprTypeChecked);
const_eval::EvalHint::ExprTypeChecked,
None);
let literal = if let Ok(value) = opt_value {
Literal::Value { value: value }
} else {
Expand Down
3 changes: 1 addition & 2 deletions src/librustc_trans/trans/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -664,10 +664,9 @@ fn const_expr_unadjusted<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
let vinfo = VariantInfo::from_ty(cx.tcx(), bt, None);
adt::const_get_field(cx, &*brepr, bv, vinfo.discr, idx.node)
},

hir::ExprIndex(ref base, ref index) => {
let (bv, bt) = try!(const_expr(cx, &**base, param_substs, fn_args, trueconst));
let iv = match eval_const_expr_partial(cx.tcx(), &index, ExprTypeChecked) {
let iv = match eval_const_expr_partial(cx.tcx(), &index, ExprTypeChecked, None) {
Ok(ConstVal::Int(i)) => i as u64,
Ok(ConstVal::Uint(u)) => u,
_ => cx.sess().span_bug(index.span,
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/astconv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1674,7 +1674,7 @@ pub fn ast_ty_to_ty<'tcx>(this: &AstConv<'tcx>,
}
hir::TyFixedLengthVec(ref ty, ref e) => {
let hint = UncheckedExprHint(tcx.types.usize);
match const_eval::eval_const_expr_partial(tcx, &e, hint) {
match const_eval::eval_const_expr_partial(tcx, &e, hint, None) {
Ok(r) => {
match r {
ConstVal::Int(i) =>
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1148,7 +1148,7 @@ fn convert_enum_def<'tcx>(tcx: &ty::ctxt<'tcx>,
debug!("disr expr, checking {}", pprust::expr_to_string(e));

let hint = UncheckedExprHint(repr_ty);
match const_eval::eval_const_expr_partial(tcx, e, hint) {
match const_eval::eval_const_expr_partial(tcx, e, hint, None) {
Ok(ConstVal::Int(val)) => Some(val as ty::Disr),
Ok(ConstVal::Uint(val)) => Some(val as ty::Disr),
Ok(_) => {
Expand Down
2 changes: 1 addition & 1 deletion src/test/compile-fail/const-eval-span.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
struct S(i32);

const CONSTANT: S = S(0);
//~^ ERROR: constant evaluation error: unsupported constant expr
//~^ ERROR: constant evaluation error: non-constant path in constant expression [E0080]

enum E {
V = CONSTANT,
Expand Down
18 changes: 18 additions & 0 deletions src/test/compile-fail/const-fn-destructuring-arg.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// test that certain things are disallowed in const fn signatures

#![feature(const_fn)]

// no destructuring
const fn i((a, b): (u32, u32)) -> u32 { a + b } //~ ERROR: E0022

fn main() {}
2 changes: 1 addition & 1 deletion src/test/compile-fail/const-fn-stability-calls-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,5 @@ extern crate const_fn_lib;
use const_fn_lib::foo;

fn main() {
let x: [usize; foo()] = []; //~ ERROR unsupported constant expr
let x: [usize; foo()] = []; //~ ERROR non-constant path in constant expr
}
19 changes: 19 additions & 0 deletions src/test/run-pass/const-fn-const-eval.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(const_fn)]

const fn add(x: usize, y: usize) -> usize {
x + y
}

const ARR: [i32; add(1, 2)] = [5, 6, 7];

pub fn main() {}
Loading