Add a libclang-style C API #337

Gnimuc · 2024-10-27T07:40:03Z

Since there is a substantial overlap with libclang’s API, I am only exposing the API I currently use in this PR.

This PR introduces two types, CXScope and CXQualType, which wrap TCppScope_t and TCppType_t, respectively. The structure of these two types is the same as libclang's CXCursor/CXType, with the only difference being that they store a handle to the interpreter instead of a CXTranslationUnit. This paves the way for upstreaming changes to libclang.

codecov · 2024-10-27T07:49:26Z

Codecov Report

Attention: Patch coverage is 30.99274% with 285 lines in your changes missing coverage. Please review.

Project coverage is 69.02%. Comparing base (c0a4dfe) to head (1bb9ead).

Files with missing lines	Patch %	Lines
lib/Interpreter/CXCppInterOp.cpp	22.46%	283 Missing ⚠️
lib/Interpreter/CppInterOp.cpp	95.83%	2 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (c0a4dfe) and HEAD (1bb9ead). Click for more details.

HEAD has 2 uploads less than BASE

Flag BASE (c0a4dfe) HEAD (1bb9ead)

4 2

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #337      +/-   ##
==========================================
- Coverage   74.29%   69.02%   -5.27%     
==========================================
  Files           8        9       +1     
  Lines        3190     3574     +384     
==========================================
+ Hits         2370     2467      +97     
- Misses        820     1107     +287

Files with missing lines	Coverage Δ
include/clang/Interpreter/CppInterOp.h	`96.29% <ø> (ø)`
lib/Interpreter/CppInterOp.cpp	`79.98% <95.83%> (+0.15%)`	⬆️
lib/Interpreter/CXCppInterOp.cpp	`22.46% <22.46%> (ø)`

... and 2 files with indirect coverage changes

Files with missing lines	Coverage Δ
include/clang/Interpreter/CppInterOp.h	`96.29% <ø> (ø)`
lib/Interpreter/CppInterOp.cpp	`79.98% <95.83%> (+0.15%)`	⬆️
lib/Interpreter/CXCppInterOp.cpp	`22.46% <22.46%> (ø)`

... and 2 files with indirect coverage changes

github-actions

clang-tidy made some suggestions

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

include/clang-c/CXCppInterOp.h

github-actions

clang-tidy made some suggestions

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

include/clang-c/CXCppInterOp.h

lib/Interpreter/CXCppInterOp.cpp

github-actions · 2024-10-27T08:37:48Z

lib/Interpreter/CXCppInterOp.cpp

+static inline bool isNull(const CXScope& S) { return !S.data[0]; }
+
+static inline clang::Decl* getDecl(const CXScope& S) {
+  return const_cast<clang::Decl*>(static_cast<const clang::Decl*>(S.data[0]));


warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]

return const_cast<clang::Decl*>(static_cast<const clang::Decl*>(S.data[0])); ^

github-actions · 2024-10-27T08:37:48Z

lib/Interpreter/CXCppInterOp.cpp

