Fix MacOS build

[ShortDevelopment]

Problem

MacOs builds using latest icu4c fail because the "placement new operator" is defined in the standard-library and in Allocator.h.

The use of __attribute__((visibility("hidden"))) (See NO_EXPORT) used to prevent these linking conflicts but the new header is now included through latest icu4c into the same compilation unit:

In file included from /Users/test/Documents/Projects/ChakraCore/lib/Backend/Encoder.cpp:5:
In file included from /Users/test/Documents/Projects/ChakraCore/lib/Backend/Backend.h:12:
In file included from /Users/test/Documents/Projects/ChakraCore/lib/Backend/../Runtime/Runtime.h:334:
In file included from /Users/test/Documents/Projects/ChakraCore/lib/Backend/../Runtime/PlatformAgnostic/ChakraPlatform.h:7:
In file included from /Users/test/Documents/Projects/ChakraCore/lib/Backend/../Runtime/PlatformAgnostic/UnicodeText.h:8:
In file included from /Users/test/Documents/Projects/ChakraCore/lib/Backend/../Runtime/PlatformAgnostic/ChakraICU.h:38:
In file included from /usr/local/opt/icu4c/include/unicode/ucol.h:1524:
In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX14.4.sdk/usr/include/c++/v1/functional:521:
In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX14.4.sdk/usr/include/c++/v1/__functional/bind.h:20:
In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX14.4.sdk/usr/include/c++/v1/tuple:1868:
/Library/Developer/CommandLineTools/SDKs/MacOSX14.4.sdk/usr/include/c++/v1/new:277:70: error: redefinition of 'operator new'
_LIBCPP_NODISCARD_AFTER_CXX17 inline _LIBCPP_INLINE_VISIBILITY void* operator new  (std::size_t, void* __p) _NOEXCEPT {return __p;}
                                                                     ^
/Users/test/Documents/Projects/ChakraCore/lib/Common/Memory/Allocator.h:457:1: note: previous definition is here
operator new(
^

Potential Solution

As this specific new overload is part of the standard-library it might be okay to just remove the custom declaration in CC and always #include <new> instead.

---

Fixes #7007

[ppenzin]

It looks like new wrapper is intentional, maybe we need to override including the system one?

[ShortDevelopment]

@ppenzin This code snippet exists since the "First Commit" and caused some build issues before (See 11a6851 and 4398657).

I don't understand why it exists in the first place because according to this gcc docs:

void* operator new(std::size_t, void*) noexcept;
    Non-allocating, “placement” single-object new, which does nothing except return its argument.
    This function cannot be replaced.

[ppenzin]

These declarations do look like a kludge, for sure. @ShortDevelopment, do you have an idea why wasm spec test failed with this change?

I wonder if there is more to this story, @rhuanjl, do you have any recollection on whether or not it is really needed for debugging? There are better methods to get placement new and delete operators if that is what this was supposed to achieve..

[rhuanjl]

These declarations do look like a kludge, for sure. @ShortDevelopment, do you have an idea why wasm spec test failed with this change?

I wonder if there is more to this story, @rhuanjl, do you have any recollection on whether or not it is really needed for debugging? There are better methods to get placement new and delete operators if that is what this was supposed to achieve..

It definitely looks like a fudge - I'm afraid I don't know its intent.

I may be misunderstanding- but I think the comment is actually saying this is Not Needed for the debugger extension (a different compile target?) but, implicitly, is needed for a normal CC build.

The test fail at first glance seems to be a real error of some kind, though would need to dig to understand it.

[rhuanjl]

@ppenzin This code snippet exists since the "First Commit" and caused some build issues before (See 11a6851 and 4398657).

I don't understand why it exists in the first place because according to this gcc docs:

void* operator new(std::size_t, void*) noexcept;

    Non-allocating, “placement” single-object new, which does nothing except return its argument.

    This function cannot be replaced.

Chakracore has never compiled with GCC, it's always needed clang on Linux/macos - I don't know if that's relevant to this issue but could be. (Thought prompted by your ref to the GCC docs)

[ShortDevelopment]

If I correctly understand this PR on clang a custom declaration of the inline placement new operator could lead to (slightly) lower build / debug times because the stl <new> header isn't parsed.

[ppenzin]

These macros aid compiler in various ways, in libcxx PR they are defining one that tells compiler placement new has been defined, the one Allocator.h has seems to imply inlining of the operator (though I don't know which compiler that is for). I think deleting custom placement operator and using a standard one is OK as long as the rest of the CI is passing.

[ShortDevelopment]

All checks are passing now although I did only add a comment and move the #include down.
I also could not reproduce the crash with the build artifacts during a quick test in a vm.

Might this be an existing bug in the codebase?

[ppenzin]

Placement of include can have some effect, though this might be just a spurious failure.

[ppenzin]

I think this should be fine. @rhuanjl what do you think?