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 22.46575% with 283 lines in your changes missing coverage. Please review.

Project coverage is 69.02%. Comparing base (9b5f4c5) to head (7be80d8).
Report is 2 commits behind head on main.

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

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #337      +/-   ##
==========================================
- Coverage   74.43%   69.02%   -5.42%     
==========================================
  Files           8        9       +1     
  Lines        3204     3574     +370     
==========================================
+ Hits         2385     2467      +82     
- Misses        819     1107     +288

Files with missing lines	Coverage Δ
lib/Interpreter/CXCppInterOp.cpp	`22.46% <22.46%> (ø)`

... and 2 files with indirect coverage changes

Files with missing lines	Coverage Δ
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.

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>

vgvassilev · 2024-10-27T18:41:55Z

lib/Interpreter/CXCppInterOp.cpp

+  if (!clang_hasDefaultConstructor(S))
+    return clang::cxscope::MakeCXScope(nullptr, getNewTU(S));
+
+  auto* CXXRD = llvm::dyn_cast_or_null<clang::CXXRecordDecl>(getDecl(S));
+  if (!CXXRD)
+    return clang::cxscope::MakeCXScope(nullptr, getNewTU(S));
+
+  const auto* Res =
+      getInterpreter(S)->getSema().LookupDefaultConstructor(CXXRD);
+  return clang::cxscope::MakeCXScope(Res, getNewTU(S));


Why can't we forward to Cpp::GetDefaultConstructor? I see this trend in several interfaces. Is there some limitation on the C++ side?

github-actions

clang-tidy made some suggestions

github-actions · 2024-10-27T18:56:28Z

unittests/CppInterOp/Utils.h

+
+void dispose_string(CXString string);
+
+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-27T18:56:28Z

unittests/CppInterOp/Utils.h

+
+void dispose_string(CXString string);
+
+CXScope make_scope(const clang::Decl* D, const CXInterpreter I);


warning: parameter 'I' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]

Suggested change

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

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

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

vgvassilev · 2024-10-29T17:42:35Z

Do we know why some builds are failing?

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

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

github-actions bot reviewed Oct 27, 2024

View reviewed changes

vgvassilev force-pushed the add-c-api2 branch from 1bb9ead to bc67a64 Compare October 27, 2024 18:38

vgvassilev reviewed Oct 27, 2024

View reviewed changes

github-actions bot reviewed Oct 27, 2024

View reviewed changes

vgvassilev force-pushed the add-c-api2 branch from bc67a64 to 573a6a8 Compare October 28, 2024 20:45

Add libclang-style C API

7be80d8

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

mcbarton force-pushed the add-c-api2 branch from 573a6a8 to 7be80d8 Compare October 28, 2024 20:58

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

vgvassilev Oct 27, 2024

github-actions bot left a comment

github-actions bot Oct 27, 2024

github-actions bot Oct 27, 2024

vgvassilev commented Oct 29, 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);


		void dispose_string(CXString string);

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

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

vgvassilev Oct 27, 2024

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

vgvassilev commented Oct 29, 2024

codecov bot commented Oct 27, 2024 •

edited

Loading