Skip to content

Conversation

@Vipul-Cariappa
Copy link
Collaborator

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

@Vipul-Cariappa 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
@codecov
Copy link

codecov bot commented Sep 5, 2025

Codecov Report

❌ Patch coverage is 18.46154% with 53 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.99%. Comparing base (2df83a9) to head (5e53412).

Files with missing lines Patch % Lines
lib/CppInterOp/CppInterOp.cpp 18.46% 53 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #708      +/-   ##
==========================================
- Coverage   79.04%   77.99%   -1.05%     
==========================================
  Files           9        9              
  Lines        3879     3940      +61     
==========================================
+ Hits         3066     3073       +7     
- Misses        813      867      +54     
Files with missing lines Coverage Δ
include/CppInterOp/CppInterOp.h 95.12% <ø> (ø)
lib/CppInterOp/CppInterOp.cpp 85.53% <18.46%> (-2.08%) ⬇️
Files with missing lines Coverage Δ
include/CppInterOp/CppInterOp.h 95.12% <ø> (ø)
lib/CppInterOp/CppInterOp.cpp 85.53% <18.46%> (-2.08%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@github-actions github-actions bot left a comment

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 14. Check the log or trigger a new build to see more.


#include "clang/AST/Attrs.inc"
#include "clang/AST/Comment.h"
#include "clang/AST/CXXInheritance.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

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"

// 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;
Copy link
Contributor

Choose a reason for hiding this comment

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

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

    sInterpreters;
    ^

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

Choose a reason for hiding this comment

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

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

    sInterpreterASTMap;
    ^

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

Choose a reason for hiding this comment

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

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

static std::mutex InterpreterStackLock;
                  ^

}

bool IsTemplateClass(TCppScope_t handle) {
auto* D = (clang::Decl*)handle;
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 C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]

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

std::string GetDocString(TCppScope_t scope) {
auto *D = static_cast<Decl*>(scope);
auto &AST = getASTContext(D);
const clang::RawComment *Comment = AST.getRawCommentForAnyRedecl(D);
Copy link
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 "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

}

auto* ND = Cpp_utils::Lookup::Named(&getSema(), name, Within);
auto* ND = Cpp_utils::Lookup::Named(&getSema(D), name, Within);
Copy link
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 "Cpp::utils::Lookup::Named" is directly included [misc-include-cleaner]

lib/CppInterOp/CppInterOp.cpp:12:

+ #include "CppInterOpInterpreter.h"

if (!ns)
return;

auto* D = (clang::Decl*)ns;
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 C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]

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

// the template function exists and >1 means overloads
bool ExistsFunctionTemplate(const std::string& name, TCppScope_t parent) {
DeclContext* Within = 0;
auto* D = (Decl*)parent;
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 C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]

  auto* D = (Decl*)parent;
            ^


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

Choose a reason for hiding this comment

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

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)) {

Copy link
Contributor

@github-actions github-actions bot left a comment

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

if (!ns)
return;

auto* D = (clang::Decl*)ns;
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 C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]

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

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