-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Support generic associated consts #30446
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ use front::map as ast_map; | |
use front::map::blocks::FnLikeNode; | ||
use middle::cstore::{self, CrateStore, InlinedItem}; | ||
use middle::{def, infer, subst, traits}; | ||
use middle::subst::Subst; | ||
use middle::def_id::DefId; | ||
use middle::pat_util::def_to_path; | ||
use middle::ty::{self, Ty}; | ||
|
@@ -48,7 +49,7 @@ fn lookup_const<'a>(tcx: &'a ty::ctxt, e: &Expr) -> Option<&'a Expr> { | |
match opt_def { | ||
Some(def::DefConst(def_id)) | | ||
Some(def::DefAssociatedConst(def_id)) => { | ||
lookup_const_by_id(tcx, def_id, Some(e.id)) | ||
lookup_const_by_id(tcx, def_id, Some(e.id), None) | ||
} | ||
Some(def::DefVariant(enum_def, variant_def, _)) => { | ||
lookup_variant_by_id(tcx, enum_def, variant_def) | ||
|
@@ -88,9 +89,17 @@ fn lookup_variant_by_id<'a>(tcx: &'a ty::ctxt, | |
} | ||
} | ||
|
||
/// * `def_id` is the id of the constant. | ||
/// * `maybe_ref_id` is the id of the expr referencing the constant. | ||
/// * `param_substs` is the monomorphization substitution for the expression. | ||
/// | ||
/// `maybe_ref_id` and `param_substs` are optional and are used for | ||
/// finding substitutions in associated constants. This generally | ||
/// happens in late/trans const evaluation. | ||
pub fn lookup_const_by_id<'a, 'tcx: 'a>(tcx: &'a ty::ctxt<'tcx>, | ||
def_id: DefId, | ||
maybe_ref_id: Option<ast::NodeId>) | ||
maybe_ref_id: Option<ast::NodeId>, | ||
param_substs: Option<&'tcx subst::Substs<'tcx>>) | ||
-> Option<&'tcx Expr> { | ||
if let Some(node_id) = tcx.map.as_local_node_id(def_id) { | ||
match tcx.map.find(node_id) { | ||
|
@@ -111,8 +120,11 @@ pub fn lookup_const_by_id<'a, 'tcx: 'a>(tcx: &'a ty::ctxt<'tcx>, | |
Some(ref_id) => { | ||
let trait_id = tcx.trait_of_item(def_id) | ||
.unwrap(); | ||
let substs = tcx.node_id_item_substs(ref_id) | ||
.substs; | ||
let mut substs = tcx.node_id_item_substs(ref_id) | ||
.substs; | ||
if let Some(param_substs) = param_substs { | ||
substs = substs.subst(tcx, param_substs); | ||
} | ||
resolve_trait_associated_const(tcx, ti, trait_id, | ||
substs) | ||
} | ||
|
@@ -158,8 +170,11 @@ pub fn lookup_const_by_id<'a, 'tcx: 'a>(tcx: &'a ty::ctxt<'tcx>, | |
// a trait-associated const if the caller gives us | ||
// the expression that refers to it. | ||
Some(ref_id) => { | ||
let substs = tcx.node_id_item_substs(ref_id) | ||
.substs; | ||
let mut substs = tcx.node_id_item_substs(ref_id) | ||
.substs; | ||
if let Some(param_substs) = param_substs { | ||
substs = substs.subst(tcx, param_substs); | ||
} | ||
resolve_trait_associated_const(tcx, ti, trait_id, | ||
substs).map(|e| e.id) | ||
} | ||
|
@@ -1013,7 +1028,7 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, | |
_ => (None, None) | ||
} | ||
} else { | ||
(lookup_const_by_id(tcx, def_id, Some(e.id)), None) | ||
(lookup_const_by_id(tcx, def_id, Some(e.id), None), None) | ||
} | ||
} | ||
Some(def::DefAssociatedConst(def_id)) => { | ||
|
@@ -1048,7 +1063,7 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>, | |
}, | ||
} | ||
} else { | ||
(lookup_const_by_id(tcx, def_id, Some(e.id)), None) | ||
(lookup_const_by_id(tcx, def_id, Some(e.id), None), None) | ||
} | ||
} | ||
Some(def::DefVariant(enum_def, variant_def, _)) => { | ||
|
@@ -1260,20 +1275,16 @@ fn resolve_trait_associated_const<'a, 'tcx: 'a>(tcx: &'a ty::ctxt<'tcx>, | |
Ok(None) => { | ||
return None | ||
} | ||
Err(e) => { | ||
tcx.sess.span_bug(ti.span, | ||
&format!("Encountered error `{:?}` when trying \ | ||
to select an implementation for \ | ||
constant trait item reference.", | ||
e)) | ||
Err(_) => { | ||
return None | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @michaelwu why did you change this to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IIRC this was to avoid hitting an error in early const eval. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm. Yes, I suspect calling |
||
} | ||
}; | ||
|
||
match selection { | ||
traits::VtableImpl(ref impl_data) => { | ||
match tcx.associated_consts(impl_data.impl_def_id) | ||
.iter().find(|ic| ic.name == ti.name) { | ||
Some(ic) => lookup_const_by_id(tcx, ic.def_id, None), | ||
Some(ic) => lookup_const_by_id(tcx, ic.def_id, None, None), | ||
None => match ti.node { | ||
hir::ConstTraitItem(_, Some(ref expr)) => Some(&*expr), | ||
_ => None, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
// Copyright 2016 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(associated_consts)] | ||
|
||
trait Foo { | ||
const ID: usize; | ||
} | ||
|
||
const X: [i32; <i32 as Foo>::ID] = [0, 1, 2]; //~ ERROR E0250 | ||
|
||
fn main() { | ||
assert_eq!(1, X); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
// Copyright 2016 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(associated_consts)] | ||
|
||
trait Foo { | ||
const ID: i32; | ||
} | ||
|
||
const X: i32 = <i32>::ID; | ||
//~^ ERROR no associated item named `ID` found for type `i32` | ||
|
||
fn main() { | ||
assert_eq!(1, X); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 <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(associated_consts)] | ||
|
||
pub enum EFoo { A, B, C, D } | ||
|
||
pub trait Foo { | ||
const X: EFoo; | ||
} | ||
|
||
struct Abc; | ||
impl Foo for Abc { | ||
const X: EFoo = EFoo::B; | ||
} | ||
|
||
struct Def; | ||
impl Foo for Def { | ||
const X: EFoo = EFoo::D; | ||
} | ||
|
||
pub fn test<A: Foo, B: Foo>(arg: EFoo) { | ||
match arg { | ||
A::X => println!("A::X"), //~ error: statics cannot be referenced in patterns [E0158] | ||
B::X => println!("B::X"), //~ error: statics cannot be referenced in patterns [E0158] | ||
_ => (), | ||
} | ||
} | ||
|
||
fn main() { | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
// 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. | ||
|
||
#![feature(associated_consts)] | ||
|
||
pub trait Foo { | ||
const Y: usize; | ||
} | ||
|
||
struct Abc; | ||
impl Foo for Abc { | ||
const Y: usize = 8; | ||
} | ||
|
||
struct Def; | ||
impl Foo for Def { | ||
const Y: usize = 33; | ||
} | ||
|
||
pub fn test<A: Foo, B: Foo>() { | ||
let _array = [4; <A as Foo>::Y]; //~ error: expected constant integer | ||
} | ||
|
||
fn main() { | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This merits a comment, I think, as to the purpose of each of these parameters. Something like:
def_id
is the id of the constantmaybe_ref_id
is the id of the expression referencing the constant, if availableparam_substs
is the monomorphization substitution for the expression, if availableThe latter two parameters are used to extract the substitutions for the constant, in the case of associated constants.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As an aside -- this all feels very hokey to me, I'd rather we just passed in the final set of substitutions, rather than passing in an expr-id and monomorphization substitution and then figuring out the substitutions. But that feels like a separate PR.
UPDATE: To be clear: I wasn't calling your diff hokey. This fn is just getting increasingly complex. In general, it just seems better to me overall if we can pass in "this definition with these substitutions" rather than "this definition referenced from this context" (the latter being how the fn is currently setup).