Skip to content

[c-api] Generate C wrappers from TableGen instead of hand-writing them. - #921

Merged
vgvassilev merged 1 commit into
compiler-research:mainfrom
vgvassilev:c-layer-rework
May 13, 2026
Merged

[c-api] Generate C wrappers from TableGen instead of hand-writing them.#921
vgvassilev merged 1 commit into
compiler-research:mainfrom
vgvassilev:c-layer-rework

Conversation

@vgvassilev

Copy link
Copy Markdown
Contributor

The old clang-c/CXCppInterOp.h and CXCppInterOp.cpp maintained ~470 lines of hand-written C forwarding code that drifted from the C++ API on every change. CTypeMap records in CppInterOpAPI.td now drive the emitter to produce CXCppInterOpDecl.inc (C-compatible declarations) and CXCppInterOpImpl.inc (C++ implementations) as two separate files with no preprocessor guards needed at include sites.

CTypeMap supports scalar passthrough, opaque pointer erasure, string and enum conversions, and collection signature rewriting (vector returns, out-params, in-params, string collections). CppInterOpArray, CppInterOpStringArray, and TemplateArgInfo use typedef struct for C compatibility, living at file scope in C and inside namespace CppImpl in C++. Functions the emitter cannot handle mechanically (GetClassTemplatedMethods, MakeFunctionCallable, GetDimensions) are marked NoCWrapper; GetClassTemplatedMethods has a hand-written wrapper in CXCppInterOp.cpp.

Adds CAPITestC.c (pure C compilation of all declarations), C API dispatch tests via dlsym, and focused tests for scalars, enums, collections, and vector in-params.

@codecov

codecov Bot commented Apr 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.67100% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.89%. Comparing base (c49ea8e) to head (9c32d55).
⚠️ Report is 54 commits behind head on main.

Files with missing lines Patch % Lines
utils/TableGen/CppInterOpEmitter.cpp 95.34% 10 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #921      +/-   ##
==========================================
+ Coverage   83.02%   85.89%   +2.87%     
==========================================
  Files          15       15              
  Lines        5079     4971     -108     
==========================================
+ Hits         4217     4270      +53     
+ Misses        862      701     -161     
Files with missing lines Coverage Δ
include/CppInterOp/CppInterOpTypes.h 93.87% <100.00%> (ø)
lib/CppInterOp/CXCppInterOp.cpp 100.00% <100.00%> (+49.71%) ⬆️
utils/TableGen/TableGen.cpp 100.00% <100.00%> (ø)
utils/TableGen/CppInterOpEmitter.cpp 96.76% <95.34%> (-3.24%) ⬇️

... and 4 files with indirect coverage changes

Files with missing lines Coverage Δ
include/CppInterOp/CppInterOpTypes.h 93.87% <100.00%> (ø)
lib/CppInterOp/CXCppInterOp.cpp 100.00% <100.00%> (+49.71%) ⬆️
utils/TableGen/TableGen.cpp 100.00% <100.00%> (ø)
utils/TableGen/CppInterOpEmitter.cpp 96.76% <95.34%> (-3.24%) ⬇️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@vgvassilev
vgvassilev force-pushed the c-layer-rework branch 3 times, most recently from fb435c4 to d7a9760 Compare April 24, 2026 15:14

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 10 out of 46. Check the log or trigger a new build to see more.


// C-compatible headers — usable from both C and C++.
#include <stdbool.h>
#include <stddef.h>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: including 'stdbool.h' has no effect in C++; consider removing it [modernize-deprecated-headers]

Suggested change
#include <stddef.h>
.

// C-compatible headers — usable from both C and C++.
#include <stdbool.h>
#include <stddef.h>
#include <stdint.h>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: inclusion of deprecated C++ header 'stddef.h'; consider using 'cstddef' instead [modernize-deprecated-headers]

Suggested change
#include <stdint.h>
>
><cstddef>

#include <stddef.h>
#include <stdint.h>

#ifdef __cplusplus

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: inclusion of deprecated C++ header 'stdint.h'; consider using 'cstdint' instead [modernize-deprecated-headers]

