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

Old vtable example fails with: "error: untyped pointers are not allowed in constant" #77995

Open
dimpolo opened this issue Oct 15, 2020 · 6 comments
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) A-raw-pointers Area: raw pointers, MaybeUninit, NonNull A-trait-system Area: Trait system C-bug Category: This is a bug. requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@dimpolo
Copy link
Contributor

dimpolo commented Oct 15, 2020

While trying to play around with raw::TraitObject I found this thread internals.rust-lang.org/... and in particular this code example play.rust-lang.org/... which does not work anymore.

Code

I tried this code:

#![feature(raw)]

use std::{mem, ptr, raw};

pub trait Trait {}

struct Struct;
impl Trait for Struct {}

#[allow(dead_code)]
const STRUCT_AS_DYN_TRAIT_VTABLE: *mut () =
    unsafe { mem::transmute::<*const dyn Trait, raw::TraitObject>(ptr::null::<Struct>()).vtable };

I expected to see this happen:
I get the vtable as a const.

Instead, this happened:
error: untyped pointers are not allowed in constant

Version it worked on

Probably before c400f75
Edit: added PR number #71665

Version with regression

rustc 1.49.0-nightly (e160e5c 2020-10-14)

Any chance we can get the example code to compile again?
Is there a different way to get the vtable?

@camelid camelid added A-raw-pointers Area: raw pointers, MaybeUninit, NonNull A-trait-system Area: Trait system C-bug Category: This is a bug. regression-from-stable-to-stable Performance or correctness regression from one stable version to another. labels Oct 15, 2020
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Oct 15, 2020
@camelid camelid added the A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) label Oct 15, 2020
@tesuji
Copy link
Contributor

tesuji commented Oct 16, 2020

Since this requires feature gate which only exists on nightly toolchain

@rustbot modify labels: -regression-from-stable-to-stable requires-nightly

@rustbot rustbot added requires-nightly This issue requires a nightly compiler in some way. and removed regression-from-stable-to-stable Performance or correctness regression from one stable version to another. labels Oct 16, 2020
@camelid
Copy link
Member

camelid commented Oct 16, 2020

Oops, thanks for fixing that @lzutao :)

@JohnTitor JohnTitor added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 16, 2020
@JohnTitor
Copy link
Member

cc @rust-lang/wg-const-eval Could someone confirm if this is an intended change?

@oli-obk
Copy link
Contributor

oli-obk commented Oct 17, 2020

This is an expected regression, but we actually want to unregress it:

// FIXME: downgrade this to a warning? It rejects some legitimate consts,
// such as `const CONST_RAW: *const Vec<i32> = &Vec::new() as *const _;`.
ecx.tcx
.sess
.span_err(ecx.tcx.span, "untyped pointers are not allowed in constant");

@RalfJung
Copy link
Member

IIRC the main concern here is that we need to implicitly mark the memory that pointer points to as immutable -- if the programmer did not expect this, they'd cause UB when mutating it.

I am surprised by this example though... the memory that needs interning here might be the vtable? That case actually should not be too hard to allow I feel. We might be able to just check alloc.mutability, and if it is already immutable, I see little harm in interning it. This means there is already some other reason this memory may not be mutated. (I should double-check with the discussions from back then though if we really want to go this path.)

@RalfJung
Copy link
Member

Or alternatively we could intern vtables already in get_vtable, and make the self.vtables cache global instead of per-interpreter. Then this memory would already be interned and there should be no ICE.

@LeSeulArtichaut LeSeulArtichaut removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Oct 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) A-raw-pointers Area: raw pointers, MaybeUninit, NonNull A-trait-system Area: Trait system C-bug Category: This is a bug. requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

8 participants