[WIP] Things required to get Python Type-Hints to work with cppyy #708

Vipul-Cariappa · 2025-09-05T10:04:31Z

Description

Please include a summary of changes, motivation and context for this PR.

Fixes # (issue)

Type of change

Please tick all options which are relevant.

Bug fix
New feature
Requires documentation updates

Testing

Please describe the test(s) that you added and ran to verify your changes.

Checklist

I have read the contribution guide recently

We need to identify which interpreter a Decl belongs to, when using multiple interpreter. We do it by checking which `clang::ASTContext` the `clang::Decl` belongs We maintain a map: `clang::ASTContext -> Cpp::InterpreterInfo`. Using this map, be identify the correct interpreter. There are 2 usecases for this: 1. We can now lock the correct interpreter making it thread safe. 2. User of `libCppInterOp` need not set the correct active interpreter using `Cpp::ActivateInterpreter`, this information can be retrived using the map.

codecov · 2025-09-05T10:12:18Z

Codecov Report

❌ Patch coverage is 73.30827% with 71 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.17%. Comparing base (d85eec0) to head (57d3266).

Files with missing lines	Patch %	Lines
lib/CppInterOp/CppInterOp.cpp	73.30%	71 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #708      +/-   ##
==========================================
- Coverage   79.83%   79.17%   -0.66%     
==========================================
  Files           9        9              
  Lines        3962     4178     +216     
==========================================
+ Hits         3163     3308     +145     
- Misses        799      870      +71

Files with missing lines	Coverage Δ
include/CppInterOp/CppInterOp.h	`95.55% <ø> (ø)`
lib/CppInterOp/CppInterOp.cpp	`86.20% <73.30%> (-1.84%)`	⬇️

Files with missing lines	Coverage Δ
include/CppInterOp/CppInterOp.h	`95.55% <ø> (ø)`
lib/CppInterOp/CppInterOp.cpp	`86.20% <73.30%> (-1.84%)`	⬇️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions

clang-tidy made some suggestions

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

github-actions · 2025-09-05T10:29:43Z

lib/CppInterOp/CppInterOp.cpp

@@ -12,6 +12,7 @@
 #include "Compatibility.h"

 #include "clang/AST/Attrs.inc"
+#include "clang/AST/Comment.h"
 #include "clang/AST/CXXInheritance.h"


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

Suggested change

#include "clang/AST/CXXInheritance.h"

#include "clang/AST/CXXInheritance.h"

#include "clang/AST/Comment.h"

github-actions · 2025-09-05T10:29:43Z

lib/CppInterOp/CppInterOp.cpp

@@ -130,15 +140,54 @@
 };

 // std::deque avoids relocations and calling the dtor of InterpreterInfo.
-static llvm::ManagedStatic<std::deque<InterpreterInfo>> sInterpreters;
+static llvm::ManagedStatic<std::deque<std::shared_ptr<InterpreterInfo>>>
+    sInterpreters;


warning: variable 'sInterpreters' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

sInterpreters; ^

github-actions · 2025-09-05T10:29:43Z

lib/CppInterOp/CppInterOp.cpp

+    sInterpreters;
+static llvm::ManagedStatic<
+    std::unordered_map<clang::ASTContext*, std::weak_ptr<InterpreterInfo>>>
+    sInterpreterASTMap;


warning: variable 'sInterpreterASTMap' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

sInterpreterASTMap; ^

github-actions · 2025-09-05T10:29:43Z

lib/CppInterOp/CppInterOp.cpp

+static llvm::ManagedStatic<
+    std::unordered_map<clang::ASTContext*, std::weak_ptr<InterpreterInfo>>>
+    sInterpreterASTMap;
+static std::mutex InterpreterStackLock;


warning: variable 'InterpreterStackLock' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

static std::mutex InterpreterStackLock; ^

github-actions · 2025-09-05T10:29:43Z

lib/CppInterOp/CppInterOp.cpp

@@ -375,6 +438,11 @@
  return llvm::StringRef(Ty.getAsString()).contains("complex");
 }

+bool IsTemplateClass(TCppScope_t handle) {
+  auto* D = (clang::Decl*)handle;


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

auto* D = (clang::Decl*)handle; ^

github-actions · 2025-09-05T10:29:44Z

lib/CppInterOp/CppInterOp.cpp

+std::string GetDocString(TCppScope_t scope) {
+  auto *D = static_cast<Decl*>(scope);
+  auto &AST = getASTContext(D);
+  const clang::RawComment *Comment = AST.getRawCommentForAnyRedecl(D);


warning: no header providing "clang::RawComment" is directly included [misc-include-cleaner]

lib/CppInterOp/CppInterOp.cpp:42:

- #if CLANG_VERSION_MAJOR >= 19 + #include <clang/AST/RawCommentList.h> + #if CLANG_VERSION_MAJOR >= 19

github-actions · 2025-09-05T10:29:44Z

lib/CppInterOp/CppInterOp.cpp

    D = GetUnderlyingScope(D);
    Within = llvm::dyn_cast<clang::DeclContext>(D);
  }

-  auto* ND = Cpp_utils::Lookup::Named(&getSema(), name, Within);
+  auto* ND = Cpp_utils::Lookup::Named(&getSema(D), name, Within);


warning: no header providing "Cpp::utils::Lookup::Named" is directly included [misc-include-cleaner]

lib/CppInterOp/CppInterOp.cpp:12:

+ #include "CppInterOpInterpreter.h"

github-actions · 2025-09-05T10:29:44Z

lib/CppInterOp/CppInterOp.cpp

+  if (!ns)
+    return;
+
+  auto* D = (clang::Decl*)ns;


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

auto* D = (clang::Decl*)ns; ^

github-actions · 2025-09-05T10:29:44Z

lib/CppInterOp/CppInterOp.cpp

 }

 // FIXME: This lookup is broken, and should no longer be used in favour of
 // `GetClassTemplatedMethods` If the candidate set returned is =1, that means
 // the template function exists and >1 means overloads
 bool ExistsFunctionTemplate(const std::string& name, TCppScope_t parent) {
  DeclContext* Within = 0;
+  auto* D = (Decl*)parent;


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

auto* D = (Decl*)parent; ^

github-actions · 2025-09-05T10:29:44Z

lib/CppInterOp/CppInterOp.cpp

@@ -1329,7 +1503,11 @@

 bool IsStaticMethod(TCppConstFunction_t method) {
  const auto* D = static_cast<const Decl*>(method);
+  if (auto *FTD = llvm::dyn_cast_or_null<FunctionTemplateDecl>(D)) {


warning: 'auto *FTD' can be declared as 'const auto *FTD' [readability-qualified-auto]

Suggested change

if (auto *FTD = llvm::dyn_cast_or_null<FunctionTemplateDecl>(D)) {

if (const auto *FTD = llvm::dyn_cast_or_null<FunctionTemplateDecl>(D)) {

Vipul-Cariappa added 4 commits September 5, 2025 09:26

Thread-Safty - Add Mutex per Interpreter

d0ed0aa

WIP

0e2e57d

[WIP] extract comments

57d3266

Vipul-Cariappa changed the title ~~Things required to get Python Type-Hints to work with cppyy~~ [WIP] Things required to get Python Type-Hints to work with cppyy Sep 5, 2025

github-actions bot reviewed Sep 5, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[WIP] Things required to get Python Type-Hints to work with cppyy #708

[WIP] Things required to get Python Type-Hints to work with cppyy #708

Vipul-Cariappa commented Sep 5, 2025

Uh oh!

codecov bot commented Sep 5, 2025

Uh oh!

github-actions bot left a comment

Uh oh!

github-actions bot Sep 5, 2025

Uh oh!

github-actions bot Sep 5, 2025

Uh oh!

github-actions bot Sep 5, 2025

Uh oh!

github-actions bot Sep 5, 2025

Uh oh!

github-actions bot Sep 5, 2025

Uh oh!

github-actions bot Sep 5, 2025

Uh oh!

github-actions bot Sep 5, 2025

Uh oh!

github-actions bot Sep 5, 2025

Uh oh!

github-actions bot Sep 5, 2025

Uh oh!

github-actions bot Sep 5, 2025

Uh oh!

Uh oh!

	#include "clang/AST/CXXInheritance.h"
	#include "clang/AST/CXXInheritance.h"
	#include "clang/AST/Comment.h"

	if (auto *FTD = llvm::dyn_cast_or_null<FunctionTemplateDecl>(D)) {
	if (const auto *FTD = llvm::dyn_cast_or_null<FunctionTemplateDecl>(D)) {

[WIP] Things required to get Python Type-Hints to work with cppyy #708

Are you sure you want to change the base?

[WIP] Things required to get Python Type-Hints to work with cppyy #708

Conversation

Vipul-Cariappa commented Sep 5, 2025

Description

Type of change

Testing

Checklist

Uh oh!

codecov bot commented Sep 5, 2025

Codecov Report

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!