Add companion classes to Kotlin reflective hierarchy registration #38062
Conversation
geoand
commented
Jan 5, 2024
•
geoand
commented
- Fixes: Serialization failure in native mode when Kotlin companion object used #37957
| // for Kotlin classes, we need to register the nested classes as well because companion classes are very often necessary at runtime | ||
| if (capabilities.isPresent(Capability.KOTLIN) && isKotlinClass(info)) { | ||
| ClassLoader classLoader = Thread.currentThread().getContextClassLoader(); | ||
| try { | ||
| Class<?>[] declaredClasses = classLoader.loadClass(info.name().toString()).getDeclaredClasses(); | ||
| for (Class<?> clazz : declaredClasses) { | ||
| DotName dotName = DotName.createSimple(clazz.getName()); | ||
| addClassTypeHierarchy(combinedIndexBuildItem, capabilities, reflectiveHierarchyBuildItem, source, | ||
| dotName, dotName, | ||
| processedReflectiveHierarchies, unindexedClasses, | ||
| finalFieldsWritable, reflectiveClass, visits); | ||
| } | ||
| } catch (ClassNotFoundException e) { | ||
| log.warnf(e, "Failed to load Class %s", info.name().toString()); | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
Can we move it to the kotlin extension? It feels weird to have this in core
There was a problem hiding this comment.
Not really, the handling of reflection is in the core
There was a problem hiding this comment.
Perhaps the kotlin extension may produce additional build items for this step to consume?
There was a problem hiding this comment.
The way I see it, that's really too much work for almost no gain.
I'm certainly not going to do it, but I won't stop anyone else from taking it up :)
There was a problem hiding this comment.
I think we can live with it. We can revisit if one day we move the Kotlin extension outside of the core repository.
| // for Kotlin classes, we need to register the nested classes as well because companion classes are very often necessary at runtime | ||
| if (capabilities.isPresent(Capability.KOTLIN) && isKotlinClass(info)) { | ||
| ClassLoader classLoader = Thread.currentThread().getContextClassLoader(); | ||
| try { | ||
| Class<?>[] declaredClasses = classLoader.loadClass(info.name().toString()).getDeclaredClasses(); | ||
| for (Class<?> clazz : declaredClasses) { | ||
| DotName dotName = DotName.createSimple(clazz.getName()); | ||
| addClassTypeHierarchy(combinedIndexBuildItem, capabilities, reflectiveHierarchyBuildItem, source, | ||
| dotName, dotName, | ||
| processedReflectiveHierarchies, unindexedClasses, | ||
| finalFieldsWritable, reflectiveClass, visits); | ||
| } | ||
| } catch (ClassNotFoundException e) { | ||
| log.warnf(e, "Failed to load Class %s", info.name().toString()); | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
I think we can live with it. We can revisit if one day we move the Kotlin extension outside of the core repository.
Failing Jobs - Building e185df8
Full information is available in the Build summary check run. Failures⚙️ JVM Tests - JDK 17 Windows #- Failing: extensions/smallrye-reactive-messaging-amqp/deployment
! Skipped: integration-tests/reactive-messaging-amqp integration-tests/virtual-threads/amqp-virtual-threads 📦 extensions/smallrye-reactive-messaging-amqp/deployment✖ ✖ ✖ ✖ ✖ |
