Skip to content
This repository has been archived by the owner on Mar 8, 2020. It is now read-only.

Commit

Permalink
address review feedback
Browse files Browse the repository at this point in the history 
 - better error handling
 - better string literals
 - doc added

Signed-off-by: Alexander Bezzubov <bzz@apache.org>
  • Loading branch information
@bzz
bzz committed Jul 4, 2019
1 parent 52e3e65 commit c8091e6
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 71 deletions.
105 changes: 68 additions & 37 deletions src/main/native/jni_utils.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,49 +20,78 @@ JNIEnv *getJNIEnv() {
return pEnv;
}

// Class fully qualified names
const char *CLS_NODE = "org/bblfsh/client/v2/Node";
const char *CLS_CTX = "org/bblfsh/client/v2/Context";
const char *CLS_OBJ = "java/lang/Object";
const char *CLS_RE = "java/lang/RuntimeException";

const char *CLS_JNODE = "org/bblfsh/client/v2/JNode";
const char *CLS_JNULL = "org/bblfsh/client/v2/JNull";
const char *CLS_JSTR = "org/bblfsh/client/v2/JString";
const char *CLS_JINT = "org/bblfsh/client/v2/JInt";
const char *CLS_JFLT = "org/bblfsh/client/v2/JFloat";
const char *CLS_JBOL = "org/bblfsh/client/v2/JBool";
const char *CLS_JUINT = "org/bblfsh/client/v2/JUint";
const char *CLS_JARR = "org/bblfsh/client/v2/JArray";
const char *CLS_JOBJ = "org/bblfsh/client/v2/JObject";

// Method signatures
const char *METHOD_JNODE_KEY_AT = "(I)Ljava/lang/String;";
const char *METHOD_JNODE_VALUE_AT = "(I)Lorg/bblfsh/client/v2/JNode;";
const char *METHOD_JOBJ_ADD =
const char CLS_NODE[] = "org/bblfsh/client/v2/Node";
const char CLS_CTX[] = "org/bblfsh/client/v2/Context";
const char CLS_OBJ[] = "java/lang/Object";
const char CLS_RE[] = "java/lang/RuntimeException";
const char CLS_JNODE[] = "org/bblfsh/client/v2/JNode";
const char CLS_JNULL[] = "org/bblfsh/client/v2/JNull";
const char CLS_JSTR[] = "org/bblfsh/client/v2/JString";
const char CLS_JINT[] = "org/bblfsh/client/v2/JInt";
const char CLS_JFLT[] = "org/bblfsh/client/v2/JFloat";
const char CLS_JBOOL[] = "org/bblfsh/client/v2/JBool";
const char CLS_JUINT[] = "org/bblfsh/client/v2/JUint";
const char CLS_JARR[] = "org/bblfsh/client/v2/JArray";
const char CLS_JOBJ[] = "org/bblfsh/client/v2/JObject";

const char METHOD_JNODE_KEY_AT[] = "(I)Ljava/lang/String;";
const char METHOD_JNODE_VALUE_AT[] = "(I)Lorg/bblfsh/client/v2/JNode;";
const char METHOD_JOBJ_ADD[] =
"(Ljava/lang/String;Lorg/bblfsh/client/v2/JNode;)Lscala/collection/"
"mutable/Buffer;";
const char *METHOD_JARR_ADD =
const char METHOD_JARR_ADD[] =
"(Lorg/bblfsh/client/v2/JNode;)Lscala/collection/mutable/Buffer;";

const char *METHOD_OBJ_TO_STR = "()Ljava/lang/String;";
const char METHOD_OBJ_TO_STR[] = "()Ljava/lang/String;";

