Commit 0165af5b authored by Jacob Potter's avatar Jacob Potter

Support collections of non-nullable interfaces properly

Fixes #143
parent 648995b0
......@@ -41,6 +41,10 @@ class JNIGenerator(spec: Spec) extends Generator(spec) {
jniHpp.add("#include " + q(spec.jniIncludeCppPrefix + spec.cppFileIdentStyle(name) + "." + spec.cppHeaderExt))
jniHpp.add("#include " + q(spec.jniBaseLibIncludePrefix + "djinni_support.hpp"))
spec.cppNnHeader match {
case Some(nnHdr) => jniHpp.add("#include " + nnHdr)
case _ =>
}
def find(ty: TypeRef) { find(ty.resolved) }
def find(tm: MExpr) {
......@@ -185,15 +189,31 @@ class JNIGenerator(spec: Spec) extends Generator(spec) {
writeJniTypeParams(w, typeParams)
w.w(s"class $jniSelf final : $baseType").bracedSemi {
w.wlOutdent(s"public:")
spec.cppNnType match {
case Some(nnPtr) =>
w.wl(s"using CppType = ${nnPtr}<$cppSelf>;")
w.wl(s"using CppOptType = std::shared_ptr<$cppSelf>;")
case _ =>
w.wl(s"using CppType = std::shared_ptr<$cppSelf>;")
}
w.wl(s"using JniType = jobject;")
w.wl
w.wl(s"using Boxed = $jniSelf;")
w.wl
w.wl(s"~$jniSelf();")
w.wl
if (spec.cppNnType.nonEmpty) {
def nnCheck(expr: String): String = spec.cppNnCheckExpression.fold(expr)(check => s"$check($expr)")
w.w("static CppType toCpp(JNIEnv* jniEnv, JniType j)").bracedSemi {
w.wl(s"""DJINNI_ASSERT_MSG(j, jniEnv, "$jniSelf::fromCpp requires a non-null Java object");""")
w.wl(s"""return ${nnCheck(s"::djinni::JniClass<$jniSelf>::get()._fromJava(jniEnv, j)")};""")
}
w.wl(s"static ::djinni::LocalRef<JniType> fromCppOpt(JNIEnv* jniEnv, const CppOptType& c) { return {jniEnv, ::djinni::JniClass<$jniSelf>::get()._toJava(jniEnv, c)}; }")
w.wl(s"static ::djinni::LocalRef<JniType> fromCpp(JNIEnv* jniEnv, const CppType& c) { return fromCppOpt(jniEnv, c); }")
} else {
w.wl(s"static CppType toCpp(JNIEnv* jniEnv, JniType j) { return ::djinni::JniClass<$jniSelf>::get()._fromJava(jniEnv, j); }")
w.wl(s"static ::djinni::LocalRef<JniType> fromCpp(JNIEnv* jniEnv, const CppType& c) { return {jniEnv, ::djinni::JniClass<$jniSelf>::get()._toJava(jniEnv, c)}; }")
}
w.wl
w.wlOutdent("private:")
w.wl(s"$jniSelf();")
......@@ -268,16 +288,18 @@ class JNIGenerator(spec: Spec) extends Generator(spec) {
w.w(")")
w.wl(";")
w.wl(s"::djinni::jniExceptionCheck(jniEnv);")
m.ret.fold()(ty => (spec.cppNnCheckExpression, isInterface(ty.resolved)) match {
m.ret.fold()(ty => {
(spec.cppNnCheckExpression, isInterface(ty.resolved)) match {
case (Some(check), true) => {
// We have a non-optional interface, assert that we're getting a non-null value
val javaParams = m.params.map(p => javaMarshal.fqParamType(p.ty) + " " + idJava.local(p.ident))
val javaParamsString: String = javaParams.mkString("(", ",", ")")
val functionString: String = s"${javaMarshal.fqTypename(ident, i)}#$javaMethodName$javaParamsString"
w.wl(s"""DJINNI_ASSERT_MSG(jret, jniEnv, "Got unexpected null return value from function $functionString");""")
w.wl(s"return ${check}(${jniMarshal.toCpp(ty, "jret")});")
w.wl(s"return ${jniMarshal.toCpp(ty, "jret")};")
}
case _ =>
}
w.wl(s"return ${jniMarshal.toCpp(ty, "jret")};")
})
}
......@@ -334,13 +356,7 @@ class JNIGenerator(spec: Spec) extends Generator(spec) {
val methodName = idCpp.method(m.ident)
val ret = m.ret.fold("")(r => "auto r = ")
val call = if (m.static) s"$cppSelf::$methodName(" else s"ref->$methodName("
writeAlignedCall(w, ret + call, m.params, ")", p => {
val v = jniMarshal.toCpp(p.ty, "j_" + idJava.local(p.ident))
(spec.cppNnCheckExpression, isInterface(p.ty.resolved)) match {
case (Some(check), true) => s"$check($v)"
case _ => v
}
})
writeAlignedCall(w, ret + call, m.params, ")", p => jniMarshal.toCpp(p.ty, "j_" + idJava.local(p.ident)))
w.wl(";")
m.ret.fold()(r => w.wl(s"return ::djinni::release(${jniMarshal.fromCpp(r, "r")});"))
})
......
......@@ -31,13 +31,7 @@ class JNIMarshal(spec: Spec) extends Marshal(spec) {
// Name for the autogenerated class containing field/method IDs and toJava()/fromJava() methods
def helperClass(name: String) = spec.jniClassIdentStyle(name)
private def helperClass(tm: MExpr): String = {
if (isOptionalInterface(tm)) {
helperClass(tm.args.head)
} else {
helperName(tm) + helperTemplates(tm)
}
}
private def helperClass(tm: MExpr): String = helperName(tm) + helperTemplates(tm)
def references(m: Meta, exclude: String = ""): Seq[SymbolReference] = m match {
case o: MOpaque => List(ImportRef(q(spec.jniBaseLibIncludePrefix + "Marshal.hpp")))
......@@ -120,7 +114,7 @@ class JNIMarshal(spec: Spec) extends Marshal(spec) {
tm.base match {
case MOptional =>
assert(tm.args.size == 1)
assert(!isInterface(tm.args.head))
//assert(!isInterface(tm.args.head))
val argHelperClass = helperClass(tm.args.head)
s"<${spec.cppOptionalTemplate}, $argHelperClass>"
case MList | MSet =>
......
......@@ -55,6 +55,8 @@ class ObjcppGenerator(spec: Spec) extends Generator(spec) {
def headerName(ident: String): String = idObjc.ty(ident) + "." + spec.objcHeaderExt
def privateBodyName(ident: String): String = idObjc.ty(ident) + "+Private." + spec.objcppExt
def nnCheck(expr: String): String = spec.cppNnCheckExpression.fold(expr)(check => s"$check($expr)")
override def generateInterface(origin: String, ident: Ident, doc: Doc, typeParams: Seq[TypeParam], i: Interface) {
val refs = new ObjcRefs()
i.methods.map(m => {
......@@ -89,13 +91,24 @@ class ObjcppGenerator(spec: Spec) extends Generator(spec) {
wrapNamespace(w, spec.objcppNamespace, w => {
w.wl(s"class $helperClass").bracedSemi {
w.wlOutdent("public:")
spec.cppNnType match {
case Some(nnPtr) =>
w.wl(s"using CppType = ${nnPtr}<$cppSelf>;")
w.wl(s"using CppOptType = std::shared_ptr<$cppSelf>;")
case _ =>
w.wl(s"using CppType = std::shared_ptr<$cppSelf>;")
}
w.wl("using ObjcType = " + (if(i.ext.objc) s"id<$self>" else s"$self*") + ";");
w.wl
w.wl(s"using Boxed = $helperClass;")
w.wl
w.wl(s"static CppType toCpp(ObjcType objc);")
if (spec.cppNnType.nonEmpty) {
w.wl(s"static ObjcType fromCppOpt(const CppOptType& cpp);")
w.wl(s"static ObjcType fromCpp(const CppType& cpp) { return fromCppOpt(cpp); }")
} else {
w.wl(s"static ObjcType fromCpp(const CppType& cpp);")
}
w.wl
w.wlOutdent("private:")
w.wl("class ObjcProxy;")
......@@ -164,13 +177,7 @@ class ObjcppGenerator(spec: Spec) extends Generator(spec) {
})
val ret = m.ret.fold("")(_ => "auto r = ")
val call = ret + (if (!m.static) "_cppRefHandle.get()->" else cppSelf + "::") + idCpp.method(m.ident) + "("
writeAlignedCall(w, call, m.params, ")", p => {
val v = objcppMarshal.toCpp(p.ty, idObjc.local(p.ident.name))
(spec.cppNnCheckExpression, isInterface(p.ty.resolved)) match {
case (Some(check), true) => s"$check($v)"
case _ => v
}
})
writeAlignedCall(w, call, m.params, ")", p => objcppMarshal.toCpp(p.ty, idObjc.local(p.ident.name)))
w.wl(";")
m.ret.fold()(r => w.wl(s"return ${objcppMarshal.fromCpp(r, "r")};"))
......@@ -198,10 +205,10 @@ class ObjcppGenerator(spec: Spec) extends Generator(spec) {
val call = s"[Handle::get() ${idObjc.method(m.ident)}"
writeAlignedObjcCall(w, ret + call, m.params, "]", p => (idObjc.field(p.ident), s"(${objcppMarshal.fromCpp(p.ty, "c_" + idCpp.local(p.ident))})"))
w.wl(";")
m.ret.fold()(ty => (spec.cppNnCheckExpression, isInterface(ty.resolved)) match {
case (Some(check), true) => {
// We have a non-optional interface, assert that we're getting a non-null value
// and put it into a non-null pointer
m.ret.fold()(ty => {
if (spec.cppNnCheckExpression.nonEmpty && isInterface(ty.resolved)) {
// We have a non-optional interface, so assert that we're getting a non-null value
// before putting it into a non-null pointer
val stringWriter = new StringWriter()
writeObjcFuncDecl(m, new IndentWriter(stringWriter))
val singleLineFunctionDecl = stringWriter.toString.replaceAll("\n *", " ")
......@@ -209,9 +216,7 @@ class ObjcppGenerator(spec: Spec) extends Generator(spec) {
w.w(s"if (r == nil)").braced {
w.wl(s"""throw std::invalid_argument("$exceptionReason");""")
}
w.wl(s"return ${check}(${objcppMarshal.toCpp(ty, "r")});")
}
case _ =>
w.wl(s"return ${objcppMarshal.toCpp(ty, "r")};")
})
}
......@@ -227,26 +232,35 @@ class ObjcppGenerator(spec: Spec) extends Generator(spec) {
w.wl(s"auto $helperClass::toCpp(ObjcType objc) -> CppType").braced {
// Handle null
w.w("if (!objc)").braced {
if (spec.cppNnType.isEmpty) {
w.wl("return nullptr;")
} else {
w.wl(s"""throw std::invalid_argument("$helperClass::toCpp requires non-nil object");""")
}
}
if (i.ext.cpp && !i.ext.objc) {
// C++ only. In this case we generate a class instead of a protocol, so
// we don't have to do any casting at all, just access cppRef directly.
w.wl(s"return objc->_cppRefHandle.get();")
w.wl("return " + nnCheck("objc->_cppRefHandle.get()") + ";")
//w.wl(s"return ${spec.cppNnCheckExpression.getOrElse("")}(objc->_cppRefHandle.get());")
} else {
// ObjC only, or ObjC and C++.
val objcExtSelf = objcppMarshal.helperClass("objc_proxy")
if (i.ext.cpp) {
// If it could be implemented in C++, we might have to unwrap a proxy object.
w.w(s"if ([(id)objc isKindOfClass:[$objcSelf class]])").braced {
w.wl(s"return (($objcSelf*)objc)->_cppRefHandle.get();")
val getProxyExpr = s"(($objcSelf*)objc)->_cppRefHandle.get()"
w.wl(s"return ${nnCheck(getProxyExpr)};")
}
}
w.wl(s"return ::djinni::get_objc_proxy<$objcExtSelf>(objc);")
val getProxyExpr = s"::djinni::get_objc_proxy<$objcExtSelf>(objc)"
w.wl(s"return ${nnCheck(getProxyExpr)};")
}
}
w.wl
w.wl(s"auto $helperClass::fromCpp(const CppType& cpp) -> ObjcType").braced {
val fromCppFunc = if (spec.cppNnType.isEmpty) "fromCpp(const CppType& cpp)"
else "fromCppOpt(const CppOptType& cpp)"
w.wl(s"auto $helperClass::$fromCppFunc -> ObjcType").braced {
// Handle null
w.w("if (!cpp)").braced {
w.wl("return nil;")
......
......@@ -56,13 +56,7 @@ class ObjcppMarshal(spec: Spec) extends Marshal(spec) {
}
def helperClass(name: String) = idCpp.ty(name)
private def helperClass(tm: MExpr): String = {
if (isOptionalInterface(tm)) {
helperClass(tm.args.head)
} else {
helperName(tm) + helperTemplates(tm)
}
}
private def helperClass(tm: MExpr): String = helperName(tm) + helperTemplates(tm)
def privateHeaderName(ident: String): String = idObjc.ty(ident) + "+Private." + spec.objcHeaderExt
......
......@@ -248,7 +248,14 @@ namespace djinni
template <template <class> class OptionalType, class T>
struct Optional
{
using CppType = OptionalType<typename T::CppType>;
// SFINAE helper: if C::CppOptType exists, opt_type<T>(nullptr) will return
// that type. If not, it returns OptionalType<C::CppType>. This is necessary
// because we special-case optional interfaces to be represented as a nullable
// std::shared_ptr<T>, not optional<shared_ptr<T>> or optional<nn<shared_ptr<T>>>.
template <typename C> static OptionalType<typename C::CppType> opt_type(...);
template <typename C> static typename C::CppOptType opt_type(typename C::CppOptType *);
using CppType = decltype(opt_type<T>(nullptr));
using JniType = typename T::Boxed::JniType;
using Boxed = Optional;
......@@ -258,10 +265,16 @@ namespace djinni
return j ? CppType(T::Boxed::toCpp(jniEnv, j)) : CppType();
}
static LocalRef<JniType> fromCpp(JNIEnv* jniEnv, const CppType& c)
static LocalRef<JniType> fromCpp(JNIEnv* jniEnv, const OptionalType<typename T::CppType> &c)
{
return c ? T::Boxed::fromCpp(jniEnv, *c) : LocalRef<JniType>{};
}
// fromCpp used for nullable shared_ptr
template <typename C = T>
static LocalRef<JniType> fromCpp(JNIEnv* jniEnv, const typename C::CppOptType & cppOpt) {
return T::Boxed::fromCppOpt(jniEnv, cppOpt);
}
};
struct ListJniInfo
......
......@@ -37,8 +37,9 @@ struct CppProxyCacheTraits {
extern template class ProxyCache<CppProxyCacheTraits>;
using CppProxyCache = ProxyCache<CppProxyCacheTraits>;
// Helper for get_cpp_proxy_impl that takes a std::shared_ptr.
template <typename ObjcType, typename CppType>
ObjcType * get_cpp_proxy(const std::shared_ptr<CppType> & cppRef) {
ObjcType * get_cpp_proxy_impl(const std::shared_ptr<CppType> & cppRef) {
return CppProxyCache::get(
cppRef,
[] (const std::shared_ptr<void> & cppRef) -> std::pair<id, void *> {
......@@ -50,4 +51,11 @@ ObjcType * get_cpp_proxy(const std::shared_ptr<CppType> & cppRef) {
);
}
// get_cpp_proxy takes any smart pointer type, as long as it can be implicitly cast
// to std::shared_ptr. This means get_cpp_proxy can also be passed non-nullable pointers.
template <typename ObjcType, typename CppPtrType>
ObjcType * get_cpp_proxy(const CppPtrType & cppRef) {
return get_cpp_proxy_impl<ObjcType, typename std::remove_reference<decltype(*cppRef)>::type>(cppRef);
}
} // namespace djinni
......@@ -161,19 +161,37 @@ struct Binary {
template<template<class> class OptionalType, class T>
class Optional {
// SFINAE helper: if C::CppOptType exists, opt_type<T>(nullptr) will return
// that type. If not, it returns OptionalType<C::CppType>. This is necessary
// because we special-case optional interfaces to be represented as a nullable
// std::shared_ptr<T>, not optional<shared_ptr<T>> or optional<nn<shared_ptr<T>>>.
template <typename C> static OptionalType<typename C::CppType> opt_type(...);
template <typename C> static typename C::CppOptType opt_type(typename C::CppOptType *);
public:
using CppType = OptionalType<typename T::CppType>;
using CppType = decltype(opt_type<T>(nullptr));
using ObjcType = typename T::Boxed::ObjcType;
using Boxed = Optional;
static CppType toCpp(ObjcType obj) {
return obj ? CppType(T::Boxed::toCpp(obj)) : CppType();
if (obj) {
return T::Boxed::toCpp(obj);
} else {
return CppType();
}
}
static ObjcType fromCpp(const CppType& opt) {
// fromCpp used for normal optionals
static ObjcType fromCpp(const OptionalType<typename T::CppType>& opt) {
return opt ? T::Boxed::fromCpp(*opt) : nil;
}
// fromCpp used for nullable shared_ptr
template <typename C = T>
static ObjcType fromCpp(const typename C::CppOptType & cppOpt) {
return T::Boxed::fromCppOpt(cppOpt);
}
};
template<class T>
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment