Skip to content

Add support for array of objects in Construct/Destruct #587

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions include/clang-c/CXCppInterOp.h
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,8 @@ CINDEX_LINKAGE void clang_deallocate(CXObject address);
* Creates an object of class \c scope and calls its default constructor. If \c
* arena is set it uses placement new.
*/
CINDEX_LINKAGE CXObject clang_construct(CXScope scope, void* arena);
CINDEX_LINKAGE CXObject clang_construct(CXScope scope, void* arena,
size_t count);

/**
* Creates a trampoline function and makes a call to a generic function or
Expand All @@ -349,7 +350,7 @@ CINDEX_LINKAGE CXObject clang_construct(CXScope scope, void* arena);
* \param self The 'this pointer' of the object.
*/
CINDEX_LINKAGE void clang_invoke(CXScope func, void* result, void** args,
size_t n, void* self);
size_t n, size_t nary, void* self);

/**
* Calls the destructor of object of type \c type. When withFree is true it
Expand All @@ -361,7 +362,8 @@ CINDEX_LINKAGE void clang_invoke(CXScope func, void* result, void** args,
*
* \param withFree Whether to call operator delete/free or not.
*/
CINDEX_LINKAGE void clang_destruct(CXObject This, CXScope S, bool withFree);
CINDEX_LINKAGE void clang_destruct(CXObject This, CXScope S,
bool withFree = true, size_t nary = 0UL);

/**
* @}
Expand Down
25 changes: 17 additions & 8 deletions include/clang/Interpreter/CppInterOp.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,10 @@ namespace Cpp {
// FIXME: Figure out how to unify the wrapper signatures.
// FIXME: Hide these implementation details by moving wrapper generation in
// this class.
using GenericCall = void (*)(void*, int, void**, void*);

// (self, args, args, result, nary)
using GenericCall = void (*)(void*, int, void**, void*, size_t);
// (self, nary, withFree)
using DestructorCall = void (*)(void*, unsigned long, int);
private:
union {
Expand Down Expand Up @@ -156,16 +159,17 @@ namespace Cpp {
// self can go in the end and be nullptr by default; result can be a nullptr
// by default. These changes should be synchronized with the wrapper if we
// decide to directly.
void Invoke(void* result, ArgList args = {}, void* self = nullptr) const {
void Invoke(void* result, ArgList args = {}, void* self = nullptr,
size_t nary = 0UL) const {
// Forward if we intended to call a dtor with only 1 parameter.
if (m_Kind == kDestructorCall && result && !args.m_Args)
return InvokeDestructor(result, /*nary=*/0UL, /*withFree=*/true);
return InvokeDestructor(result, nary, /*withFree=*/true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: return statement within a void function should not have a specified return value [readability-avoid-return-with-void-value]

Suggested change
return InvokeDestructor(result, nary, /*withFree=*/true);
{ InvokeDestructor(result, nary, /*withFree=*/true); return;
}


#ifndef NDEBUG
assert(AreArgumentsValid(result, args, self) && "Invalid args!");
ReportInvokeStart(result, args, self);
#endif // NDEBUG
m_GenericCall(self, args.m_ArgSize, args.m_Args, result);
m_GenericCall(self, args.m_ArgSize, args.m_Args, result, nary);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: do not access members of unions; use (boost::)variant instead [cppcoreguidelines-pro-type-union-access]

      m_GenericCall(self, args.m_ArgSize, args.m_Args, result, nary);
      ^

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: narrowing conversion from 'size_t' (aka 'unsigned long') to signed type 'int' is implementation-defined [cppcoreguidelines-narrowing-conversions]

      m_GenericCall(self, args.m_ArgSize, args.m_Args, result, nary);
                          ^

}
/// Makes a call to a destructor.
///\param[in] object - the pointer of the object whose destructor we call.
Expand Down Expand Up @@ -753,20 +757,25 @@ namespace Cpp {
CPPINTEROP_API std::vector<long int> GetDimensions(TCppType_t type);

/// Allocates memory for a given class.
CPPINTEROP_API TCppObject_t Allocate(TCppScope_t scope);
/// \c count is used to indicate the number of objects to allocate for.
CPPINTEROP_API TCppObject_t Allocate(TCppScope_t scope,
TCppIndex_t count = 1UL);

/// Deallocates memory for a given class.
CPPINTEROP_API void Deallocate(TCppScope_t scope, TCppObject_t address);
CPPINTEROP_API void Deallocate(TCppScope_t scope, TCppObject_t address,
TCppIndex_t count = 1UL);

/// Creates an object of class \c scope and calls its default constructor. If
/// \c arena is set it uses placement new.
/// \c count is used to indicate the number of objects to construct.
CPPINTEROP_API TCppObject_t Construct(TCppScope_t scope,
void* arena = nullptr);
void* arena = nullptr,
TCppIndex_t count = 1UL);

