stevenschlansker commented on code in PR #2338:
URL: https://github.com/apache/fory/pull/2338#discussion_r2153328948
##########
java/fory-format/src/main/java/org/apache/fory/format/encoder/BaseBinaryEncoderBuilder.java:
##########
@@ -164,15 +167,21 @@ protected Expression serializeFor(
serializeFor(
ordinal, newInputObject, writer, rewrittenType, arrowField,
visitedCustomTypes);
return new If(
- ExpressionUtils.eqNull(inputObject),
+ new Expression.IsNull(inputObject),
new Invoke(writer, "setNullAt", ordinal),
doSerialize);
} else if (rawType == Optional.class) {
TypeRef<?> elemType = TypeUtils.getTypeArguments(typeRef).get(0);
Invoke orNull =
- new Invoke(inputObject, "orElse", TypeUtils.OBJECT_TYPE, new
Expression.Null(elemType));
+ new Invoke(
+ inputObject,
+ "orElse",
+ "",
+ TypeUtils.OBJECT_TYPE,
+ true,
+ new Expression.Null(elemType));
Expression unwrapped =
- new If(ExpressionUtils.eqNull(inputObject), new
Expression.Null(elemType), orNull);
Review Comment:
It seems there is usually a local variable holding whether the input value
is null, but `ExpressionUtils.eqNull` does not use it - `Expression.IsNull`
does. But then I found it was somewhat broken, so there's a few fixes included
to make this work better.
This reduces dead code computing if a field is null that is not used, then
we re-compute whether it is null again later for the `if()` statement
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
