Skip to content
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

typeck: Fix const generic in repeat param ICE. #61698

Merged
merged 1 commit into from
Jun 12, 2019
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
35 changes: 22 additions & 13 deletions src/librustc_typeck/astconv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2144,6 +2144,17 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx> + 'o {
result_ty
}

/// Returns the `DefId` of the constant parameter that the provided expression is a path to.
pub fn const_param_def_id(&self, expr: &hir::Expr) -> Option<DefId> {
match &expr.node {
ExprKind::Path(hir::QPath::Resolved(_, path)) => match path.res {
Res::Def(DefKind::ConstParam, did) => Some(did),
_ => None,
},
_ => None,
}
}

pub fn ast_const_to_const(
&self,
ast_const: &hir::AnonConst,
Expand Down Expand Up @@ -2174,19 +2185,17 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx> + 'o {
}
}

if let ExprKind::Path(ref qpath) = expr.node {
if let hir::QPath::Resolved(_, ref path) = qpath {
if let Res::Def(DefKind::ConstParam, def_id) = path.res {
let node_id = tcx.hir().as_local_node_id(def_id).unwrap();
let item_id = tcx.hir().get_parent_node(node_id);
let item_def_id = tcx.hir().local_def_id(item_id);
let generics = tcx.generics_of(item_def_id);
let index = generics.param_def_id_to_index[&tcx.hir().local_def_id(node_id)];
let name = tcx.hir().name(node_id).as_interned_str();
const_.val = ConstValue::Param(ty::ParamConst::new(index, name));
}
}
};
if let Some(def_id) = self.const_param_def_id(expr) {
davidtwco marked this conversation as resolved.
Show resolved Hide resolved
// Find the name and index of the const parameter by indexing the generics of the
// parent item and construct a `ParamConst`.
let node_id = tcx.hir().as_local_node_id(def_id).unwrap();
let item_id = tcx.hir().get_parent_node(node_id);
let item_def_id = tcx.hir().local_def_id(item_id);
let generics = tcx.generics_of(item_def_id);
let index = generics.param_def_id_to_index[&tcx.hir().local_def_id(node_id)];
let name = tcx.hir().name(node_id).as_interned_str();
const_.val = ConstValue::Param(ty::ParamConst::new(index, name));
}

tcx.mk_const(const_)
}
Expand Down
34 changes: 22 additions & 12 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2439,6 +2439,11 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
ty
}

/// Returns the `DefId` of the constant parameter that the provided expression is a path to.
pub fn const_param_def_id(&self, hir_c: &hir::AnonConst) -> Option<DefId> {
AstConv::const_param_def_id(self, &self.tcx.hir().body(hir_c.body).value)
}

pub fn to_const(&self, ast_c: &hir::AnonConst, ty: Ty<'tcx>) -> &'tcx ty::Const<'tcx> {
AstConv::ast_const_to_const(self, ast_c, ty)
}
Expand Down Expand Up @@ -4414,19 +4419,24 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
}
ExprKind::Repeat(ref element, ref count) => {
let count_def_id = tcx.hir().local_def_id_from_hir_id(count.hir_id);
let param_env = ty::ParamEnv::empty();
let substs = InternalSubsts::identity_for_item(tcx.global_tcx(), count_def_id);
let instance = ty::Instance::resolve(
tcx.global_tcx(),
param_env,
count_def_id,
substs,
).unwrap();
let global_id = GlobalId {
instance,
promoted: None
let count = if self.const_param_def_id(count).is_some() {
Ok(self.to_const(count, self.tcx.type_of(count_def_id)))
} else {
let param_env = ty::ParamEnv::empty();
let substs = InternalSubsts::identity_for_item(tcx.global_tcx(), count_def_id);
let instance = ty::Instance::resolve(
tcx.global_tcx(),
param_env,
count_def_id,
substs,
).unwrap();
let global_id = GlobalId {
instance,
promoted: None
};

tcx.const_eval(param_env.and(global_id))
davidtwco marked this conversation as resolved.
Show resolved Hide resolved
};
let count = tcx.const_eval(param_env.and(global_id));

let uty = match expected {
ExpectHasType(uty) => {
Expand Down
12 changes: 12 additions & 0 deletions src/test/ui/const-generics/issue-61336-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#![feature(const_generics)]
//~^ WARN the feature `const_generics` is incomplete and may cause the compiler to crash

fn f<T: Copy, const N: usize>(x: T) -> [T; N] {
[x; N]
//~^ ERROR array lengths can't depend on generic parameters
}

fn main() {
let x: [u32; 5] = f::<u32, 5>(3);
assert_eq!(x, [3u32; 5]);
}
14 changes: 14 additions & 0 deletions src/test/ui/const-generics/issue-61336-1.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
warning: the feature `const_generics` is incomplete and may cause the compiler to crash
--> $DIR/issue-61336-1.rs:1:12
|
LL | #![feature(const_generics)]
| ^^^^^^^^^^^^^^

error: array lengths can't depend on generic parameters
--> $DIR/issue-61336-1.rs:5:9
|
LL | [x; N]
| ^

error: aborting due to previous error

16 changes: 16 additions & 0 deletions src/test/ui/const-generics/issue-61336.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#![feature(const_generics)]
//~^ WARN the feature `const_generics` is incomplete and may cause the compiler to crash

fn f<T: Copy, const N: usize>(x: T) -> [T; N] {
[x; N]
Copy link
Member

Choose a reason for hiding this comment

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

Could we also have a test where we make sure that this does the right thing? E.g. pass in N explicitly and check the return value.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added another test. It will error later in the compiler with a "array lengths can't depend on generic parameters" error, emitted when creating the HAIR. From a quick look, it appears that the MIR/HAIR data structures all contain a u64 for the count, so it isn't trivial to change one or two lines and make this work, probably best left for a follow-up (which I'd be interested in trying to tackle). It's an improvement over an ICE in any case.

Copy link
Member

@varkor varkor Jun 10, 2019

Choose a reason for hiding this comment

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

Ah, okay. Yes, this is certainly better than an ICE, and this gives us a good minimal example to fix. Let's open an issue for it after this is merged.

}

fn g<T, const N: usize>(x: T) -> [T; N] {
[x; N]
//~^ ERROR the trait bound `T: std::marker::Copy` is not satisfied [E0277]
}

fn main() {
let x: [u32; 5] = f::<u32, 5>(3);
assert_eq!(x, [3u32; 5]);
}
18 changes: 18 additions & 0 deletions src/test/ui/const-generics/issue-61336.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
warning: the feature `const_generics` is incomplete and may cause the compiler to crash
--> $DIR/issue-61336.rs:1:12
|
LL | #![feature(const_generics)]
| ^^^^^^^^^^^^^^

error[E0277]: the trait bound `T: std::marker::Copy` is not satisfied
--> $DIR/issue-61336.rs:9:5
|
LL | [x; N]
| ^^^^^^ the trait `std::marker::Copy` is not implemented for `T`
|
= help: consider adding a `where T: std::marker::Copy` bound
= note: the `Copy` trait is required because the repeated element will be copied

error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.