-
Notifications
You must be signed in to change notification settings - Fork 3k
Reflection-free deserializers - Better handle basic types #47553
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
4f79ea1
to
ab8d361
Compare
static boolean isBoxedPrimitive(String typeName) { | ||
return "java.lang.Character".equals(typeName) || "java.lang.Short".equals(typeName) | ||
|| "java.lang.Integer".equals(typeName) || "java.lang.Long".equals(typeName) | ||
|| "java.lang.Float".equals(typeName) || "java.lang.Double".equals(typeName) | ||
|| "java.lang.Boolean".equals(typeName); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ladicek I was wondering if we should have a util in Jandex for this. I couldn't find one but maybe there is one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't have one, but it makes sense. I'll add PrimitiveType.isBox(Type)
in the next version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏽
That could probably be used in a lot of places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ladicek awesome!
static ResultHandle getDefaultValue(BytecodeCreator bytecodeCreator, Type type) { | ||
if (type.kind() != Kind.PRIMITIVE) { | ||
return bytecodeCreator.loadNull(); | ||
} | ||
|
||
return switch (type.name().toString()) { | ||
case "byte" -> bytecodeCreator.load(DEFAULT_BYTE); | ||
case "boolean" -> bytecodeCreator.load(DEFAULT_BOOLEAN); | ||
case "char" -> bytecodeCreator.load(DEFAULT_CHAR); | ||
case "double" -> bytecodeCreator.load(DEFAULT_DOUBLE); | ||
case "float" -> bytecodeCreator.load(DEFAULT_FLOAT); | ||
case "int" -> bytecodeCreator.load(DEFAULT_INT); | ||
case "long" -> bytecodeCreator.load(DEFAULT_LONG); | ||
case "short" -> bytecodeCreator.load(DEFAULT_SHORT); | ||
default -> throw new IllegalStateException("Type " + type + " should be handled by the switch"); | ||
}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ladicek do you see a better way to handle that? I need to assign a value to the ResultHandle
, including for primitive types and I ended up with this but I was wondering if there was a better way to handle that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we have a method to load a default value based on given type. What you have is close to the best. (I'd personally avoid the static fields and just pass (byte) 0
, false
, '\0'
, 0.0
, 0.0F
, 0
, 0L
, (short) 0
, but there's almost zero difference.)
(I actually think we don't have that in Gizmo 2 either. I'll make sure we add it, because it makes sense.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ladicek yes, Gizmo 2 is exactly where I expected this to go. I thought it was useful input from real world usage.
/cc @mariofusco |
Status for workflow
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job, thank you!
Fixes #47478