Add `BestOverloadFunctionMatch` & `IsFunction` Remove `BestTemplateFunctionMatch` #514

Vipul-Cariappa · 2025-03-05T12:41:37Z

Description

Resolve and instantiate (if templated) overload from vector of overloads for the given template parameter types and argument types.
With these changes, we replicate clang's overload resolution and template instantiation.

Fixes # (issue)

Fixes 10 test cases in cppyy.
Can close #352 and #511.

Type of change

Please tick all options which are relevant.

Bug fix
New feature
Requires documentation updates

Testing

Included tests

Checklist

I have read the contribution guide recently

github-actions

clang-tidy made some suggestions

github-actions · 2025-03-05T13:04:21Z

lib/Interpreter/CppInterOp.cpp

+      ExprValueKind ExprKind = ExprValueKind::VK_PRValue;
+      if (Type->isReferenceType())
+        ExprKind = ExprValueKind::VK_LValue;
+      Args.push_back(new (getSema().getASTContext())


warning: initializing non-owner argument of type 'ValueParamT' (aka 'clang::Expr *') with a newly created 'gsl::owner<>' [cppcoreguidelines-owning-memory]

Args.push_back(new (getSema().getASTContext()) ^

codecov · 2025-03-05T13:14:07Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.71%. Comparing base (16c67f9) to head (2b81002).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #514      +/-   ##
==========================================
+ Coverage   72.45%   72.71%   +0.26%     
==========================================
  Files           9        9              
  Lines        3591     3618      +27     
==========================================
+ Hits         2602     2631      +29     
+ Misses        989      987       -2

Files with missing lines	Coverage Δ
include/clang/Interpreter/CppInterOp.h	`96.29% <ø> (ø)`
lib/Interpreter/CppInterOp.cpp	`83.00% <100.00%> (+0.34%)`	⬆️

Files with missing lines	Coverage Δ
include/clang/Interpreter/CppInterOp.h	`96.29% <ø> (ø)`
lib/Interpreter/CppInterOp.cpp	`83.00% <100.00%> (+0.34%)`	⬆️

🚀 New features to boost your workflow:

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

from compiler-research/CppInterOp#514

lib/Interpreter/CppInterOp.cpp

vgvassilev · 2025-03-06T08:39:25Z

lib/Interpreter/CppInterOp.cpp

+      ExprValueKind ExprKind = ExprValueKind::VK_PRValue;
+      if (Type->isReferenceType())
+        ExprKind = ExprValueKind::VK_LValue;
+      Args.push_back(new (getSema().getASTContext())


Construct the Expr on the stack and pass it as a reference. This way if we start using something outside of the types and expression values valgrind will be able to tell us (upon future changes of the implementation in clang). To do this you will need an std::vector<Expr> Exprs with the same lifetime as Args and Args taking Exprs addresses elementwise.

Exprs.push_back fails. I guess it is because move constructor is deleted. Error message:

[ 68%] Building CXX object lib/Interpreter/CMakeFiles/clangCppInterOp.dir/CppInterOp.cpp.o In file included from /usr/include/c++/14.2.1/x86_64-pc-linux-gnu/bits/c++allocator.h:33, from /usr/include/c++/14.2.1/bits/allocator.h:46, from /usr/include/c++/14.2.1/bits/stl_tree.h:64, from /usr/include/c++/14.2.1/set:62, from /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/CppInterOp/include/clang/Interpreter/CppInterOp.h:15, from /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/CppInterOp/lib/Interpreter/CppInterOp.cpp:10: /usr/include/c++/14.2.1/bits/new_allocator.h: In instantiation of ‘void std::__new_allocator<_Tp>::construct(_Up*, _Args&& ...) [with _Up = clang::OpaqueValueExpr; _Args = {clang::OpaqueValueExpr}; _Tp = clang::OpaqueValueExpr]’: /usr/include/c++/14.2.1/bits/alloc_traits.h:575:17: required from ‘static void std::allocator_traits<std::allocator<_Tp1> >::construct(allocator_type&, _Up*, _Args&& ...) [with _Up = clang::OpaqueValueExpr; _Args = {clang::OpaqueValueExpr}; _Tp = clang::OpaqueValueExpr; allocator_type = std::allocator<clang::OpaqueValueExpr>]’ 575 | __a.construct(__p, std::forward<_Args>(__args)...); | ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/vector.tcc:117:30: required from ‘std::vector<_Tp, _Alloc>::reference std::vector<_Tp, _Alloc>::emplace_back(_Args&& ...) [with _Args = {clang::OpaqueValueExpr}; _Tp = clang::OpaqueValueExpr; _Alloc = std::allocator<clang::OpaqueValueExpr>; reference = clang::OpaqueValueExpr&]’ 117 | _Alloc_traits::construct(this->_M_impl, this->_M_impl._M_finish, | ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 118 | std::forward<_Args>(__args)...); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/stl_vector.h:1301:21: required from ‘void std::vector<_Tp, _Alloc>::push_back(value_type&&) [with _Tp = clang::OpaqueValueExpr; _Alloc = std::allocator<clang::OpaqueValueExpr>; value_type = clang::OpaqueValueExpr]’ 1301 | { emplace_back(std::move(__x)); } | ~~~~~~~~~~~~^~~~~~~~~~~~~~~~ /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/CppInterOp/lib/Interpreter/CppInterOp.cpp:1082:22: required from here 1082 | Exprs.push_back(OpaqueValueExpr(SourceLocation::getFromRawEncoding(1), Type.getNonReferenceType(), ExprKind)); | ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/new_allocator.h:191:11: error: use of deleted function ‘clang::OpaqueValueExpr::OpaqueValueExpr(clang::OpaqueValueExpr&&)’ 191 | { ::new((void *)__p) _Up(std::forward<_Args>(__args)...); } | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In file included from /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/llvm-project/clang/include/clang/AST/DeclCXX.h:22, from /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/llvm-project/clang/include/clang/AST/GlobalDecl.h:18, from /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/CppInterOp/lib/Interpreter/Compatibility.h:8, from /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/CppInterOp/lib/Interpreter/CppInterOp.cpp:12: /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/llvm-project/clang/include/clang/AST/Expr.h:1173:7: note: ‘clang::OpaqueValueExpr::OpaqueValueExpr(clang::OpaqueValueExpr&&)’ is implicitly deleted because the default definition would be ill-formed: 1173 | class OpaqueValueExpr : public Expr { | ^~~~~~~~~~~~~~~ /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/llvm-project/clang/include/clang/AST/Expr.h:1173:7: error: use of deleted function ‘clang::Expr::Expr(clang::Expr&&)’ /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/llvm-project/clang/include/clang/AST/Expr.h:116:3: note: declared here 116 | Expr(Expr &&) = delete; | ^~~~ /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/llvm-project/clang/include/clang/AST/Expr.h:1173:7: note: use ‘-fdiagnostics-all-candidates’ to display considered candidates 1173 | class OpaqueValueExpr : public Expr { | ^~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/new_allocator.h:191:11: note: use ‘-fdiagnostics-all-candidates’ to display considered candidates 191 | { ::new((void *)__p) _Up(std::forward<_Args>(__args)...); } | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In file included from /usr/include/c++/14.2.1/vector:65, from /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/CppInterOp/include/clang/Interpreter/CppInterOp.h:17: /usr/include/c++/14.2.1/bits/stl_uninitialized.h: In instantiation of ‘constexpr bool std::__check_constructible() [with _ValueType = clang::OpaqueValueExpr; _Tp = clang::OpaqueValueExpr&&]’: /usr/include/c++/14.2.1/bits/stl_uninitialized.h:182:4: required from ‘_ForwardIterator std::uninitialized_copy(_InputIterator, _InputIterator, _ForwardIterator) [with _InputIterator = move_iterator<clang::OpaqueValueExpr*>; _ForwardIterator = clang::OpaqueValueExpr*]’ 182 | = _GLIBCXX_USE_ASSIGN_FOR_INIT(_ValueType2, _From); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/stl_uninitialized.h:373:37: required from ‘_ForwardIterator std::__uninitialized_copy_a(_InputIterator, _InputIterator, _ForwardIterator, allocator<_Tp>&) [with _InputIterator = move_iterator<clang::OpaqueValueExpr*>; _ForwardIterator = clang::OpaqueValueExpr*; _Tp = clang::OpaqueValueExpr]’ 373 | return std::uninitialized_copy(__first, __last, __result); | ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/stl_uninitialized.h:399:2: required from ‘_ForwardIterator std::__uninitialized_move_if_noexcept_a(_InputIterator, _InputIterator, _ForwardIterator, _Allocator&) [with _InputIterator = clang::OpaqueValueExpr*; _ForwardIterator = clang::OpaqueValueExpr*; _Allocator = allocator<clang::OpaqueValueExpr>]’ 398 | return std::__uninitialized_copy_a | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ 399 | (_GLIBCXX_MAKE_MOVE_IF_NOEXCEPT_ITERATOR(__first), | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 400 | _GLIBCXX_MAKE_MOVE_IF_NOEXCEPT_ITERATOR(__last), __result, __alloc); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/vector.tcc:676:60: required from ‘void std::vector<_Tp, _Alloc>::_M_realloc_append(_Args&& ...) [with _Args = {clang::OpaqueValueExpr}; _Tp = clang::OpaqueValueExpr; _Alloc = std::allocator<clang::OpaqueValueExpr>]’ 676 | __new_finish = std::__uninitialized_move_if_noexcept_a( | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^ 677 | __old_start, __old_finish, | ~~~~~~~~~~~~~~~~~~~~~~~~~~ 678 | __new_start, _M_get_Tp_allocator()); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/vector.tcc:123:21: required from ‘std::vector<_Tp, _Alloc>::reference std::vector<_Tp, _Alloc>::emplace_back(_Args&& ...) [with _Args = {clang::OpaqueValueExpr}; _Tp = clang::OpaqueValueExpr; _Alloc = std::allocator<clang::OpaqueValueExpr>; reference = clang::OpaqueValueExpr&]’ 123 | _M_realloc_append(std::forward<_Args>(__args)...); | ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/stl_vector.h:1301:21: required from ‘void std::vector<_Tp, _Alloc>::push_back(value_type&&) [with _Tp = clang::OpaqueValueExpr; _Alloc = std::allocator<clang::OpaqueValueExpr>; value_type = clang::OpaqueValueExpr]’ 1301 | { emplace_back(std::move(__x)); } | ~~~~~~~~~~~~^~~~~~~~~~~~~~~~ /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/CppInterOp/lib/Interpreter/CppInterOp.cpp:1082:22: required from here 1082 | Exprs.push_back(OpaqueValueExpr(SourceLocation::getFromRawEncoding(1), Type.getNonReferenceType(), ExprKind)); | ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/stl_uninitialized.h:90:56: error: static assertion failed: result type must be constructible from input type 90 | static_assert(is_constructible<_ValueType, _Tp>::value, | ^~~~~ /usr/include/c++/14.2.1/bits/stl_uninitialized.h:90:56: note: ‘std::integral_constant<bool, false>::value’ evaluates to false make[2]: *** [lib/Interpreter/CMakeFiles/clangCppInterOp.dir/build.make:79: lib/Interpreter/CMakeFiles/clangCppInterOp.dir/CppInterOp.cpp.o] Error 1 make[1]: *** [CMakeFiles/Makefile2:426: lib/Interpreter/CMakeFiles/clangCppInterOp.dir/all] Error 2 make: *** [Makefile:136: all] Error 2

Similarly, Exprs.emplace_back also fails with the following error message.

[ 68%] Building CXX object lib/Interpreter/CMakeFiles/clangCppInterOp.dir/CppInterOp.cpp.o In file included from /usr/include/c++/14.2.1/vector:65, from /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/CppInterOp/include/clang/Interpreter/CppInterOp.h:17, from /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/CppInterOp/lib/Interpreter/CppInterOp.cpp:10: /usr/include/c++/14.2.1/bits/stl_uninitialized.h: In instantiation of ‘constexpr bool std::__check_constructible() [with _ValueType = clang::OpaqueValueExpr; _Tp = clang::OpaqueValueExpr&&]’: /usr/include/c++/14.2.1/bits/stl_uninitialized.h:182:4: required from ‘_ForwardIterator std::uninitialized_copy(_InputIterator, _InputIterator, _ForwardIterator) [with _InputIterator = move_iterator<clang::OpaqueValueExpr*>; _ForwardIterator = clang::OpaqueValueExpr*]’ 182 | = _GLIBCXX_USE_ASSIGN_FOR_INIT(_ValueType2, _From); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/stl_uninitialized.h:373:37: required from ‘_ForwardIterator std::__uninitialized_copy_a(_InputIterator, _InputIterator, _ForwardIterator, allocator<_Tp>&) [with _InputIterator = move_iterator<clang::OpaqueValueExpr*>; _ForwardIterator = clang::OpaqueValueExpr*; _Tp = clang::OpaqueValueExpr]’ 373 | return std::uninitialized_copy(__first, __last, __result); | ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/stl_uninitialized.h:399:2: required from ‘_ForwardIterator std::__uninitialized_move_if_noexcept_a(_InputIterator, _InputIterator, _ForwardIterator, _Allocator&) [with _InputIterator = clang::OpaqueValueExpr*; _ForwardIterator = clang::OpaqueValueExpr*; _Allocator = allocator<clang::OpaqueValueExpr>]’ 398 | return std::__uninitialized_copy_a | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ 399 | (_GLIBCXX_MAKE_MOVE_IF_NOEXCEPT_ITERATOR(__first), | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 400 | _GLIBCXX_MAKE_MOVE_IF_NOEXCEPT_ITERATOR(__last), __result, __alloc); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/vector.tcc:676:60: required from ‘void std::vector<_Tp, _Alloc>::_M_realloc_append(_Args&& ...) [with _Args = {clang::SourceLocation, clang::QualType, clang::ExprValueKind&}; _Tp = clang::OpaqueValueExpr; _Alloc = std::allocator<clang::OpaqueValueExpr>]’ 676 | __new_finish = std::__uninitialized_move_if_noexcept_a( | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^ 677 | __old_start, __old_finish, | ~~~~~~~~~~~~~~~~~~~~~~~~~~ 678 | __new_start, _M_get_Tp_allocator()); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/vector.tcc:123:21: required from ‘std::vector<_Tp, _Alloc>::reference std::vector<_Tp, _Alloc>::emplace_back(_Args&& ...) [with _Args = {clang::SourceLocation, clang::QualType, clang::ExprValueKind&}; _Tp = clang::OpaqueValueExpr; _Alloc = std::allocator<clang::OpaqueValueExpr>; reference = clang::OpaqueValueExpr&]’ 123 | _M_realloc_append(std::forward<_Args>(__args)...); | ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/CppInterOp/lib/Interpreter/CppInterOp.cpp:1082:25: required from here 1082 | Exprs.emplace_back(SourceLocation::getFromRawEncoding(1), Type.getNonReferenceType(), ExprKind); | ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/stl_uninitialized.h:90:56: error: static assertion failed: result type must be constructible from input type 90 | static_assert(is_constructible<_ValueType, _Tp>::value, | ^~~~~ /usr/include/c++/14.2.1/bits/stl_uninitialized.h:90:56: note: ‘std::integral_constant<bool, false>::value’ evaluates to false make[2]: *** [lib/Interpreter/CMakeFiles/clangCppInterOp.dir/build.make:79: lib/Interpreter/CMakeFiles/clangCppInterOp.dir/CppInterOp.cpp.o] Error 1 make[1]: *** [CMakeFiles/Makefile2:426: lib/Interpreter/CMakeFiles/clangCppInterOp.dir/all] Error 2

Do we need to specify an allocator provided by clang?

I seem to remember that we could create an expression on the stack but apparently I am confusing myself. We should probably put it in some scratch area, call placement new there and then destroy it...

Not sure how I would go about destroying it. The default delete operator is marked unreachable (reference: https://github.com/llvm/llvm-project/blob/release/19.x/clang/include/clang/AST/Stmt.h#L106-L109). And the delete operators defined are empty (reference: https://github.com/llvm/llvm-project/blob/release/19.x/clang/include/clang/AST/Stmt.h#L1289-L1293).
I can call into one of these delete operators if required?

vgvassilev · 2025-03-06T08:41:02Z

lib/Interpreter/CppInterOp.cpp

+        S.AddMethodCandidate(
+            MD, DeclAccessPair::make(MD, MD->getAccess()), MD->getParent(),
+            C.getTypeDeclType(MD->getParent()),
+            Expr::Classification::makeSimpleLValue(), Args, Overloads);
+      } else if (auto* FD = dyn_cast<FunctionDecl>(D)) {


AddOverloadCandidate already calls AddMethodCandidate if we got a CXXMethodDecl...

vgvassilev · 2025-03-06T08:43:28Z

lib/Interpreter/CppInterOp.cpp

+
+    OverloadCandidateSet Overloads(
+        SourceLocation(), OverloadCandidateSet::CandidateSetKind::CSK_Normal);
+    for (void* i : candidates) {


Why not using Sema::AddFunctionCandidates instead of this loop?

We will still require a loop to populate UnresolvedSetImpl from candidates. Also, when trying it locally, the following assertion fails https://github.com/llvm/llvm-project/blob/release/19.x/clang/lib/Sema/SemaOverload.cpp#L5692 for one of the test cases.

Maybe that assertion is coming from an interface on our end that needs fixing?

We will still require a loop to populate UnresolvedSetImpl from candidates.

It seems to have an append operator taking a range...

It seems to have an append operator taking a range...

The iterator is a UnresolvedSetIterator(reference: https://clang.llvm.org/doxygen/classclang_1_1UnresolvedSetImpl.html#ad1e4dde3cd343da5f9918eb0caedd0a7)

Maybe that assertion is coming from an interface on our end that needs fixing?

I have been debugging this. I am not able to detect the cause. Our interfaces are being used correctly, as far as I can see.

vgvassilev · 2025-03-06T08:49:17Z

lib/Interpreter/CppInterOp.cpp

@@ -1027,61 +1056,78 @@ namespace Cpp {
  }

  TCppFunction_t
-  BestTemplateFunctionMatch(const std::vector<TCppFunction_t>& candidates,
+  BestOverloadFunctionMatch(const std::vector<TCppFunction_t>& candidates,


This function feels very similar to Sema::BuildCallExpr can we use that instead and unwrap the result from the call expression? I think the cost would be an AST node allocation per call but we currently do that anyway...

I will recommend against it. We will need to create a C++ function call expression for it to work.
I did adapt this function from Sema::BuildCallExpr.

Yes, it will but we do that anyway. If you prefer not to do it, then write a comment that this is copied from there. FWIW, that did not seem the case when I looked...

Vipul-Cariappa

I will make the necessary changes in my next commit.
For the comments where I have not replied.

Vipul-Cariappa · 2025-03-06T09:57:43Z

lib/Interpreter/CppInterOp.cpp

+      ExprValueKind ExprKind = ExprValueKind::VK_PRValue;
+      if (Type->isReferenceType())
+        ExprKind = ExprValueKind::VK_LValue;
+      Args.push_back(new (getSema().getASTContext())


Exprs.push_back fails. I guess it is because move constructor is deleted. Error message:

[ 68%] Building CXX object lib/Interpreter/CMakeFiles/clangCppInterOp.dir/CppInterOp.cpp.o In file included from /usr/include/c++/14.2.1/x86_64-pc-linux-gnu/bits/c++allocator.h:33, from /usr/include/c++/14.2.1/bits/allocator.h:46, from /usr/include/c++/14.2.1/bits/stl_tree.h:64, from /usr/include/c++/14.2.1/set:62, from /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/CppInterOp/include/clang/Interpreter/CppInterOp.h:15, from /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/CppInterOp/lib/Interpreter/CppInterOp.cpp:10: /usr/include/c++/14.2.1/bits/new_allocator.h: In instantiation of ‘void std::__new_allocator<_Tp>::construct(_Up*, _Args&& ...) [with _Up = clang::OpaqueValueExpr; _Args = {clang::OpaqueValueExpr}; _Tp = clang::OpaqueValueExpr]’: /usr/include/c++/14.2.1/bits/alloc_traits.h:575:17: required from ‘static void std::allocator_traits<std::allocator<_Tp1> >::construct(allocator_type&, _Up*, _Args&& ...) [with _Up = clang::OpaqueValueExpr; _Args = {clang::OpaqueValueExpr}; _Tp = clang::OpaqueValueExpr; allocator_type = std::allocator<clang::OpaqueValueExpr>]’ 575 | __a.construct(__p, std::forward<_Args>(__args)...); | ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/vector.tcc:117:30: required from ‘std::vector<_Tp, _Alloc>::reference std::vector<_Tp, _Alloc>::emplace_back(_Args&& ...) [with _Args = {clang::OpaqueValueExpr}; _Tp = clang::OpaqueValueExpr; _Alloc = std::allocator<clang::OpaqueValueExpr>; reference = clang::OpaqueValueExpr&]’ 117 | _Alloc_traits::construct(this->_M_impl, this->_M_impl._M_finish, | ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 118 | std::forward<_Args>(__args)...); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/stl_vector.h:1301:21: required from ‘void std::vector<_Tp, _Alloc>::push_back(value_type&&) [with _Tp = clang::OpaqueValueExpr; _Alloc = std::allocator<clang::OpaqueValueExpr>; value_type = clang::OpaqueValueExpr]’ 1301 | { emplace_back(std::move(__x)); } | ~~~~~~~~~~~~^~~~~~~~~~~~~~~~ /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/CppInterOp/lib/Interpreter/CppInterOp.cpp:1082:22: required from here 1082 | Exprs.push_back(OpaqueValueExpr(SourceLocation::getFromRawEncoding(1), Type.getNonReferenceType(), ExprKind)); | ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/new_allocator.h:191:11: error: use of deleted function ‘clang::OpaqueValueExpr::OpaqueValueExpr(clang::OpaqueValueExpr&&)’ 191 | { ::new((void *)__p) _Up(std::forward<_Args>(__args)...); } | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In file included from /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/llvm-project/clang/include/clang/AST/DeclCXX.h:22, from /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/llvm-project/clang/include/clang/AST/GlobalDecl.h:18, from /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/CppInterOp/lib/Interpreter/Compatibility.h:8, from /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/CppInterOp/lib/Interpreter/CppInterOp.cpp:12: /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/llvm-project/clang/include/clang/AST/Expr.h:1173:7: note: ‘clang::OpaqueValueExpr::OpaqueValueExpr(clang::OpaqueValueExpr&&)’ is implicitly deleted because the default definition would be ill-formed: 1173 | class OpaqueValueExpr : public Expr { | ^~~~~~~~~~~~~~~ /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/llvm-project/clang/include/clang/AST/Expr.h:1173:7: error: use of deleted function ‘clang::Expr::Expr(clang::Expr&&)’ /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/llvm-project/clang/include/clang/AST/Expr.h:116:3: note: declared here 116 | Expr(Expr &&) = delete; | ^~~~ /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/llvm-project/clang/include/clang/AST/Expr.h:1173:7: note: use ‘-fdiagnostics-all-candidates’ to display considered candidates 1173 | class OpaqueValueExpr : public Expr { | ^~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/new_allocator.h:191:11: note: use ‘-fdiagnostics-all-candidates’ to display considered candidates 191 | { ::new((void *)__p) _Up(std::forward<_Args>(__args)...); } | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In file included from /usr/include/c++/14.2.1/vector:65, from /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/CppInterOp/include/clang/Interpreter/CppInterOp.h:17: /usr/include/c++/14.2.1/bits/stl_uninitialized.h: In instantiation of ‘constexpr bool std::__check_constructible() [with _ValueType = clang::OpaqueValueExpr; _Tp = clang::OpaqueValueExpr&&]’: /usr/include/c++/14.2.1/bits/stl_uninitialized.h:182:4: required from ‘_ForwardIterator std::uninitialized_copy(_InputIterator, _InputIterator, _ForwardIterator) [with _InputIterator = move_iterator<clang::OpaqueValueExpr*>; _ForwardIterator = clang::OpaqueValueExpr*]’ 182 | = _GLIBCXX_USE_ASSIGN_FOR_INIT(_ValueType2, _From); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/stl_uninitialized.h:373:37: required from ‘_ForwardIterator std::__uninitialized_copy_a(_InputIterator, _InputIterator, _ForwardIterator, allocator<_Tp>&) [with _InputIterator = move_iterator<clang::OpaqueValueExpr*>; _ForwardIterator = clang::OpaqueValueExpr*; _Tp = clang::OpaqueValueExpr]’ 373 | return std::uninitialized_copy(__first, __last, __result); | ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/stl_uninitialized.h:399:2: required from ‘_ForwardIterator std::__uninitialized_move_if_noexcept_a(_InputIterator, _InputIterator, _ForwardIterator, _Allocator&) [with _InputIterator = clang::OpaqueValueExpr*; _ForwardIterator = clang::OpaqueValueExpr*; _Allocator = allocator<clang::OpaqueValueExpr>]’ 398 | return std::__uninitialized_copy_a | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ 399 | (_GLIBCXX_MAKE_MOVE_IF_NOEXCEPT_ITERATOR(__first), | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 400 | _GLIBCXX_MAKE_MOVE_IF_NOEXCEPT_ITERATOR(__last), __result, __alloc); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/vector.tcc:676:60: required from ‘void std::vector<_Tp, _Alloc>::_M_realloc_append(_Args&& ...) [with _Args = {clang::OpaqueValueExpr}; _Tp = clang::OpaqueValueExpr; _Alloc = std::allocator<clang::OpaqueValueExpr>]’ 676 | __new_finish = std::__uninitialized_move_if_noexcept_a( | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^ 677 | __old_start, __old_finish, | ~~~~~~~~~~~~~~~~~~~~~~~~~~ 678 | __new_start, _M_get_Tp_allocator()); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/vector.tcc:123:21: required from ‘std::vector<_Tp, _Alloc>::reference std::vector<_Tp, _Alloc>::emplace_back(_Args&& ...) [with _Args = {clang::OpaqueValueExpr}; _Tp = clang::OpaqueValueExpr; _Alloc = std::allocator<clang::OpaqueValueExpr>; reference = clang::OpaqueValueExpr&]’ 123 | _M_realloc_append(std::forward<_Args>(__args)...); | ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/stl_vector.h:1301:21: required from ‘void std::vector<_Tp, _Alloc>::push_back(value_type&&) [with _Tp = clang::OpaqueValueExpr; _Alloc = std::allocator<clang::OpaqueValueExpr>; value_type = clang::OpaqueValueExpr]’ 1301 | { emplace_back(std::move(__x)); } | ~~~~~~~~~~~~^~~~~~~~~~~~~~~~ /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/CppInterOp/lib/Interpreter/CppInterOp.cpp:1082:22: required from here 1082 | Exprs.push_back(OpaqueValueExpr(SourceLocation::getFromRawEncoding(1), Type.getNonReferenceType(), ExprKind)); | ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/stl_uninitialized.h:90:56: error: static assertion failed: result type must be constructible from input type 90 | static_assert(is_constructible<_ValueType, _Tp>::value, | ^~~~~ /usr/include/c++/14.2.1/bits/stl_uninitialized.h:90:56: note: ‘std::integral_constant<bool, false>::value’ evaluates to false make[2]: *** [lib/Interpreter/CMakeFiles/clangCppInterOp.dir/build.make:79: lib/Interpreter/CMakeFiles/clangCppInterOp.dir/CppInterOp.cpp.o] Error 1 make[1]: *** [CMakeFiles/Makefile2:426: lib/Interpreter/CMakeFiles/clangCppInterOp.dir/all] Error 2 make: *** [Makefile:136: all] Error 2

Similarly, Exprs.emplace_back also fails with the following error message.

[ 68%] Building CXX object lib/Interpreter/CMakeFiles/clangCppInterOp.dir/CppInterOp.cpp.o In file included from /usr/include/c++/14.2.1/vector:65, from /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/CppInterOp/include/clang/Interpreter/CppInterOp.h:17, from /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/CppInterOp/lib/Interpreter/CppInterOp.cpp:10: /usr/include/c++/14.2.1/bits/stl_uninitialized.h: In instantiation of ‘constexpr bool std::__check_constructible() [with _ValueType = clang::OpaqueValueExpr; _Tp = clang::OpaqueValueExpr&&]’: /usr/include/c++/14.2.1/bits/stl_uninitialized.h:182:4: required from ‘_ForwardIterator std::uninitialized_copy(_InputIterator, _InputIterator, _ForwardIterator) [with _InputIterator = move_iterator<clang::OpaqueValueExpr*>; _ForwardIterator = clang::OpaqueValueExpr*]’ 182 | = _GLIBCXX_USE_ASSIGN_FOR_INIT(_ValueType2, _From); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/stl_uninitialized.h:373:37: required from ‘_ForwardIterator std::__uninitialized_copy_a(_InputIterator, _InputIterator, _ForwardIterator, allocator<_Tp>&) [with _InputIterator = move_iterator<clang::OpaqueValueExpr*>; _ForwardIterator = clang::OpaqueValueExpr*; _Tp = clang::OpaqueValueExpr]’ 373 | return std::uninitialized_copy(__first, __last, __result); | ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/stl_uninitialized.h:399:2: required from ‘_ForwardIterator std::__uninitialized_move_if_noexcept_a(_InputIterator, _InputIterator, _ForwardIterator, _Allocator&) [with _InputIterator = clang::OpaqueValueExpr*; _ForwardIterator = clang::OpaqueValueExpr*; _Allocator = allocator<clang::OpaqueValueExpr>]’ 398 | return std::__uninitialized_copy_a | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ 399 | (_GLIBCXX_MAKE_MOVE_IF_NOEXCEPT_ITERATOR(__first), | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 400 | _GLIBCXX_MAKE_MOVE_IF_NOEXCEPT_ITERATOR(__last), __result, __alloc); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/vector.tcc:676:60: required from ‘void std::vector<_Tp, _Alloc>::_M_realloc_append(_Args&& ...) [with _Args = {clang::SourceLocation, clang::QualType, clang::ExprValueKind&}; _Tp = clang::OpaqueValueExpr; _Alloc = std::allocator<clang::OpaqueValueExpr>]’ 676 | __new_finish = std::__uninitialized_move_if_noexcept_a( | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^ 677 | __old_start, __old_finish, | ~~~~~~~~~~~~~~~~~~~~~~~~~~ 678 | __new_start, _M_get_Tp_allocator()); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/vector.tcc:123:21: required from ‘std::vector<_Tp, _Alloc>::reference std::vector<_Tp, _Alloc>::emplace_back(_Args&& ...) [with _Args = {clang::SourceLocation, clang::QualType, clang::ExprValueKind&}; _Tp = clang::OpaqueValueExpr; _Alloc = std::allocator<clang::OpaqueValueExpr>; reference = clang::OpaqueValueExpr&]’ 123 | _M_realloc_append(std::forward<_Args>(__args)...); | ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/vipul-cariappa/Documents/Workspace/cpp-py/compiler-research/CppInterOp/lib/Interpreter/CppInterOp.cpp:1082:25: required from here 1082 | Exprs.emplace_back(SourceLocation::getFromRawEncoding(1), Type.getNonReferenceType(), ExprKind); | ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/include/c++/14.2.1/bits/stl_uninitialized.h:90:56: error: static assertion failed: result type must be constructible from input type 90 | static_assert(is_constructible<_ValueType, _Tp>::value, | ^~~~~ /usr/include/c++/14.2.1/bits/stl_uninitialized.h:90:56: note: ‘std::integral_constant<bool, false>::value’ evaluates to false make[2]: *** [lib/Interpreter/CMakeFiles/clangCppInterOp.dir/build.make:79: lib/Interpreter/CMakeFiles/clangCppInterOp.dir/CppInterOp.cpp.o] Error 1 make[1]: *** [CMakeFiles/Makefile2:426: lib/Interpreter/CMakeFiles/clangCppInterOp.dir/all] Error 2

Do we need to specify an allocator provided by clang?

Vipul-Cariappa · 2025-03-06T10:26:24Z

lib/Interpreter/CppInterOp.cpp

@@ -1027,61 +1056,78 @@ namespace Cpp {
  }

  TCppFunction_t
-  BestTemplateFunctionMatch(const std::vector<TCppFunction_t>& candidates,
+  BestOverloadFunctionMatch(const std::vector<TCppFunction_t>& candidates,


I will recommend against it. We will need to create a C++ function call expression for it to work.
I did adapt this function from Sema::BuildCallExpr.

Vipul-Cariappa · 2025-03-06T10:26:48Z

lib/Interpreter/CppInterOp.cpp

+
+    OverloadCandidateSet Overloads(
+        SourceLocation(), OverloadCandidateSet::CandidateSetKind::CSK_Normal);
+    for (void* i : candidates) {


We will still require a loop to populate UnresolvedSetImpl from candidates. Also, when trying it locally, the following assertion fails https://github.com/llvm/llvm-project/blob/release/19.x/clang/lib/Sema/SemaOverload.cpp#L5692 for one of the test cases.

vgvassilev · 2025-03-07T08:21:49Z

lib/Interpreter/CppInterOp.cpp

@@ -1027,61 +1056,73 @@ namespace Cpp {
  }

  TCppFunction_t
-  BestTemplateFunctionMatch(const std::vector<TCppFunction_t>& candidates,
+  BestOverloadFunctionMatch(const std::vector<TCppFunction_t>& candidates,


Another note is that we should check the memory growth when we call this function N times. Ideally it should be 0 for failed overload resolution candidate set. In reality it should be close to 0...

vgvassilev

Nicely done. Please run clang-format and squash on merge.

Vipul-Cariappa · 2025-03-09T10:33:58Z

Hi @mcbarton, the Windows CI is failing in the build step of CppInterOp. I am not sure what is causing this. Can you please look into this. This PR is good to go otherwise. I expect it to fail while building cppyy, with an error message similar to the Ubuntu/OSX CI run (but I guess we don't test cppyy on Windows).

…nctionMatch` Resolve and instantiate (if templated) overload from vector of overloads, for the given template parameter types and argument types With this changes we replicate clang's overload resolution and template instantiation

Using `AddOverloadCandidate` over `AddMethodCandidate` and `AddTemplateOverloadCandidate` over `AddMethodTemplateCandidate`

Vipul-Cariappa · 2025-03-09T10:55:44Z

@mcbarton, I have fixed the CI problem. Now it looks like a code problem. Sorry, I should have read the error message properly before pinging you.

@vgvassilev, it looks like MSVC does not support Variable Length Array, reference: https://stackoverflow.com/a/33423538.
How to proceed?

from compiler-research/CppInterOp#514

Vipul-Cariappa requested review from vgvassilev and aaronj0 March 5, 2025 12:41

github-actions bot reviewed Mar 5, 2025

View reviewed changes

Vipul-Cariappa added a commit to Vipul-Cariappa/cppyy-backend-compiler-research that referenced this pull request Mar 6, 2025

fix according InterOp's overload resolution changes

5d8b95c

from compiler-research/CppInterOp#514

Vipul-Cariappa mentioned this pull request Mar 6, 2025

fix according InterOp's overload resolution changes compiler-research/cppyy-backend#131

Merged

vgvassilev reviewed Mar 6, 2025

View reviewed changes

Vipul-Cariappa commented Mar 6, 2025

View reviewed changes

Vipul-Cariappa requested a review from vgvassilev March 6, 2025 14:33

vgvassilev reviewed Mar 7, 2025

View reviewed changes

Vipul-Cariappa requested a review from vgvassilev March 8, 2025 14:33

vgvassilev approved these changes Mar 8, 2025

View reviewed changes

Vipul-Cariappa requested a review from mcbarton March 9, 2025 10:30

Vipul-Cariappa added 8 commits March 9, 2025 16:05

make cling happy

cc75f31

make codecov happy

63917bd

skip using specilised function for methods

9e5f2f7

Using `AddOverloadCandidate` over `AddMethodCandidate` and `AddTemplateOverloadCandidate` over `AddMethodTemplateCandidate`

adapted from Sema::BuildCallExpr

d3d913e

fix Expr memory ownership

097603a

fix cmake to accept variable length array

0a54e71

brief memory leak at clang

323b5fc

Vipul-Cariappa force-pushed the dev/FindBestViableOverload branch from 7a235e1 to 323b5fc Compare March 9, 2025 10:35

fix cmake for windows (-Wno-vla)

2a5c211

avoid variable length array

2b81002

Vipul-Cariappa merged commit 59b7c11 into compiler-research:main Mar 9, 2025
49 of 73 checks passed

Vipul-Cariappa deleted the dev/FindBestViableOverload branch March 9, 2025 14:59

Vipul-Cariappa added a commit to Vipul-Cariappa/cppyy-backend-compiler-research that referenced this pull request Mar 9, 2025

fix according InterOp's overload resolution changes

a3eab24

from compiler-research/CppInterOp#514

Vipul-Cariappa added a commit to compiler-research/cppyy-backend that referenced this pull request Mar 10, 2025

fix according InterOp's overload resolution changes (#131)

51257cb

from compiler-research/CppInterOp#514

This was referenced Mar 10, 2025

Improve template instantiate #511

Closed

while instantiating template use non reference types compiler-research/cppyy-backend#130

Closed

Add BestOverloadFunctionMatch & IsFunction Remove BestTemplateFunctionMatch #514

Add BestOverloadFunctionMatch & IsFunction Remove BestTemplateFunctionMatch #514

Uh oh!

Conversation

Vipul-Cariappa commented Mar 5, 2025

Description

Fixes # (issue)

Type of change

Testing

Checklist

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot Mar 5, 2025

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Vipul-Cariappa left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vgvassilev left a comment

Choose a reason for hiding this comment

Uh oh!

Vipul-Cariappa commented Mar 9, 2025

Uh oh!

Vipul-Cariappa commented Mar 9, 2025

Uh oh!

Uh oh!

Uh oh!

Add `BestOverloadFunctionMatch` & `IsFunction` Remove `BestTemplateFunctionMatch` #514

Add `BestOverloadFunctionMatch` & `IsFunction` Remove `BestTemplateFunctionMatch` #514

codecov bot commented Mar 5, 2025 •

edited

Loading