`extern type` cannot support `size_of_val` and `align_of_val`

[joshtriplett]

Based on discussion on #43467 and in @rust-lang/lang meetings, an opaque extern type type cannot support size_of_val or align_of_val. We believe that we should panic or abort in this case. We also believe that we should have a compile-time lint that detects this whenever possible, since at some level the compiler should know at compile time if code invokes size_of_val or align_of_val on an extern type.

I've opened this separate issue to document that decision, and will propose FCP on it to give time for final comments.

[joshtriplett]

@rfcbot fcp merge

[rfcbot]

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged teams:

• @aturon
• @cramertj
• @eddyb
• @joshtriplett
• @nikomatsakis
• @nrc
• @pnkfelix
• @scottmcm
• @withoutboats

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[kennytm]

we should have a compile-time lint that detects this whenever possible

will the lint be emitted before or after monomorphization?

P.S. cc rust-lang/rfcs#2310

[eddyb]

I'm specifically in favor of "before", only emitting anything after monomorphization under some general umbrella of "this code will always panic at runtime" warnings.

[joshtriplett]

It would be nice to handle generic functions that get monomorphized with an extern type, though. That will come up.

[rkruppe]

The RFC text (and the discussion even more so, IIRC) mention the possibility of adding a new trait for encoding this constraint. I assume this has been dropped because that trait would have to be included in bounds by default like Sized?

[Ericson2314]

@rkruppe yes sadly.... rust-lang/rfcs#2310 was opened as backup plan. See the end of #43467.

[comex]

I'm a bit confused why this is in rust-lang/rust instead of rust-lang/rfcs. (I have rust-lang/rfcs set as watched, so I get email copies of all issues and PRs there, but not here.)

[joshtriplett]

Because it's a bug about extern type behavior.

[nikomatsakis]

@comex

I'm a bit confused why this is in rust-lang/rust instead of rust-lang/rfcs. (I have rust-lang/rfcs set as watched, so I get email copies of all issues and PRs there, but not here.)

Because this is not a new RFC, it's nailing down a specific unresolved question around extern types (though we should have linked from the tracking issue to here -- fixed). This is pretty standard practice.

[jethrogb]

Why lint when you can have type safety? Two convincing comments from the other discussion thread:

@mikeyhew #43467 (comment)

OK, I just want to say that I am very strongly of the opinion that Rust should use built-in traits like DynSized to express the difference in capabilities between extern types and the dynamically-sized types that currently exist in Rust (i.e. trait objects and slices). All of the alternatives that I have seen – panicking, returning Option from size_of_val, post-monomorphisation lints – are less powerful, and the issues with ?-traits that people keep bringing up need to be tested and not just speculated about. We need to at least try doing things the builtin-traits way and see what it's like, and see what the ergonomic impact is like, and see if we can reduce it, before settling for something inferior.

Maybe I'm overreacting, I just got the sense from reading some of the comments in this thread that something might be done in order to get extern types out the door, that might put us in a backward-compatibility trap later on. Now that I have more time to work on Rust, I'm planning on writing an eRFC to add DST-specific builtin traits like DynSized and SizeFromMeta, so we can start experimenting with them and Custom DST.

@Ericson2314 #43467 (comment)

1. I don't think "always use the smallest tool for the job" applies here. The decreased power of lints is directly worse for uses. C.f. non-null lints v.s. Option in other languages. Lints are easily lost amid other warnings, and the fact is only some users will care. This means while individual code bases might obey them, the ecosystem as a whole can not be trusted to uphold the invariants the lints try to maintain. This a real loss for fostering an ecosystem of DynSize abstractions, or whatever the niche feature is, as for such niche things, being able to sync up few and scattered programmers and form a community is all the more important.

2. Ecosystem-wide enforcement is also good for the "regular users don't need to care" goal. If some library happens to use truly unsized types, and the consumer is unaware, they could face some nasty unexpected packages. With ?DynSize they do get bothered with a compile time error they didn't expect, but that is much less nasty to deal with with than a run-time bug. If they don't want to learn ?DynSized, they can go use a different library; better to have the opportunity to do that up front than after you're tied to the library too deeply because it took a while to excise the {size,align}_of_val panic.

And my own perspective: there are various places where I'd like to be generic over things that the language doesn't allow. That goes hand in hand with the type system. For example, you can't be generic over lengths when using arrays or calling conventions when using function pointers. Let's not add more features in that could reasonably be described correctly in the type system but aren't.

[joshtriplett]

@jethrogb

Why lint when you can have type safety?