// TODO(bzz): cache classes&methods in JNI_OnLoad should speed this up
void checkJvmException(std::string msg) {
JNIEnv *env = getJNIEnv();
if (env->ExceptionCheck()) {
// env->ExceptionDescribe(); // prints to stdout, un-comment for debuging
auto err = env->ExceptionOccurred();
auto err = env->ExceptionOccurred();
if (err) {
env->ExceptionClear();

auto exceptionCls = env->FindClass(CLS_RE);
if (env->ExceptionCheck()) {
env->ExceptionClear();
env->Throw(env->ExceptionOccurred());
return;
}

jclass cls = env->FindClass(CLS_OBJ);
if (env->ExceptionCheck()) {
env->ExceptionClear();
env->ThrowNew(
exceptionCls,
msg.append(" - failed to find class ").append(CLS_OBJ).data());
return;
}

jmethodID toString = env->GetMethodID(cls, "toString", METHOD_OBJ_TO_STR);
if (env->ExceptionCheck()) {
env->ExceptionClear();
env->ThrowNew(exceptionCls,
msg.append(" - failed to find method toString").data());
return;
}

jmethodID toString = env->GetMethodID(env->FindClass(CLS_OBJ), "toString",
METHOD_OBJ_TO_STR);
jstring s = (jstring)env->CallObjectMethod(err, toString);
if (env->ExceptionCheck() || !s) {
env->ThrowNew(exceptionCls,
msg.append(" - failed co call method toString").data());
return;
}

const char *utf = env->GetStringUTFChars(s, 0);
env->ReleaseStringUTFChars(s, utf);

env->ExceptionClear();
// new RuntimeException(msg.data(), err)
jmethodID initId = env->GetMethodID(
cls, "<init>", "(Ljava/lang/String;Ljava/lang/Throwable;)V");
jthrowable exception = (jthrowable)env->NewObject(
exceptionCls, initId, msg.append(": ").append(utf).data(), err);

auto exception = env->FindClass(CLS_RE);
env->ThrowNew(exception, msg.append(": ").append(utf).data());
env->Throw(exception);
}
}

Expand All @@ -72,8 +101,10 @@ jobject NewJavaObject(JNIEnv *env, const char *className, const char *initSign,
checkJvmException(std::string("failed to find a class ").append(className));

jmethodID initId = env->GetMethodID(cls, "<init>", initSign);
checkJvmException(
std::string("failed to call <inti> constructor for ").append(className));
checkJvmException(std::string("failed to call a constructor with signature ")
.append(initSign)
.append(" for the class name ")
.append(className));

va_list varargs;
va_start(varargs, initSign);
Expand All @@ -87,7 +118,7 @@ jobject NewJavaObject(JNIEnv *env, const char *className, const char *initSign,

jfieldID getField(JNIEnv *env, jobject obj, const char *name) {
jclass cls = env->GetObjectClass(obj);
checkJvmException("failed get an object class");
checkJvmException("failed get the class of an object");

jfieldID jfid = env->GetFieldID(cls, name, "J");
checkJvmException(std::string("failed get a field ").append(name));
Expand All @@ -101,7 +132,7 @@ static jmethodID MethodID(JNIEnv *env, const char *method,
checkJvmException(std::string("failed to find a class ").append(className));

jmethodID mId = env->GetMethodID(cls, method, signature);
checkJvmException(std::string("failed get a method ")
checkJvmException(std::string("failed to get method ")
.append(className)
.append(".")
.append(method));
Expand All @@ -112,13 +143,13 @@ static jmethodID MethodID(JNIEnv *env, const char *method,
jint IntMethod(JNIEnv *env, const char *method, const char *signature,
const char *className, const jobject *object) {
jmethodID mId = MethodID(env, method, signature, className);
checkJvmException(std::string("failed get a method ")
checkJvmException(std::string("failed to get method ")
.append(className)
.append(".")
.append(method));

jint res = env->CallIntMethod(*object, mId);
checkJvmException(std::string("failed call a method ")
checkJvmException(std::string("failed to call method ")
.append(className)
.append(".")
.append(method)
Expand All @@ -131,7 +162,7 @@ jint IntMethod(JNIEnv *env, const char *method, const char *signature,
jobject ObjectMethod(JNIEnv *env, const char *method, const char *signature,
const char *className, const jobject *object, ...) {
jmethodID mId = MethodID(env, method, signature, className);
checkJvmException(std::string("failed get a method ")
checkJvmException(std::string("failed to get method ")
.append(className)
.append(".")
.append(method));
Expand Down
52 changes: 29 additions & 23 deletions src/main/native/jni_utils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,28 +4,34 @@
#include <jni.h>
#include <string>

extern const char *CLS_NODE;
extern const char *CLS_CTX;
extern const char *CLS_OBJ;
extern const char *CLS_RE;

extern const char *CLS_JNODE;
extern const char *CLS_JNULL;
extern const char *CLS_JSTR;
extern const char *CLS_JINT;
extern const char *CLS_JFLT;
extern const char *CLS_JBOL;
extern const char *CLS_JUINT;
extern const char *CLS_JARR;
extern const char *CLS_JOBJ;

extern const char *METHOD_JNODE_KEY_AT;
extern const char *METHOD_JNODE_VALUE_AT;
extern const char *METHOD_JOBJ_ADD;
extern const char *METHOD_JARR_ADD;

extern const char *METHOD_OBJ_TO_STR;

// Fully qualified Java class names
extern const char CLS_NODE[];
extern const char CLS_CTX[];
extern const char CLS_OBJ[];
extern const char CLS_RE[];

// Fully qualified class names for Bablefish UAST types
extern const char CLS_JNODE[];
extern const char CLS_JNULL[];
extern const char CLS_JSTR[];
extern const char CLS_JINT[];
extern const char CLS_JFLT[];
extern const char CLS_JBOOL[];
extern const char CLS_JUINT[];
extern const char CLS_JARR[];
extern const char CLS_JOBJ[];

// Method signatures
extern const char METHOD_JNODE_KEY_AT[];
extern const char METHOD_JNODE_VALUE_AT[];
extern const char METHOD_JOBJ_ADD[];
extern const char METHOD_JARR_ADD[];
extern const char METHOD_OBJ_TO_STR[];

// Checks though JNI, if there is a pending excption on JVM side.
//
// Throws new RuntimeExpection to JVM in case there is,
// uses the origial one as a cause and the given string as a message.
void checkJvmException(std::string);

JNIEnv *getJNIEnv();
Expand All @@ -38,4 +44,4 @@ jint IntMethod(JNIEnv *, const char *, const char *, const char *,
jobject ObjectMethod(JNIEnv *, const char *, const char *, const char *,
const jobject *, ...);

#endif
#endif
6 changes: 3 additions & 3 deletions src/main/native/org_bblfsh_client_v2_libuast_Libuast.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ class Node : public uast::Node<Node *> {
return NODE_INT;
} else if (env->IsInstanceOf(obj, env->FindClass(CLS_JFLT))) {
return NODE_FLOAT;
} else if (env->IsInstanceOf(obj, env->FindClass(CLS_JBOL))) {
} else if (env->IsInstanceOf(obj, env->FindClass(CLS_JBOOL))) {
return NODE_BOOL;
} else if (env->IsInstanceOf(obj, env->FindClass(CLS_JUINT))) {
return NODE_BOOL;
Expand Down Expand Up @@ -378,8 +378,8 @@ class Interface : public uast::NodeCreator<Node *> {
return create(NODE_FLOAT, i);
}
Node *NewBool(bool v) {
jobject i = NewJavaObject(getJNIEnv(), CLS_JBOL, "(Z)V", v);
checkJvmException("failed to create new " + std::string(CLS_JBOL));
jobject i = NewJavaObject(getJNIEnv(), CLS_JBOOL, "(Z)V", v);
checkJvmException("failed to create new " + std::string(CLS_JBOOL));
return create(NODE_BOOL, i);
}
};
Expand Down
2 changes: 1 addition & 1 deletion src/main/scala/org/bblfsh/client/v2/Context.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ case class Context(nativeContext: Long) {
@native def encode(n: Node): ByteBuffer
@native def dispose()

@native def filter() // TODO(bzz)
@native def filter()

// TODO(bzz): add loading of the root node, after clarifying when it's needed
// https://github.com/bblfsh/client-python/blob/master/bblfsh/pyuast.cc#L364
Expand Down
16 changes: 9 additions & 7 deletions src/main/scala/org/bblfsh/client/v2/JNode.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ import java.io.Serializable
import scala.collection.mutable

/** UAST nodes representation on the JVM side.
*
* Mirrors https://godoc.org/github.com/bblfsh/sdk/uast/nodes
*/
*
* Mirrors https://godoc.org/github.com/bblfsh/sdk/uast/nodes
*/
sealed abstract class JNode {
/* Dynamic dispatch is a convenience to be called from JNI */
def children: Seq[JNode] = this match {
case JObject(l) => l map (_._2)
case JArray(l) => l
Expand All @@ -19,8 +20,7 @@ sealed abstract class JNode {
case JObject(l) => l.size
case JArray(l) => l.size
case JString(l) => l.length
case JNothing => 0
case _ => 1
case _ => 0
}

def keyAt(i: Int): String = this match {
Expand All @@ -45,7 +45,9 @@ case object JNothing extends JNode // 'zero' value for JNode
case class JNull() extends JNode
case class JString(str: String) extends JNode
case class JFloat(num: Double) extends JNode
case class JUint(num: Long) extends JNode
case class JUint(num: Long) extends JNode {
def get(): Long = java.lang.Integer.toUnsignedLong(num.toInt)
}
case class JInt(num: Long) extends JNode
case class JBool(value: Boolean) extends JNode

Expand Down Expand Up @@ -75,4 +77,4 @@ case object JArray {
ja.arr ++= ns
ja
}
}
}

0 comments on commit c8091e6

Please sign in to comment.