[AuthZ] Middle ground between Requirement and Resource Handler

I like the resource handler. well done.
I am not much of a fan of the plain requirements since it encourages you to source the authZ data solely from the claims (CanUpdateTableB and other "funny" stuff).
Requirements would be much more useful if they would support DI - so we could inject our IPermissionRepository into it.
@blowdart  liked the idea and told me to open an issue.

@leastprivilege Can you elaborate a bit more about what you wanted?  We aren't quite clear what you had in mind for the IPermissionRepository?
Can you outline the usage?

So in a nutshell - it would be great if policies could go into DI.
The current design injects the user's claims as the only data source - which encourages people to treat claims as permission/authorization data (which they are not). Being able to inject other dependencies like a repository over a permission system would improve that situation.
Right now you have to new up the policies explicitly (and thus pass in dependencies explicitly)
does that make sense?

Are you just asking for something like this where you can plug in your own PolicyManager which maps names to requirements in way that doesn't necessarily involve the mechanical PolicyBuilder calls?
class AuthZPolicy {
 IEnumerable<IAuthZRequirement> Requirements { get; }
 IEnumerable<string> AuthenticationSchemes { get; }
}

interface IPolicyManager
 AuthZPolicy GetPolicy(string policyName)

The default AuthZ service would use the IPolicyManager to resolve policies instead of directly via the AuthZOptions.  Although, our default PolicyManager would continue to do this

Blocked until @leastprivilege is back from holiday