If the lint is deny-by-default (which I think it should be), it'd effectively act the same as a type error. The second comment you quote seems based on the idea that it'd be a warning and not an error.

So the only difference is whether to provide a more general trait-based mechanism or to handle the specific case at hand. I haven't seen any fundamental opposition to the idea of introducing those traits in the future if we have a more general case that needs them, but if we want those traits for some future features that we haven't yet introduced to the language, let's design those traits alongside those future language features.

[kennytm]

Would it be possible to make substituting an extern type in generics and using them in struct fields an error altogether (feature-gate it), until the situation of DynSized reached consensus? extern type could still be used for FFI as originally designed. This would side-step the entire size_of_val problem for stabilizing extern type.

// ok:

fn f1(a: &ExternType) {}
fn f2() -> *mut ExternType {}
fn f3<T: ?Sized>(a: &T, b: *const ExternType) {}
fn f4<A, R, F: Fn(A) -> R>(func: F, a: A) -> R {}
let w1 = size_of::<*const ExternType>();
let w2 = size_of_val(&f2());
let w3 = f4(|x| x, &*f2());
//^ substituting `&ExternType` is ok.

// errors (gated):

fn g1<T: ?Sized>(a: &T) {}
g1(&*f2()); //~ ERROR: Cannot substitute an extern type to `T`

fn g2() -> Box<ExternType> {} //~ ERROR: Cannot substitute an extern type to `T`

struct S1(ExternType);  //~ ERROR: Cannot use an extern type as struct field.

struct S2<R: ?Sized>(R);
let s2: S2<ExternType>; //~ ERROR: Cannot substitute an extern type to `R`

struct S3;
impl Deref for S3 {
    type Target = ExternType; //~ ERROR: Cannot use extern type as associated type.
    fn deref(&self) -> &ExternType {}
}

[Ericson2314]

If the lint is deny-by-default (which I think it should be), it'd effectively act the same as a type error. The second comment you quote seems based on the idea that it'd be a warning and not an error.

In terms of trust, only lints that cannot be turned off allow foreign code to be trusted to the same degree.

The generics thing that @jethrogb points out is good too. Let's assume we have a monomorphization-time lint as you proposed, @joshtriplett (I agree with you that that's better than only having a pre-monomorphization time lint). Then, even thought its just as safe (same things will be prevented in the end), the user experience is still worse. The library could be fine over it's testsuite, but not fine when instantiated downstream. And for upstream, even if the test do catch such bad instantiations, type errors pop up sooner than monomorization-time lints for a quicker debug cycle.

[Ericson2314]

@kennytm I mentioned before in a buried comment that we can stabilize extern type and change the types of {size,align}-of-val to have the ?DynSize bound without stabilizing DynSized. This only makes polymorphic code with a ?DynSize bound require unstable Rust, a very niche use-case but precisely the one hurt by using lints. Problem solved!

In many ways, that is effectively the same as what you propose for stable code, unstable DynSize is just the easiest way to implement it :).

[kennytm]

@Ericson2314 That's a much bigger change because you need to introduce a new concept DynSized, while feature-gating substitution + struct field is much easier as an incremental step.

Furthermore we are not even committed to whether ?Sized should opt-out of DynSized yet!

[Ericson2314]

@kennytm DynSized is not actually that big of a change despite all the controversy because the traits system so accurately models what's going on. It's already been implemented in a PR in the past, for example. Feature-gating substitution + struct field I'd wager would be harder because it's ad-hoc, unless we've happened to do exactly that lint before. [Closest thing I can think of is packed structs, and that's not as close.]

Furthermore we are not even committed to whether ?Sized should opt-out of DynSized yet!

Nothing I propose forces a final decision, since DynSized is still unstable. Here's a case analysis demonstrating.

• Case: DynSize is "positive" trait, {size,align}_of_val has DynSize bound. Not possible no matter what:
  1. breaks existing code instantiating {size,align}_of_val with type variable.
• Case: No DynSize, {size,align}_of_val has existing type. Possible by doing:
  1. Remove all mention of DynSize (OK since unstable).
  2. Add panicking behavior so {size,align}_of_val is defined on extern type.
• Case: DynSize is "positive" trait, {size,align}_of_val has existing type (but is deprecated). Possible by doing:
  1. Same steps as above
  2. Add back DynSize as normal "postive" trait.
  3. Make the {size,align}_of_val replacements using DynSized
  4. (Optional) Deprecate {size,align}_of_val

[glaebhoerl]

How does this relate to #48055? It seems like that is another case where we'd assume for arbitrary T: !Sized that we nonetheless know the size at runtime and can move it around by memcpy - potentially a much bigger 'hole' in the system than size_of_val and align_of_val?

