Enable template features by aaronj0 · Pull Request #215 · compiler-research/CppInterOp

aaronj0 · 2024-04-03T16:25:10Z

This PR consolidates interfaces in CppInterOp that allows:

Exposes information required for template function scope proxy creation - arg types, method counts, names
Perform template function lookups using clang::LookupResult
Updates existing interfaces to also handle FunctionTemplateDecl*
Improves the instantiation process
Enables auto instantiation and a basic overload selection mechanism in BestTemplateFunctionMatch

github-actions

clang-tidy made some suggestions

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

include/clang/Interpreter/CppInterOp.h

github-actions · 2024-04-03T16:31:03Z

lib/Interpreter/CppInterOp.cpp

+  std::vector<TCppFunction_t> GetTemplatedFuncs(TCppScope_t klass)
+  {
+
+    if (!klass)


warning: implicit conversion 'Cpp::TCppScope_t' (aka 'void *') -> bool [readability-implicit-bool-conversion]

Suggested change

if (!klass)

if (klass == nullptr)

Can you disable that clang-tidy check?

lib/Interpreter/CppInterOp.cpp

vgvassilev · 2024-04-03T17:21:21Z

@maximusron, here is how we can apply git-clang-format on multiple commits: vgvassilev/clad#847

Can you update the CI action with that here too?

aaronj0 · 2024-04-03T17:26:38Z

Oh yeah I will do that, was wondering why the clang-format was throwing errors

lib/Interpreter/CppInterOp.cpp

include/clang/Interpreter/CppInterOp.h

lib/Interpreter/CppInterOp.cpp

include/clang/Interpreter/CppInterOp.h

github-actions

clang-tidy made some suggestions

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

include/clang/Interpreter/CppInterOp.h

lib/Interpreter/CppInterOp.cpp

github-actions

clang-tidy made some suggestions

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

lib/Interpreter/CppInterOp.cpp

unittests/CppInterOp/FunctionReflectionTest.cpp

lib/Interpreter/CppInterOp.cpp

github-actions

clang-tidy made some suggestions

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

unittests/CppInterOp/FunctionReflectionTest.cpp

github-actions

clang-tidy made some suggestions

lib/Interpreter/CppInterOp.cpp

unittests/CppInterOp/ScopeReflectionTest.cpp

lib/Interpreter/CppInterOp.cpp

vgvassilev

Can you rebase this PR?

lib/Interpreter/CppInterOp.cpp

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.

lib/Interpreter/CppInterOp.cpp

unittests/CppInterOp/FunctionReflectionTest.cpp

github-actions

clang-tidy made some suggestions

lib/Interpreter/CppInterOp.cpp

aaronj0 · 2024-04-07T18:28:25Z

@vgvassilev the builds are failing because cppyy-backend has not been updated to the new usage of GetClassMethods where we have migrated to passing the result by reference as an out parameter.

compiler-research/cppyy-backend#93

vgvassilev

Here is another round of review.

include/clang/Interpreter/CppInterOp.h

lib/Interpreter/CppInterOp.cpp

include/clang/Interpreter/CppInterOp.h

lib/Interpreter/CppInterOp.cpp

include/clang/Interpreter/CppInterOp.h

lib/Interpreter/CppInterOp.cpp

github-actions

clang-tidy made some suggestions

lib/Interpreter/CppInterOp.cpp

vgvassilev

This is looking good...

include/clang/Interpreter/CppInterOp.h

vgvassilev · 2024-04-08T13:18:41Z

lib/Interpreter/CppInterOp.cpp

-  std::vector<TCppFunction_t> GetClassMethods(TCppScope_t klass)
-  {
-
+  template <typename DeclType>


We do not need a template, we can take a DeclKind as a parameter.

lib/Interpreter/CppInterOp.cpp

vgvassilev

Can you rebase?

lib/Interpreter/CppInterOp.cpp

unittests/CppInterOp/FunctionReflectionTest.cpp

github-actions

clang-tidy made some suggestions

unittests/CppInterOp/FunctionReflectionTest.cpp

vgvassilev

LGTM!

vgvassilev · 2024-04-12T13:35:08Z

The clang-format report seems bogus. This change requires changes to cppyy and that's why the other tests failed.

github-actions bot reviewed Apr 3, 2024

View reviewed changes

aaronj0 marked this pull request as draft April 3, 2024 16:57

aaronj0 marked this pull request as ready for review April 3, 2024 17:18

vgvassilev reviewed Apr 3, 2024

View reviewed changes

github-actions bot reviewed Apr 3, 2024

View reviewed changes

vgvassilev reviewed Apr 3, 2024

View reviewed changes

lib/Interpreter/CppInterOp.cpp Outdated Show resolved Hide resolved

github-actions bot reviewed Apr 4, 2024

View reviewed changes

lib/Interpreter/CppInterOp.cpp Outdated Show resolved Hide resolved

lib/Interpreter/CppInterOp.cpp Outdated Show resolved Hide resolved

unittests/CppInterOp/ScopeReflectionTest.cpp Show resolved Hide resolved

vgvassilev reviewed Apr 4, 2024

View reviewed changes

lib/Interpreter/CppInterOp.cpp Outdated Show resolved Hide resolved

lib/Interpreter/CppInterOp.cpp Outdated Show resolved Hide resolved

lib/Interpreter/CppInterOp.cpp Outdated Show resolved Hide resolved

lib/Interpreter/CppInterOp.cpp Show resolved Hide resolved

aaronj0 mentioned this pull request Apr 4, 2024

Consolidate the template instantiation logic. #128

Merged

vgvassilev reviewed Apr 7, 2024

View reviewed changes

lib/Interpreter/CppInterOp.cpp Outdated Show resolved Hide resolved

github-actions bot reviewed Apr 7, 2024

View reviewed changes

aaronj0 force-pushed the template-fix branch from 7d810f9 to 37fabc1 Compare April 7, 2024 18:09

github-actions bot reviewed Apr 7, 2024

View reviewed changes

lib/Interpreter/CppInterOp.cpp Outdated Show resolved Hide resolved

lib/Interpreter/CppInterOp.cpp Outdated Show resolved Hide resolved

lib/Interpreter/CppInterOp.cpp Outdated Show resolved Hide resolved

lib/Interpreter/CppInterOp.cpp Outdated Show resolved Hide resolved

vgvassilev reviewed Apr 7, 2024

View reviewed changes

github-actions bot reviewed Apr 8, 2024

View reviewed changes

lib/Interpreter/CppInterOp.cpp Show resolved Hide resolved

aaronj0 force-pushed the template-fix branch from 636775e to 9f1b696 Compare April 8, 2024 12:43

vgvassilev reviewed Apr 8, 2024

View reviewed changes

aaronj0 force-pushed the template-fix branch from d5ea0d5 to eb39402 Compare April 9, 2024 11:34

vgvassilev reviewed Apr 10, 2024

View reviewed changes

lib/Interpreter/CppInterOp.cpp Outdated Show resolved Hide resolved

unittests/CppInterOp/FunctionReflectionTest.cpp Outdated Show resolved Hide resolved

github-actions bot reviewed Apr 10, 2024

View reviewed changes

unittests/CppInterOp/FunctionReflectionTest.cpp Show resolved Hide resolved

aaronj0 force-pushed the template-fix branch 2 times, most recently from fd89d8f to 2c0e719 Compare April 10, 2024 12:43

aaronj0 added 3 commits April 12, 2024 13:29

Add interfaces enabling template function logic

c565a2a

Update interfaces to pass return by ref(out param)

84fd0a6

Remove redundant interfaces

5e8f4d5

aaronj0 added 5 commits April 12, 2024 13:29

Check scope before casting to CXXRecordDecl

5ce073e

Update tests to use new GetClassMethods interface

0f4e8ea

Remove unused filter param

488ef5f

Refactoring, move common code to static function

423b3e1

Update function documentation and tests

22c1e0d

aaronj0 force-pushed the template-fix branch from 2c0e719 to 22c1e0d Compare April 12, 2024 11:30

vgvassilev approved these changes Apr 12, 2024

View reviewed changes

vgvassilev merged commit 1e4b64f into compiler-research:main Apr 12, 2024

This was referenced Apr 12, 2024

Template fix patch based on updated InterOp API compiler-research/cppyy-backend#93

Merged

[WIP]Function interfaces don't work with template function decl #210

Closed

aaronj0 mentioned this pull request Apr 28, 2024

[WIP] Converters not created for templated method arguments #211

Closed

Conversation

aaronj0 commented Apr 3, 2024

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot Apr 3, 2024

Choose a reason for hiding this comment

Uh oh!

vgvassilev Apr 3, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vgvassilev commented Apr 3, 2024

Uh oh!

aaronj0 commented Apr 3, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!