Working idea is enabling something like this which would activate the requirement using services from DI and pass in any additional arguments to the ctor
    AddAuthorization(options => options.AddPolicy("eighteen", policy => policy.AddRequirement<AgeRequirement>(18, new FileInfo("file.json"), ...)

looks good!

@leastprivilege After playing around with this some, I actually think this is no longer the right thing to do.  The intent of the current design is for the requirements to be basically data and simple.  The handlers are the consumers of the requirements and already live in DI and can depend on services.  So if you need a service to process a particular policy/requirements.  You can do that today already since you need to manually put your handler into DI.
For example:
    public class FooRequirement : IAuthorizationRequirement { }
    public class FooHandler : AuthorizationHandler<FooRequirement> {
       public FooHandler(DbContextAuth db) { }

       void Handle(AuthorizationContext context, FooRequirement requirement) {
          if (db.IsAuthorized(context.User)) context.Succeed(requirement);
       }
    }

cc @blowdart @divega @lodejard

@HaoK do you know of any working examples that demonstrate this? I'm trying to follow the example you gave in your last comment but my Handler never gets instantiated. All the examples I can find use the same class to inherit from IAuthorizationRequirement and AuthorizationHandler so I can't use DI

@robianmcd Did you find out how to do this? I'm in exactly the same situation. My handler is never instantiated and I can't find a working example anywhere. @HaoK Are you able to clarify on how to do this?

@Jargon64 no I never figured it out. I just gave up on using requirements :(

@robianmcd I've actually got a little further as I found this: https://docs.asp.net/en/latest/security/authorization/dependencyinjection.html
I can see that my handler is being instantiated now but the Handle method is never called. Beginning to think this is a bug so may open a ticket about it.

@Jargon64 I ran into the same problem. This solved it for me http://stackoverflow.com/a/37288885/373655 .

@robianmcd This looks interesting. I'm working on a different task at the moment but will definitely try this soon.
My concern with this solution is that I'm only using a custom authorization handler for a specific endpoint of my API. The rest is handled by the Identity system so I'm wondering if there's a way to have the workaround authenticated identity created for a user requesting a specific endpoint only?
At least I know why it's not calling the Handle method now and points me in the right direction. Thanks :)


ColorGlyphRunEnumerator.CurrentRun returns Intpr instead of ColorGlyphRun

Current CI builds seem to have some code generation issues. ColorGlyphRunEnumerator.CurrentRun returns Intptr instead of ColorGlyphRun. Atleast the marshaling code is available in the current master branch.
It seems the current CI builds dont map this correctly.
I have the following code:
var result = factory2.TryTranslateColorGlyphRun(
baselineOriginX,
baselineOriginY,
glyphRun,
glyphRunDescription,
measuringMode,
null,
0,
out var colorGlyphRunEnumerator);

if (result == Result.Ok)
{
    while (true)
    {
        colorGlyphRunEnumerator.MoveNext(out var hasRun);

        if (!hasRun)
        {
            break;
        }

        var colorGlyphRun = colorGlyphRunEnumerator.CurrentRun;

        brush = new SolidColorBrush(_renderTarget, colorGlyphRun.RunColor);

        _renderTarget.DrawGlyphRun(
             new RawVector2 { X = colorGlyphRun.BaselineOriginX, Y = colorGlyphRun.BaselineOriginY },
                            glyphRun,
                            brush,
                            measuringMode);
     }
         colorGlyphRunEnumerator.Dispose();
     }
    else
    {
         _renderTarget.DrawGlyphRun(baselineOrigin, glyphRun, brush, measuringMode);
    }
}

@jkoritzinsky could you have a look?

Will do!

From: Alexandre Mutel<EMAIL_ADDRESS>Sent: Thursday, August 23, 2018 8:27:15 AM
To: sharpdx/SharpDX
Cc: Jeremy Koritzinsky; Assign
Subject: Re: [sharpdx/SharpDX] ColorGlyphRunEnumerator.CurrentRun returns Intpr instead of ColorGlyphRun (#1067)
Assigned #1067https://github.com/sharpdx/SharpDX/issues/1067 to @jkoritzinskyhttps://github.com/jkoritzinsky.
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHubhttps://github.com/sharpdx/SharpDX/issues/1067#event-1805253320, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABf6UFEI_zTmqlkcZncuDehfT6RkerPNks5uTq2ygaJpZM4WJYFs.

Hmm okay didn't realize there was a method for that. Will have a try.

@Gillibald this will be fixed (with the expected property name like before) in the next version of SharpDX.

Thanks for this 👍


Add a default database adapter for production environment

The rails template is missing the adapter configuration for production in database.yml file
Therefore, I got the ActiveRecord::AdapterNotSpecified when building Docker image on production

On production, as default Rails is using DATABASE_URL format which is we can set the adapter in the URL, for example: postgres://mdhxjqtgwtrcgu:xxx.compute-1.amazonaws.com:5432/yyy

After checking with @hoangmirs and @andyduong1920, it seems there are 2 wordarounds:

As Andy suggested we can add the DATABASE_URL env variable with the postgres:// adapter. But that requires changing the CI/CD scripts
Hoang suggested to add only the adapter: postgresin the database.yml file (inside the production section). This is working too.

IMO while handling this issue, we should try to understand what the pre-compile assets script is doing and see if we cannot remove this script or update it so that it does not need a DB connection 🤔

Solution found by @hoangmirs 🥳
In the
Just adding the DATABASE_URL in the dummy env variables (bin/docker-assets-precompile)!
This prevents the fix to affect another place than the root issue (legacy of this file).
if rails_env == 'production'
  env_keys = YAML.load_file('config/application.yml')['docker_build'].keys
  env_keys.each { |name| ENV[name] = 'dummy_value' }

  ENV['DATABASE_URL'] = 'postgres://postgres:postgres@postgres:5432/postgres'
end

exit system('bin/rails i18n:js:export && bin/rails assets:precompile')


Ensure we resolve tags once.

As I've been refactoring things, I noticed a handful of places where a single command potentially used a name.Tag multiple times for remote access.  There is an exceedingly small window here for a race to happen where the first tag access points to one image, and the subsequent access points to another.
I've already fixed a handful of these, and this change fixes one more (in sget IIRC), but also tries to add some defensive logic in a few places where we were already doing the right thing.
The defensive logic I added is to clobber ref with digest after resolving the reference:
       digest, err := ociremote.ResolveDigest(ref, regOpts.ClientOpts(ctx)...)
       if err != nil {
               return err
       }
       // Overwrite "ref" with a digest to avoid a race where we use a tag
       // multiple times, and it potentially points to different things at
       // each access.
       ref = digest // nolint

This ensures that regardless of which reference is used below resolution, we always get what we resolved at this point.
There are some other superficial cleanups lumped in, which I noticed as I skimmed the code.
Signed-off-by: Matt Moore<EMAIL_ADDRESS>Ticket Link
Release Note
NONE

We might wanna alter ~most of the library to take digests (name.Digest, v1.Descriptor)


Split recovery image and system image into separate artifacts

Currently we are using the same image for the recovery partition than for the active (COS_ACTIVE) loop device. For a plain system like the one we are using now it might be reasonable, however for a derivative service specific appliance this approach does not have much sense.
I'd say that generically speaking having production services into the recovery partition is a bad idea (even a dangerous idea) and we should facilitate having some control about what is the recovery system and what is the production system.
For that I would simply create different images, system/cos and recovery/cos. system/cos used for the COS_ACTIVE and COS_PASSIVE  loop devices and the  recovery/cos one used for the COS_SYSTEM partition. Also the immutability approach could be expressed with different defaults for the recovery system than for the COS_ACTIVE system.
In addition to that and linked with #217 I believe having a recovery image contained in a squashfs filesystem is better for few reasons:

Immutability as it was well known live image, no persistence at all and whole system gets overlayed.
No need of applying any sort of custom layout code at boot (just use the well established and robust live boot procedure)
Compressed images so less space in disk, I'd say the performance penalty for a recovery system is just fine.
It can be populated without having to mount any filesystem (relevant for containerized builds)

Action items

[ ] Split system/cos into recovery/cos and system/cos, so that during upgrades it can be specified a different image to upgrade recovery from
[ ] Make the recovery script accept an image to use to recovery from, besides the current behavior which is replicating the recovery to the ACTIVE and PASSIVE
[ ] Move the recovery layout to squashfs instead of an image

The move to squashfs is more complex than it looks like: at the moment we are relabeling image files and copying them over instead of recreating them, and more over we should ship a squashfs within the final image, containing another image which is squash'fsed.

WIP branch at https://github.com/rancher-sandbox/cOS-toolkit/tree/split_recovery


Fix for short._ and manual imports.

Addresses issue #86 by modifying import short._ to work properly in the scope that the docs suggest.
Formerly, the test 'manualImports was defined after an import bundle.all._, allowing errors in the implementation to get through.
Note that the example from the docs that imported:
import bundle.{attrs => attr, styles => css, _}
import bundle.tags._

Now requires two additional imports:
import bundle.implicits._
import scalatags.DataConverters._

This looks good, thanks!


Can not get the right injected threadpool in TestNG

TestNG Version 7.1.0

Note: only the latest version is supported

Expected behavior
TestNG testNG = new TestNG();
testNG.setExecutorFactory(new DemoThreadPool());
IExecutorFactory factory = retrieveInjectionExecutorMechanism(testNG);
Assert.assertEquals(factory.getClass(), DemoThreadPool.class);

private IExecutorFactory retrieveInjectionExecutorMechanism(TestNG testNG) {
    try {
        Field field = TestNG.class.getDeclaredField("m_configuration");
        field.setAccessible(true);
        IConfiguration cfg = (IConfiguration) field.get(testNG);
        return cfg.getExecutorFactory();
    } catch (NoSuchFieldException | IllegalAccessException e) {
        throw new RuntimeException(e);
    }
}

Actual behavior
The assert failed because of the configuration's thread pool was different from what I set by setExecutorFactory
Is the issue reproductible on runner?
yes, of course.

[ ] Shell
[X] Maven
[ ] Gradle
[ ] Ant
[ ] Eclipse
[X] IntelliJ
[ ] NetBeans

Test case sample

Please, share the test case (as small as possible) which shows the issue

The example you shared as a test is incorrect. The below example shows this working.
You missed invoking testNG.initializeEverything(); before your assertion.
import java.util.Comparator;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.TimeUnit;
import org.testng.IDynamicGraph;
import org.testng.ISuite;
import org.testng.ITestNGMethod;
import org.testng.thread.IExecutorFactory;
import org.testng.thread.ITestNGThreadPoolExecutor;
import org.testng.thread.IThreadWorkerFactory;

public class DemoThreadPool implements IExecutorFactory {

  @Override
  public ITestNGThreadPoolExecutor newSuiteExecutor(String name, IDynamicGraph<ISuite> graph,
      IThreadWorkerFactory<ISuite> factory, int corePoolSize, int maximumPoolSize,
      long keepAliveTime, TimeUnit unit, BlockingQueue<Runnable> workQueue,
      Comparator<ISuite> comparator) {
    return null;
  }

  @Override
  public ITestNGThreadPoolExecutor newTestMethodExecutor(String name,
      IDynamicGraph<ITestNGMethod> graph, IThreadWorkerFactory<ITestNGMethod> factory,
      int corePoolSize, int maximumPoolSize, long keepAliveTime, TimeUnit unit,
      BlockingQueue<Runnable> workQueue, Comparator<ITestNGMethod> comparator) {
    return null;
  }
}

Test example
import java.lang.reflect.Field;
import org.testng.Assert;
import org.testng.TestNG;
import org.testng.annotations.Test;
import org.testng.internal.IConfiguration;
import org.testng.thread.IExecutorFactory;

public class IssueTest {

  @Test
  public void runTest() {
    TestNG testNG = new TestNG();
    testNG.setExecutorFactory(new DemoThreadPool());
    testNG.initializeEverything(); // This line was missing
    IExecutorFactory factory = retrieveInjectionExecutorMechanism(testNG);
    Assert.assertEquals(factory.getClass(), DemoThreadPool.class);
  }

  private IExecutorFactory retrieveInjectionExecutorMechanism(TestNG testNG) {
    try {
      Field field = TestNG.class.getDeclaredField("m_configuration");
      field.setAccessible(true);
      IConfiguration cfg = (IConfiguration) field.get(testNG);
      return cfg.getExecutorFactory();
    } catch (NoSuchFieldException | IllegalAccessException e) {
      throw new RuntimeException(e);
    }
  }
}

Thank you for your example, I tried your solution and I think it works well.
There is one thing I noticed is that there is another example in code:
@Test(description = "GITHUB-2207")
 public void testInjectorFactoryCanBeConfiguredViaProperties() {
    Map<String, String> params = new HashMap<>();
    params.put(
        CommandLineArgs.DEPENDENCY_INJECTOR_FACTORY, TestInjectorFactory.class.getName());
    TestNG testNG = new TestNG();
    testNG.configure(params);
    IInjectorFactory resolvedInjectorFactory = retrieveInjectionMechanism(testNG);
    Assert.assertEquals(resolvedInjectorFactory.getClass(), TestInjectorFactory.class);
  }

This example can work without testNG.initializeEverything();
public void testPluginDependencyInjector() {
        TestNG testNG = new TestNG();
        testNG.setInjectorFactory(new TestInjectorFactory());

        IInjectorFactory resolvedInjectorFactory = retrieveInjectionMechanism(testNG);
        Assert.assertEquals(resolvedInjectorFactory.getClass(), TestInjectorFactory.class);
    }

And I check the source code and I found that the setter method for 'IInjectorFactory ' and 'IExecutorFactory' is a little bit different in TestNG.java
public void setExecutorFactory(IExecutorFactory factory) {
    this.m_executorFactory = factory;
  }

 public void setInjectorFactory(IInjectorFactory factory) {
    this.m_configuration.setInjectorFactory(factory);
  }

I don't know it's a designed feature or not. And when set the executor and doesn't do init, the executor in IConfig and TestNG may be different, I don't know if this would be a problem when using this feature.


RFC: Pointer metadata & VTable

Add generic APIs that allow manipulating the metadata of fat pointers:

Naming the metadata’s type  (as an associated type)
Extracting metadata from a pointer
Reconstructing a pointer from a data pointer and metadata
Representing vtables, the metadata for trait objects, as a type with some limited API

This RFC does not propose a mechanism for defining custom dynamically-sized types, but tries to stay compatible with future proposals that do.
HTML view

A previous iteration of this RFC is also visible at https://github.com/rust-lang/rfcs/pull/2579. It was based on @Gankro’s proposal https://gist.github.com/Gankro/b053cb4d1cb3bcaec070de89734720f7

First of all, thanks for opening this rfc. It's the right way to fix the raw::TraitObject API, and is a big step toward custom DST.
My only criticism is I think the Vtable type should be generic over the trait object type, as mentioned in the alternatives section. Having different types for each trait object type would help catch errors at compile time. Also, using &'static would preclude us from implementing trait object types like dyn Trait1 + Trait2 using multiple vtable pointers, and i think it would be nice to avoid making that decision in this RFC.
I like the name Pointee, I think it's an improvement over Referent because it's more clear what it means.

cc @ubsan

@mikeyhew I’ve very open to adding a type parameter to VTable, I’ll just wait to get some more feedback on the RFC as-is before making significant changes.
As to supporting super-fat pointers with multiple vtable pointers, as mentioned in the alternatives section I believe this design doesn’t prevent it. Types that don’t exist yet and are added to the language in the future (possibly custom DSTs) can have a different metadata type. For dyn A + B that could be [&'static VTable; 2], for example. This proposal does however force dyn C with trait C: A + B  {} to keep only having a single vtable pointer.

Is there any way to ensure minimal compiler time is wasted performing monomorphization on useless vtable type params? If not, I would rather the API just be less safe.

@Gankro the word "useless" sounds a little strong there... the purpose it to make sure you can't use a vtable from a *const dyn Trait to make a *const dyn SomeOtherTrait, at least not without doing something obviously hacky like transmute.

You're supposing a situation where I somehow am writing code with two vtable types floating around, in which case I have two data pointers, and nothing can stop me from swapping the data pointers, producing the exact same effect.

No idea, compiler team is better to ask about this sort of thing

This is a very good point. If used heavily this could become problematic.
@Gankro's point about the parameter being useless makes me think that we shouldn't waste any time thinking about the performance of this and just not do it. Two raw pointers with the same trait but different concrete type can still be swapped, because their vtable type is the same.
I also don't see us getting different vtable layouts for different traits. Any fancy new additions to the type system will still be able to change their fat pointer type to something other than Vtable (like FancyVtable) if the Pointee impls are autogenerated by the compiler anyway.

@Gankro

You're supposing a situation where I somehow am writing code with two vtable types floating around, in which case I have two data pointers, and nothing can stop me from swapping the data pointers, producing the exact same effect.

Well sure, but invalid vtables are different than invalid data pointers. I argue that the metadata for a *const dyn Trait should always be valid, whereas it's understood that the data pointer may not be.

Can you give some example code, that you imagine someone will actually write, which you are concerned will compile without these extra types?
Keeping in mind this is an ultra-low-level API that likely only a handful of people will ever use, and only when building larger abstractions like TraitVec<SomeTrait> or ThinBox<Trait>.

Can you give some example code, that you imagine someone will actually write, which you are concerned will compile without these extra types?

Let's say you wrote some code using this API on a dyn Trait1. Then later in some refactor, you switch from dyn Trait1 to dyn Trait2 in one place but forget to in the other place.
let vtable = (ptr::null::<()>() as *const dyn Trait1).metadata();
let ptr: *const dyn Trait2 = ptr::from_raw_parts(vtable, ptr::null::<()>())

// We now have a `*const dyn Trait2` with invalid metadata
// If we guarantee that `*const dyn Trait` should always have valid metadata,
// then this is bad. Calling `ptr.raw_pointer_method()`, possible right now with
// `#![feature(arbitrary_self_types)]` would cause UB.

That doesn't work anyway, unless you're specifically looking for the vtable corresponding to impl Trait1 for (). It seems just as likely that you might switch types rather than traits, which isn't really fixable.

@rpjohnst

That doesn't work anyway, unless you're specifically looking for the vtable corresponding to impl Trait1 for ().

You're saying the above code won't compile anyway? I'm not sure I follow. The point is that it produces a *const dyn Trait with a vtable for the wrong trait, which should not compile.

It seems just as likely that you might switch types rather than traits, which isn't really fixable

Right, that's sort of what @Gankro said: it's just as easy to have a data pointer that points to invalid data. But like I've been saying, for a *const dyn Trait having an invalid vtable pointer is worse than having an invalid data pointer.

This is why I requested an example someone will actually write, which your example doesn't strike me as (feel free to correct me by fleshing it out into something recognizable as actual code). I cannot imagine why someone would construct *const dyn Trait without any intent to, you know, access it as an actual implementor of Trait. You can't do loads, stores, or offsets with such a pointer because the pointee is ?Sized. Literally all you can do is access the vtable methods, in which case getting the data pointer wrong is a much more plausible failure mode.

I can keep explaining my position, but I think the point's been made that having different metadata types for different trait object types is useful.
Obviously I don't want compile times to explode, but I'm just not sure this is going to be that big of an issue. Where would it come up that code would be monomorphized if Vtable had a type parameter, and not be monomorphized if it didn't? It would have to be code that operates on vtables, and not the trait objects that they came from, and I can't imagine there would be very much of that. Maybe @Gankro or someone else can shed some light on possible situations where there would be an impact.

You are correct that use of this API would generally be wrapped up in a generic abstraction (especially the entry-point which would likely be fully generic over the concrete types because Rust currently lacks any way to claim ownership without pass-by-value, and DSTs cannot be passed by value, short of passing them in a Box which largely defeats the purpose of this API).
But we want to minimize the set of things that the compiler thinks are monomorphic, and really encourage shared/small code, which adding more generics can only hurt.

I’ve pushed some more changes, including code for ThinBox which hopefully makes a more compelling example.
Interestingly, the only reason ThinBox is restricted to trait objects rather than supporting any DST is in order to use VTable::align: knowing the value’s alignment is required in order to construct a pointer to it.
This problem is not specific to ThinBox. It was brought up before by @kennytm in the context of a custom DSTs discussion:
https://internals.rust-lang.org/t/pre-erfc-lets-fix-dsts/6663/2

DynAligned doesn’t work. The minimal promise a type can give about alignment, if any, is AlignFromMeta . This is because in a DST struct
struct X<T: DynAligned> {
    a: u8,
    b: T,
}

if you need to read &x.b , you’ll first need to find the offset of b , which in turn requires knowing the alignment of b without knowing its pointer value.

Since being able to compute the alignment based based on only a (DST) type and the pointer metadata is already a constraint of the language, how about exposing an API doing that?
// std::mem
// Very much subject to bikeshed
pub fn align_for_metadata::<T: ?Sized>(meta: <T as Pointee>::Metadata) -> usize {…}

Or
trait Pointee {
    // Already in the RFC:
    type Metadata: Copy + Send + Sync + Ord + Hash + 'static;

    // Added:
    fn align(meta: Self::Metadata) -> usize;
}

@F001 Which trait do you mean? There is no trait named TraitObject in this RFC.

@SimonSapin The first one (pub fn std::mem::align_for_meta) doesn't seem compatible with Custom DST since external code cannot override a free function.
BTW, the alignment of an extern type is undefined. Though I'd implement <extern type>::align() to panic 🙂.

Is it undefined? What about code like this, then? (Though this is getting into discussion for https://github.com/rust-lang/rust/issues/43467 rather than here…)
#![feature(extern_types)]
extern { pub type Foo; }
pub struct Bar(u8, Foo);
pub fn baz(x: &Bar) -> &Foo {
    &x.1
}

(Playground)

@SimonSapin Yeah that's getting off-topic, the extern-type-in-a-struct discussed before starting from https://github.com/rust-lang/rust/issues/43467#issuecomment-377639742 I think. This code works today because we're still emitting the fallback values size_of_val(x) == 0 and align_of_val(x) == 1 instead of abort()/panic!().
What I mean is that Pointee::align may diverge, but that's a very special case and is simpler than non-panicking alternatives like returning an Option<usize>.

I think we're getting off topic here: this RFC shouldn't provide an API for calculating the alignment, because there is not as much agreement on how to do so. Some of us would prefer to have traits like AlignFromMeta and AlignFromRef, whereas others would be OK with having a function that panics.

I think we're getting off topic here: this RFC shouldn't provide an API for calculating the alignment, because there is not as much agreement on how to do so. Some of us would prefer to have traits like AlignFromMeta and AlignFromRef, whereas others would be OK with having a function that panics.

This RFC is literally useless without alignment, as noted many times in the text and examples

Right, both ThinBox and VecOfDst absolutely require knowing the alignment of values.

A few questions:

I think that the metadata type of trait objects in this RFC is &'static VTable, not VTable. Should VTable be an extern type so that users can't assume anything about its representation?
I'm not certain the bounds on Metadata are useful. When would you want to write code generic over pointees with different metadatas that manipulates the metadata?

The point of the bounds is to provide forward compatibility with Custom DSTs. For sure you can't impl Pointee for X with this RFC alone, and with built-in types the list would be irrelevant. But once we get Custom DST the same list will be needed anyway, so I don't see any harm being more specific upfront.

@kennytm why is the list needed? what relies on those guarantees?

@withoutboats This is needed for Custom DSTs because *T should be equivalent to (*(), T::Metadata) (also note that all from_raw_parts functions in this RFC are safe).
For example, if Metadata does not require Copy, a Custom DST could make Vec<usize> a Metadata and then we can "copy" the Vec via:
// all safe!
let ptr = <*const CustomDst>::from_raw_parts(&(), vec![1,2,3]);
let ptr2 = ptr;
let ptr3 = ptr;
let vec2 = metadata(ptr);
let vec3 = metadata(ptr);

The whole list Sized + Copy + Send + Sync + Ord + Hash + Unpin + 'static is derived from various such impls on pointers and references.
We could restrict the impl<T: ?Sized> Copy for *const T to where T::Metadata: Copy, but this will break existing generic code. We could prevent Custom DSTs from having non-Copy metadata but that's equivalent to adding this list.
Again, the list is irrelevant if no one can impl Pointee, it is mostly for future compatibility with Custom DST.
(A copy of the relevant section from kennytm/rfcs#2)
When manipulating generic DSTs, we often need to use its metadata type. This can be exposed via associated type if the DST implements some trait.
type Pointee {
    type Metadata: Sized;
}

That is, every type &Dst will be represented as the tuple (&Something, Dst::Metadata) in memory.
We need to ensure all existing traits implemented for pointers and references (&T, *T, Box<T> etc) are not affected by custom DST. This imposes many restrictions on Metadata:

Copy — &T and *T are both Copy

Send — &T is Send as long as T is Sync, without considering T::Metadata.

Sync — &T is Sync as long as T is Sync, without considering T::Metadata.

Ord — *T is ordered by the pointer value + metadata together. Demonstration:
let a: &[u8] = &[1, 2, 3];
let b: &[u8] = &a[..2];
let a_ptr: *const [u8] = a;
let b_ptr: *const [u8] = b;
// the thin-pointer part are equal...
assert_eq!(a_ptr as *const u8, b_ptr as *const u8);
// but together with the metadata, they are different.
assert!(a_ptr > b_ptr);

Hash — *T hashes the pointer value + metadata together.

Unpin — &T is always Unpin (Copy does not imply Unpin).

'static — *T should outlive T, thus T::Metadata should outlive T. There is no 'self lifetime, nor it makes sense to complicate the matters by making Metadata a GAT, thus the 'static bound.

Thus, the final constraint would be:
trait Pointee {
    type Metadata: Sized + Copy + Send + Sync + Ord + Hash + Unpin + 'static;
}

In principle Metadata should be further bound by UnwindSafe and RefUnwindSafe, but these traits are defined in libstd instead of libcore, so they cannot be included in the bound. One may need to explicitly opt-out of RefUnwindSafe according to the metadata type.
impl<T: ?Sized> !RefUnwindSafe for T {}
impl<T: ?Sized> RefUnwindSafe for T where T::Metadata: UnwindSafe {}

Fortunately, RefUnwindSafe is only excluded for UnsafeCell<T>, and UnwindSafe is only excluded for &mut T, both of which are handled by the Copy bound already.
The Metadata type should also be bound by Freeze (i.e. cell-free), but Freeze is a private trait, thus cannot be exposed to public. Again, Copy already eliminated the possibility of having cells.

When would you want to write code generic over pointees with different metadatas that manipulates the metadata?

It’s rather niche, and likely we’ll end up with a small number of library that do that, but for example ThinBox and VecOfDst both could (almost https://github.com/rust-lang/rfcs/pull/2580#issuecomment-434726299) be generalized to work with any DST.

@kennytm your comments make sense, thanks! and it is concerning that this represents a forward compat hazard with adding new autotraits like Unpin, something we'll have to keep in mind.
However, I don't think the metadata influences the implementations of Ord and Hash, do they? So why are those necessary?

@withoutboats For now the both Hash and Ord impl for pointers compare their content as if opaque bytes (e.g. for Hash the implementation just transmutes to (usize, usize) and hash the tuple). I don't think transmute works when Custom DST is added.
Since we need to keep these impls without adding new where bounds:
impl<T: ?Sized> Hash for *const T { ... }
impl<T: ?Sized> Ord for *const T { ... }

and we need to keep comparing the metadata to maintain the runtime behavior, we'll need to add the Ord + Hash constraints.

For Unpin — interesting. I believe Pin<&mut CustomDst> not capturing the metadata in the pin is irrelevant. The interesting case is
Pin<&mut &CustomDst>  // this is a _thin_ pointer to a fat pointer

The inner reference is always Unpin currently, meaning we could safely Pin::get_mut to obtain the &mut &CustomDst, and we could mem::swap it with a different &CustomDst. If CustomDst::Metadata: !Unpin, this would have violated the pin guarantee.
Fortunately there's no safe way convert an arbitrary Pin<&mut T> to a Pin<&mut &CustomDst> (unlike Copy) so the damage is quite limited if Unpin is not on the list.

@kennytm I was thinking of reference types, not pointer types. The behavior of pointer types is interesting, maybe even unfortunate..

Because of impl<T: ?Sized> Eq for *const T the metadata has to be Eq too.

Ord already implies PartialEq + PartialOrd + Eq.

Would it be OK if we changed the Metadata type for dyn Trait to be this?
struct DynTraitMeta<T: ?Sized> {
    vtable: &'static Vtable,
    // not exactly sure what to put here, but this works for now. It must contain `T`.
    _marker: PhantomData<*const T>,
}

This type could be kept unstable, so that it could be changed in the future (potentially to &'static Vtable directly, if that's really what people want).
I'm uneasy about using &'static Vtable directly, not only because it means the type would be the same for all dyn Trait types (more on that later), but also because using &'static exposes an implementation detail that we don't need to expose — the fact that the metadata is a single, valid pointer to a type that we also need to expose. I think we can get away with keeping the metadata opaque, as long as we provide methods to get the size and alignment.

guarantee that the metadata for any *const dyn Trait is always valid

Extending data invariants like this has repercussions beyond just this RFC. For example, it can make unsafe code invalid even if it doesn't interact with the abstractions introduced here, and if we take this step we should do due digilence to check that doesn't affect any unsafe code we want to allow. Not necessarily a problem, but it's extra work that needs to be motivated.

and therefore have safe dynamic dispatch even when *const dyn Trait is the type of self

I don't understand how anything substantial can be made safe this way. The data pointer part of a raw pointer can be invalid, so even if one defines a method with raw pointer receiver type and can call it that way, that method couldn't really do anything with the trait object (if it's safe). While there are some bits and pieces of information one might want to put into a trait that are solely about the type it's implemented on rather than a specific object,

there are other more direct ways to support this (e.g., allowing access to associated constants through trait objects)
if an actual trait object is available, there's no problem even today, so this only helps if no trait object is available (in which case one could use e.g. a *const dyn Trait with the data pointer being null)
in those cases, people would have to write code passing around unusual types like Vtable<dyn Trait> or *const dyn Trait and it's not clear to me how that is any better than existing workarounds that pass around an unusual type like &'static OneOffTableOfImplSpecificData

have <*const dyn Trait>::from_raw_parts be safe functions

We have an opportunity to make something that sounds unsafe — messing around with pointer metadata and vtables — actually be a completely safe thing to do. I think that's pretty cool, and it's a great example of the kind of programming Rust lets you do.

This function is trivial and all the uses I know of are embedded into a context where its use is crucial to other, far more subtle, unsafe code. For example, making this function safe does not help at all when converting the *const dyn Trait constructed with from_raw_parts into a &'a dyn Trait. Can you give an example of useful code that could be written using only this function (and other safe functions)?
If not, I do not see any reason why this would make anything safer. As a general principle, usage of nominally-safe functions deeply embedded in unsafe code is often critical for the soundness of the whole code, so the advantage of making more functionality safe lies in enabling entirely safe code, not in being able to move one important line out of the unsafe block.

And there are no downsides here. The only one that was suggested — potentially extra monomorphization because of an extra type parameter — doesn't hold any water. And even if it did, it seems like something that would come up so rarely that it wouldn't matter, or that could be fixed by future optimizations.

Sorry, just asserting that is not convincing. That type parameters lead to monomorphization today is a fact. That monomorphization generally causes binary size issues is also well established. That only a neglegible amount of code would be redundantly monomorphized is not obvious to me -- there are entire families of data structures that could be built on this abstraction, not to mention all the generic utilities that may be used while working on those vtable pointers.
Appeals to future optimizations / "sufficiently smart compilers" also aren't great, as the extent and efficancy of these optimizations is still somewhat unknown (as they aren't implemented and we also don't yet know all the details of the code we'd want to optimize). Not to mention that even if the overhead would eventually become zero, it's still not great to have the overhead for the medium-term future.

The question of whether metadata value in a raw pointer should always be valid is completely independent of whether the metadata type for dyn Trait of different traits should be the same type.
from_raw_parts cannot be safe either way. Even if the type signature of that function includes the trait of dyn Trait, it does not include the concrete type that implements the trait. The vtable for *const u32 as *const dyn Display is not interchangeable with the vtable for *const String as *const dyn Display.

@SimonSapin

The vtable for *const u32 as *const dyn Display is not interchangeable with the vtable for *const String as *const dyn Display

Actually, it is. (Sort of.)
If a *const dyn Trait has the vtable for <String as Trait> when it actually points to a u32, the vtable is still valid. It is the data pointer that is not valid, and that's the distinction I'm trying to make here.

@rkruppe

Extending data invariants like this has repercussions beyond just this RFC. For example, it can make unsafe code invalid even if it doesn't interact with the abstractions introduced here, and if we take this step we should do due digilence to check that doesn't affect any unsafe code we want to allow. Not necessarily a problem, but it's extra work that needs to be motivated.

Fair point. I'm with you on that.

And there are no downsides here. The only one that was suggested — potentially extra monomorphization because of an extra type parameter — doesn't hold any water. And even if it did, it seems like something that would come up so rarely that it wouldn't matter, or that could be fixed by future optimizations.

Sorry, just asserting that is not convincing. That type parameters lead to monomorphization today is a fact. That monomorphization generally causes binary size issues is also well established. That only a neglegible amount of code would be redundantly monomorphized is not obvious to me -- there are entire families of data structures that could be built on this abstraction, not to mention all the generic utilities that may be used while working on those vtable pointers.

OK. Well I'm not sure what it will take to convince you, but I'll try.
Like I said earlier in this thread, all those data structures and generic utilities will already be generic on the pointee type. Or they might be specific to one trait object type. This one extra struct having the same type parameter that everything else does won't result in any extra functions being monomorphized in either of those cases, because either the code would be monomorphized anyway (in the case of code that is generic over the pointee type), or it would not be monomorphized even with the type parameter (in the case of code that is specific to one dyn Trait type).
If you're still not convinced, give me an example of code that you would want to write that you think would not be monomorphized if the metadata type didn't have the type parameter, and would be monomorphized if it did.

give me an example of code that you would want to write that you think would not be monomorphized if the metadata type didn't have the type parameter, and would be monomorphized if it did.

Even the exampled sketched in the RFC, ThinBox, has instances of this. While the ThinBox type itself is generic over the trait object type, it stores a NonNull<WithVtable<()>> and the trait object type is just stashed in a PhantomData. That means some of the code ThinBox uses can be monomorphic (other parts still need to know the trait object type to do unsizing/drop_in_place/etc. though some of that could be made obsolete with further RFCs).
Specifically, let's look at fn data_ptr, which I pick mostly because it's already in the RFC (I expect that a full implementation of ThinBox will have more instances -- to say nothing of other, more complex data structures like a heterogeneous list of trait objects). While data_ptr is polymorphic, that's only due to taking ThinBox<dyn Foo> and being nested in its impl, its body only needs to operate on a *const () based on the size and align from the vtable and so can be extracted into a monomorphic function. That leaves the polymorphic code as tiny as extracting self.ptr: NonNull<WithVtable<()>> (which will be a nop or just a move in the binary, and also a nop in LLVM IR once a certain refactoring finally finishes) and passing it to the monomorphic code.
If the type of the vtable is parametrized by the trait object, one can still extract the arithmetic into monomorphic code, but it's less natural (need to extract size+align and pass them separately) and the polymorphic code contains some more instructions which are then replicated for all trait object types.

@rkruppe thanks for replying with an example. I'm super busy right now but I'll try to read through it and get back to you next week

Great RFC @SimonSapin. I really like the idea of starting with top most super trait (Pointee) and then working "down" the trait hierarchy towards custom DSTs.

Nominating for @rust-lang/lang feedback at @SimonSapin's request. I've not had time to dig into this myself. But it also seems like something that could be folded into the @rust-lang/wg-unsafe-code-guidelines's ambit.

Sorry for taking so long to get back to this!
@rkruppe you do make some good points, I guess it's more of a grey area than I thought. I think at this point it would be good to put it down as an unresolved question whether the metadata type for dyn Trait should be the same or different from dyn OtherTrait. In the meantime, I would propose we go with the DynTraitMeta<dyn Trait> approach that I posted earlier, and keep that type unstable. If we give access to the &'static VTable field it contains, then people can experiment writing code with &'static VTable to see if it reduces compile times or binary size. If it turns out there is a big impact because of extra monomorphization, and the API isn't practically any safer to use, then we can consider changing the API to make the metadata type be &'static VTable directly.
Let me know what you think about that.

I think it doesn’t make sense to keep a type unstable if stable code can manipulate values of that type.

@SimonSapin no VTable would still be unstable, I just meant we should make it available (but unstable) so that people can experiment with it and see if it helps reduce monomorphization.

If <dyn SomeTrait as Pointee>::Metadata is DynTraitMeta<dyn SomeTrait> and is unstable, that means pub fn metadata<T: ?Sized>(ptr: *const T) -> <T as Pointee>::Metadata should also be unstable. Otherwise stable code would be able to obtain a value of that type, which makes it de-facto stable. At that point almost everything proposed in this RFC is unstable.
Keeping everything from a new RFC unstable for a while so that people can experiment is the normal process.

@SimonSapin I had the idea that in some distant future, we could let people create <dyn Trait>::Metadata values in stable code, but force them to use <dyn Trait>::Metadata and not the actual type that that refers to, so that we would be free to change it from e.g. DynTraitMeta to &'static VTable without breaking backward compatibility. I don't know, maybe that wouldn't work, but that's just what I was thinking. But anyway, I guess this is so far off in the future that it's not useful to discuss it at this point, so sorry for cluttering up the RFC thread with it.

We discussed this RFC in our @rust-lang/lang meeting yesterday. We had a bit of trouble deciding what to do about it. The problem is not so much in the content of the RFC as it is in the process questions -- specifically, we are trying to adopt a more directed structure, where we don't start a bunch of things and then leave them half-done, but instead only start things when we are dedicated to seeing them through. One key part of that is creating working groups around the tasks we are highlighting, and trying to coordinate with e.g. the compiler team for implementation work etc.
This RFC did not obviously align with any of our existing working groups or roadmap goals. Admittedly, those need to be more crisply laid out! I had hoped to point you at the lang-team repository to find a good listing of those things, but I realize now that content isn't really there. I guess the best source for it is my blog post.
One thing we have talked about but clearly haven't quite settled is what to do with existing RFCs, and how to leave room for "spontaneity" -- like this RFC -- while still respecting our overall bandwidth and priorities.
Anyway, all of this preamble is to say, one thing that would help us -- and forgive me if it is present on thread -- is to try to place this RFC in the context of bigger goals. For example:

What is the motivation do make this change now -- does it unblock work?

We thought maybe it would help with stabilizing extern types, by potentially providing traits that could be used for std::null beyond Sized

If you were going to put this in a working group, what would it be?

It seemed to me like it would most naturally fall into a "Custom DST" working group, but perhaps that isn't quite right.

There was a specific concern about the timing with respect to dyn (A+B) types -- i.e., should we try to work out how those work first, to ensure that this RFC is compatible with that? At minimum, we should explore the space for that design, and then ensure compatibility of this RFC with all the options on the table (which don't seem to me to be that many, I suspect it would work out fine). Has any such work been done?

I think ultimately the options on the table are:

Create a working group around this RFC to work out the future compat questions, get it implemented, etc. It's not clear that we have enough people to allocate to make sure that gets done, but maybe there is enough energy around -- e.g., from @SimonSapin etc. -- but it would still take at least some time for "reviewing".
Add this RFC to a "shortlist" of things to pick up as we proceed -- the more we can link it to our priorities, the better.
Postpone for a longer period.

I'm going to leave this nominated so that we check again on the answers to these questions in the next meeting.

The original motivation for this RFC is to unblock libraries that are generic over various kinds of DSTs, such a Vec-like collection that stores dyn Trait items of different sizes in one contiguous allocation. This arguably extremely niche, and maybe by itself not worth prioritizing this RFC over an other.
A side effect of this design is the ability to precisely express at the trait level pointers to this type are thin. Today T: Sized provides this property, but excludes extern types. This would indeed allow APIs like ptr::null to be extended to support extern types, and so help push extern types themselves along.

Pointer metadata is all about DSTs. So this RFC is very relevant to a Custom DSTs working group in my opinion, although it is a very small part of what’s in scope for that group. I’ve tried to make this RFC minimal to hopefully not depend on as many big unresolved questions as previous proposals in this space, and achieve incremental progress.
Out of the ones listed in your blog post, I would say this RFC is closest to the realm of the Traits working group. (Because Metadata = usize for slices and Metadata = () for thin pointers are comparitively trivial cases, which only leaves dyn Trait types.)

At this time I am not volunteering to join a working group or champion the design exploration work for trait objects with multiple traits or for trait object upcasting. However the RFC does mention that if we ever decide to have “super-thick” pointers in the language they would fit in the proposed design by having a different Pointee::Metadata associated type, possibly [&'static VTable; N].

As to next steps, it sounds like this is more about the lang team deciding what process they want to have, rather than about this RFC specifically.

@nikomatsakis I'm surprised this hit process concerns. I really like this RFC precisely because it unblocks so much work to take place in parallel to the core teams efforts. Little RFCs like this, that would stay unstable for a while while they trickle into more comprehensive plans, are precisely the type of RFC I'd like to see more.

@nikomatsakis How does the team want to proceed here?

weirdly-sized-types-wg!

This RFC did not obviously align with any of our existing working groups or roadmap goals.

The 2019 roadmap includes “Finish long-standing features”. I argue that resolving a 1.0-era tracking issue (https://github.com/rust-lang/rust/issues/27751) fits that description.
This RFC is not adding fundamentally new features to the language such as new kinds of types. Trait objects are already there, only without an official way to decompose or recompose them to/from components, so far.

If you were going to put this in a working group, what would it be?

The topic I'd suggest would be unsized types in general. That would also include the work on unsized values that @qnighy has been working on, coming up with a solution for DerefMove, relaxing Sized bounds on types and functions in the standard library (e.g. MaybeUninit), adding the Pointee trait and related APIs from this RFC, extern types, multi-trait trait objects (dyn (A + B)), specifying whether *const dyn Trait must have a valid vtable or not, exploring DST enums and unions, and maybe, at some point, Custom DSTs.

The RFC currently proposes that the pointer metadata associated type for trait objects is a static reference to some struct, <dyn SomeTrait as Pointee>::Metadata = &'static VTable.
But this raises some uncomfortable questions: what is size_of::<VTable>()? What does it mean to dereference this reference?
Instead, we could have the associated type be directly a struct (that contains a pointer or reference):
pub struct SomethingSomething(&'static ());

But now this value is not the vtable, so the struct should not be named VTable.  It contains a pointer to the vtable, so maybe name it VTablePtr? Or maybe it we should rather avoid the “vtable” term and use a tautological name, some variation of DynTraitMetadata or DynMeta.

DynTraitMetadataPtr?

The standard library contains this “interesting” snippet:
https://github.com/rust-lang/rust/blob/1.36.0/src/liballoc/rc.rs#L723
// Sets the data pointer of a `?Sized` raw pointer.
//
// For a slice/trait object, this sets the `data` field and leaves the rest
// unchanged. For a sized raw pointer, this simply sets the pointer.
unsafe fn set_data_ptr<T: ?Sized, U>(mut ptr: *mut T, data: *mut U) -> *mut T {
    ptr::write(&mut ptr as *mut _ as *mut *mut u8, data as *mut u8);
    ptr
}

This is the kind of work-around that the absence of a proper <*mut T>::from_raw_parts API leads to.

Is there any plan to make the drop_in_place pointer reachable from the vtable pointer? I played a bit with TraitObject earlier today and the drop code was an issue.

@nox With this RFC accepted, you could use <*mut dyn Trait>::from_raw_parts(data, vtable).drop_in_place()

I just wanted to mention here a create of mine which (ab)use the layout of trait object to embed a virtual table pointer inside the struct:  https://github.com/ogoffart/vptr
Stable method such as from_raw_parts and metadata would make the implementation easier,  but at least the later would need to be a const fn to be able to build this extra table at compile time in static memory.

But at this point you do already assumptions about unspecified parts about
for rust creates assembly/handles the stack. Without such assumptions it's
indisguishable between the compiler retuning a reference into the tuple and
it returning a (arbitrary) const value.
But I get your point people already rely on such thinks. Which is what I
missed I though people only really on such things for pointers. Sad that we
missed making zero-sized types more efficient but then it probably doesn't
matter much ;=)
On Thu, Sep 10, 2020, 19:53 Ralf Jung<EMAIL_ADDRESS>wrote:

@RalfJung commented on this pull request.
In text/0000-ptr-meta.md
https://github.com/rust-lang/rfcs/pull/2580#discussion_r486527651:

+/// * type size

+/// * type alignment
+/// * a pointer to the type’s drop_in_place impl (may be a no-op for plain-old-data)
+/// * pointers to all the methods for the type’s implementation of the trait
+///
+/// Note that the first three are special because they’re necessary to allocate, drop,
+/// and deallocate any trait object.
+///
+/// The layout of vtables is still unspecified, so this type is a more-type-safe
+/// convenience for accessing those 3 special values. Note however that DynMetadata does
+/// not actually know the trait it’s associated with, indicating that, at very least,
+/// the location of size, align, and drop_in_place is identical for all
+/// trait object vtables in a single program.
+#[derive(Copy, Clone)]
+pub struct DynMetadata {

vtable_ptr: &'static (),

all &() have the same compiler predetermined address
That is certainly not true today, and it would be hard to make true. When
you take a reference to the first field of a ((), i32), you'll get a
reference that actually points to that memory where the pair is stored.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/rust-lang/rfcs/pull/2580#discussion_r486527651, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AB2HJEPTVCOUUY6ZLCTEEELSFEHAXANCNFSM4F7WKGAA
.

@rustonaut you suggested to introduce a new assumption ("all &() references have the same underlying value"). I just explained why that assumption is (a) not true today, so you'd have to do some work to achieve it, and (b) hard to achieve. I'd also add (c) even if we fix rustc itself, there's still going to be way too much unsafe code out there that does not satisfy your new assumption.

My main point was that I wasn't sure if the assumption upholds that a
arbitrary typed (static) pointer can safely be converted to an &'static (). And with safely I don't mean it  happens to work with the current
implementation but that it's okay wrt. all accepted RFCs and less formal
"specifications"/"agreements" about how it works. I shouldn't ever have
mentioned anything about const value references. This was just misleading
the discussion.
But yes I now full agree on your point (c) people rely on it working the
way it currently happens to work independent of weather this is a
implementation detail or not.
On Fri, Sep 11, 2020, 12:07 Ralf Jung<EMAIL_ADDRESS>wrote:

@rustonaut https://github.com/rustonaut you suggested to introduce a
new assumption ("all &() references have the same underlying value"). I
just explained why that assumption is (a) not true today, so you'd have to
do some work to achieve it, and (b) hard to achieve. I'd also add (c) even
if we fix rustc itself, there's still going to be way too much unsafe code
out there that does not satisfy your new assumption.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rust-lang/rfcs/pull/2580#issuecomment-691002680, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AB2HJEKBUKDYSBB6R7MLYVLSFHZEPANCNFSM4F7WKGAA
.

My main point was that I wasn't sure if the assumption upholds that a
arbitrary typed (static) pointer can safely be converted to an &'static ().

An arbitrary static pointer may be converted, yes. But if the pointer points to memory that gets deallocated eventually, then the cast is incorrect.

Looking  more into Thin pointer to DST I did:

Realize that most of it (all important) parts can most times be implemented as a library with a custom derive.
In that context the practical additional type safety added by having thin points on the from_raw_parts methods or
by having DynMetadata<dyn Trait> seemed often very small. Especially because thin pointers can (mostly) be
done as a library having their own more safe from_raw_parts wrapper.
We might want to have a fn get_dyn_align<T:?Sized>(meta: <T as Pointee>::Metadata) -> Option<usize> method
because forward compatibility issues (I post more about this below)

This leaves me with following questions:

How likely is it that we want to extend DynMetadata at some point to contain a method like fn vtable(self) -> ...?

How likely does that method profit from being typed over the dyn Trait it's from?

How high is the (roughly estimated) cost of having DynMetadata<dyn Trait>?

If the cost for DynMetadata<dyn Trait> is high or the chance we ever want to add a typed VTable is low I would
to go with DynMetadata like currently proposed. Even if we add typed vtables in the future it's a very niche feature which probably could live with getting a untyped VTable and then "unsafe" casting it to a typed one.

We might want to have a fn get_dyn_align<T:?Sized>(meta: <T as Pointee>::Metadata) -> Option<usize> method
because forward compatibility issues (I post more about this below)

Following is slightly off-topic but outlines a potential future problem we should consider shortly after accepting this RFC which is strongly related to this RFC
The problem comes from the unknown padding of dynamic sized fields in structs which are based on dyn Trait. If you want to handle them you need to fix the alignment. But a (e.g. derive based) "create my own composed DST" library should preferably generalize over any form of DST independent of weather or not it's based on dyn Trait, [T] or str.
Because of this such a library needs to "sometimes" fix the alignment depending on type of DST it's based one.
With DynMetadata we could implement following:
#[inline]
fn get_dyn_alignment<T: Copy + 'static>(meta: T) -> Option<usize> {
    (&meta as &dyn Any).downcast_ref::<DynMetadata>()
        .map(|m|m.align())
}

And given that the success of the downcast is predetermined at compiler time with optimizations enabled (>=opt-level=2) this will normally compile to either a call to m.align() or None.  Which in turn means that you can have code which often at compiler time realizes that it must do alignment fixing or that it can "eliminate" it (const propagation + dead code elimination ...).
So the problem with that is two fold:

If we ever add more ways to customize DST's we likely also will have DST with dynamic alignment which do not have a DynMetadata metadata which means that a libraries (derived based) general handling of DST's will just fall over, silently with
unsafe violations.

It doesn't work  with DynMetadata<dyn Trait>

While I do not think this should be part of this RFC but a follow up one. I though it's best to raise some awareness about this issue. I think what is best is to add the get_dyn_align() function in some forward compatible way to the core library. But how exactly to do this doesn't matter for this RFC at all.

In the conversation here: https://rust-lang.zulipchat.com/#narrow/stream/122651-general/topic/Aliasing.20guarantees.20of.20*mut.20T/near/212127940
It came up that maybe creating a fat-pointer with invalid metadata should be unsafe.
One reason for this was that there are discussions of allowing dynamic dispatch on methods with self: *mut Self or self: *const Self (which is fine as the dispatch only uses the vtable from the metadata and does not access the pointer).
In that context <*const/mut T>::from_raw_parts needs to be unsafe. (Or we use DynMetadata<dyn Trait> instead of DynMetadata).

It should be safe to do dynamic dispatch with the wrong vtable if self is a raw pointer. This is equivalent to passing a raw pointer of a type that does not represent the memory to a regular function. The only thing that must hold is that the vtable itself contains valid function pointers with the right types. Any maybe also that the alignment field is a power of two and less than isize::max_value().

But if the correct vtable has 100 entryies (e.g. for dyn MyTrait) and we new end up using a vtable created for dyn Debug without knowing that and call the last method in the vtable wouldn't we do a out of bounds access because we assume the vtable has 100 entries but it only has 1 entry?

I mean the problem is not that we accidentally use a vtable from A: Disply for B: Display, that would be fine I guess. But that we use a vtable for a completely different kind of trait.

Based on some discussion in the design meeting today:

Is *const () the way we want to spell "generic un-typed pointer" here? Is there any way we can make this forward-compatible with a potential future version of c_void that's in core rather than just std?

If possible, I'd like to see the 'static bound dropped, in favor of ensuring the 'static bound via reasoning from the lifetimes of objects where we need that. @nikomatsakis observed in the meeting that it should be possible to get there by reasoning from T: 'static. My interest here is in future types where the metadata may have the same lifetime as the data, rather than being able to outlive it.

@joshtriplett

Is this about the field inside DynMetadata? Note that it is private, so to some extent its type does not matter. The RFC could specify instead:
struct DynMetadata {
    // Opaque
}

However I wanted to make it apparent that DynMetadata is a pointer to a vtable, not the table itself.

I have no strong opinion there, I’m happy to change the bounds per team consensus.

Hello all -- so, as some previous comments alluded to, we had a lang team design meeting to discuss this RFC yesterday (minutes, with link to recording). I just took some time to read through the comment thread and prepare some notes, which I've collected into this hackmd. I'll inline their contents here. I think that we are generally happy with the state of the RFC, however, so I'm going to kick off an fcp merge, although there may be some small edits or things moved to unresolved questions in the course of this final discussion:
@rfcbot fcp merge
Notes from thread
Here are my notes on various issues that I saw discussed in the discussion thread.  If there are major things missing, I'd appreciate folks letting me know. In particular @rustonaut I wasn't quite sure if I captured your points. I will say that some of these issues feel like they are @rust-lang/libs concerns more (notably whether DynMetadata should be generic or not).

Naming of Pointee vs Referent

Former is maybe clearer
Latter is more proper

Should we use *const () for data pointers, or is there a better type?
DynMetadata generic over the dyn type?

Downside: potentially more monomorphization costs, depends how much code is generic

but code might be generic anyway

Downside: code will be a bit more complex
Downside: no perfect way to prevent DynMetadata<u64>
Advantage: permits us to (in theory) adapt the representation depending on the dyn type

but we could use other types in the future if necessary (e.g., by having different Metadata for dyn Foo and dyn Foo + Bar), and we are unlikely to make "super-wide" pointers in any case

Advantage: may help prevent mistakes

but it wouldn't prevent you from mixing up the data pointer
in code where this is important, could add newtypes for this purpose

remove 'static from the bounds

as discussed in the lang team meeting, it isn't really necessary, since T: 'static implies T::Metadata: 'static anyway by Rust's rules

how committed are we do the list of bounds

two potential reasons we would be committed

user-given impls, which are currently disallowed
generic code like fn foo<T, U>() where U: Pointee<Metadata=T>; implied bounds would be important here if we accepted that, but that is not implemented and some variants we've considered wouldn't help here

but what would happen if we didn't have Unpin?

we'd have had to write a more complex impl of Unpin for Box, like impl<T: ?Sized> Unpin for Box<T> where T: Pointee<Metadata: Unpin>

observation:

Presently, T::Metadata doesn't have to be the actual metadata, but it would be nice if it is, and it will be important for custom DST

why Hash and Ord?

because of impls on *const T (link)

one could imagine wanting to add a trait like Metadata with operations like "get alignment", but those could be added to the Pointee trait as well, or to some sub-trait

this is more "Custom DST" work, but we do want to preserve compatibility with this direction
note that we will need them to access fields like struct Foo<T: ?Sized> { data1: u8, data2: T, }, since the offset of data2 must be dynamically computed in this scenario

@rfcbot concern remove-static-bound
As discussed in the lang meeting, the 'static bound on Metadata is not necessary and should be removed.

since T: 'static implies T::Metadata: 'static anyway by Rust's rules

But what about T: 'a? Like what should happen below?
use std::marker::PhantomData;
use std::hash::Hash;

struct Weird<'a> {
    x: PhantomData<&'a ()>,
}

pub trait Pointee {
    type Metadata: Copy + Send + Sync + Ord + Hash + Unpin /* + 'static */;
}

impl<'a> Pointee for Weird<'a> {
    type Metadata = PhantomData<&'a mut &'a ()>;
}

fn ok<'a, 'b: 'a>(
    mut a: *const Weird<'a>,
    b: *const Weird<'b>,
) {
    // should the next line be rejected?
    a = b;
}

fn not_ok<'a, 'b: 'a>(
    mut a: (*const (), <Weird<'a> as Pointee>::Metadata),
    b: (*const (), <Weird<'b> as Pointee>::Metadata),
) {
    // the next line will be E0623 lifetime mismatch due to the invariance
    a = b;
}

@SimonSapin

* Is this about the field inside `DynMetadata`?

No, this is about the type of the data argument of from_raw_parts

@kennytm

But what about T: 'a? Like what should happen below?

T: 'a => T::Metadata: 'a in general, yes.

However, @kennytm, I am now reading your example more closely, and I see I didn't address the substance of it really. The point is that because *const T (and &T) are covariant with respect to T, that implies that the Metadata might change too as T is adjusted, but we don't know that Metadata this will be valid. That's...a good point.

Now I'm wondering about other sorts of subtyping that we have in Rust, and whether they to can cause problems, though I can't immediately see how (in particular, we have subtyping from fn types with bound lifetimes, though I often wish we didn't).
So conceivably you could do something like
impl Pointee for Weird<fn(&u8)> {
    type Metadata = X;
}

impl<T> Pointee for Weird<T> {
    type Metadata = Y;
}

and that would get us into some awkward trouble using a similar construction, where you "upcast" from *const Weird<fn(&u8)> to (say) Weird<fn(&'static u8)> (which works).

So the TL;DR is that I don't think that a 'static bound actually solves the problem you're concerned about, but indeed it is a real problem that we will have to address in some way.

@nikomatsakis as far as I understand fn(T) -> .. is contravariant with respect to T, and https://github.com/sunshowers/lifetime-variance-example/blob/master/src/lib.rs suggests that that is fine, what can materially go wrong with the playground example you linked?

@guswynn  the problem is that one could, if we permitted user-defined Pointee impls, conceivably change the Metadata types for those two types. In that case, if you had a *const Weird<fn(&u8)> and you upcast it to the supertype *const Weird<fn(&'static u8)> (for example), the metadata would have changed. That doesn't work because subtype and supertype have to have the same representation (including metadata).

@nikomatsakis does any set of dangerous impl's require full-specialization? as far as I can tell, only min_specialization wouldn't permit this overlap: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=74c83f9a0748c1043e479c0547243968 (actually, #![feature(specialization)] doesn't even work here either, and I'm not sure the status of specializable associated types, https://github.com/rust-lang/rust/issues/31844 looks like it has some unresolved details)

why Hash and Ord? because of impls on *const T

How much does it matter that these impls care about the data portion?  Is it even desirable?  IIRC it's an anti-pattern to care about it on *const dyn Trait anyway since it might have different values in different places.
I guess we're stuck with it for Ord because of *const [T]?

Yes it’s a problem that vtable pointers can have different addresses for the same vtable generated independently in separate compilation units, but it’s still a compatibility constraint that Hash and Ord are implemented for *const dyn Trait. At they moment they also hash/compare the vtable pointer address, even though that might give unexpected results. I’m not sure how compatible it would be to change that behavior to ignore vtable pointers entirely.

Wouldn't rustc be able to use a linkonce_odr comdat for vtables to merge them, just like C++ does for classes that don't have a specific owning .cpp file?
https://gcc.godbolt.org/z/M16voj

That doesn't work across dynamic libraries for example.

That doesn't work across dynamic libraries for example.

That's not entirely true, GCC seems to think it does:
https://gcc.gnu.org/onlinedocs/gcc-5.2.0/gccint/WHOPR.html

The whole program mode assumptions are slightly more complex in C++, where inline functions in headers are put into COMDAT sections. COMDAT function and variables can be defined by multiple object files and their bodies are unified at link-time and dynamic link-time.

The whole program mode assumptions are slightly more complex in C++, where inline functions in headers are put into COMDAT sections. COMDAT function and variables can be defined by multiple object files and their bodies are unified at link-time and dynamic link-time.

This is only true on ELF platforms. Windows and macOS don't use ELF and have much more limited run-time linking.

I’ve pushed a few changes:

Remove the 'static bound as requested. This should resolve the concern filed by @nikomatsakis
Parameterize DynMetadata so it’s used like DynMetadata<dyn SomeTrait>. After recent discussions I have a mild preference for it but I left a unresolved question about changing it back. Mention DynMetadata<u64> as a type you can name but not obtain a meaningful value of.
Add an unresolved question about adding into_raw_parts, potentially instead of the metadata function. This was suggested in https://github.com/rust-lang/rfcs/pull/2984#issuecomment-752065665

I’m still in interested in opinions on unresolved questions, in particular the API design details.
Ping @Amanieu, @BurntSushi, @cramertj, @pnkfelix, and @withoutboats for checkboxes in https://github.com/rust-lang/rfcs/pull/2580#issuecomment-722398370.

@joshtriplett

Is *const () the way we want to spell "untyped pointer" here? Is there any way we can make this forward-compatible with a potential future version of c_void that's in core rather than just std? (Note that if we end up going with one of the proposals that uses a typed pointer here, that's fine, but if we're going to use an untyped pointer, I'd rather not have multiple incompatible untyped pointer types.)

Sorry I misunderstood this earlier. That’s a good question.
c_void does exist in core these days, but at the language level it’s just another Sized type like () (except of size 1). What makes it special is only its use convention in libc and other FFI crates.
If the language had truly untyped pointers they would indeed be perfect for from_raw_parts, but it doesn’t and I’m not aware of a proposal to add them. I picked *const () as what felt like the closest thing that exist (more so than enum c_void { #[doc(hidden)] __variant1, #[doc(hidden)] __variant2 }) but I’m open to changing it.

@rfcbot resolve remove-static-bound

@rfcbot resolve remove-static-bound

Implementation PR: https://github.com/rust-lang/rust/pull/81172

Implementation PR: https://github.com/rust-lang/rust/pull/81172

This RFC seems limited to providing the necessary data for traits; at least the demonstration, and the proposed vtable field make me think so.
If so, I think that care should be taken to ensure it can be extended to provide a unified way of disassembling a pointer into meta-data + data pointer, and reassembling it later, (such as here).

Should DynMetadata not have a type parameter?

While for a trait the size/align can be hidden into the v-table, for a slice it would not be the case, so a SliceMetadata<[T]>(usize) needs to be typed to be able to provide size/align. Typing DynMetadata for symmetry therefore seems better.
There's also an argument for type-safety to be made. When reassembling the fat pointer, the data-pointer is an erased *const () (or *const u8 or whatever); having the meta-data be typed prevents type-mistakes.
If there is a motivation for erased DynMetadata, it may be preferable to provide a second type (erased).

Concern: is there a usecase for providing size/align without a data-pointer?
In the examples provided, the data-pointer is always available whenever the size and alignment of the type are necessary. This means that it would perfectly possible to just "re-assemble" the fat pointer and then query it, and it's unclear whether there's a clear usecase for accessing this information purely from the DynMetadata.
One reason to avoid it would be a desire to, later down the road, offer better support !Sized types. That is, if we want to support the following code:
struct Unsized { head: usize, tail: [usize], }

fn main() {
    let concrete: ThinBox<Unsized> = /*...*/;
    let interface: ThinBox<Debug> = concrete;

}

The v-table of <Unsized as Debug> cannot contain the size of Unsized, there's a single v-table for an unlimited number of possible sizes. Instead, the v-table size determination method must be to reach through the data-pointer.
Today Rust disallows creating pointers to traits from pointers to !Sized, this restriction could be lifted at some point unless we paint ourselves into a corner. Does offering size/align on an instance of DynMetadata paints us into a corner?

This RFC seems limited to providing the necessary data for traits; at least the demonstration, and the proposed vtable field make me think so.
If so, I think that care should be taken to ensure it can be extended to provide a unified way of disassembling a pointer into meta-data + data pointer, and reassembling it later, (such as here).

Should DynMetadata not have a type parameter?

While for a trait the size/align can be hidden into the v-table, for a slice it would not be the case, so a SliceMetadata<[T]>(usize) needs to be typed to be able to provide size/align. Typing DynMetadata for symmetry therefore seems better.
There's also an argument for type-safety to be made. When reassembling the fat pointer, the data-pointer is an erased *const () (or *const u8 or whatever); having the meta-data be typed prevents type-mistakes.
If there is a motivation for erased DynMetadata, it may be preferable to provide a second type (erased).

Concern: is there a usecase for providing size/align without a data-pointer?
In the examples provided, the data-pointer is always available whenever the size and alignment of the type are necessary. This means that it would perfectly possible to just "re-assemble" the fat pointer and then query it, and it's unclear whether there's a clear usecase for accessing this information purely from the DynMetadata.
One reason to avoid it would be a desire to, later down the road, offer better support !Sized types. That is, if we want to support the following code:
struct Unsized { head: usize, tail: [usize], }

fn main() {
    let concrete: ThinBox<Unsized> = /*...*/;
    let interface: ThinBox<Debug> = concrete;

}

The v-table of <Unsized as Debug> cannot contain the size of Unsized, there's a single v-table for an unlimited number of possible sizes. Instead, the v-table size determination method must be to reach through the data-pointer.
Today Rust disallows creating pointers to traits from pointers to !Sized, this restriction could be lifted at some point unless we paint ourselves into a corner. Does offering size/align on an instance of DynMetadata paints us into a corner?

@matthieu-m some of what you’re asking for is in fact in the RFC. Note that code snippets in the "Guide-level" section are only an example of what could be built on top, the actual proposal is in the next section.

In the examples provided, the data-pointer is always available whenever the size and alignment of the type are necessary.

No, ThinBox needs to know align_of for the value in order to compute the offset to the data.

@matthieu-m some of what you’re asking for is in fact in the RFC. Note that code snippets in the "Guide-level" section are only an example of what could be built on top, the actual proposal is in the next section.

In the examples provided, the data-pointer is always available whenever the size and alignment of the type are necessary.

No, ThinBox needs to know align_of for the value in order to compute the offset to the data.

@matthieu-m some of what you’re asking for is in fact in the RFC. Note that code snippets in the "Guide-level" section are only an example of what could be built on top, the actual proposal is in the next section.

In the reference-level explanation I only see:
#[derive(Copy, Clone)]
pub struct DynMetadata<DynTrait: ?Sized> {
    // Private fields
    vtable_ptr: ptr::NonNull<()>,
    phantom: PhantomData<DynTrait>
}

And no equivalent for slices.
This is why it's not clear to me whether you intend DynMetadata<[T]> to be a possibility, or if I missed a SliceMetadata<[T]> somewhere...

In the examples provided, the data-pointer is always available whenever the size and alignment of the type are necessary.

No, ThinBox needs to know align_of for the value in order to compute the offset to the data.

Indeed.
I think this could be solved by having an intermediary layer of DynMetadata in some way, one which makes no assumption about the ability to retrieve the alignment without a data-pointer:

Either with a separate struct.
Or with a separate bound on the impl.

I think it would be worth exploring before stabilizing, to ensure maximum forward-compatibility.

On another note, thanks for pushing forward on this topic!

@matthieu-m some of what you’re asking for is in fact in the RFC. Note that code snippets in the "Guide-level" section are only an example of what could be built on top, the actual proposal is in the next section.

In the reference-level explanation I only see:
#[derive(Copy, Clone)]
pub struct DynMetadata<DynTrait: ?Sized> {
    // Private fields
    vtable_ptr: ptr::NonNull<()>,
    phantom: PhantomData<DynTrait>
}

And no equivalent for slices.
This is why it's not clear to me whether you intend DynMetadata<[T]> to be a possibility, or if I missed a SliceMetadata<[T]> somewhere...

In the examples provided, the data-pointer is always available whenever the size and alignment of the type are necessary.

No, ThinBox needs to know align_of for the value in order to compute the offset to the data.

Indeed.
I think this could be solved by having an intermediary layer of DynMetadata in some way, one which makes no assumption about the ability to retrieve the alignment without a data-pointer:

Either with a separate struct.
Or with a separate bound on the impl.

I think it would be worth exploring before stabilizing, to ensure maximum forward-compatibility.

On another note, thanks for pushing forward on this topic!

This is the relevant part:

/// For statically-sized types (that implement the Sized traits)
/// as well as for extern types,
/// pointers are said to be “thin”: metadata is zero-sized and its type is ().
///
/// Pointers to [dynamically-sized types][dst] are said to be “fat”
/// and have non-zero-sized metadata:
///
/// * For structs whose last field is a DST, metadata is the metadata for the last field
/// * For the str type, metadata is the length in bytes as usize
/// * For slice types like [T], metadata is the length in items as usize
/// * For trait objects like dyn SomeTrait, metadata is [DynMetadata<Self>][DynMetadata]
///   (e.g. DynMetadata<dyn SomeTrait>).

Metadata for slices is usize, not a dedicated type.

I don’t understand what you mean by “an intermediary layer”, or how that would help with computing the data pointer without already having the data pointer when the computation is based on an offset which depends on alignment.

This is the relevant part:

/// For statically-sized types (that implement the Sized traits)
/// as well as for extern types,
/// pointers are said to be “thin”: metadata is zero-sized and its type is ().
///
/// Pointers to [dynamically-sized types][dst] are said to be “fat”
/// and have non-zero-sized metadata:
///
/// * For structs whose last field is a DST, metadata is the metadata for the last field
/// * For the str type, metadata is the length in bytes as usize
/// * For slice types like [T], metadata is the length in items as usize
/// * For trait objects like dyn SomeTrait, metadata is [DynMetadata<Self>][DynMetadata]
///   (e.g. DynMetadata<dyn SomeTrait>).

Metadata for slices is usize, not a dedicated type.

I don’t understand what you mean by “an intermediary layer”, or how that would help with computing the data pointer without already having the data pointer when the computation is based on an offset which depends on alignment.

Metadata for slices is usize, not a dedicated type.

I see; I had indeed skipped the comment section, not expecting to find it here.
I think there would be value in specifying a stronger typed interface, and introducing a Metadata<T: ?Sized> trait, as I did here.
With the minimal implementation proposed here implementing a ThinBox<T: ?Sized> is going to be challenging; the writer essentially has to use specialization to handle trait, slices, and Sized items separately. Specialization which is not available on stable.
However, by pulling the specialization bits within std, then the compiler can provide the 3 separate structs implementing Metadata<T> and the user can then uniformly use such a Metadata<T> to implement a ThinBox<T: ?Sized>.
For example, using my above meta module, I implemented ThinBox in a type-agnostic way: it handles traits, slices, and of course regular types without issues.

I don’t understand what you mean by “an intermediary layer”, or how that would help with computing the data pointer without already having the data pointer when the computation is based on an offset which depends on alignment.

I apologize for not being clear. I was considering the case where one would want trait of unsized, in which case the data-pointer is necessary to compute either size or alignment, and therefore propose two layers:

A Metadata<T> trait without size/align/layout.
A AugmentedMetadata<T> trait with size/align/layout.

In the case of ThinBox, a choice would have to be made about whether using the AugmentedMetada<T>, requiring that T be usize-aligned, or somehow storing the alignment somewhere.

Metadata for slices is usize, not a dedicated type.

I see; I had indeed skipped the comment section, not expecting to find it here.
I think there would be value in specifying a stronger typed interface, and introducing a Metadata<T: ?Sized> trait, as I did here.
With the minimal implementation proposed here implementing a ThinBox<T: ?Sized> is going to be challenging; the writer essentially has to use specialization to handle trait, slices, and Sized items separately. Specialization which is not available on stable.
However, by pulling the specialization bits within std, then the compiler can provide the 3 separate structs implementing Metadata<T> and the user can then uniformly use such a Metadata<T> to implement a ThinBox<T: ?Sized>.
For example, using my above meta module, I implemented ThinBox in a type-agnostic way: it handles traits, slices, and of course regular types without issues.

I don’t understand what you mean by “an intermediary layer”, or how that would help with computing the data pointer without already having the data pointer when the computation is based on an offset which depends on alignment.

I apologize for not being clear. I was considering the case where one would want trait of unsized, in which case the data-pointer is necessary to compute either size or alignment, and therefore propose two layers:

A Metadata<T> trait without size/align/layout.
A AugmentedMetadata<T> trait with size/align/layout.

In the case of ThinBox, a choice would have to be made about whether using the AugmentedMetada<T>, requiring that T be usize-aligned, or somehow storing the alignment somewhere.

@SimonSapin I created a crate https://crates.io/crates/rfc2580 packaging a slightly revised version of this RFC, and adapting the ThinBox example to match.
I like the clean Pointee trait proposed by this RFC much better than my original MetaPointee, and by using an associated type for MetaData rather than a generic argument, type inference works much better, avoiding the boiler-plate when using the into_raw_parts and from_raw_parts functions.
Also, I must congratulate you on your ThinBox code, it worked out of the box (baring adjustments to the new API).

@SimonSapin I created a crate https://crates.io/crates/rfc2580 packaging a slightly revised version of this RFC, and adapting the ThinBox example to match.
I like the clean Pointee trait proposed by this RFC much better than my original MetaPointee, and by using an associated type for MetaData rather than a generic argument, type inference works much better, avoiding the boiler-plate when using the into_raw_parts and from_raw_parts functions.
Also, I must congratulate you on your ThinBox code, it worked out of the box (baring adjustments to the new API).

No, ThinBox needs to know align_of for the value in order to compute the offset to the data.

Actually, it doesn't. Since the size of the meta-data is known at compile-time -- based on T -- it's possible to use a middle-pointer technique instead:

Create a large allocation, similar to WithMeta.
Compute the middle pointer as the allocated pointer + max(sizeof(T::MetaData), align(value))
Write the value at the value at the middle pointer.
Write the meta-data just before the middle pointer.
Have ThinBox memorize the middle pointer.

This simplifies the code for Deref:

The meta-data is at -sizeof(T::MetaData) bytes before the middle pointer.
The data is right at the middle pointer.

The only moment where the alignment of the value is needed again is in Drop, but then we can just use Deref to get a complete fat-pointer and use size_of_value and align_of_value.
An example implementation can be found in the examples of https://crates.io/crates/rfc2580/0.2.0 .

As a result, I once again raise the question of whether the size, align and layout functions are necessary.
Conservatively, I would advise not exposing them -- even if they can be implemented -- for the moment.

No, ThinBox needs to know align_of for the value in order to compute the offset to the data.

Actually, it doesn't. Since the size of the meta-data is known at compile-time -- based on T -- it's possible to use a middle-pointer technique instead:

Create a large allocation, similar to WithMeta.
Compute the middle pointer as the allocated pointer + max(sizeof(T::MetaData), align(value))
Write the value at the value at the middle pointer.
Write the meta-data just before the middle pointer.
Have ThinBox memorize the middle pointer.

This simplifies the code for Deref:

The meta-data is at -sizeof(T::MetaData) bytes before the middle pointer.
The data is right at the middle pointer.

The only moment where the alignment of the value is needed again is in Drop, but then we can just use Deref to get a complete fat-pointer and use size_of_value and align_of_value.
An example implementation can be found in the examples of https://crates.io/crates/rfc2580/0.2.0 .

As a result, I once again raise the question of whether the size, align and layout functions are necessary.
Conservatively, I would advise not exposing them -- even if they can be implemented -- for the moment.

Oh that middle pointer is a really cool trick. Congrats.
I feel that fn size_for_meta<T>(meta: <T as Pointee>::Metadata) -> usize and align_for_meta intrinsics would still be useful for other cases where such tricks might not be sufficient, such as a collections storing multiple DST values in a contiguous allocation analogous to Vec<dyn Any>.
On the other hand it’s less clear to me what not having {size,align}_for_meta enables.
While I tried to make this RFC not prevent future language extensions such as custom DSTs, to be honest I somewhat expect Rust to never have custom DSTs, or any DST much more complex than already exist. Rust already spent a lot "complexity budget" on other language features, and at some point pushing the type system further starts to have diminishing returns.

Oh that middle pointer is a really cool trick. Congrats.
I feel that fn size_for_meta<T>(meta: <T as Pointee>::Metadata) -> usize and align_for_meta intrinsics would still be useful for other cases where such tricks might not be sufficient, such as a collections storing multiple DST values in a contiguous allocation analogous to Vec<dyn Any>.
On the other hand it’s less clear to me what not having {size,align}_for_meta enables.
While I tried to make this RFC not prevent future language extensions such as custom DSTs, to be honest I somewhat expect Rust to never have custom DSTs, or any DST much more complex than already exist. Rust already spent a lot "complexity budget" on other language features, and at some point pushing the type system further starts to have diminishing returns.

On the other hand it’s less clear to me what not having {size,align}_for_meta enables.

I am mostly concerned about the current limitation that you cannot go from &[u8] to &Debug, because &[u8] is already a DST and one cannot build a DST of DST. It seems like a limitation we should lift at some point.
At the same time, it's been 5 years already so maybe the fact that it's not been tackled means that in practice it's not too much of a problem, and we shouldn't think too much.

I feel that fn size_for_meta<T>(meta: <T as Pointee>::Metadata) -> usize and align_for_meta intrinsics would still be useful for other cases where such tricks might not be sufficient, such as a collections storing multiple DST values in a contiguous allocation analogous to Vec<dyn Any>.

I am not sure. As long as you can access the (full) object, you can use mem::size_of_val. I've already implemented a Vec (in C++), and the implementation actually used 2 vectors (in a single allocation): a vector of offsets followed by a vector of items, to be able to jump straight to the item.
I'd imagine that in Rust you would either have:

A vector of (metadata + offset), followed by a vector of "data" with potential padding.
A vector of offsets pointing to the "middle" of each item in the following vector of (metadata + data).

And in neither case you'd need to obtain the alignment from just the metadata.
I can imagine one just implementing a contiguous vector of (metadata + padding + data) where you'd need the alignment from metadata alone to find the data, but then you'd need a linear scan to get to the Nth item which isn't terribly efficient.
Then again, with most of my experience coming from C++, I'm not used to being able to separate the v-ptr from its data, so maybe I am just not imaginative enough.

In any case, thank you very much for working on this. It's necessary for all kinds of custom storage, so I'm really keen on toying with the result.

On the other hand it’s less clear to me what not having {size,align}_for_meta enables.

I am mostly concerned about the current limitation that you cannot go from &[u8] to &Debug, because &[u8] is already a DST and one cannot build a DST of DST. It seems like a limitation we should lift at some point.
At the same time, it's been 5 years already so maybe the fact that it's not been tackled means that in practice it's not too much of a problem, and we shouldn't think too much.

I feel that fn size_for_meta<T>(meta: <T as Pointee>::Metadata) -> usize and align_for_meta intrinsics would still be useful for other cases where such tricks might not be sufficient, such as a collections storing multiple DST values in a contiguous allocation analogous to Vec<dyn Any>.

I am not sure. As long as you can access the (full) object, you can use mem::size_of_val. I've already implemented a Vec (in C++), and the implementation actually used 2 vectors (in a single allocation): a vector of offsets followed by a vector of items, to be able to jump straight to the item.
I'd imagine that in Rust you would either have:

A vector of (metadata + offset), followed by a vector of "data" with potential padding.
A vector of offsets pointing to the "middle" of each item in the following vector of (metadata + data).

And in neither case you'd need to obtain the alignment from just the metadata.
I can imagine one just implementing a contiguous vector of (metadata + padding + data) where you'd need the alignment from metadata alone to find the data, but then you'd need a linear scan to get to the Nth item which isn't terribly efficient.
Then again, with most of my experience coming from C++, I'm not used to being able to separate the v-ptr from its data, so maybe I am just not imaginative enough.

In any case, thank you very much for working on this. It's necessary for all kinds of custom storage, so I'm really keen on toying with the result.

Huzzah! The @rust-lang/lang and @rust-lang/libs teams have decided to accept this RFC.
To track further discussion, subscribe to the tracking issue here:
rust-lang/rust#81513

Huzzah! The @rust-lang/lang and @rust-lang/libs teams have decided to accept this RFC.
To track further discussion, subscribe to the tracking issue here:
rust-lang/rust#81513

Is fn size_for_meta<T>(meta: <T as Pointee>::Metadata) -> usize still going to be implemented? Being able to work out the size of a type without needing to have a valid reference/pointer to it would be very helpful for my use-case.

Are there plans for fn size_for_meta<T>(meta: <T as Pointee>::Metadata) -> usize to be implemented? As far as I know, I need it for my use-case.

Is fn size_for_meta<T>(meta: <T as Pointee>::Metadata) -> usize still going to be implemented? Being able to work out the size of a type without needing to have a valid reference/pointer to it would be very helpful for my use-case.

if Rust ever supports C++ virtual classes (imho something that would be useful for FFI), that won't work since the size of the type is indirectly stored in the pointee, the metadata would be a ZST.

if Rust ever supports C++ virtual classes (imho something that would be useful for FFI), that won't work since the size of the type is indirectly stored in the pointee, the metadata would be a ZST.

Unless ABI compatiblity is needed, wouldn't the type pointer be stored in the metadata, as is done for dun Trait?

if Rust ever supports C++ virtual classes (imho something that would be useful for FFI), that won't work since the size of the type is indirectly stored in the pointee, the metadata would be a ZST.

Unless ABI compatiblity is needed, wouldn't the type pointer be stored in the metadata, as is done for dyn Trait?

almost the whole point of Rust supporting C++ virtual classes is ABI compatibility -- needed for FFI, so &ACppClass would have to be a pointer with ZST metadata and would have to match C++ * ACppClass's ABI


How to handle errors in useContractFunction?

There seems to be no way to extract an error message while using useContractFunction, it seems that one can only refer to state to check if it is equal to "Exception", but in most cases, I'd want to handle the error thrown by the smart contract in order to show it to the user. At the moment, the error does get logged in my console but I have no idea how I can extract it or use it in order to show a specific error message to the user.
Example of what I see logged:

Is there any way I can handle this error or access the error.message within my code whilst using useContractFunction?

There is a state.errorMessage next to state.status, does it contain what you need?

@rzadp yes you are correct, I've got no idea how I missed that. Thanks so much!

@rzadp What about add property errorCode to object status. I could be wrong, but I don't think you should rely on the error message, it might change. The error code will remain the same.
And in the code it looks more concise:
if (status.errorCode === '4001') than:
if (status.errorMessage === 'MetaMask Tx Signature: User denied transaction signature.'

@rzadp What about add property errorCode to object state? I could be wrong, but I don't think you should rely on the error message, it might change. The error code will remain the same.
And in the code it looks more concise: if (state.errorCode === '4001') than: if (state.errorMessage === 'MetaMask Tx Signature: User denied transaction signature.')

@FanManutd Agree, could you create a new issue with this request please?


Add /var/go to server volume list

/var/go folder can be used to store credentials / ssh keys. Adding it as a volume to the list.

Thanks! Done.