From 7ac41c3b51455fbab5cf4b6c0e598584a55d230b Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Tue, 12 May 2026 00:38:21 +0200 Subject: [PATCH] Preserve using-declaration access for inherited methods When `GetClassDecls` walked a class's decls and hit a `UsingShadowDecl` whose target is a method, it pushed the *target* `CXXMethodDecl` from the base class. The target's `getAccess()` returns the access in the base (e.g. `protected`); the elevated access carried by the using-declaration (`public` in the introducing class) was silently lost. Consumers like CPyCppyy filter on `IsPublicMethod`, so a republished overload such as class Base { protected: void foo(int, int); }; class Derived: public Base { public: using Base::foo; void foo(int); }; would never reach the Python proxy - calling `derived.foo(1, 2)` from cppyy raised `TypeError: takes at most 1 arguments (2 given)` (cppyy issue https://github.com/compiler-research/cppyy/issues/220). Push the `UsingShadowDecl` itself (for the non-constructor case; constructor-using-shadows still go through `findInheritingConstructor`) and teach `CheckMethodAccess` to consult `USD->getAccess()`. A small `UnwrapUsingShadowToFunction` helper unwraps the shadow at the entry of every API that downcasts a `TCppFunction_t` to `FunctionDecl` (return type, arg counts/types/names/defaults, signature, IsConst/Static/Virtual/Constructor/Destructor/Templated/Explicit, function address, operator arity). `MakeFunctionCallable` keeps the unwrapped target as the wrapped function but threads a `relaxAccessControl` flag into `make_wrapper` when the caller passed a using-shadow: the generated wrapper still references the target by its original qualified name (e.g. `((Base*)obj)->Base::foo(...)`), so access control has to be relaxed during wrapper compilation for it to compile. The runtime call is well-defined because the using-declaration made the member reachable through the derived class. Adds a `FunctionReflection_GetClassMethods_UsingShadowAccess` regression test covering both the public-promoted case from the issue and a protected-using control case (which must stay hidden). Fixes: https://github.com/compiler-research/cppyy/issues/220 --- lib/CppInterOp/CppInterOp.cpp | 95 ++++++++--- .../CppInterOp/FunctionReflectionTest.cpp | 151 ++++++++++++++++++ 2 files changed, 222 insertions(+), 24 deletions(-) diff --git a/lib/CppInterOp/CppInterOp.cpp b/lib/CppInterOp/CppInterOp.cpp index 270487c4e..49c936ece 100644 --- a/lib/CppInterOp/CppInterOp.cpp +++ b/lib/CppInterOp/CppInterOp.cpp @@ -1021,6 +1021,27 @@ static const clang::Decl* GetUnderlyingScopeImpl(const clang::Decl* D) { 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(D)) + if (auto* Target = USD->getTargetDecl()) + if (isa(Target) || isa(Target)) + return Target; + return D; +} + +static const clang::Decl* UnwrapUsingShadowToFunction(const clang::Decl* D) { + if (const auto* USD = dyn_cast_or_null(D)) + if (const auto* Target = USD->getTargetDecl()) + if (isa(Target) || isa(Target)) + return Target; + return D; +} + DeclRef GetUnderlyingScope(ConstDeclRef DRef) { INTEROP_TRACE(DRef); if (!DRef) @@ -1408,7 +1429,10 @@ static void GetClassDecls(ConstDeclRef DRef, std::vector& methods) { auto* CUSD = dyn_cast(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; } @@ -1549,7 +1573,7 @@ std::vector GetFunctionsUsingName(ConstDeclRef DRef, TypeRef GetFunctionReturnType(ConstFuncRef func) { INTEROP_TRACE(func); - const auto* D = unwrap(func); + const auto* D = UnwrapUsingShadowToFunction(unwrap(func)); if (const auto* FD = llvm::dyn_cast_or_null(D)) { QualType Type = FD->getReturnType(); if (Type->isUndeducedAutoType()) { @@ -1604,7 +1628,7 @@ void GetFnTypeSignature(ConstTypeRef fn_type, std::vector& sig) { // exclude it, which is what callers introspecting the argument list want. size_t GetFunctionNumArgs(ConstFuncRef func) { INTEROP_TRACE(func); - const auto* D = unwrap(func); + const auto* D = UnwrapUsingShadowToFunction(unwrap(func)); if (const auto* FD = llvm::dyn_cast_or_null(D)) return INTEROP_RETURN(FD->getNumNonObjectParams()); @@ -1616,7 +1640,7 @@ size_t GetFunctionNumArgs(ConstFuncRef func) { size_t GetFunctionRequiredArgs(ConstFuncRef func) { INTEROP_TRACE(func); - const auto* D = unwrap(func); + const auto* D = UnwrapUsingShadowToFunction(unwrap(func)); if (const auto* FD = llvm::dyn_cast_or_null(D)) return INTEROP_RETURN(FD->getMinRequiredExplicitArguments()); @@ -1629,7 +1653,7 @@ size_t GetFunctionRequiredArgs(ConstFuncRef func) { TypeRef GetFunctionArgType(ConstFuncRef func, size_t iarg) { INTEROP_TRACE(func, iarg); - const auto* D = unwrap(func); + const auto* D = UnwrapUsingShadowToFunction(unwrap(func)); if (const auto* FTD = llvm::dyn_cast_or_null(D)) D = FTD->getTemplatedDecl(); @@ -1655,7 +1679,7 @@ std::string GetFunctionSignature(ConstFuncRef func) { if (!func) return INTEROP_RETURN(""); - const auto* D = unwrap(func); + const auto* D = UnwrapUsingShadowToFunction(unwrap(func)); const clang::FunctionDecl* FD; if (llvm::dyn_cast(D)) @@ -1700,13 +1724,14 @@ bool IsTemplateInstantiationOrSpecialization(const Decl* D) { bool IsFunctionDeleted(ConstFuncRef function) { INTEROP_TRACE(function); - const auto* FD = cast(unwrap(function)); + const auto* FD = cast( + UnwrapUsingShadowToFunction(unwrap(function))); return INTEROP_RETURN(FD->isDeleted()); } bool IsTemplatedFunction(ConstFuncRef func) { INTEROP_TRACE(func); - const auto* D = unwrap(func); + const auto* D = UnwrapUsingShadowToFunction(unwrap(func)); return INTEROP_RETURN(IsTemplatedFunction(D) || IsTemplateInstantiationOrSpecialization(D)); } @@ -1912,6 +1937,13 @@ BestOverloadFunctionMatch(const std::vector& candidates, // the provided AccessSpecifier. bool CheckMethodAccess(ConstFuncRef method, AccessSpecifier AS) { const auto* D = unwrap(method); + // Must NOT unwrap here: the using-shadow is the only carrier of the effective + // access (e.g. `public` in the derived class), while the target only knows + // its base access (e.g. `protected`). + if (const auto* USD = llvm::dyn_cast_or_null(D)) { + if (llvm::isa_and_nonnull(USD->getTargetDecl())) + return USD->getAccess() == AS; + } if (const auto* CXXMD = llvm::dyn_cast_or_null(D)) { return CXXMD->getAccess() == AS; } @@ -1921,7 +1953,7 @@ bool CheckMethodAccess(ConstFuncRef method, AccessSpecifier AS) { bool IsMethod(ConstFuncRef method) { INTEROP_TRACE(method); - const auto* D = unwrap(method); + const auto* D = UnwrapUsingShadowToFunction(unwrap(method)); if (const auto* FTD = dyn_cast_or_null(D)) D = FTD->getTemplatedDecl(); return INTEROP_RETURN(dyn_cast_or_null(D)); @@ -1945,7 +1977,7 @@ bool IsPrivateMethod(ConstFuncRef method) { bool IsConstructor(ConstFuncRef method) { INTEROP_TRACE(method); - const auto* D = unwrap(method); + const auto* D = UnwrapUsingShadowToFunction(unwrap(method)); if (const auto* FTD = dyn_cast(D)) return INTEROP_RETURN(IsConstructor(FTD->getTemplatedDecl())); return INTEROP_RETURN(llvm::isa_and_nonnull(D)); @@ -1953,13 +1985,13 @@ bool IsConstructor(ConstFuncRef method) { bool IsDestructor(ConstFuncRef method) { INTEROP_TRACE(method); - const auto* D = unwrap(method); + const auto* D = UnwrapUsingShadowToFunction(unwrap(method)); return INTEROP_RETURN(llvm::isa_and_nonnull(D)); } bool IsStaticMethod(ConstFuncRef method) { INTEROP_TRACE(method); - const auto* D = unwrap(method); + const auto* D = UnwrapUsingShadowToFunction(unwrap(method)); if (const auto* FTD = llvm::dyn_cast_or_null(D)) D = FTD->getTemplatedDecl(); @@ -1975,7 +2007,7 @@ bool IsExplicit(ConstFuncRef method) { if (!method) return INTEROP_RETURN(false); - const auto* D = unwrap(method); + const auto* D = UnwrapUsingShadowToFunction(unwrap(method)); if (const auto* FTD = llvm::dyn_cast_or_null(D)) D = FTD->getTemplatedDecl(); @@ -2032,7 +2064,7 @@ static void* GetFunctionAddress(const FunctionDecl* FD) { void* GetFunctionAddress(FuncRef method) { INTEROP_TRACE(method); - auto* D = unwrap(method); + auto* D = UnwrapUsingShadowToFunction(unwrap(method)); if (auto* FD = llvm::dyn_cast_or_null(D)) { if ((IsTemplateInstantiationOrSpecialization(FD) || FD->getTemplatedKind() == FunctionDecl::TK_MemberSpecialization) && @@ -2048,7 +2080,7 @@ void* GetFunctionAddress(FuncRef method) { bool IsVirtualMethod(ConstFuncRef method) { INTEROP_TRACE(method); - const auto* D = unwrap(method); + const auto* D = UnwrapUsingShadowToFunction(unwrap(method)); if (const auto* CXXMD = llvm::dyn_cast_or_null(D)) { return INTEROP_RETURN(CXXMD->isVirtual()); } @@ -4037,7 +4069,8 @@ int get_wrapper_code(compat::Interpreter& I, const FunctionDecl* FD, } 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); @@ -4075,6 +4108,12 @@ JitCall::GenericCall make_wrapper(compat::Interpreter& I, // We should be able to call private default constructors. if (auto Ctor = dyn_cast(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; void* wrapper = compile_wrapper(I, wrapper_name, wrapper_code, withAccessControl); if (wrapper) { @@ -4286,10 +4325,16 @@ static JitCall::DestructorCall make_dtor_wrapper(compat::Interpreter& interp, CPPINTEROP_API JitCall MakeFunctionCallable(InterpRef I, ConstFuncRef func) { INTEROP_TRACE(I, func); - const auto* D = unwrap(func); - if (!D) + const auto* InputD = unwrap(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(InputD); + const auto* D = UnwrapUsingShadowToFunction(InputD); + auto* interp = unwrap(I); // FIXME: Unify with make_wrapper. @@ -4302,14 +4347,16 @@ CPPINTEROP_API JitCall MakeFunctionCallable(InterpRef I, ConstFuncRef func) { } if (const auto* Ctor = dyn_cast(D)) { - if (auto Wrapper = make_wrapper(*interp, cast(D))) + if (auto Wrapper = + make_wrapper(*interp, cast(D), isUsingShadow)) return INTEROP_RETURN(JitCall(JitCall::kConstructorCall, Wrapper, wrap(Ctor))); // FIXME: else error we failed to compile the wrapper. return INTEROP_RETURN(JitCall{}); } - if (auto Wrapper = make_wrapper(*interp, cast(D))) { + if (auto Wrapper = + make_wrapper(*interp, cast(D), isUsingShadow)) { return INTEROP_RETURN(JitCall(JitCall::kGenericCall, Wrapper, wrap(cast(D)))); } @@ -5200,7 +5247,7 @@ bool IsTypeDerivedFrom(ConstTypeRef derived, ConstTypeRef base) { std::string GetFunctionArgDefault(ConstFuncRef func, size_t param_index) { INTEROP_TRACE(func, param_index); - const auto* D = unwrap(func); + const auto* D = UnwrapUsingShadowToFunction(unwrap(func)); const clang::ParmVarDecl* PI = nullptr; if (const auto* FD = llvm::dyn_cast_or_null(D)) @@ -5243,7 +5290,7 @@ bool IsConstMethod(ConstFuncRef method) { if (!method) return INTEROP_RETURN(false); - const auto* D = unwrap(method); + const auto* D = UnwrapUsingShadowToFunction(unwrap(method)); if (const auto* func = dyn_cast(D)) return INTEROP_RETURN(func->getMethodQualifiers().hasConst()); @@ -5252,7 +5299,7 @@ bool IsConstMethod(ConstFuncRef method) { std::string GetFunctionArgName(ConstFuncRef func, size_t param_index) { INTEROP_TRACE(func, param_index); - const auto* D = unwrap(func); + const auto* D = UnwrapUsingShadowToFunction(unwrap(func)); const clang::ParmVarDecl* PI = nullptr; if (const auto* FD = llvm::dyn_cast_or_null(D)) @@ -5282,7 +5329,7 @@ Operator GetOperatorFromSpelling(const std::string& op) { OperatorArity GetOperatorArity(ConstFuncRef op) { INTEROP_TRACE(op); - const auto* D = unwrap(op); + const auto* D = UnwrapUsingShadowToFunction(unwrap(op)); if (const auto* FD = llvm::dyn_cast(D)) { if (FD->isOverloadedOperator()) { switch (FD->getOverloadedOperator()) { diff --git a/unittests/CppInterOp/FunctionReflectionTest.cpp b/unittests/CppInterOp/FunctionReflectionTest.cpp index 48e8609ed..c9f7dcc11 100644 --- a/unittests/CppInterOp/FunctionReflectionTest.cpp +++ b/unittests/CppInterOp/FunctionReflectionTest.cpp @@ -163,6 +163,157 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, FunctionReflection_GetClassMethods) { EXPECT_EQ(get_method_name(templ_methods2[6]), "inline TT::~TT()"); } +// 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 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 derived_methods; + Cpp::GetClassMethods(Decls[1], derived_methods); + + // Find the using-promoted foo (the two-argument overload). + bool found_using_promoted = false; + Cpp::FuncRef using_promoted; + for (auto m : derived_methods) { + if (Cpp::GetName(Cpp::DeclRef{m.data}) != "foo") + continue; + if (Cpp::GetFunctionNumArgs(m) == 2) { + found_using_promoted = true; + using_promoted = m; + 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"; + + // Resolving the address of the using-promoted overload must transparently + // unwrap the using-shadow to its target before emitting code. This is the + // only caller exercising the non-const UnwrapUsingShadowToFunction overload + // (the reflection-only APIs above all go through the const overload), and the + // resolved address must match the one obtained directly from the base method. +#ifndef _WIN32 // GetFunctionAddress is disabled on Windows; see its own test. + if (!TypeParam::isOutOfProcess && found_using_promoted) { + std::vector base_methods; + Cpp::GetClassMethods(Decls[0], base_methods); + Cpp::FuncRef base_foo; + for (auto m : base_methods) { + if (Cpp::GetName(Cpp::DeclRef{m.data}) == "foo" && + Cpp::GetFunctionNumArgs(m) == 2) + base_foo = m; + } + ASSERT_TRUE(base_foo); + + void* shadow_addr = Cpp::GetFunctionAddress(using_promoted); + EXPECT_TRUE(shadow_addr); + EXPECT_EQ(shadow_addr, Cpp::GetFunctionAddress(base_foo)); + } +#endif + + std::vector hidden_methods; + Cpp::GetClassMethods(Decls[2], hidden_methods); + + bool found_hidden = false; + for (auto m : hidden_methods) { + if (Cpp::GetName(Cpp::DeclRef{m.data}) == "foo" && + Cpp::GetFunctionNumArgs(m) == 2) { + found_hidden = true; + EXPECT_FALSE(Cpp::IsPublicMethod(m)); + EXPECT_TRUE(Cpp::IsProtectedMethod(m)); + } + } + EXPECT_TRUE(found_hidden); +} + +// Companion to the access test above, covering the *call* path: a method +// promoted into a derived class with a public `using Base::name;` must be +// invocable through the derived class even though the target still carries the +// base class's protected access. The generated wrapper references the target by +// its original (protected) qualified name, so MakeFunctionCallable threads a +// relaxAccessControl flag into wrapper compilation for this case. Exercise it +// end to end: compile the wrapper and actually invoke it. +TYPED_TEST(CPPINTEROP_TEST_MODE, + FunctionReflection_MakeFunctionCallable_UsingShadow) { +#ifdef EMSCRIPTEN + GTEST_SKIP() << "Test fails for Emscripten builds"; +#endif +#if defined(CPPINTEROP_USE_CLING) && defined(_WIN32) + GTEST_SKIP() << "Disabled on Cling/Windows."; +#endif + if (TypeParam::isOutOfProcess) + GTEST_SKIP() << "Test fails for OOP JIT builds"; + + std::vector Decls; + std::string code = R"( + class MyBase { + protected: + int foo(int a, int b) { return a * 100 + b; } + }; + class MyDerived : public MyBase { + public: + using MyBase::foo; // promoted to public + int foo(int a) { return a; } + }; + )"; + + // `-include new` is needed for the constructor wrapper's placement new. + GetAllTopLevelDecls(code, Decls, /*filter_implicitGenerated=*/false, + /*interpreter_args=*/{"-include", "new"}); + + std::vector derived_methods; + Cpp::GetClassMethods(Decls[1], derived_methods); + + // Locate the using-promoted foo (the two-argument overload from the base). + Cpp::FuncRef using_promoted; + for (auto m : derived_methods) { + if (Cpp::GetName(Cpp::DeclRef{m.data}) == "foo" && + Cpp::GetFunctionNumArgs(m) == 2) + using_promoted = m; + } + ASSERT_TRUE(using_promoted); + + // Compiling this wrapper exercises the relaxAccessControl path: the generated + // body calls MyBase::foo through its qualified (protected) name, so access + // control has to be disabled for the wrapper to compile. + Cpp::JitCall Call = Cpp::MakeFunctionCallable(using_promoted); + ASSERT_EQ(Call.getKind(), Cpp::JitCall::kGenericCall); + + // Construct a MyDerived and call the promoted overload through it. + auto Ctor = Cpp::MakeFunctionCallable(Cpp::GetDefaultConstructor(Decls[1])); + void* object = nullptr; + Ctor.Invoke((void*)&object, {}, /*self=*/nullptr); + ASSERT_TRUE(object); + + int a = 3; + int b = 7; + int result = 0; + std::array args = {(void*)&a, (void*)&b}; + Call.Invoke(&result, {args.data(), /*args_size=*/2}, object); + EXPECT_EQ(result, (a * 100) + b); + + Cpp::Destruct(object, Decls[1]); +} + TYPED_TEST(CPPINTEROP_TEST_MODE, FunctionReflection_ConstructorInGetClassMethods) { std::vector Decls;