From f26e0996c752dfbb5768ac5b0e355638a32b9d72 Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Mon, 4 Mar 2024 18:33:07 +0100 Subject: [PATCH] JNI: Fix error with overload resolution when passing string types The variadic templates are supposed to be removed from the overload set when any of the parameters is a literal string type, as otherwise we get conflicts with the legacy overload taking class names and signatures as const char *. The detection of a literal string types was missing a few specializations, so that we ended up with the wrong overload being called, and class names getting interpreted as method names instead. Add the missing specializations, and add more test coverage for using the old overloads. Task-number: QTBUG-122235 Change-Id: I5488f2009c8f62d74fac6754844f57cf64011414 Reviewed-by: Assam Boudjelthia Reviewed-by: Rami Potinkara Reviewed-by: Lauri Pohjanheimo (cherry picked from commit 10afa38aa44231b3617984fdbca66d9699e2825f) Reviewed-by: Qt Cherry-pick Bot --- src/corelib/kernel/qjnitypes_impl.h | 5 +- .../testdata/QtJniObjectTestClass.java | 3 ++ .../kernel/qjniobject/tst_qjniobject.cpp | 14 +++++ .../kernel/qjnitypes/tst_qjnitypes.cpp | 52 +++++++++++++++++++ 4 files changed, 73 insertions(+), 1 deletion(-) diff --git a/src/corelib/kernel/qjnitypes_impl.h b/src/corelib/kernel/qjnitypes_impl.h index 16bf308dabb..d9635090238 100644 --- a/src/corelib/kernel/qjnitypes_impl.h +++ b/src/corelib/kernel/qjnitypes_impl.h @@ -134,9 +134,12 @@ struct CTString // Helper types that allow us to disable variadic overloads that would conflict // with overloads that take a const char*. template struct IsStringType : std::false_type {}; -template<> struct IsStringType : std::true_type {}; +template<> struct IsStringType : std::true_type {}; +template<> struct IsStringType : std::true_type {}; template struct IsStringType> : std::true_type {}; template struct IsStringType : std::true_type {}; +template struct IsStringType : std::true_type {}; +template struct IsStringType : std::true_type {}; template struct Traits { diff --git a/tests/auto/corelib/kernel/qjniobject/testdata/src/org/qtproject/qt/android/testdata/QtJniObjectTestClass.java b/tests/auto/corelib/kernel/qjniobject/testdata/src/org/qtproject/qt/android/testdata/QtJniObjectTestClass.java index 14cc8d265d3..5b983407a3c 100644 --- a/tests/auto/corelib/kernel/qjniobject/testdata/src/org/qtproject/qt/android/testdata/QtJniObjectTestClass.java +++ b/tests/auto/corelib/kernel/qjniobject/testdata/src/org/qtproject/qt/android/testdata/QtJniObjectTestClass.java @@ -127,6 +127,9 @@ public class QtJniObjectTestClass public static String staticStringMethod() { return A_STRING_OBJECT; } public String stringMethod() { return staticStringMethod(); } + // -------------------------------------------------------------------------------------------- + public static String staticEchoMethod(String value) { return value; } + // -------------------------------------------------------------------------------------------- public static Throwable staticThrowableMethod() { return A_THROWABLE_OBJECT; } public Throwable throwableMethod() { return staticThrowableMethod(); } diff --git a/tests/auto/corelib/kernel/qjniobject/tst_qjniobject.cpp b/tests/auto/corelib/kernel/qjniobject/tst_qjniobject.cpp index 2f7652dc3b6..6af3c300141 100644 --- a/tests/auto/corelib/kernel/qjniobject/tst_qjniobject.cpp +++ b/tests/auto/corelib/kernel/qjniobject/tst_qjniobject.cpp @@ -117,6 +117,7 @@ private slots: void callback_data(); void callback(); + void callStaticOverloadResolution(); void cleanupTestCase(); }; @@ -2009,6 +2010,19 @@ void tst_QJniObject::callback() QCOMPARE(result, int(parameterType)); } +// Make sure the new callStaticMethod overload taking a class, return type, +// and argument as template parameters, doesn't break overload resolution +// and that the class name doesn't get interpreted as the function name. +void tst_QJniObject::callStaticOverloadResolution() +{ + const QString value = u"Hello World"_s; + QJniObject str = QJniObject::fromString(value); + const auto result = QJniObject::callStaticMethod( + QtJniTypes::Traits::className(), + "staticEchoMethod", str.object()).toString(); + QCOMPARE(result, value); +} + QTEST_MAIN(tst_QJniObject) #include "tst_qjniobject.moc" diff --git a/tests/auto/corelib/kernel/qjnitypes/tst_qjnitypes.cpp b/tests/auto/corelib/kernel/qjnitypes/tst_qjnitypes.cpp index f5a0ae7d751..bf582041f32 100644 --- a/tests/auto/corelib/kernel/qjnitypes/tst_qjnitypes.cpp +++ b/tests/auto/corelib/kernel/qjnitypes/tst_qjnitypes.cpp @@ -22,6 +22,7 @@ private slots: void initTestCase(); void nativeMethod(); void construct(); + void stringTypeCantBeArgument(); }; struct QtJavaWrapper {}; @@ -227,6 +228,57 @@ void tst_QJniTypes::construct() QCOMPARE(str3.toString(), text); } +template +static constexpr bool isValidArgument(Arg &&...) noexcept +{ + return QtJniTypes::ValidSignatureTypesDetail...>; +} + +enum class Overload +{ + ClassNameAndMethod, + OnlyMethod, +}; + +template = true +#endif +> +static constexpr auto callStaticMethod(const char *className, const char *methodName, Args &&...) +{ + Q_UNUSED(className); + Q_UNUSED(methodName); + return Overload::ClassNameAndMethod; +} + +template = true +#endif +> +static constexpr auto callStaticMethod(const char *methodName, Args &&...) +{ + Q_UNUSED(methodName); + return Overload::OnlyMethod; +} + +void tst_QJniTypes::stringTypeCantBeArgument() +{ + const char *methodName = "staticEchoMethod"; + + static_assert(!isValidArgument(QtJniTypes::Traits::className())); + static_assert(!isValidArgument("someFunctionName")); + static_assert(!isValidArgument(methodName)); + static_assert(!isValidArgument(QtJniTypes::Traits::className(), + "someFunctionName", methodName, 42)); + + static_assert(callStaticMethod("class name", "method name", 42) + == Overload::ClassNameAndMethod); + static_assert(callStaticMethod("method name", 42) + == Overload::OnlyMethod); +} + QTEST_MAIN(tst_QJniTypes) #include "tst_qjnitypes.moc"