4

Constify `Box<T, A>` methods by woppopo · Pull Request #91884 · rust-lang/...

 2 years ago
source link: https://github.com/rust-lang/rust/pull/91884
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.

_old_layout: Layout,

_new_layout: Layout,

) -> Result<NonNull<[u8]>, AllocError> {

unimplemented!()

Copy link

Contributor

@bjorn3 bjorn3 28 days ago

Please implement these.

Copy link

Contributor

Author

@woppopo woppopo 26 days ago

This is a type for testing, so I omitted some implementations. Should I still implement these?

Copy link

Contributor

@bjorn3 bjorn3 26 days ago

It is fine to leave them unimplemented for now, but I think they should be implemented before merging. You may be able to copy the default implementation of these methods.

woppopo reacted with thumbs up emoji

Copy link

Contributor

Author

@woppopo woppopo 25 days ago

I have implemented these, but the methods aren't called in the testcases, we can't check if they are implemented correctly. Also we may need to implement const_deallocate to the compiler.

Copy link

Contributor

@oli-obk oli-obk 25 days ago

Const deallocation is simply leaking the thing. The mir interpreter cleans up all dangling allocations

Also we may need to implement const_deallocate to the compiler.

I just saw you open a PR to add this, let me emphasize: This PR cannot contain any use of that intrinsic. The nature of ConstAlloc/CtfeAlloc is to leak when dropping, which means you can't realistically use it in this test allocator impl.

The nature of ConstAlloc/CtfeAlloc is to leak when dropping

And why is that?
I don't think this should be the case, at least for allocations that are created and then deallocated within the same const-evalaution. (Of course, at runtime deallocation is a NOP, since static memory cannot be deallocated and it is shared among all the copies of the constant anyway.)

Well, looks like const_eval_select can resolve this problem faily easily.

I don't think that is enough -- if one const works on something that was allocated in a different const, deallocation should still be a NOP.


About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK