Skip to content

Preserve using-declaration access for inherited methods #995

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
guitargeek wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Preserve using-declaration access for inherited methods #995

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
104 changes: 78 additions & 26 deletions lib/CppInterOp/CppInterOp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1025,6 +1025,28 @@
return D->getCanonicalDecl();
}

// If D is a UsingShadowDecl whose target is a function-like decl,
// return that target; otherwise return D unchanged. The using-shadow
// is the only carrier of the "effective access" introduced into the
// derived class, so callers that need access info should consult D
// before unwrapping.
static clang::Decl* UnwrapUsingShadowToFunction(clang::Decl* D) {
if (auto* USD = dyn_cast_or_null<UsingShadowDecl>(D))
if (auto* Target = USD->getTargetDecl())
if (isa<FunctionDecl>(Target) || isa<FunctionTemplateDecl>(Target))
return Target;
return D;
}

static const clang::Decl*
UnwrapUsingShadowToFunction(const clang::Decl* D) {
if (const auto* USD = dyn_cast_or_null<UsingShadowDecl>(D))
if (const auto* Target = USD->getTargetDecl())
if (isa<FunctionDecl>(Target) || isa<FunctionTemplateDecl>(Target))
return Target;
return D;
}

TCppScope_t GetUnderlyingScope(TCppScope_t scope) {
INTEROP_TRACE(scope);
if (!scope)
Expand Down Expand Up @@ -1396,7 +1418,10 @@

auto* CUSD = dyn_cast<ConstructorUsingShadowDecl>(DI);
if (!CUSD) {
methods.push_back(MD);
// Push the using-shadow rather than the target so that the
// effective access (USD->getAccess()) reachable from the
// introducing class is preserved for downstream consumers.
methods.push_back(USD);
continue;
}

Expand Down Expand Up @@ -1506,7 +1531,7 @@

TCppType_t GetFunctionReturnType(TCppFunction_t func) {
INTEROP_TRACE(func);
auto* D = (clang::Decl*)func;
auto* D = UnwrapUsingShadowToFunction((clang::Decl*)func);
if (auto* FD = llvm::dyn_cast_or_null<clang::FunctionDecl>(D)) {
QualType Type = FD->getReturnType();
if (Type->isUndeducedAutoType()) {
Expand Down Expand Up @@ -1535,7 +1560,7 @@

TCppIndex_t GetFunctionNumArgs(TCppFunction_t func) {
INTEROP_TRACE(func);
auto* D = (clang::Decl*)func;
auto* D = UnwrapUsingShadowToFunction((clang::Decl*)func);
if (auto* FD = llvm::dyn_cast_or_null<FunctionDecl>(D))
return INTEROP_RETURN(FD->getNumParams());

Expand All @@ -1547,7 +1572,8 @@

TCppIndex_t GetFunctionRequiredArgs(TCppConstFunction_t func) {
INTEROP_TRACE(func);
const auto* D = static_cast<const clang::Decl*>(func);
const auto* D =
UnwrapUsingShadowToFunction(static_cast<const clang::Decl*>(func));
if (auto* FD = llvm::dyn_cast_or_null<FunctionDecl>(D))
return INTEROP_RETURN(FD->getMinRequiredArguments());

Expand All @@ -1559,7 +1585,7 @@

TCppType_t GetFunctionArgType(TCppFunction_t func, TCppIndex_t iarg) {
INTEROP_TRACE(func, iarg);
auto* D = (clang::Decl*)func;
auto* D = UnwrapUsingShadowToFunction((clang::Decl*)func);

if (auto* FD = llvm::dyn_cast_or_null<clang::FunctionDecl>(D)) {
if (iarg < FD->getNumParams()) {
Expand All @@ -1576,7 +1602,7 @@
if (!func)
return INTEROP_RETURN("<unknown>");

auto* D = (clang::Decl*)func;
auto* D = UnwrapUsingShadowToFunction((clang::Decl*)func);
clang::FunctionDecl* FD;

if (llvm::dyn_cast<FunctionDecl>(D))
Expand Down Expand Up @@ -1621,14 +1647,14 @@

bool IsFunctionDeleted(TCppConstFunction_t function) {
INTEROP_TRACE(function);
const auto* FD =
cast<const FunctionDecl>(static_cast<const clang::Decl*>(function));
const auto* FD = cast<const FunctionDecl>(
UnwrapUsingShadowToFunction(static_cast<const clang::Decl*>(function)));
return INTEROP_RETURN(FD->isDeleted());
}

bool IsTemplatedFunction(TCppFunction_t func) {
INTEROP_TRACE(func);
auto* D = (Decl*)func;
auto* D = UnwrapUsingShadowToFunction((Decl*)func);
return INTEROP_RETURN(IsTemplatedFunction(D) ||
IsTemplateInstantiationOrSpecialization(D));
}
Expand Down Expand Up @@ -1809,6 +1835,13 @@
// the provided AccessSpecifier.
bool CheckMethodAccess(TCppFunction_t method, AccessSpecifier AS) {
auto* D = (Decl*)method;
// For methods introduced into a derived class via `using Base::name;`,
// the effective access is the access of the using-declaration, not of
// the underlying target method.
if (auto* USD = llvm::dyn_cast_or_null<UsingShadowDecl>(D)) {
if (llvm::isa_and_nonnull<CXXMethodDecl>(USD->getTargetDecl()))
return USD->getAccess() == AS;
}

@vgvassilev vgvassilev May 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why not using UnwrapUsingShadowToFunction?

if (auto* CXXMD = llvm::dyn_cast_or_null<CXXMethodDecl>(D)) {
return CXXMD->getAccess() == AS;
}
Expand All @@ -1818,8 +1851,8 @@

bool IsMethod(TCppConstFunction_t method) {
INTEROP_TRACE(method);
return INTEROP_RETURN(
dyn_cast_or_null<CXXMethodDecl>(static_cast<const clang::Decl*>(method)));
return INTEROP_RETURN(dyn_cast_or_null<CXXMethodDecl>(
UnwrapUsingShadowToFunction(static_cast<const clang::Decl*>(method))));
}

bool IsPublicMethod(TCppFunction_t method) {
Expand All @@ -1840,21 +1873,24 @@

bool IsConstructor(TCppConstFunction_t method) {
INTEROP_TRACE(method);
const auto* D = static_cast<const Decl*>(method);
const auto* D =
UnwrapUsingShadowToFunction(static_cast<const Decl*>(method));
if (const auto* FTD = dyn_cast<FunctionTemplateDecl>(D))
return INTEROP_RETURN(IsConstructor(FTD->getTemplatedDecl()));
return INTEROP_RETURN(llvm::isa_and_nonnull<CXXConstructorDecl>(D));
}

bool IsDestructor(TCppConstFunction_t method) {
INTEROP_TRACE(method);
const auto* D = static_cast<const Decl*>(method);
const auto* D =
UnwrapUsingShadowToFunction(static_cast<const Decl*>(method));
return INTEROP_RETURN(llvm::isa_and_nonnull<CXXDestructorDecl>(D));
}

bool IsStaticMethod(TCppConstFunction_t method) {
INTEROP_TRACE(method);
const auto* D = static_cast<const Decl*>(method);
const auto* D =
UnwrapUsingShadowToFunction(static_cast<const Decl*>(method));
if (const auto* FTD = llvm::dyn_cast_or_null<FunctionTemplateDecl>(D))
D = FTD->getTemplatedDecl();

Expand All @@ -1870,7 +1906,8 @@
if (!method)
return INTEROP_RETURN(false);

const auto* D = static_cast<const Decl*>(method);
const auto* D =
UnwrapUsingShadowToFunction(static_cast<const Decl*>(method));

if (const auto* FTD = llvm::dyn_cast_or_null<FunctionTemplateDecl>(D))
D = FTD->getTemplatedDecl();
Expand Down Expand Up @@ -1927,7 +1964,7 @@

TCppFuncAddr_t GetFunctionAddress(TCppFunction_t method) {
INTEROP_TRACE(method);
auto* D = static_cast<Decl*>(method);
auto* D = UnwrapUsingShadowToFunction(static_cast<Decl*>(method));
if (auto* FD = llvm::dyn_cast_or_null<FunctionDecl>(D)) {
if ((IsTemplateInstantiationOrSpecialization(FD) ||
FD->getTemplatedKind() == FunctionDecl::TK_MemberSpecialization) &&
Expand All @@ -1943,7 +1980,7 @@

bool IsVirtualMethod(TCppFunction_t method) {
INTEROP_TRACE(method);
auto* D = (Decl*)method;
auto* D = UnwrapUsingShadowToFunction((Decl*)method);
if (auto* CXXMD = llvm::dyn_cast_or_null<CXXMethodDecl>(D)) {
return INTEROP_RETURN(CXXMD->isVirtual());
}
Expand Down Expand Up @@ -3625,7 +3662,8 @@
}

JitCall::GenericCall make_wrapper(compat::Interpreter& I,
const FunctionDecl* FD) {
const FunctionDecl* FD,
bool relaxAccessControl = false) {
auto& WrapperStore = getInterpInfo(&I).WrapperStore;

auto R = WrapperStore.find(FD);
Expand Down Expand Up @@ -3663,6 +3701,12 @@
// We should be able to call private default constructors.
if (auto Ctor = dyn_cast<CXXConstructorDecl>(FD))
withAccessControl = !Ctor->isDefaultConstructor();
// Members introduced into a derived class with a public using-declaration
// are reachable through the derived class, but the generated wrapper still
// calls the target through its original (e.g. protected) qualified name.
// Disable access control for this specific case so the wrapper compiles.
if (relaxAccessControl)
withAccessControl = false;

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

View check run for this annotation

Codecov / codecov/patch

lib/CppInterOp/CppInterOp.cpp#L3709 

Added line #L3709 was not covered by tests
void* wrapper =
compile_wrapper(I, wrapper_name, wrapper_code, withAccessControl);
if (wrapper) {
Expand Down Expand Up @@ -3875,10 +3919,16 @@
CPPINTEROP_API JitCall MakeFunctionCallable(TInterp_t I,
TCppConstFunction_t func) {
INTEROP_TRACE(I, func);
const auto* D = static_cast<const clang::Decl*>(func);
if (!D)
const auto* InputD = static_cast<const clang::Decl*>(func);
if (!InputD)
return INTEROP_RETURN(JitCall{});

// If the caller passed a using-shadow, unwrap to the target function but
// remember the fact: the generated wrapper still references the target's
// original (e.g. protected) name, so it needs access control relaxed.
const bool isUsingShadow = isa<UsingShadowDecl>(InputD);
const auto* D = UnwrapUsingShadowToFunction(InputD);

auto* interp = static_cast<compat::Interpreter*>(I);

// FIXME: Unify with make_wrapper.
Expand All @@ -3890,13 +3940,15 @@
}

if (const auto* Ctor = dyn_cast<CXXConstructorDecl>(D)) {
if (auto Wrapper = make_wrapper(*interp, cast<FunctionDecl>(D)))
if (auto Wrapper =
make_wrapper(*interp, cast<FunctionDecl>(D), isUsingShadow))
return INTEROP_RETURN(JitCall(JitCall::kConstructorCall, Wrapper, Ctor));
// FIXME: else error we failed to compile the wrapper.
return INTEROP_RETURN(JitCall{});
}

if (auto Wrapper = make_wrapper(*interp, cast<FunctionDecl>(D))) {
if (auto Wrapper =
make_wrapper(*interp, cast<FunctionDecl>(D), isUsingShadow)) {
return INTEROP_RETURN(
JitCall(JitCall::kGenericCall, Wrapper, cast<FunctionDecl>(D)));
}
Expand Down Expand Up @@ -4720,7 +4772,7 @@
std::string GetFunctionArgDefault(TCppFunction_t func,
TCppIndex_t param_index) {
INTEROP_TRACE(func, param_index);
auto* D = (clang::Decl*)func;
auto* D = UnwrapUsingShadowToFunction((clang::Decl*)func);
clang::ParmVarDecl* PI = nullptr;

if (auto* FD = llvm::dyn_cast_or_null<clang::FunctionDecl>(D))
Expand Down Expand Up @@ -4762,7 +4814,7 @@
if (!method)
return INTEROP_RETURN(false);

auto* D = (clang::Decl*)method;
auto* D = UnwrapUsingShadowToFunction((clang::Decl*)method);
if (auto* func = dyn_cast<CXXMethodDecl>(D))
return INTEROP_RETURN(func->getMethodQualifiers().hasConst());

Expand All @@ -4771,7 +4823,7 @@

std::string GetFunctionArgName(TCppFunction_t func, TCppIndex_t param_index) {
INTEROP_TRACE(func, param_index);
auto* D = (clang::Decl*)func;
auto* D = UnwrapUsingShadowToFunction((clang::Decl*)func);
clang::ParmVarDecl* PI = nullptr;

if (auto* FD = llvm::dyn_cast_or_null<clang::FunctionDecl>(D))
Expand Down Expand Up @@ -4800,7 +4852,7 @@

OperatorArity GetOperatorArity(TCppFunction_t op) {
INTEROP_TRACE(op);
Decl* D = static_cast<Decl*>(op);
Decl* D = UnwrapUsingShadowToFunction(static_cast<Decl*>(op));
if (auto* FD = llvm::dyn_cast<FunctionDecl>(D)) {
if (FD->isOverloadedOperator()) {
switch (FD->getOverloadedOperator()) {
Expand Down
58 changes: 58 additions & 0 deletions unittests/CppInterOp/FunctionReflectionTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,64 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, FunctionReflection_GetClassMethods) {
clang_Interpreter_dispose(I);
}

// Regression for cppyy issue
// https://github.com/compiler-research/cppyy/issues/220: a method introduced
// into a derived class with `using Base::name;` in a public section must
// report public access (taken from the using-declaration), not the access of
// the underlying target in the base class.
TYPED_TEST(CPPINTEROP_TEST_MODE,
FunctionReflection_GetClassMethods_UsingShadowAccess) {
std::vector<Decl*> Decls;
std::string code = R"(
class MyBase {
protected:
void foo(int, int) {}
};
class MyDerived : public MyBase {
public:
using MyBase::foo; // promoted to public
void foo(int) {}
};
class HiddenDerived : public MyBase {
protected:
using MyBase::foo; // stays protected
};
)";

GetAllTopLevelDecls(code, Decls);

std::vector<Cpp::TCppFunction_t> derived_methods;
Cpp::GetClassMethods(Decls[1], derived_methods);

// Find the using-promoted foo (the two-argument overload).
bool found_using_promoted = false;
for (auto m : derived_methods) {
if (Cpp::GetName(m) != "foo")
continue;
if (Cpp::GetFunctionNumArgs(m) == 2) {
found_using_promoted = true;
EXPECT_TRUE(Cpp::IsPublicMethod(m));
EXPECT_FALSE(Cpp::IsProtectedMethod(m));
EXPECT_FALSE(Cpp::IsConstructor(m));
}
}
EXPECT_TRUE(found_using_promoted)
<< "using-promoted base method missing from GetClassMethods";

std::vector<Cpp::TCppFunction_t> hidden_methods;
Cpp::GetClassMethods(Decls[2], hidden_methods);

bool found_hidden = false;
for (auto m : hidden_methods) {
if (Cpp::GetName(m) == "foo" && Cpp::GetFunctionNumArgs(m) == 2) {
found_hidden = true;
EXPECT_FALSE(Cpp::IsPublicMethod(m));
EXPECT_TRUE(Cpp::IsProtectedMethod(m));
}
}
EXPECT_TRUE(found_hidden);
}

TYPED_TEST(CPPINTEROP_TEST_MODE,
FunctionReflection_ConstructorInGetClassMethods) {
std::vector<Decl*> Decls;
Expand Down
Loading