Skip to content

Add "type_name" support in emulate_intrinsic() #61399

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

Closed
wants to merge 26 commits into from
Closed
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
d3d82b0
Add "type_name" support in emulate_intrinsic()
TankhouseAle May 31, 2019
271cf97
Fix whitespace, change "value" to "name_val"
TankhouseAle May 31, 2019
67b3fde
Add a test, as suggested.
TankhouseAle May 31, 2019
d0cc41d
Do an actual check with assert_eq, as suggested.
TankhouseAle May 31, 2019
d6e63a3
Adapt implementation to "deterministic type_name"
TankhouseAle May 31, 2019
cce1392
We need "tcx.tcx", not just "tcx"
TankhouseAle Jun 1, 2019
82215e2
The last commit certainly didn't do what I thought it did.
TankhouseAle Jun 1, 2019
0f0ad45
Can't have ConstValue without ConstValue!
TankhouseAle Jun 1, 2019
892bc65
Move a brace up to be more consistent.
TankhouseAle Jun 2, 2019
19a3814
Add the suggested utility function to type_name.rs
TankhouseAle Jun 3, 2019
11da537
Use the new function to implement type_name
TankhouseAle Jun 3, 2019
78d5c38
Use Immediate and Pointer, but not ConstValue
TankhouseAle Jun 3, 2019
fe40101
Actually for real use Immediate and Pointer
TankhouseAle Jun 3, 2019
dfb8dbd
Make Tidy happy
TankhouseAle Jun 3, 2019
04736f5
Make Tidy happy
TankhouseAle Jun 3, 2019
c326ef1
Literally just remove a single space
TankhouseAle Jun 3, 2019
a58a557
Don't need to use Pointer if we do name_id.into()
TankhouseAle Jun 3, 2019
04946c7
Add line break as suggested
TankhouseAle Jun 3, 2019
9a0c77c
Use alloc_type_name() in type_name()
TankhouseAle Jun 3, 2019
9b2d648
Maybe it will merge this way?
TankhouseAle Jun 3, 2019
0a9047b
Change this back temporarily
TankhouseAle Jun 3, 2019
c3ef5a5
Merge pull request #2 from rust-lang/master
TankhouseAle Jun 3, 2019
43ba61a
Put my changes back in
TankhouseAle Jun 3, 2019
1c8ce6d
Fix duplicated bounds printing in rustdoc (#4)
TankhouseAle Jun 3, 2019
430f4a6
Update from master
TankhouseAle Jun 3, 2019
bbb209d
Merge pull request #8 from rust-lang/master
TankhouseAle Jun 3, 2019
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
21 changes: 20 additions & 1 deletion src/librustc_mir/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use rustc::ty;
use rustc::ty::layout::{LayoutOf, Primitive, Size};
use rustc::mir::BinOp;
use rustc::mir::interpret::{
EvalResult, InterpError, Scalar,
EvalResult, InterpError, Scalar, ConstValue
};

use super::{
Expand Down Expand Up @@ -78,6 +78,25 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> InterpretCx<'a, 'mir, 'tcx, M>
let id_val = Scalar::from_uint(type_id, dest.layout.size);
self.write_scalar(id_val, dest)?;
}

"type_name" => {
let const_val = type_name(self.tcx.tcx, substs.type_at(0)).val;
if let ConstValue::Slice {
data: slice_val,
start: _,
end: _,
} = const_val
{
// unsafe for from_utf8_unchecked()
unsafe {
Copy link
Contributor

Choose a reason for hiding this comment

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

please get rid of str_to_immediate, it is only used in miri's implementation of type_name and thus not needed.

Instead, you can write

let alloc_id = self.tcx.alloc_map.lock().create_memory_alloc(data);
let name_val = Immediate::new_slice(Scalar::Ptr(ptr.into()), data.len(), self);

The code as currently written would be invalid when encountering a ConstValue::Slice where start != 0 or end - start != data.len(). The way type_name is written, that doesn't happen, so... idk, add some comments explaining why that's ok?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it would also simplify things if you pulled out

let path = AbsolutePathPrinter { tcx, path: String::new() }.print_type(ty).unwrap().path;
let len = path.len();
let alloc = Allocation::from_byte_aligned_bytes(path.into_bytes(), ());
let alloc = tcx.intern_const_alloc(alloc);
into a separate function (with pub(super) visibility) and called that here? That way we'd not have the ConstValue::Slice wrapper and could simplify the code shown above to

let alloc_id = self.tcx.alloc_map.lock().create_memory_alloc(data);
let name_val = Immediate::new_slice(Scalar::Ptr(ptr.into()), data.len(), self);

Copy link
Contributor Author

@TankhouseAle TankhouseAle Jun 2, 2019

Choose a reason for hiding this comment

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

😆 the reddit post you linked to shows a typecast example

Yeah. As I believe I stated in there though, I don't think those are particularly realistic use cases for what someone who is using compile-time intrinsics at all is likely to want to do with this kind of function.

I was quite literally unaware the "breaking parametricity" thing was a concern that anyone had at all before being told about it by a few people in that thread, as the general concept of doing something like that hadn't ever crossed my mind.

With regards to the changes, I'll work on them now (I didn't notice you had replied until right after I sent off the minor commit I just made, haha.)

Thanks for the review!

let str_val = core::str::from_utf8_unchecked(&slice_val.bytes);
let name_val = self.str_to_immediate(str_val)?;
self.write_immediate(name_val, dest)?;
}
} else {
bug!("type_name() somehow failed to return a ConstValue::Slice");
}
}
| "ctpop"
| "cttz"
| "cttz_nonzero"
Expand Down
37 changes: 37 additions & 0 deletions src/test/run-pass/ctfe/const-fn-type-name.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// run-pass

#![feature(core_intrinsics)]
#![feature(const_fn)]
#![allow(dead_code)]

const fn type_name_wrapper<T>(_: &T) -> &'static str {
unsafe { core::intrinsics::type_name::<T>() }
}

struct Struct<TA, TB, TC> {
a: TA,
b: TB,
c: TC,
}

type StructInstantiation = Struct<i8, f64, bool>;

const CONST_STRUCT: StructInstantiation = StructInstantiation {
a: 12,
b: 13.7,
c: false,
};

const CONST_STRUCT_NAME: &'static str = type_name_wrapper(&CONST_STRUCT);

fn main() {
let non_const_struct = StructInstantiation {
a: 87,
b: 65.99,
c: true,
};

let non_const_struct_name = type_name_wrapper(&non_const_struct);

assert_eq!(CONST_STRUCT_NAME, non_const_struct_name);
}