diff --git a/lib/CppInterOp/CppInterOp.cpp b/lib/CppInterOp/CppInterOp.cpp index 0c3fee7e4..2407dd159 100644 --- a/lib/CppInterOp/CppInterOp.cpp +++ b/lib/CppInterOp/CppInterOp.cpp @@ -1025,6 +1025,28 @@ static clang::Decl* GetUnderlyingScope(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; +} + TCppScope_t GetUnderlyingScope(TCppScope_t scope) { INTEROP_TRACE(scope); if (!scope) @@ -1396,7 +1418,10 @@ static void GetClassDecls(TCppScope_t klass, 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; } @@ -1506,7 +1531,7 @@ std::vector GetFunctionsUsingName(TCppScope_t scope, 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(D)) { QualType Type = FD->getReturnType(); if (Type->isUndeducedAutoType()) { @@ -1535,7 +1560,7 @@ TCppType_t GetFunctionReturnType(TCppFunction_t func) { 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(D)) return INTEROP_RETURN(FD->getNumParams()); @@ -1547,7 +1572,8 @@ TCppIndex_t GetFunctionNumArgs(TCppFunction_t func) { TCppIndex_t GetFunctionRequiredArgs(TCppConstFunction_t func) { INTEROP_TRACE(func); - const auto* D = static_cast(func); + const auto* D = + UnwrapUsingShadowToFunction(static_cast(func)); if (auto* FD = llvm::dyn_cast_or_null(D)) return INTEROP_RETURN(FD->getMinRequiredArguments()); @@ -1559,7 +1585,7 @@ TCppIndex_t GetFunctionRequiredArgs(TCppConstFunction_t func) { 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(D)) { if (iarg < FD->getNumParams()) { @@ -1576,7 +1602,7 @@ std::string GetFunctionSignature(TCppFunction_t func) { if (!func) return INTEROP_RETURN(""); - auto* D = (clang::Decl*)func; + auto* D = UnwrapUsingShadowToFunction((clang::Decl*)func); clang::FunctionDecl* FD; if (llvm::dyn_cast(D)) @@ -1621,14 +1647,14 @@ bool IsTemplateInstantiationOrSpecialization(Decl* D) { bool IsFunctionDeleted(TCppConstFunction_t function) { INTEROP_TRACE(function); - const auto* FD = - cast(static_cast(function)); + const auto* FD = cast( + UnwrapUsingShadowToFunction(static_cast(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)); } @@ -1809,6 +1835,13 @@ BestOverloadFunctionMatch(const std::vector& candidates, // 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(D)) { + if (llvm::isa_and_nonnull(USD->getTargetDecl())) + return USD->getAccess() == AS; + } if (auto* CXXMD = llvm::dyn_cast_or_null(D)) { return CXXMD->getAccess() == AS; } @@ -1818,8 +1851,8 @@ bool CheckMethodAccess(TCppFunction_t method, AccessSpecifier AS) { bool IsMethod(TCppConstFunction_t method) { INTEROP_TRACE(method); - return INTEROP_RETURN( - dyn_cast_or_null(static_cast(method))); + return INTEROP_RETURN(dyn_cast_or_null( + UnwrapUsingShadowToFunction(static_cast(method)))); } bool IsPublicMethod(TCppFunction_t method) { @@ -1840,7 +1873,8 @@ bool IsPrivateMethod(TCppFunction_t method) { bool IsConstructor(TCppConstFunction_t method) { INTEROP_TRACE(method); - const auto* D = static_cast(method); + const auto* D = + UnwrapUsingShadowToFunction(static_cast(method)); if (const auto* FTD = dyn_cast(D)) return INTEROP_RETURN(IsConstructor(FTD->getTemplatedDecl())); return INTEROP_RETURN(llvm::isa_and_nonnull(D)); @@ -1848,13 +1882,15 @@ bool IsConstructor(TCppConstFunction_t method) { bool IsDestructor(TCppConstFunction_t method) { INTEROP_TRACE(method); - const auto* D = static_cast(method); + const auto* D = + UnwrapUsingShadowToFunction(static_cast(method)); return INTEROP_RETURN(llvm::isa_and_nonnull(D)); } bool IsStaticMethod(TCppConstFunction_t method) { INTEROP_TRACE(method); - const auto* D = static_cast(method); + const auto* D = + UnwrapUsingShadowToFunction(static_cast(method)); if (const auto* FTD = llvm::dyn_cast_or_null(D)) D = FTD->getTemplatedDecl(); @@ -1870,7 +1906,8 @@ bool IsExplicit(TCppConstFunction_t method) { if (!method) return INTEROP_RETURN(false); - const auto* D = static_cast(method); + const auto* D = + UnwrapUsingShadowToFunction(static_cast(method)); if (const auto* FTD = llvm::dyn_cast_or_null(D)) D = FTD->getTemplatedDecl(); @@ -1927,7 +1964,7 @@ static TCppFuncAddr_t GetFunctionAddress(const FunctionDecl* FD) { TCppFuncAddr_t GetFunctionAddress(TCppFunction_t method) { INTEROP_TRACE(method); - auto* D = static_cast(method); + auto* D = UnwrapUsingShadowToFunction(static_cast(method)); if (auto* FD = llvm::dyn_cast_or_null(D)) { if ((IsTemplateInstantiationOrSpecialization(FD) || FD->getTemplatedKind() == FunctionDecl::TK_MemberSpecialization) && @@ -1943,7 +1980,7 @@ TCppFuncAddr_t GetFunctionAddress(TCppFunction_t method) { 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(D)) { return INTEROP_RETURN(CXXMD->isVirtual()); } @@ -3625,7 +3662,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); @@ -3663,6 +3701,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) { @@ -3875,10 +3919,16 @@ static JitCall::DestructorCall make_dtor_wrapper(compat::Interpreter& interp, CPPINTEROP_API JitCall MakeFunctionCallable(TInterp_t I, TCppConstFunction_t func) { INTEROP_TRACE(I, func); - const auto* D = static_cast(func); - if (!D) + const auto* InputD = static_cast(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 = static_cast(I); // FIXME: Unify with make_wrapper. @@ -3890,13 +3940,15 @@ CPPINTEROP_API JitCall MakeFunctionCallable(TInterp_t I, } 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, 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, cast(D))); } @@ -4720,7 +4772,7 @@ bool IsTypeDerivedFrom(TCppType_t derived, TCppType_t base) { 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(D)) @@ -4762,7 +4814,7 @@ bool IsConstMethod(TCppFunction_t method) { if (!method) return INTEROP_RETURN(false); - auto* D = (clang::Decl*)method; + auto* D = UnwrapUsingShadowToFunction((clang::Decl*)method); if (auto* func = dyn_cast(D)) return INTEROP_RETURN(func->getMethodQualifiers().hasConst()); @@ -4771,7 +4823,7 @@ bool IsConstMethod(TCppFunction_t method) { 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(D)) @@ -4800,7 +4852,7 @@ Operator GetOperatorFromSpelling(const std::string& op) { OperatorArity GetOperatorArity(TCppFunction_t op) { INTEROP_TRACE(op); - Decl* D = static_cast(op); + Decl* D = UnwrapUsingShadowToFunction(static_cast(op)); if (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 c67c4ee46..249f67186 100644 --- a/unittests/CppInterOp/FunctionReflectionTest.cpp +++ b/unittests/CppInterOp/FunctionReflectionTest.cpp @@ -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 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; + 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 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 Decls;