Suggested change
#ifdef __cplusplus
>
><cstdint>

#include <cstddef>
#include <cstdint>
#include <set>
#include <string>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: included header set is not used directly [misc-include-cleaner]

Suggested change
#include <string>
>

#include <cstdint>
#include <set>
#include <string>
#include <vector>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: included header string is not used directly [misc-include-cleaner]

Suggested change
#include <vector>
>

#include <string>
#include <vector>

namespace CppImpl {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: included header vector is not used directly [misc-include-cleaner]

Suggested change
namespace CppImpl {
>

void** data;
size_t size;
} CppInterOpArray;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: use 'using' instead of 'typedef' [modernize-use-using]

Suggested change
.
using CppInterOpArray = struct CppInterOpArray {
void** data;
size_t size;
}

char** data;
size_t size;
} CppInterOpStringArray;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: use 'using' instead of 'typedef' [modernize-use-using]

Suggested change
.
using CppInterOpStringArray = struct CppInterOpStringArray {
char** data;
size_t size;
}

: m_Type(type), m_IntegralValue(integral_value) {}
#endif
} TemplateArgInfo;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: use 'using' instead of 'typedef' [modernize-use-using]

Suggested change
.
using TemplateArgInfo = struct TemplateArgInfo {
void* m_Type;
const char* m_IntegralValue;
#ifdef __cplusplus
TemplateArgInfo(void* type, const char* integral_value = nullptr)
: m_Type(type), m_IntegralValue(integral_value) {}
#endif
}

return clang::cxscope::MakeCXScope(CXXRD->getDestructor(), getNewTU(S));
// GetClassTemplatedMethods returns bool AND fills a vector out-param.
// The C wrapper drops the bool (caller checks arr.size > 0 instead).
CPPINTEROP_API Cpp::CppInterOpArray

Copy link
Copy Markdown
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 "CPPINTEROP_API" is directly included [misc-include-cleaner]

CPPINTEROP_API Cpp::CppInterOpArray
^

@vgvassilev
vgvassilev force-pushed the c-layer-rework branch 2 times, most recently from 5215f6b to 8ec7eba Compare April 24, 2026 16:20

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 10 out of 36. Check the log or trigger a new build to see more.

return clang::cxscope::MakeCXScope(CXXRD->getDestructor(), getNewTU(S));
// GetClassTemplatedMethods returns bool AND fills a vector out-param.
// The C wrapper drops the bool (caller checks arr.size > 0 instead).
CPPINTEROP_API Cpp::CppInterOpArray

Copy link
Copy Markdown
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 "CppImpl::CppInterOpArray" is directly included [misc-include-cleaner]

lib/CppInterOp/CXCppInterOp.cpp:6:

- #include <cstdlib>
+ #include <CppInterOp/CppInterOpTypes.h>
+ #include <cstdlib>

// The C wrapper drops the bool (caller checks arr.size > 0 instead).
CPPINTEROP_API Cpp::CppInterOpArray
cppinterop_GetClassTemplatedMethods(const char* name, void* parent) {
std::vector<Cpp::TCppFunction_t> out;

Copy link
Copy Markdown
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 "CppImpl::TCppFunction_t" is directly included [misc-include-cleaner]

  std::vector<Cpp::TCppFunction_t> out;
                   ^

CPPINTEROP_API Cpp::CppInterOpArray
cppinterop_GetClassTemplatedMethods(const char* name, void* parent) {
std::vector<Cpp::TCppFunction_t> out;
Cpp::GetClassTemplatedMethods(std::string(name), parent, out);

Copy link
Copy Markdown
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 "std::string" is directly included [misc-include-cleaner]

lib/CppInterOp/CXCppInterOp.cpp:8:

- #include <vector>
+ #include <string>
+ #include <vector>

Cpp::GetClassTemplatedMethods(std::string(name), parent, out);
Cpp::CppInterOpArray arr = {nullptr, out.size()};
if (arr.size) {
arr.data = static_cast<void**>(malloc(arr.size * sizeof(void*)));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: assigning newly created 'gsl::owner<>' to non-owner 'void **' [cppcoreguidelines-owning-memory]

    arr.data = static_cast<void**>(malloc(arr.size * sizeof(void*)));
    ^

Cpp::GetClassTemplatedMethods(std::string(name), parent, out);
Cpp::CppInterOpArray arr = {nullptr, out.size()};
if (arr.size) {
arr.data = static_cast<void**>(malloc(arr.size * sizeof(void*)));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: do not manage memory manually; consider a container or a smart pointer [cppcoreguidelines-no-malloc]

    arr.data = static_cast<void**>(malloc(arr.size * sizeof(void*)));
                                   ^

Cpp::CppInterOpArray arr = {nullptr, out.size()};
if (arr.size) {
arr.data = static_cast<void**>(malloc(arr.size * sizeof(void*)));
memcpy(arr.data, out.data(), arr.size * sizeof(void*));

Copy link
Copy Markdown
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 'const void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion]

    memcpy(arr.data, out.data(), arr.size * sizeof(void*));
                     ^

Cpp::CppInterOpArray arr = {nullptr, out.size()};
if (arr.size) {
arr.data = static_cast<void**>(malloc(arr.size * sizeof(void*)));
memcpy(arr.data, out.data(), arr.size * sizeof(void*));

Copy link
Copy Markdown
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]

    memcpy(arr.data, out.data(), arr.size * sizeof(void*));
           ^

#include <cstdlib>
#include <cstring>

using Cpp::CppInterOpArray;

Copy link
Copy Markdown
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 "CppImpl::CppInterOpArray" is directly included [misc-include-cleaner]

lib/CppInterOp/CXCppInterOpGenerated.cpp:5:

- #include <cstdlib>
+ #include <CppInterOp/CppInterOpTypes.h>
+ #include <cstdlib>

#include <cstring>

using Cpp::CppInterOpArray;
using Cpp::CppInterOpStringArray;

Copy link
Copy Markdown
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 "CppImpl::CppInterOpStringArray" is directly included [misc-include-cleaner]

using Cpp::CppInterOpStringArray;
           ^


using Cpp::CppInterOpArray;
using Cpp::CppInterOpStringArray;
using Cpp::TemplateArgInfo;

Copy link
Copy Markdown
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 "CppImpl::TemplateArgInfo" is directly included [misc-include-cleaner]

using Cpp::TemplateArgInfo;
           ^

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 10 out of 26. Check the log or trigger a new build to see more.

#include "CppInterOp/CXCppInterOpDecl.inc"

// Hand-written wrappers not in the .inc file.
CPPINTEROP_API Cpp::CppInterOpArray

Copy link
Copy Markdown
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 "CPPINTEROP_API" is directly included [misc-include-cleaner]

CPPINTEROP_API Cpp::CppInterOpArray
^

#include "CppInterOp/CXCppInterOpDecl.inc"

// Hand-written wrappers not in the .inc file.
CPPINTEROP_API Cpp::CppInterOpArray

Copy link
Copy Markdown
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 "CppImpl::CppInterOpArray" is directly included [misc-include-cleaner]

unittests/CppInterOp/CAPITest.cpp:7:

- #include <cstdlib>
+ #include <CppInterOp/CppInterOpTypes.h>
+ #include <cstdlib>

TYPED_TEST(CppInterOpTest, CAPI_Construct) {
if (TypeParam::isOutOfProcess)
GTEST_SKIP() << "Test fails for OOP JIT builds";
std::vector<const char*> args = {"-include", "new"};

Copy link
Copy Markdown
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 "std::vector" is directly included [misc-include-cleaner]

unittests/CppInterOp/CAPITest.cpp:9:

+ #include <vector>

Cpp::Declare("namespace CAPIStrNs { enum E1 { P, Q }; enum E2 { R }; }");

auto* ns = cppinterop_GetNamed("CAPIStrNs", nullptr);
Cpp::CppInterOpStringArray enums = cppinterop_GetEnums(ns);

Copy link
Copy Markdown
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 "CppImpl::CppInterOpStringArray" is directly included [misc-include-cleaner]

  Cpp::CppInterOpStringArray enums = cppinterop_GetEnums(ns);
       ^

Cpp::CppInterOpStringArray enums = cppinterop_GetEnums(ns);
EXPECT_EQ(enums.size, 2U);
// Verify the enum names are present (order may vary).
bool foundE1 = false, foundE2 = false;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: multiple declarations in a single statement reduces readability [readability-isolate-declaration]

Suggested change
bool foundE1 = false, foundE2 = false;
bool foundE1 = false;
bool foundE2 = false;


TYPED_TEST(CppInterOpTest, CAPI_CreateInterpreterInParam) {
// Test the vector in-param pattern: CreateInterpreter(ptr, size, ptr, size).
const char* args[] = {"-std=c++17"};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: do not declare C-style arrays, use std::array<> instead [cppcoreguidelines-avoid-c-arrays]

  const char* args[] = {"-std=c++17"};
        ^

TYPED_TEST(CppInterOpTest, CAPI_CreateInterpreterInParam) {
// Test the vector in-param pattern: CreateInterpreter(ptr, size, ptr, size).
const char* args[] = {"-std=c++17"};
auto* I = cppinterop_CreateInterpreter(args, 1, nullptr, 0);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]

  auto* I = cppinterop_CreateInterpreter(args, 1, nullptr, 0);
                                         ^


namespace {
/// RAII wrapper for a dlopen handle used by the C API dispatch tests.
class DlHandle {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: class 'DlHandle' defines a non-default destructor, a copy constructor and a copy assignment operator but does not define a move constructor or a move assignment operator [cppcoreguidelines-special-member-functions]

.
        ^

DlHandle& operator=(const DlHandle&) = delete;
explicit DlHandle(const char* path) {
#ifndef _WIN32
H = dlopen(path, RTLD_LOCAL | RTLD_NOW);

Copy link
Copy Markdown
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 "RTLD_LOCAL" is directly included [misc-include-cleaner]

2
                       ^

DlHandle& operator=(const DlHandle&) = delete;
explicit DlHandle(const char* path) {
#ifndef _WIN32
H = dlopen(path, RTLD_LOCAL | RTLD_NOW);

Copy link
Copy Markdown
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 "RTLD_NOW" is directly included [misc-include-cleaner]

2
                                    ^

@vgvassilev
vgvassilev force-pushed the c-layer-rework branch 3 times, most recently from 7a5e50f to bf639b0 Compare May 13, 2026 14:57
The old clang-c/CXCppInterOp.h and CXCppInterOp.cpp maintained ~470
lines of hand-written C forwarding code that drifted from the C++ API
on every change. CTypeMap records in CppInterOpAPI.td now drive the
emitter to produce CXCppInterOpDecl.inc (C-compatible declarations)
and CXCppInterOpImpl.inc (C++ implementations) as two separate files
with no preprocessor guards needed at include sites.

CTypeMap supports scalar passthrough, opaque pointer erasure, string
and enum conversions, and collection signature rewriting (vector
returns, out-params, in-params, string collections). CppInterOpArray,
CppInterOpStringArray, and TemplateArgInfo use typedef struct for C
compatibility, living at file scope in C and inside namespace CppImpl
in C++. Functions the emitter cannot handle mechanically
(GetClassTemplatedMethods, MakeFunctionCallable, GetDimensions) are
marked NoCWrapper; GetClassTemplatedMethods has a hand-written wrapper
in CXCppInterOp.cpp.

Adds CAPITestC.c (pure C compilation of all declarations), C API
dispatch tests via dlsym, and focused tests for scalars, enums,
collections, and vector in-params.
@vgvassilev
vgvassilev merged commit 5197645 into compiler-research:main May 13, 2026
26 checks passed
@vgvassilev
vgvassilev deleted the c-layer-rework branch May 13, 2026 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant