-
Notifications
You must be signed in to change notification settings - Fork 133
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
Default value fixes in M3 compiler and reactivator #827
base: master
Are you sure you want to change the base?
Default value fixes in M3 compiler and reactivator #827
Conversation
7937efb
to
ca7de4f
Compare
...egend-pure-functions-base/legend-pure-runtime-java-extension-compiled-functions-base/pom.xml
Outdated
Show resolved
Hide resolved
...src/test/java/org/finos/legend/pure/m3/tests/elements/property/AbstractTestDefaultValue.java
Outdated
Show resolved
Hide resolved
...in/java/org/finos/legend/pure/m3/compiler/postprocessing/processor/AssociationProcessor.java
Outdated
Show resolved
Hide resolved
...ain/java/org/finos/legend/pure/runtime/java/compiled/generation/processors/support/Pure.java
Outdated
Show resolved
Hide resolved
ca7de4f
to
dea7e4b
Compare
@@ -721,13 +723,17 @@ public static Object newObject(Bridge bridge, org.finos.legend.pure.m3.coreinsta | |||
{ | |||
Class<?> c = ((CompiledExecutionSupport) es).getClassLoader().loadClass(JavaPackageAndImportBuilder.platformJavaPackage() + "." + Pure.elementToPath(aClass, "_", true) + "_Impl"); | |||
Any result = (Any) c.getConstructor(String.class).newInstance(name); | |||
MutableList<String> userAssignedParameters = Lists.mutable.empty(); |
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.
MutableList<String> userAssignedParameters = Lists.mutable.empty(); | |
MutableSet<String> userAssignedParameters = Sets.mutable.empty(); |
RichIterable<CoreInstance> classProperties = _Class.getSimpleProperties(aClass, ((CompiledExecutionSupport) es).getProcessorSupport()); | ||
classProperties.forEach(new CheckedProcedure<CoreInstance>() | ||
{ | ||
@Override | ||
public void safeValue(CoreInstance p) throws Exception | ||
{ | ||
if (!userAssignedParameters.contains(p.getName())) | ||
{ | ||
Property<?, ?> prop = (Property<?, ?>) p; | ||
DefaultValue defaultValue = prop._defaultValue(); | ||
if (defaultValue != null) | ||
{ | ||
Object res = reactivate(defaultValue._functionDefinition()._expressionSequence().getFirst(), new PureMap(Maps.fixedSize.empty()), bridge, es); | ||
Method method = c.getMethod("_" + prop._name(), RichIterable.class); | ||
if (res instanceof RichIterable) | ||
{ | ||
method.invoke(result, res); | ||
} | ||
else | ||
{ | ||
method.invoke(result, Lists.fixedSize.of(res)); | ||
} | ||
} | ||
} |
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 have methods now on AbstractCompiledCoreInstance for dealing with default values. Maybe result above can be cast directly to AbstractCompiledCoreInstance? In case it needs to be cast to Any above, I'm casting it in the suggestion below.
RichIterable<CoreInstance> classProperties = _Class.getSimpleProperties(aClass, ((CompiledExecutionSupport) es).getProcessorSupport()); | |
classProperties.forEach(new CheckedProcedure<CoreInstance>() | |
{ | |
@Override | |
public void safeValue(CoreInstance p) throws Exception | |
{ | |
if (!userAssignedParameters.contains(p.getName())) | |
{ | |
Property<?, ?> prop = (Property<?, ?>) p; | |
DefaultValue defaultValue = prop._defaultValue(); | |
if (defaultValue != null) | |
{ | |
Object res = reactivate(defaultValue._functionDefinition()._expressionSequence().getFirst(), new PureMap(Maps.fixedSize.empty()), bridge, es); | |
Method method = c.getMethod("_" + prop._name(), RichIterable.class); | |
if (res instanceof RichIterable) | |
{ | |
method.invoke(result, res); | |
} | |
else | |
{ | |
method.invoke(result, Lists.fixedSize.of(res)); | |
} | |
} | |
} | |
AbstractCompiledCoreInstance instance = (AbstractCompiledCoreInstance) result; | |
instance.getDefaultValueKeys().forEach(new CheckedProcedure<String>() | |
{ | |
@Override | |
public void safeValue(String p) throws Exception | |
{ | |
if (!userAssignedParameters.contains(p)) | |
{ | |
RichIterable<?> defaultValues = instance.getDefaultValue(p, es); | |
if ((defaultValues != null) && defaultValues.notEmpty()) | |
{ | |
Method method = c.getMethod("_" + prop._name(), RichIterable.class); | |
method.invoke(result, defaultValues); | |
} | |
} |
Fixes a bug in the compiled mode version of reactivator where evaluation of calls to new does not set default values defined on the class being instantiated. More specifically this bug occurs when the reactivator doesn't need to regenerate and compile Java code and can evaluate the call to new directly.
Fails compilation on association properties with default values. This was not supported in compiled mode and a new compiler check is added to not allow default values on association properties