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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
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
50 changes: 43 additions & 7 deletions include/CppInterOp/CppInterOp.h
Original file line number Diff line number Diff line change
@@ -103,6 +103,7 @@ class JitCall {
enum Kind : char {
kUnknown = 0,
kGenericCall,
kConstructorCall,
kDestructorCall,
};
struct ArgList {
@@ -115,25 +116,32 @@ class JitCall {
// FIXME: Figure out how to unify the wrapper signatures.
// FIXME: Hide these implementation details by moving wrapper generation in
// this class.
// (self, nargs, args, result, nary)
using GenericCall = void (*)(void*, size_t, void**, void*);
using DestructorCall = void (*)(void*, unsigned long, int);
// (result, nary, nargs, args, is_arena)
using ConstructorCall = void (*)(void*, size_t, size_t, void**, void*);
// (self, nary, withFree)
using DestructorCall = void (*)(void*, size_t, int);

private:
union {
GenericCall m_GenericCall;
ConstructorCall m_ConstructorCall;
DestructorCall m_DestructorCall;
};
Kind m_Kind;
TCppConstFunction_t m_FD;
JitCall() : m_GenericCall(nullptr), m_Kind(kUnknown), m_FD(nullptr) {}
JitCall(Kind K, GenericCall C, TCppConstFunction_t FD)
: m_GenericCall(C), m_Kind(K), m_FD(FD) {}
JitCall(Kind K, ConstructorCall C, TCppConstFunction_t Ctor)
: m_ConstructorCall(C), m_Kind(K), m_FD(Ctor) {}
JitCall(Kind K, DestructorCall C, TCppConstFunction_t Dtor)
: m_DestructorCall(C), m_Kind(K), m_FD(Dtor) {}

/// Checks if the passed arguments are valid for the given function.
CPPINTEROP_API bool AreArgumentsValid(void* result, ArgList args,
void* self) const;
CPPINTEROP_API bool AreArgumentsValid(void* result, ArgList args, void* self,
size_t nary = 1UL) const;

/// This function is used for debugging, it reports when the function was
/// called.
@@ -169,6 +177,12 @@ class JitCall {
InvokeDestructor(result, /*nary=*/0UL, /*withFree=*/true);
return;
}
// Forward if we intended to call a constructor (nary cannot be inferred, so
// we stick to constructing a single object)
else if (m_Kind == kConstructorCall && result && !args.m_Args) {
InvokeConstructor(result, /*nary=*/1UL, args, self);
return;
}
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 'else' after 'return' [llvm-else-after-return]

Suggested change
}
if (m_Kind == kConstructorCall && result && !args.m_Args) {
InvokeConstructor(result, /*nary=*/1UL, args, false);
return;
}

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 'else' after 'return' [llvm-else-after-return]

Suggested change
}
if (m_Kind == kConstructorCall && result && !args.m_Args) {
InvokeConstructor(result, /*nary=*/1UL, args, self);
return;
}


#ifndef NDEBUG
assert(AreArgumentsValid(result, args, self) && "Invalid args!");
@@ -192,6 +206,23 @@ class JitCall {
#endif // NDEBUG
m_DestructorCall(object, nary, withFree);
}

/// Makes a call to a constructor.
///\param[in] result - the memory address at which we construct the object
/// (placement new).
///\param[in] nary - Use array new if we have to construct an array of
/// objects (nary > 1).
///\param[in] args - a pointer to a argument list and argument size.
// FIXME: Change the type of withFree from int to bool in the wrapper code.
void InvokeConstructor(void* result, unsigned long nary = 1,
ArgList args = {}, void* is_arena = nullptr) const {
assert(m_Kind == kConstructorCall && "Wrong overload!");
#ifndef NDEBUG
assert(AreArgumentsValid(result, args, nullptr, nary) && "Invalid args!");
ReportInvokeStart(result, args, nullptr);
#endif // NDEBUG
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_ConstructorCall(result, nary, args.m_ArgSize, args.m_Args, (int)is_arena);
    ^

m_ConstructorCall(result, nary, args.m_ArgSize, args.m_Args, is_arena);
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_ConstructorCall(result, nary, args.m_ArgSize, args.m_Args, is_arena);
    ^

}
};

///\returns the version string information of the library.
@@ -779,19 +810,24 @@ enum : long int {
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,
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: enum '(unnamed enum at /github/workspace/include/CppInterOp/CppInterOp.h:802:1)' uses a larger base type ('long', size: 8 bytes) than necessary for its value set, consider using 'std::int8_t' (1 byte) as the base type to reduce its size [performance-enum-size]

enum : long int {
^

Copy link
Contributor

Choose a reason for hiding this comment

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

We should adjust the types to match the signature types to match the generated code types and disable the clang-tidy complaints if we have to.

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.
CPPINTEROP_API TCppObject_t Construct(TCppScope_t scope, void* arena = nullptr);
/// \c count is used to indicate the number of objects to construct.
CPPINTEROP_API TCppObject_t Construct(TCppScope_t scope, 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
///
6 changes: 4 additions & 2 deletions include/clang-c/CXCppInterOp.h
Original file line number Diff line number Diff line change
@@ -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 = 1UL);

/**
* Creates a trampoline function and makes a call to a generic function or
@@ -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);

/**
* @}
12 changes: 6 additions & 6 deletions lib/CppInterOp/CXCppInterOp.cpp
Original file line number Diff line number Diff line change
@@ -599,12 +599,12 @@ 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,
@@ -615,9 +615,9 @@ void clang_invoke(CXScope func, void* result, void** args, size_t n,

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);
}
Loading