Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Improve template instantiation for GetMethodTemplate #326

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

aaronj0
Copy link
Collaborator

@aaronj0 aaronj0 commented Sep 20, 2024

No description provided.

Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@vgvassilev
Copy link
Contributor

I suspect this needs tests. Otherwise lgtm.

Copy link

codecov bot commented Sep 22, 2024

Codecov Report

Attention: Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.

Project coverage is 74.30%. Comparing base (12e6e69) to head (8315f8c).
Report is 4 commits behind head on main.

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

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #326      +/-   ##
==========================================
+ Coverage   73.71%   74.30%   +0.58%     
==========================================
  Files           8        8              
  Lines        3021     3191     +170     
==========================================
+ Hits         2227     2371     +144     
- Misses        794      820      +26     
Files with missing lines Coverage Δ
lib/Interpreter/CppInterOp.cpp 79.86% <85.71%> (+0.29%) ⬆️

... and 6 files with indirect coverage changes

Files with missing lines Coverage Δ
lib/Interpreter/CppInterOp.cpp 79.86% <85.71%> (+0.29%) ⬆️

... and 6 files with indirect coverage changes

@aaronj0
Copy link
Collaborator Author

aaronj0 commented Sep 25, 2024

I suspect this needs tests. Otherwise lgtm.

@vgvassilev This allows the calls like cppyy.ll.cast["float"](1) to succeed, and enables 2 tests on all platforms that can be seen in the CI. A unittest has also been added.

Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

Copy link
Contributor

@vgvassilev vgvassilev left a comment

Choose a reason for hiding this comment

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

Lgtm!

@aaronj0 aaronj0 merged commit f59f2cf into compiler-research:main Sep 26, 2024
59 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants