Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Components with overriden new operator. #1131

Closed
jdumas opened this issue Mar 26, 2024 · 6 comments
Closed

Components with overriden new operator. #1131

jdumas opened this issue Mar 26, 2024 · 6 comments
Assignees
Labels
feature request feature request not yet accepted solved available upstream or in a branch

Comments

@jdumas
Copy link

jdumas commented Mar 26, 2024

Hi. I'm trying to use EnTT with JoltPhysics. Jolt by default overrides its new/delete operators, which causes compilation issues with EnTT's usage of placement new operator here:

return std::apply([value](auto &&...curr) { return new(value) Type(std::forward<decltype(curr)>(curr)...); }, internal::uses_allocator_construction<Type>::args(allocator, std::forward<Args>(args)...));

As proposed in this discussion, the solution would be to replace new(value) with ::new(value) in EnTT's codebase.

@skypjack skypjack self-assigned this Mar 27, 2024
@skypjack skypjack added the feature request feature request not yet accepted label Mar 27, 2024
@skypjack
Copy link
Owner

Have you tried it already in your project? Can you confirm that it works as expected?

@jdumas
Copy link
Author

jdumas commented Mar 27, 2024

Yes! But if there are other places where EnTT is using a placement new, those may also need to be updated. Happy to work on a MWE without Jolt if you think that's useful.

@skypjack
Copy link
Owner

The only other class that uses placement new is any afaik.
I don't think a MWE with Jolt is required here actually. A standalone test that doesn't use external libraries would be way better to avoid future regression probably.
Do you have bandwidth to pack something in this sense for the utility in memory.hpp? A rough example would be enough, I can then fix it and throw it in myself but it would definitely save me a lot of time!

@jdumas
Copy link
Author

jdumas commented Mar 27, 2024

Sure. As I said I was planning to provide a MWE without Jolt. So here it is:

#include <entt/entt.hpp>

#include <stdlib.h>
#include <cstdlib>

namespace MyLib {

void* Allocate(size_t inSize)
{
    return malloc(inSize);
}

void Free(void* inBlock)
{
    free(inBlock);
}

void* AlignedAllocate(size_t inSize, size_t inAlignment)
{
#if defined(_WIN32)
    return _aligned_malloc(inSize, inAlignment);
#else
    void* block = nullptr;
    posix_memalign(&block, inAlignment, inSize);
    return block;
#endif
}

void AlignedFree(void* inBlock)
{
#if defined(_WIN32)
    _aligned_free(inBlock);
#else
    free(inBlock);
#endif
}

class Foo
{
public:
    Foo(int x_)
        : x(x_)
    {}

public:
    void* operator new(size_t inCount) { return MyLib::Allocate(inCount); }
    void operator delete(void* inPointer) noexcept { MyLib::Free(inPointer); }
    void* operator new[](size_t inCount) { return MyLib::Allocate(inCount); }
    void operator delete[](void* inPointer) noexcept { MyLib::Free(inPointer); }
    void* operator new(size_t inCount, std::align_val_t inAlignment)
    {
        return MyLib::AlignedAllocate(inCount, static_cast<size_t>(inAlignment));
    }
    void operator delete(void* inPointer, [[maybe_unused]] std::align_val_t inAlignment) noexcept
    {
        MyLib::AlignedFree(inPointer);
    }
    void* operator new[](size_t inCount, std::align_val_t inAlignment)
    {
        return MyLib::AlignedAllocate(inCount, static_cast<size_t>(inAlignment));
    }
    void operator delete[](void* inPointer, [[maybe_unused]] std::align_val_t inAlignment) noexcept
    {
        MyLib::AlignedFree(inPointer);
    }

protected:
    int x;
};

} // namespace MyLib

int main(void)
{
    entt::registry r;
    auto e = r.create();
    r.emplace<MyLib::Foo>(e, 42); // <-- doesn't compile
    return 0;
}

@skypjack
Copy link
Owner

As I said I was planning to provide a MWE without Jolt

You're right and I read it wrong. 😅
Thanks for the example, I'll use it as the basis for a non-regression test. 👍

@jdumas
Copy link
Author

jdumas commented Mar 28, 2024

All good, thanks for looking into it!

@skypjack skypjack added the solved available upstream or in a branch label Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request feature request not yet accepted solved available upstream or in a branch
Projects
None yet
Development

No branches or pull requests

2 participants