/// Calls the destructor of object of type \c type. When withFree is true it
/// calls operator delete/free.
CPPINTEROP_API void Destruct(TCppObject_t This, TCppScope_t type,
bool withFree = true);
bool withFree = true, TCppIndex_t count = 0UL);

/// @name Stream Redirection
///
Expand Down
16 changes: 8 additions & 8 deletions lib/Interpreter/CXCppInterOp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -598,25 +598,25 @@ void clang_deallocate(CXObject address) { ::operator delete(address); }

namespace Cpp {
void* Construct(compat::Interpreter& interp, TCppScope_t scope,
void* arena /*=nullptr*/);
void* arena /*=nullptr*/, TCppIndex_t count);
} // namespace Cpp

CXObject clang_construct(CXScope scope, void* arena) {
CXObject clang_construct(CXScope scope, void* arena, size_t count) {
return Cpp::Construct(*getInterpreter(scope),
static_cast<void*>(getDecl(scope)), arena);
static_cast<void*>(getDecl(scope)), arena, count);
}

void clang_invoke(CXScope func, void* result, void** args, size_t n,
void* self) {
size_t nary, void* self) {
Cpp::MakeFunctionCallable(getInterpreter(func), getDecl(func))
.Invoke(result, {args, n}, self);
.Invoke(result, {args, n}, self, nary);
}

namespace Cpp {
void Destruct(compat::Interpreter& interp, TCppObject_t This,
clang::Decl* Class, bool withFree);
clang::Decl* Class, bool withFree, size_t nary);
} // namespace Cpp

