Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion include/CppInterOp/CppInterOp.h
Original file line number Diff line number Diff line change
Expand Up @@ -863,7 +863,8 @@ InstantiateTemplateFunctionFromString(const char* function_template);
CPPINTEROP_API TCppFunction_t
BestOverloadFunctionMatch(const std::vector<TCppFunction_t>& candidates,
const std::vector<TemplateArgInfo>& explicit_types,
const std::vector<TemplateArgInfo>& arg_types);
const std::vector<TemplateArgInfo>& arg_types,
TCppType_t invoking_object_type = nullptr);

CPPINTEROP_API void GetAllCppNames(TCppScope_t scope,
std::set<std::string>& names);
Expand Down
124 changes: 112 additions & 12 deletions lib/CppInterOp/CppInterOp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1274,7 +1274,8 @@
TCppFunction_t
BestOverloadFunctionMatch(const std::vector<TCppFunction_t>& candidates,
const std::vector<TemplateArgInfo>& explicit_types,
const std::vector<TemplateArgInfo>& arg_types) {
const std::vector<TemplateArgInfo>& arg_types,
TCppType_t invoking_object_type) {
auto& S = getSema();
auto& C = S.getASTContext();

Expand All @@ -1288,10 +1289,33 @@
struct WrapperExpr : public OpaqueValueExpr {
WrapperExpr() : OpaqueValueExpr(clang::Stmt::EmptyShell()) {}
};
auto* Exprs = new WrapperExpr[arg_types.size()];
// Check if we need to prepend the invoking object (for member functions)
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: initializing non-owner 'WrapperExpr *' with a newly created 'gsl::owner<>' [cppcoreguidelines-owning-memory]

  auto* Exprs = new WrapperExpr[num_exprs];
  ^

size_t num_exprs = arg_types.size();
bool has_invoking_object = (invoking_object_type != nullptr);
if (has_invoking_object)
num_exprs++;
auto* Exprs = new WrapperExpr[num_exprs];
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: initializing non-owner 'WrapperExpr *' with a newly created 'gsl::owner<>' [cppcoreguidelines-owning-memory]

  auto* Exprs = new WrapperExpr[num_exprs];
  ^

llvm::SmallVector<Expr*> Args;
Args.reserve(arg_types.size());
Args.reserve(num_exprs);
size_t idx = 0;
// If we have an invoking object, create a synthetic expression for it first
// This represents the object on which the member function is called
if (has_invoking_object) {
QualType ObjType = QualType::getFromOpaquePtr(invoking_object_type);

// Determine the expression kind based on the type
ExprValueKind ExprKind =
ExprValueKind::VK_LValue; // Default for T& and const T&
if (ObjType->isRValueReferenceType())
ExprKind = ExprValueKind::VK_XValue; // For T&&

// Create the synthetic expression for the invoking object
new (&Exprs[idx]) OpaqueValueExpr(SourceLocation::getFromRawEncoding(1),
ObjType.getNonReferenceType(), ExprKind);
Args.push_back(&Exprs[idx]);
++idx;
}
// Now add the regular function arguments
for (auto i : arg_types) {
QualType Type = QualType::getFromOpaquePtr(i.m_Type);
// XValue is an object that can be "moved" whereas PRValue is temporary
Expand Down Expand Up @@ -1330,27 +1354,103 @@
OverloadCandidateSet Overloads(
SourceLocation(), OverloadCandidateSet::CandidateSetKind::CSK_Normal);

// Separate object expression from regular arguments for member functions
llvm::ArrayRef<Expr*> CallArgs = Args;
Expr* ObjectArg = nullptr;
if (has_invoking_object && !Args.empty()) {
ObjectArg = Args[0];
CallArgs = llvm::ArrayRef<Expr*>(Args).drop_front();
}

for (void* i : candidates) {
Decl* D = static_cast<Decl*>(i);
llvm::errs() << "Candidate decl kind: " << D->getDeclKindName();
if (auto* ND = dyn_cast<NamedDecl>(D))
llvm::errs() << " name: " << ND->getNameAsString();
llvm::errs() << " | isa<FunctionDecl>: " << isa<FunctionDecl>(D)
<< " | isa<CXXMethodDecl>: " << isa<CXXMethodDecl>(D)
<< " | isa<FunctionTemplateDecl>: "
<< isa<FunctionTemplateDecl>(D) << "\n";

// Special handling for member functions when object type is provided
if (has_invoking_object && ObjectArg) {
if (auto* MD = dyn_cast<CXXMethodDecl>(D)) {
S.AddMethodCandidate(MD, DeclAccessPair::make(MD, MD->getAccess()),
MD->getParent(), ObjectArg->getType(),
ObjectArg->Classify(C), CallArgs, Overloads);
continue;
}
}

// Default behavior: regular function handling (backward compatible)
if (auto* FD = dyn_cast<FunctionDecl>(D)) {
if (auto* MD = dyn_cast<CXXMethodDecl>(FD)) {
if (!has_invoking_object && !MD->isStatic()) {
llvm::errs() << "Skipping non-static method as non-member candidate: "
<< MD->getQualifiedNameAsString() << "\n";
continue;
}
}

llvm::errs() << "Adding overload candidate: "
<< FD->getQualifiedNameAsString() << "\n";
S.AddOverloadCandidate(FD, DeclAccessPair::make(FD, FD->getAccess()),
Args, Overloads);
} else if (auto* FTD = dyn_cast<FunctionTemplateDecl>(D)) {
// AddTemplateOverloadCandidate is causing a memory leak
// It is a known bug at clang
// call stack: AddTemplateOverloadCandidate -> MakeDeductionFailureInfo
// source:
// https://github.com/llvm/llvm-project/blob/release/19.x/clang/lib/Sema/SemaOverload.cpp#L731-L756
S.AddTemplateOverloadCandidate(
FTD, DeclAccessPair::make(FTD, FTD->getAccess()),
&ExplicitTemplateArgs, Args, Overloads);
// Special handling for member function templates when object type is
// provided
if (has_invoking_object && ObjectArg && FTD->getTemplatedDecl() &&
isa<CXXMethodDecl>(FTD->getTemplatedDecl())) {
llvm::errs() << "Adding method template candidate: "
<< FTD->getQualifiedNameAsString() << "\n";
S.AddMethodTemplateCandidate(
FTD, DeclAccessPair::make(FTD, FTD->getAccess()),
cast<CXXRecordDecl>(FTD->getDeclContext()), &ExplicitTemplateArgs,
ObjectArg->getType(), ObjectArg->Classify(C), CallArgs, Overloads);
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: isa_and_nonnull<> is preferred over an explicit test for null followed by calling isa<> [llvm-prefer-isa-or-dyn-cast-in-conditionals]

Suggested change
ObjectArg->getType(), ObjectArg->Classify(C), CallArgs, Overloads);
if (isa_and_nonnull<CXXMethodDecl>(FTD->getTemplatedDecl())) {

} else {
// If the templated declaration is a non-static method and we do not
// have an invoking object, skip adding it as a non-member candidate.
if (FTD->getTemplatedDecl() &&
isa<CXXMethodDecl>(FTD->getTemplatedDecl())) {
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: isa_and_nonnull<> is preferred over an explicit test for null followed by calling isa<> [llvm-prefer-isa-or-dyn-cast-in-conditionals]

Suggested change
isa<CXXMethodDecl>(FTD->getTemplatedDecl())) {
if (isa_and_nonnull<CXXMethodDecl>(FTD->getTemplatedDecl())) {

auto* MD = cast<CXXMethodDecl>(FTD->getTemplatedDecl());

Check warning on line 1415 in lib/CppInterOp/CppInterOp.cpp

View check run for this annotation

Codecov / codecov/patch

lib/CppInterOp/CppInterOp.cpp#L1415

Added line #L1415 was not covered by tests
// Only skip when the templated decl was defined *within* the class.
// Out-of-class definitions live in the translation unit and should be
// allowed as non-member candidates
if (isa<CXXRecordDecl>(FTD->getDeclContext())) {
if (!has_invoking_object && !MD->isStatic()) {
llvm::errs()

Check warning on line 1421 in lib/CppInterOp/CppInterOp.cpp

View check run for this annotation

Codecov / codecov/patch

lib/CppInterOp/CppInterOp.cpp#L1419-L1421

Added lines #L1419 - L1421 were not covered by tests
<< "Skipping non-static method template as non-member "
"candidate: "
<< MD->getQualifiedNameAsString() << "\n";
continue;

Check warning on line 1425 in lib/CppInterOp/CppInterOp.cpp

View check run for this annotation

Codecov / codecov/patch

lib/CppInterOp/CppInterOp.cpp#L1423-L1425

Added lines #L1423 - L1425 were not covered by tests
}
}
}

llvm::errs() << "Adding template overload candidate: "
<< FTD->getQualifiedNameAsString() << "\n";
// AddTemplateOverloadCandidate is causing a memory leak
// It is a known bug at clang
// call stack: AddTemplateOverloadCandidate -> MakeDeductionFailureInfo
// source:
// https://github.com/llvm/llvm-project/blob/release/19.x/clang/lib/Sema/SemaOverload.cpp#L731-L756
S.AddTemplateOverloadCandidate(
FTD, DeclAccessPair::make(FTD, FTD->getAccess()),
&ExplicitTemplateArgs, Args, Overloads);
}
}
}

OverloadCandidateSet::iterator Best;
Overloads.BestViableFunction(S, SourceLocation(), Best);

FunctionDecl* Result = Best != Overloads.end() ? Best->Function : nullptr;
FunctionDecl* Result = nullptr;

// If overload resolution succeeded or found an ambiguous match, use it
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: deleting a pointer through a type that is not marked 'gsl::owner<>'; consider using a smart pointer instead [cppcoreguidelines-owning-memory]

  delete[] Exprs;
  ^
Additional context

lib/CppInterOp/CppInterOp.cpp:1296: variable declared here

  auto* Exprs = new WrapperExpr[num_exprs];
  ^

if (Best != Overloads.end()) {
Result = Best->Function;
}

delete[] Exprs;
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: deleting a pointer through a type that is not marked 'gsl::owner<>'; consider using a smart pointer instead [cppcoreguidelines-owning-memory]

  delete[] Exprs;
  ^
Additional context

lib/CppInterOp/CppInterOp.cpp:1296: variable declared here

  auto* Exprs = new WrapperExpr[num_exprs];
  ^

return Result;
}
Expand Down
Loading
Loading