+  auto* I = getInterpreter(scope);
+  if (const Cpp::JitCall JC = Cpp::MakeFunctionCallableImpl(I, getDecl(Ctor))) {
+    if (arena) {
+      JC.Invoke(&arena, {}, (void*)~0); // Tell Invoke to use placement new.


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

JC.Invoke(&arena, {}, (void*)~0); // Tell Invoke to use placement new. ^

github-actions · 2024-10-27T08:37:48Z

lib/Interpreter/CXCppInterOp.cpp

+  auto* I = getInterpreter(scope);
+  if (const Cpp::JitCall JC = Cpp::MakeFunctionCallableImpl(I, getDecl(Ctor))) {
+    if (arena) {
+      JC.Invoke(&arena, {}, (void*)~0); // Tell Invoke to use placement new.


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

JC.Invoke(&arena, {}, (void*)~0); // Tell Invoke to use placement new. ^

vgvassilev

Looks like we are quite close. Can we silence the codecov somehow without duplicating tests? Can we move the refactorings in CppInterOp.cpp as a separate PR?

vgvassilev · 2024-10-27T10:25:11Z

include/clang-c/CXCppInterOp.h

+ *
+ * \param prepend Whether to prepend the directory to the search path.
+ */
+void clang_interpreter_addSearchPath(CXInterpreter I, const char* dir,


Do we want to shorten these interfaces. Maybe Cpp_AddSearchPath?

Since we only expose opaque pointers in the C API, I believe libclang uses this naming style to warn users about the type of the input argument. However, for historical reasons, libclang appears to lack strict rules on capitalization.

The naming convention is like clang_ + ClassName + methodName or clang_ + functionName.

Here are some examples:

typedef void *CXIndexAction; CINDEX_LINKAGE CXIndexAction clang_IndexAction_create(CXIndex CIdx); CINDEX_LINKAGE void clang_IndexAction_dispose(CXIndexAction); CINDEX_LINKAGE CXIdxClientContainer clang_index_getClientContainer(const CXIdxContainerInfo *); CINDEX_LINKAGE unsigned clang_CXXMethod_isDeleted(CXCursor C); CINDEX_LINKAGE CXString clang_Module_getName(CXModule Module); CINDEX_LINKAGE CXCursor clang_getCanonicalCursor(CXCursor); CINDEX_LINKAGE unsigned clang_PrintingPolicy_getProperty(CXPrintingPolicy Policy, enum CXPrintingPolicyProperty Property); CINDEX_LINKAGE void clang_PrintingPolicy_setProperty(CXPrintingPolicy Policy, enum CXPrintingPolicyProperty Property, unsigned Value); CINDEX_LINKAGE CXPrintingPolicy clang_getCursorPrintingPolicy(CXCursor); CINDEX_LINKAGE void clang_PrintingPolicy_dispose(CXPrintingPolicy Policy); CINDEX_LINKAGE CXString clang_getCursorPrettyPrinted(CXCursor Cursor, CXPrintingPolicy Policy);

vgvassilev · 2024-10-27T10:28:38Z

include/clang/Interpreter/CppInterOp.h

@@ -262,16 +264,16 @@ namespace Cpp {

  /// This is similar to GetName() function, but besides
  /// the name, it also gets the template arguments.
-  CPPINTEROP_API std::string GetCompleteName(TCppType_t klass);
+  CPPINTEROP_API std::string GetCompleteName(TCppScope_t klass);


Thanks! We should probably adopt the CX enum style so that the compiler could help us with finding these wrong cases. Related to compiler-research/CPyCppyy#8.

lib/Interpreter/CXCppInterOp.cpp

lib/Interpreter/CppInterOp.cpp

Gnimuc · 2024-10-27T11:14:59Z

Looks like we are quite close. Can we silence the codecov somehow without duplicating tests? Can we move the refactorings in CppInterOp.cpp as a separate PR?

Yes. I'll make another PR for the changes in CppInterOp.cpp.

also fix argument type typos and c-style casts

- Add `CXScope` and `CXQualType` as a bridge between libclang types and CppInterOp's types - Add C API for interpreter and scope manipulations

github-actions

clang-tidy made some suggestions

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

github-actions · 2024-10-27T15:18:33Z

unittests/CppInterOp/InterpreterTest.cpp

@@ -98,6 +100,19 @@ TEST(InterpreterTest, CreateInterpreter) {
                   "#endif");
  EXPECT_TRUE(Cpp::GetNamed("cpp17"));
  EXPECT_FALSE(Cpp::GetNamed("cppUnknown"));
+
+  // C API
+  const char* args[] = {"-std=c++14"};


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

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

github-actions · 2024-10-27T15:18:33Z

unittests/CppInterOp/InterpreterTest.cpp

+
+  // C API
+  const char* args[] = {"-std=c++14"};
+  auto CXI = clang_createInterpreter(args, 1);


warning: 'auto CXI' can be declared as 'auto *CXI' [llvm-qualified-auto]

Suggested change

auto CXI = clang_createInterpreter(args, 1);

auto *CXI = clang_createInterpreter(args, 1);

github-actions · 2024-10-27T15:18:33Z

unittests/CppInterOp/InterpreterTest.cpp

+
+  // C API
+  const char* args[] = {"-std=c++14"};
+  auto CXI = clang_createInterpreter(args, 1);


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 CXI = clang_createInterpreter(args, 1); ^

github-actions · 2024-10-27T15:18:33Z

unittests/CppInterOp/InterpreterTest.cpp

+  clang_Interpreter_dispose(CXI);
+
+  CXI = clang_createInterpreterFromRawPtr(I);
+  auto CLI = clang_Interpreter_getClangInterpreter(CXI);


warning: 'auto CLI' can be declared as 'auto *CLI' [llvm-qualified-auto]

Suggested change

auto CLI = clang_Interpreter_getClangInterpreter(CXI);

auto *CLI = clang_Interpreter_getClangInterpreter(CXI);

github-actions · 2024-10-27T15:18:33Z

unittests/CppInterOp/InterpreterTest.cpp

+  CXI = clang_createInterpreterFromRawPtr(I);
+  auto CLI = clang_Interpreter_getClangInterpreter(CXI);
+  EXPECT_TRUE(CLI);
+  auto I2 = clang_Interpreter_takeInterpreterAsPtr(CXI);


warning: 'auto I2' can be declared as 'auto *I2' [llvm-qualified-auto]

Suggested change

auto I2 = clang_Interpreter_takeInterpreterAsPtr(CXI);

auto *I2 = clang_Interpreter_takeInterpreterAsPtr(CXI);

github-actions · 2024-10-27T15:18:33Z

unittests/CppInterOp/Utils.cpp

+
+void dispose_string(CXString string) {
+  if (string.private_flags == 1 && string.data)
+    free(const_cast<void*>(string.data));


warning: calling legacy resource function without passing a 'gsl::owner<>' [cppcoreguidelines-owning-memory]

free(const_cast<void*>(string.data)); ^

github-actions · 2024-10-27T15:18:33Z

unittests/CppInterOp/Utils.cpp

+
+void dispose_string(CXString string) {
+  if (string.private_flags == 1 && string.data)
+    free(const_cast<void*>(string.data));


warning: do not manage memory manually; use RAII [cppcoreguidelines-no-malloc]

free(const_cast<void*>(string.data)); ^

github-actions · 2024-10-27T15:18:33Z

unittests/CppInterOp/Utils.cpp

+
+void dispose_string(CXString string) {
+  if (string.private_flags == 1 && string.data)
+    free(const_cast<void*>(string.data));


warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]

free(const_cast<void*>(string.data)); ^

github-actions · 2024-10-27T15:18:34Z

unittests/CppInterOp/Utils.cpp

+    free(const_cast<void*>(string.data));
+}
+
+CXScope make_scope(const clang::Decl* D, const CXInterpreter I) {


warning: 'I' declared with a const-qualified typedef; results in the type being 'CXInterpreterImpl *const' instead of 'const CXInterpreterImpl *' [misc-misplaced-const]

CXScope make_scope(const clang::Decl* D, const CXInterpreter I) { ^

Additional context

include/clang-c/CXCppInterOp.h:25: typedef declared here

typedef struct CXInterpreterImpl* CXInterpreter; ^

github-actions · 2024-10-27T15:18:34Z

unittests/CppInterOp/Utils.h

 #include <memory>
 #include <vector>
-#include "llvm/Support/Valgrind.h"
+#include "clang-c/CXCppInterOp.h"
+#include "clang-c/CXString.h"


warning: #includes are not sorted properly [llvm-include-order]

Suggested change

#include "clang-c/CXString.h"

#include "clang-c/CXCppInterOp.h"

#include "clang-c/CXString.h"

#include "llvm/Support/Valgrind.h"

#include <memory>

#include <vector>

Gnimuc mentioned this pull request Oct 27, 2024

Add a libclang-style C API and improve type safety #304

Closed

18 tasks

github-actions bot reviewed Oct 27, 2024

View reviewed changes

vgvassilev reviewed Oct 27, 2024

View reviewed changes

Add new API for multiple interpreter instance support

9c695a8

also fix argument type typos and c-style casts

Gnimuc force-pushed the add-c-api2 branch 2 times, most recently from 1d48612 to 66ccc69 Compare October 27, 2024 14:44

Add libclang-style C API

1bb9ead

- Add `CXScope` and `CXQualType` as a bridge between libclang types and CppInterOp's types - Add C API for interpreter and scope manipulations

Gnimuc force-pushed the add-c-api2 branch from 66ccc69 to 1bb9ead Compare October 27, 2024 14:56

github-actions bot reviewed Oct 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a libclang-style C API #337

Add a libclang-style C API #337

Gnimuc commented Oct 27, 2024

codecov bot commented Oct 27, 2024 •

edited

Loading

github-actions bot left a comment

github-actions bot left a comment

github-actions bot Oct 27, 2024

github-actions bot Oct 27, 2024

github-actions bot Oct 27, 2024

vgvassilev left a comment

vgvassilev Oct 27, 2024

Gnimuc Oct 27, 2024

vgvassilev Oct 27, 2024

Gnimuc commented Oct 27, 2024

github-actions bot left a comment

github-actions bot Oct 27, 2024

github-actions bot Oct 27, 2024

github-actions bot Oct 27, 2024

github-actions bot Oct 27, 2024

github-actions bot Oct 27, 2024

github-actions bot Oct 27, 2024

github-actions bot Oct 27, 2024

github-actions bot Oct 27, 2024

github-actions bot Oct 27, 2024

github-actions bot Oct 27, 2024

	auto CXI = clang_createInterpreter(args, 1);
	auto *CXI = clang_createInterpreter(args, 1);

	auto CLI = clang_Interpreter_getClangInterpreter(CXI);
	auto *CLI = clang_Interpreter_getClangInterpreter(CXI);

	auto I2 = clang_Interpreter_takeInterpreterAsPtr(CXI);
	auto *I2 = clang_Interpreter_takeInterpreterAsPtr(CXI);

Add a libclang-style C API #337

Are you sure you want to change the base?

Add a libclang-style C API #337

Conversation

Gnimuc commented Oct 27, 2024

codecov bot commented Oct 27, 2024 • edited Loading

Codecov Report

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Oct 27, 2024

Choose a reason for hiding this comment

github-actions bot Oct 27, 2024

Choose a reason for hiding this comment

github-actions bot Oct 27, 2024

Choose a reason for hiding this comment

vgvassilev left a comment

Choose a reason for hiding this comment

vgvassilev Oct 27, 2024

Choose a reason for hiding this comment

Gnimuc Oct 27, 2024

Choose a reason for hiding this comment

vgvassilev Oct 27, 2024

Choose a reason for hiding this comment

Gnimuc commented Oct 27, 2024

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Oct 27, 2024

Choose a reason for hiding this comment

github-actions bot Oct 27, 2024

Choose a reason for hiding this comment

github-actions bot Oct 27, 2024

Choose a reason for hiding this comment

github-actions bot Oct 27, 2024

Choose a reason for hiding this comment

github-actions bot Oct 27, 2024

Choose a reason for hiding this comment

github-actions bot Oct 27, 2024

Choose a reason for hiding this comment

github-actions bot Oct 27, 2024

Choose a reason for hiding this comment

github-actions bot Oct 27, 2024

Choose a reason for hiding this comment

github-actions bot Oct 27, 2024

Choose a reason for hiding this comment

github-actions bot Oct 27, 2024

Choose a reason for hiding this comment

codecov bot commented Oct 27, 2024 •

edited

Loading