[comex]

Well, not necessarily out of the box, without a way to get an owned T. But &move T could be problematic if ever implemented, and someone might decide that functions like ptr::read should have their Sized bounds removed…

Anyway, definitely seems like a good idea to restrict that feature to DynSized types (good thing it's not even implemented yet).

[briansmith]

If the lint is deny-by-default (which I think it should be), it'd effectively act the same as a type error. The second comment you quote seems based on the idea that it'd be a warning and not an error.

I propose that it be (1) deny-by-default, (2) not allowed to be changed from that default, and (3) formatted to look like a type error instead of a lint message. Then it would be indistinguishable, to the user, from a type error.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[briansmith]

I still think that when we have the option of enforcing a typing rule in the type checker vs. the lint mechanism, we should enforce it in the type checker. I understand and don't necessarily disagree with the objections to DynSized but there is an alternative: special-case these intrinsics and extern type in the type system. It's not very elegant but the whole point of static type systems is to prevent static typing errors at compile time and not at runtime.

[SimonSapin]

There is precedent for such no-so-elegant special-casing: instanciating std::mem::transmute<T, U> errors if T and U do not have the same size.

[SimonSapin]

Hmmm on the other hand to avoid monomorphization-time errors transmute conservatively errors when a type parameter is involved and it can’t determine “generically” the size equality. This principle might be overly restrictive for size_of_val which would not be usable at all in a generic context at all, since we don’t know before monomorphization if a type parameter will be an extern type.

[rkruppe]

However, the transmute special casing is sound, since transmute flat out refuses to work work on type parameters. size_of_val has to keep working (without any trait bounds), so special casing size_of_val and extern type doesn't actually buy any hard guarantees compared to a lint. At most it's a matter of presentation (@briansmith previously suggested to "[format] it like a type error"), but that's not unambigously a good thing IMO: it increases the size and complexity of "the type checker" from the perspective of, and may even give a false sense of reassurance (or conversely, diminish trust in the type system once users notice that this check isn't sound).

Edit: this assumes we don't want monomorphization-time errors, but those are pretty firmly established as a big no-no at this point.

[kennytm]

transmute's check is pre-monomorphization and will give up when the size of T and U is not a fixed number:

#![crate_type = "lib"]
use std::mem::transmute;
fn g<T>(a: [[T; 2]; 2]) -> [T; 4] {
    unsafe { transmute(a) }
}

error[E0512]: transmute called with types of different sizes
 --> src/lib.rs:6:14
  |
6 |     unsafe { transmute(a) }
  |              ^^^^^^^^^
  |
  = note: source type: [[T; 2]; 2] (size can vary because of T)
  = note: target type: [T; 4] (size can vary because of T)

The same cannot be applied to size_of_val, we can't throw a compiler-error if the type's "extern-type-ness" is unknown before monomorphization as it will break existing code:

unsafe fn reinterpret_as_bytes<T: ?Sized>(a: &T) -> &[u8] {
    let size = size_of_val(a);
    //~^ ERROR
    // we don't know `T` would be an `extern type` without monomorphizing
    slice::from_raw_parts(a as *const T as *const u8, size)
}

[mikeyhew]

Has any consideration been given to @kennytm's suggestion of disallowing extern types in generics? Pointers to extern types would work just fine as type arguments and associated types, since they are Sized. It would just be a temporary solution while we're figuring out the best way to support them in the long term, but it would let us stabilize extern types now in a minimal, hopefully usable way.

[Ericson2314]

@mikeyhew Note that adding an unstable opt-out DynSize trait has exactly that effect, and is in fact probably the easiest way to implement that. Normal use is not impacted, but (shallow [1]) generic code is impossible. If we really want, we can hack the error messages to not mention the unstable trait.

[1]: e.g. "shallow" because if you push the type parameter deeper, e.g. x: [(Phantom<T>, usize)], size_of_val(x), it's fine.

[briansmith]

it increases the size and complexity of "the type checker"

The current proposal is to have part of the type checker moved to the lint mechanism. Thus we can say "look at how simple and clear the type checker is" and "look at how consistent* the type system is" when we want to think those things because we just say "the lint mechanism isn't part of the type checker so don't look at it." Then on the other hand we'd say "we still check this statically" because the lint mechanism does the type checking that was left out of the type checker. However, the value in putting part of the type checker into the lint checker is purely aesthetic: the fact is that the type system isn't really consistent: using these intrinstics on extern types is a type error either way.

My understanding is that the code generator has to be specialized to generate the panic! for these types, so the effect is to make the linter more complicated and the code generator more complicated in order to allow us to pretend that the type system is better than it really is. It doesn't seem like the right trade-off to me.

I understand that people don't want the type checker to reject things after monomorphization but in a programmer's actual usage of this stuff, it makes no difference whether the type checker rejects the program after monomorphization or the linter rejects the program after monomorphization. The program is rejected either way.

* I'm not sure "consistent" is the best word.

[rkruppe]

using these intrinstics on extern types is a type error either way.

It is a programmer error, but for every error we (the designers of the relevant language feature or API) have the choice of which mechanisms we use to combat that error. The type system is a useful tool for that, but it's not the only one, and in this case it has drawbacks (the drawbacks of momonomorphization time errors, if we restrict ourselves to your proposal rather than the one @mikeyhew brought up) and few upsides.

Additionally, even if the type system and lint approaches have equivalent complexity, the cost of a complex lint is far smaller than the cost of a type system feature of equivalent complexity. Lints are optional, their existence and how they work exactly is a quality-of-implementation issue. They can have bugs without causing UB (if false negative) or hard-breaking correct code (if false positive). They don't have to be specified in as much detail or be reproduced exactly in other implementations. They can be ignored in efforts to verify the language or the compiler.

make [...] the code generator more complicated

The code generator complexity seems quite manageable to me. Even if emitting a panic in codegen directly is problematic (it might, since we don't currently have any intrinsics that can panic AFAIK), we can achieve the same effect by adding a trivial parametricity-breaking intrinsic is_extern_type::<T: ?Sized> (which would only take around 10 lines of repetive code throughout rustc and libcore) and do the panicking in in the stable wrapper function:

fn size_of_val<T: ?Sized>(val: &T) -> usize {
    if is_extern_type::<T>() {
        panic!(...)
    } else {
        intrinsics::size_of_val(val)
    }
}

The intrinsic would continue to return 0 or some other nonsense for extern types, but it's perma-unstable anyway.

[...] in order to allow us to pretend that the type system is better than it really is.

I see why you're putting it that way, but from my perspective it's quite the opposite: making this a lint is an admission that our type system isn't covering this class of errors! We're just making the conscious decision to have a less powerful type system in pursuit of other goals.

I understand that people don't want the type checker to reject things after monomorphization but in a programmer's actual usage of this stuff, it makes no difference whether the type checker rejects the program after monomorphization or the linter rejects the program after monomorphization. The program is rejected either way.

One disadvantage of monomorphization time errors is that a buggy library can cause hard errors when compiling client code with no way to work around it other than patching the library. In contrast, a deny-by-default lint can be turned off and is "capped" by Cargo when compiling dependencies, so if the buggy code is never actually executed, one can still productively use the rest of the library. This is similar to how the compiler reports runtime panics that it can identify by constant evaluation, but explicitly isn't allowed to actually break the build in those cases.

[RalfJung]

we can achieve the same effect by adding a trivial parametricity-breaking intrinsic is_extern_type::<T: ?Sized> (which would only take around 10 lines of repetive code throughout rustc and libcore) and do the panicking in in the stable wrapper function:

As another proposal, the intrinsic could actually return Option<usize> and the library function could unwrap that. Seems like a nicer solution to me :)

[kennytm]

What would the compiler generate when computing the offset of an unsized field, which involves align_of_val, as I've asked before in #43467 (comment)? The align_of_val intrinsic would be used indirectly, and thus needs to make an abort/panic/dummy value. We can't sidestep the problem with Option<usize>.

[RalfJung]

Oh, right -- sorry I forgot about that.

I don't think a dummy value would be a good idea though.

[mikeyhew]

@Ericson2314

disallowing extern types in generics

Note that adding an unstable opt-out DynSize trait has exactly that effect, and is in fact probably the easiest way to implement that

You're right. I was saying we disallow extern types as type arguments and associated types completely, but now that I think about it again, that wouldn't work — I forgot that to get a *const SomeExternType, you are using that extern type as a type argument. So using a trait like DynSized really does seem like the only way to allow its use in pointer types but not anywhere else, and catch the error as early as possible.

Keeping the DynSized trait unstable would mean that people could use *const SomeExternType in stable code, but that's it – the name of the trait would not be stabilized, the fact that there is a trait wouldn't even be stabilized. Only the fact that you can use extern types as the type arguments to the *const _ and *mut _ type constructors, and anywhere else we allow them. If the decision is made later to remove the trait later, and use post-monomorphization errors or panic at runtime, that would be a backward-compatible (if unadvisable) change. So would be replacing DynSized with a new trait hierarchy when custom DST comes along. However, if we do post-monomorphization errors or panic now, then using traits later would be a breaking change.