void clang_destruct(CXObject This, CXScope S, bool withFree) {
Cpp::Destruct(*getInterpreter(S), This, getDecl(S), withFree);
void clang_destruct(CXObject This, CXScope S, bool withFree, size_t nary) {
Cpp::Destruct(*getInterpreter(S), This, getDecl(S), withFree, nary);
}
76 changes: 58 additions & 18 deletions lib/Interpreter/CppInterOp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1896,13 +1896,19 @@ namespace Cpp {
void make_narg_ctor(const FunctionDecl* FD, const unsigned N,
std::ostringstream& typedefbuf,
std::ostringstream& callbuf,
const std::string& class_name, int indent_level) {
const std::string& class_name, int indent_level,
bool array = false) {
// Make a code string that follows this pattern:
//
// ClassName(args...)
// OR
// ClassName[nary](args...) // array of objects
//

callbuf << class_name << "(";
callbuf << class_name;
if (array)
callbuf << "[nary]";
callbuf << "(";
for (unsigned i = 0U; i < N; ++i) {
const ParmVarDecl* PVD = FD->getParamDecl(i);
QualType Ty = PVD->getType();
Expand Down Expand Up @@ -2085,16 +2091,43 @@ namespace Cpp {
std::ostringstream& buf, int indent_level) {
// Make a code string that follows this pattern:
//
// (*(ClassName**)ret) = (obj) ?
// new (*(ClassName**)ret) ClassName(args...) : new ClassName(args...);
//
// // array of objects construction
// if (nary > 1) {
// (*(ClassName**)ret) = (obj) ? new (*(ClassName**)ret)
// ClassName[nary](args...) : new ClassName[nary](args...);
// }
// else {
// (*(ClassName**)ret) = (obj) ? new (*(ClassName**)ret)
// ClassName(args...) : new ClassName(args...);
// }
{
std::ostringstream typedefbuf;
std::ostringstream callbuf;
//
// Write the return value assignment part.
//
indent(callbuf, indent_level);
callbuf << "if (nary > 1) {\n";
indent(callbuf, indent_level);
callbuf << "(*(" << class_name << "**)ret) = ";
callbuf << "(obj) ? new (*(" << class_name << "**)ret) ";
make_narg_ctor(FD, N, typedefbuf, callbuf, class_name, indent_level,
true);

callbuf << ": new ";
//
// Write the actual expression.
//
make_narg_ctor(FD, N, typedefbuf, callbuf, class_name, indent_level,
true);
//
// End the new expression statement.
//
callbuf << ";\n";
indent(callbuf, indent_level);
callbuf << "}\n";
callbuf << "else {\n";
indent(callbuf, indent_level);
callbuf << "(*(" << class_name << "**)ret) = ";
callbuf << "(obj) ? new (*(" << class_name << "**)ret) ";
make_narg_ctor(FD, N, typedefbuf, callbuf, class_name, indent_level);
Expand All @@ -2108,6 +2141,8 @@ namespace Cpp {
// End the new expression statement.
//
callbuf << ";\n";
indent(callbuf, --indent_level);
callbuf << "}\n";
//
// Output the whole new expression and return statement.
//
Expand Down Expand Up @@ -2626,7 +2661,8 @@ namespace Cpp {
"__attribute__((annotate(\"__cling__ptrcheck(off)\")))\n"
"extern \"C\" void ";
buf << wrapper_name;
buf << "(void* obj, int nargs, void** args, void* ret)\n"
buf << "(void* obj, int nargs, void** args, void* ret, unsigned long "
"nary)\n"
"{\n";
++indent_level;
if (min_args == num_params) {
Expand Down Expand Up @@ -3577,17 +3613,18 @@ namespace Cpp {
}
}

TCppObject_t Allocate(TCppScope_t scope) {
return (TCppObject_t)::operator new(Cpp::SizeOf(scope));
TCppObject_t Allocate(TCppScope_t scope, TCppIndex_t count) {
return (TCppObject_t)::operator new(Cpp::SizeOf(scope) * count);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "operator new" is directly included [misc-include-cleaner]

lib/Interpreter/CppInterOp.cpp:39:

- #if CLANG_VERSION_MAJOR >= 19
+ #include <new>
+ #if CLANG_VERSION_MAJOR >= 19

}

void Deallocate(TCppScope_t scope, TCppObject_t address) {
::operator delete(address);
void Deallocate(TCppScope_t scope, TCppObject_t address, TCppIndex_t count) {
size_t bytes = Cpp::SizeOf(scope) * count;
::operator delete(address, bytes);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "operator delete" is directly included [misc-include-cleaner]

    ::operator delete(address, bytes);
      ^

}

// FIXME: Add optional arguments to the operator new.
TCppObject_t Construct(compat::Interpreter& interp, TCppScope_t scope,
void* arena /*=nullptr*/) {
void* arena /*=nullptr*/, TCppIndex_t count /*=1UL*/) {
auto* Class = (Decl*) scope;
// FIXME: Diagnose.
if (!HasDefaultConstructor(Class))
Expand All @@ -3596,7 +3633,8 @@ namespace Cpp {
auto* const Ctor = GetDefaultConstructor(interp, Class);
if (JitCall JC = MakeFunctionCallable(&interp, Ctor)) {
if (arena) {
JC.Invoke(&arena, {}, (void*)~0); // Tell Invoke to use placement new.
JC.Invoke(&arena, {}, (void*)~0,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]

        JC.Invoke(&arena, {}, (void*)~0,
                              ^

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr]

        JC.Invoke(&arena, {}, (void*)~0,
                              ^

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: multilevel pointer conversion from 'void **' to 'void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion]

        JC.Invoke(&arena, {}, (void*)~0,
                  ^

count); // Tell Invoke to use placement new.
return arena;
}

Expand All @@ -3607,22 +3645,24 @@ namespace Cpp {
return nullptr;
}

TCppObject_t Construct(TCppScope_t scope, void* arena /*=nullptr*/) {
return Construct(getInterp(), scope, arena);
TCppObject_t Construct(TCppScope_t scope, void* arena /*=nullptr*/,
TCppIndex_t count /*=0UL*/) {
return Construct(getInterp(), scope, arena, count);
}

void Destruct(compat::Interpreter& interp, TCppObject_t This, Decl* Class,
bool withFree) {
bool withFree, TCppIndex_t nary) {
if (auto wrapper = make_dtor_wrapper(interp, Class)) {
(*wrapper)(This, /*nary=*/0, withFree);
(*wrapper)(This, nary, withFree);
return;
}
// FIXME: Diagnose.
}

void Destruct(TCppObject_t This, TCppScope_t scope, bool withFree /*=true*/) {
void Destruct(TCppObject_t This, TCppScope_t scope, bool withFree /*=true*/,
TCppIndex_t count /*=1UL*/) {
auto* Class = static_cast<Decl*>(scope);
Destruct(getInterp(), This, Class, withFree);
Destruct(getInterp(), This, Class, withFree, count);
}

class StreamCaptureInfo {
Expand Down
Loading
Loading