[clang][repl] fix new on Mac M1
#69072
Conversation
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Maksim Levental (makslevental) ChangesOn Mac M1, if you load something that transitively loads This is because Full diff: https://github.com/llvm/llvm-project/pull/69072.diff 2 Files Affected:
diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp
index 7968c62cbd3e7b3..ddfbc9ac01c6743 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -248,7 +248,7 @@ Interpreter::~Interpreter() {
// can't find the precise resource directory in unittests so we have to hard
// code them.
const char *const Runtimes = R"(
- void* operator new(__SIZE_TYPE__, void* __p) noexcept;
+ #include <new>
void *__clang_Interpreter_SetValueWithAlloc(void*, void*, void*);
void __clang_Interpreter_SetValueNoAlloc(void*, void*, void*);
void __clang_Interpreter_SetValueNoAlloc(void*, void*, void*, void*);
diff --git a/clang/test/Interpreter/execute.cpp b/clang/test/Interpreter/execute.cpp
index 6e73ed3927e8155..d54ab99749c1bac 100644
--- a/clang/test/Interpreter/execute.cpp
+++ b/clang/test/Interpreter/execute.cpp
@@ -20,4 +20,6 @@ auto r2 = printf("S[f=%f, m=0x%llx]\n", s.f, reinterpret_cast<unsigned long long
inline int foo() { return 42; }
int r3 = foo();
+#include <memory>
+
%quit
|
makslevental
force-pushed
the
clang_repl_include_path
branch
4 times, most recently
from
737bef4 to
940da2d
Compare
|
@vgvassilev this seems acceptable? |
makslevental
force-pushed
the
clang_repl_include_path
branch
from
940da2d to
891cdd5
Compare
| @@ -24,6 +24,7 @@ | |||
| #include "llvm/ExecutionEngine/Orc/LLJIT.h" | |||
| #include "llvm/Support/ManagedStatic.h" | |||
| #include "llvm/Support/TargetSelect.h" | |||
| #include "llvm/TargetParser/Host.h" | |||
There was a problem hiding this comment.
The changes in this file should not be needed.
| @@ -148,12 +161,12 @@ TEST(InterpreterTest, UndoCommand) { | |||
| auto Interp = createInterpreter(ExtraArgs, DiagPrinter.get()); | |||
|
|
|||
| // Fail to undo. | |||
| auto Err1 = Interp->Undo(); | |||
| auto Err1 = Interp->Undo(2); | |||
| @@ -257,14 +256,9 @@ const char *const Runtimes = R"( | |||
| void __clang_Interpreter_SetValueNoAlloc(void*, void*, void*, long double); | |||
| void __clang_Interpreter_SetValueNoAlloc(void*,void*,void*,unsigned long long); | |||
| template <class T, class = T (*)() /*disable for arrays*/> | |||
| void __clang_Interpreter_SetValueCopyArr(T* Src, void* Placement, unsigned long Size) { | |||
There was a problem hiding this comment.
We can’t outline templates because we are introducing an odr violation. We should move new behind a new non-templates operation which we can forward declare and use within the template.
There was a problem hiding this comment.
We can’t outline templates because we are introducing an odr violation.
This is not correct - templates (subject to conditions satisfied here) are exempt from odr:
There can be more than one definition of a [...] templated entity ([temp.pre]) [...] in a program provided that each definition appears in a different translation unit and the definitions satisfy the following requirements.
https://timsong-cpp.github.io/cppwp/n4861/basic.def.odr#13
We should move new behind a new non-templates operation which we can forward declare and use within the template.
I don't know what you're describing here; feel free to take over this PR if you have a concrete design in mind.
…C++. This patch brings back the basic support for C by inserting the required for value printing runtime only when we are in C++ mode. Additionally, it defines a new overload of operator placement new because we can't really forward declare it in a library-agnostic way. Fixes the issue described in llvm#69072.
…C++. This patch brings back the basic support for C by inserting the required for value printing runtime only when we are in C++ mode. Additionally, it defines a new overload of operator placement new because we can't really forward declare it in a library-agnostic way. Fixes the issue described in llvm#69072.
…C++ (#76218) This patch brings back the basic support for C by inserting the required for value printing runtime only when we are in C++ mode. Additionally, it defines a new overload of operator placement new because we can't really forward declare it in a library-agnostic way. Fixes the issue described in #69072.
…C++ (llvm#76218) This patch brings back the basic support for C by inserting the required for value printing runtime only when we are in C++ mode. Additionally, it defines a new overload of operator placement new because we can't really forward declare it in a library-agnostic way. Fixes the issue described in llvm#69072. (cherry picked from commit 1566f1f)
On Mac M1, if you load something that transitively loads
#include <new>, it will fail becauseThis is because
MacOSX12.3.sdk/usr/include/c++/v1/__configsets_LIBCPP_INLINE_VISIBILITYto be__attribute__((__visibility__("hidden"))) __attribute__((internal_linkage)), which makes thenewin<new>incompatible with the hardcoded declvoid* operator new(__SIZE_TYPE__, void* __p) noexcept;inclang/lib/Interpreter/Interpreter::Runtimes. I believe the correct thing to do is replace that decl with#include <new>(as I've done here).