IIRC, the problems with DynSized were as follows:

• ?-traits are confusing and un-ergonomic. By keeping the trait unstable and using it only as an implementation detail, we avoid this problem
• There are bugs in the type-checker that make it a backwards-incompatible change. Hopefully those can be fixed, or the breakage isn't too bad? Otherwise I'm not sure what to do here.

Whatever we end up doing, these are the things we need to disallow, at minimum:

• calling size_of_val/align_of_val
• anything that would require knowing the offset of the tail field of a struct, if that field is an extern type and there are non-zero-sized fields ahead of it. For example:

  extern { type Opaque };
  struct Foo {
      x: i32,
      tail: Opaque
  }
  // foo: &Foo
  // error because we don't know how much padding there is between fields `x` and `tail`
  let tail: &Opaque = &foo.tail;
  

[comex]

Regarding the compile-time lint, I'd just like to highlight my suggestion from a different thread for how it could be implemented as a more general feature:

rust-lang/rfcs#2310 (comment)

In short, right now putting #[deprecated] on an impl does nothing; if it were fixed to lint when that impl is selected during type checking, that would be enough to implement the extern type lint as a pure library feature. (Well, there would also have to be a DynSized auto trait that extern types automatically opt out of, but that's desirable anyway and could be left unstable for now. It would not require adding DynSized as an implicit bound or anything like that.)

[briansmith]

Given a value x of any type, how could I check whether size_of_val(x) would panic before evaluating size_of_val(x)?

[briansmith]

In other words, how can I write this function:

fn checked_size_of_val<T>(x: T) -> Option<usize> {
    if [would `size_of_val(x)` panic?] {
       None
    } else {
       Some(size_of_val(x)
    }
}

In particular, how can we make this function work in the case where panic=abort?

[briansmith]

One thing that's mentioned in the comment above that didn't get enough discussion, IMO, is the suggestion to just deprecate and remove size_of_val and align_of_val, e.g. in Rust 2018. Of course that's easy for me to say since I don't use them, but I do think it's notable that C++ and many other languages lack these functions and still manage to get by without them just fine. It's not clear to me that these two functions earn their existence given this issue.

[SimonSapin]

They are used in multiple places of the standard library, in generic code that supports ?Sized types. (Typically when (de)allocating memory.)

[whitequark]

Of course that's easy for me to say since I don't use them, but I do think it's notable that C++ and many other languages lack these functions and still manage to get by without them just fine.

That's not the whole truth--every practical C++ compiler does support nonstandard intrinsics with those functions.

[comex]

@whitequark What intrinsics are you referring to? I imagine the equivalent in C++ would be something like

class Base {
    int a;
};

class Derived : public Base {
    int b;
};

void foo() {
    Base *obj = new Derived;
    size_t size = get_real_size(obj);
               // ^^^^^^^^^^^^^ some intrinsic?
    assert(size == sizeof(Derived));
    assert(size != sizeof(Base));
}

However, I've never heard of functionality that lets you do this. If you have RTTI enabled you can get a std::type_info for Derived, so I imagine an implementation could add methods to std::type_info to get the size and alignment, but from a quick search, I can't find documentation of such an extension existing under popular compilers. Without RTTI enabled, I've seen enough vtables in IDA to know that on most platforms, the size and alignment aren't stored anywhere :)

(If the object was allocated on the heap, there are nonstandard allocator functions that an approximation of the allocated size, like malloc_usable_size on Linux and malloc_size on Darwin, but those are different.)

[whitequark]

@comex __alignof__

[mikeyhew]

@whitequark I don't think sizeof and alignof in C++ work the same way. In C++, the closest equivalent to a trait object that I know of is a pointer or reference to a superclass that has a virtual method, if you do sizeof(superclass_reference) it will just give you the size of the superclass type, not the actual size of the value, which, if it is a subclass, could have extra fields. Here is an example I threw together to test it out with clang v5 and --std=c++17:

#include <iostream>
using namespace std;

struct SuperClass {
    virtual void some_method() {}
};

struct SubClass : public SuperClass {
    int field;
};

int main() {
    SubClass foo;
    SuperClass& bar = foo;

    cout << "sizeof(foo) = " << sizeof(foo) << endl;
    // sizeof(foo) = 16
    cout << "sizeof(bar) = " << sizeof(bar) << endl;
    // sizeof(bar) = 8
}

[whitequark]

Oh I see, thanks for explanation and sorry for misleading comment.

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.