Skip to content
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

Java Compilation fails for this function #863

Closed
gbrgr opened this issue Aug 11, 2022 · 12 comments
Closed

Java Compilation fails for this function #863

gbrgr opened this issue Aug 11, 2022 · 12 comments
Labels

Comments

@gbrgr
Copy link
Contributor

gbrgr commented Aug 11, 2022

Bug Report

Steps to Reproduce:

I have the following PURE implementation of a function:

function meta::nothing::inspect<T|m>(result: Result<T|m>[1], fn: FunctionDefinition<{T[m]->Any[*]}>[1]): Result<T|m>[1] {
  if($result.errors->isEmpty(), 
    {| $fn->eval($result.values->evaluateAndDeactivate()); $result;}, 
    | $result
  );
}

The class Result looks as follows:

Class meta::nothing::Result<T|n> {
  <<equality.Key>> values: T[n];
  <<equality.Key>> errors: Any[*];
}

The compiler generates the following Java Code for this function:

public static <T> org.finos.legend.pure.generated.Root_meta_nothing_Result<? extends T> Root_meta_nothing_inspect_Result_1__FunctionDefinition_1__Result_1_(final org.finos.legend.pure.generated.Root_meta_nothing_Result<? extends T> _result, final org.finos.legend.pure.m3.coreinstance.meta.pure.metamodel.function.FunctionDefinition<? extends java.lang.Object> _fn,final ExecutionSupport es)
{
    return ((org.finos.legend.pure.generated.Root_meta_nothing_Result<? extends T>)
            (CompiledSupport.isEmpty(_result._errors()) ?
                    new LambdaZero<org.finos.legend.pure.generated.Root_meta_nothing_Result<? extends T>>() {
                        public org.finos.legend.pure.generated.Root_meta_nothing_Result<? extends T> execute() {
                            ((RichIterable<? extends java.lang.Object>)(Object)CompiledSupport.toPureCollection(CoreGen.evaluate(es, (org.finos.legend.pure.m3.coreinstance.meta.pure.metamodel.function.Function)_fn, _result._values())));
                            return _result;
                        }
            }.execute():_result)
    );
}

However, the compiler complains that the inner statement

((RichIterable<? extends java.lang.Object>)(Object)CompiledSupport.toPureCollection(CoreGen.evaluate(es, (org.finos.legend.pure.m3.coreinstance.meta.pure.metamodel.function.Function)_fn, _result._values())));

"is not a statement".

Environment:

Tested using legend-engine-3.11.1-SNAPSHOT and openjdk 11.0.15 2022-04-19

@aziemchawdhary-gs
Copy link
Contributor

What happens if you assign the result of $fn->eval($result.values->evaluateAndDeactivate()) to a variable? Does the generated Java compile once you do the following?

function meta::nothing::inspect<T|m>(result: Result<T|m>[1], fn: FunctionDefinition<{T[m]->Any[*]}>[1]): Result<T|m>[1] {
  if($result.errors->isEmpty(), 
    {| let x = $fn->eval($result.values->evaluateAndDeactivate()); $result;}, 
    | $result
  );
}

The generated code is 'correct': your Pure code has a statement which does nothing with the result of an expression. Are you expecting the call to $fn->eval(...) to perform some side-effects such as debug printing?

@gbrgr
Copy link
Contributor Author

gbrgr commented Aug 11, 2022

Thanks @aziemchawdhary-gs. If I make that change as you suggest, I run into the following error:

public class test_extension_code
{
    public static MutableMap<String, SharedPureFunction<?>> __functions = Maps.mutable.empty();
    static
    {
        __functions.put("meta$nothing$expand$21$system$imports$import__test_extension_code_pure_1$5", new DefaultPureLambdaFunction1<java.lang.Object, org.finos.legend.pure.generated.Root_meta_nothing_Result<? extends T>>()
...

(Code does not compile, as the symbol T is not known)

(Other code omitted). I still feel that the code should compile even without that change as, as you suspect, the function has side effects (debug printing).

Here is the full PURE code for the record:

import meta::nothing::*;

Class meta::nothing::Result<T|n> {
  <<equality.Key>> values: T[n];
  <<equality.Key>> errors: Any[*];
}

function meta::nothing::okOrElse<T|m>(value: T[m], else: FunctionDefinition<{->Result<T|m>[1]}>[1]): Result<T|m>[1] {
  if ($value->isEmpty(),
    | $else->eval(),
    {|
      ^Result<T|m>(
        values=if($value->size() > 1, | $value->toOneMany(), | $value->toOne())
      );
    }
  );
}

function meta::nothing::unwrap<T>(value: T[0..1]): T[1] {
  $value->expect('Call of `unwrap` on empty object.');
}

function meta::nothing::expect<T>(value: T[0..1], message: String[1]): T[1] {
  if ($value->isEmpty(),
    | fail($message),
    | $value->toOne()
  );
}


function meta::nothing::error(message: FunctionDefinition<{->String[1]}>[1]): String[1] {
  let source = $message
    ->evaluateAndDeactivate()
    ->sourceInformation()
    ->toOne();

  let m = $source.source + ' at ' + $source.line->toString()->toOne() + ':';
  $m + $source.column->toString() + ': ' + $message->eval();
}

function meta::nothing::okOr<T|m>(value: T[m], message: FunctionDefinition<{->String[1]}>[1]): Result<T|m>[1] {
  $value->okOrElse(| ^Result<T|m>(errors=[error($message)]));
}

function meta::nothing::ok<T>(value: T[1]): Result<T|0..1>[1] {
  ^Result<T|0..1>(values=$value);
}

function meta::nothing::okm<T>(values: T[*]): Result<T|*>[1] {
  ^Result<T|*>(values=$values);
}

function meta::nothing::error<T>(errors: Any[*], object: T[1]): Result<T|0..1>[1] {
  ^Result<T|0..1>(errors=$errors);
}

function meta::nothing::andThen<S,T|n>(result: Result<S|0..1>[1], fn: FunctionDefinition<{S[1]->Result<T|n>[1]}>[1]): Result<T|n>[1] {
  if($result.errors->isEmpty(), 
    | $fn->eval($result.values->evaluateAndDeactivate()), 
    | ^Result<T|n>(errors=$result.errors)
  );
}

function meta::nothing::andMany<S,T|n>(result: Result<S|0..1>[1], fn: FunctionDefinition<{S[1]->Result<T|n>[*]}>[1]): Result<T|n>[*] {
  if($result.errors->isEmpty(), 
    | $fn->eval($result.values->evaluateAndDeactivate()), 
    | ^Result<T|n>(errors=$result.errors)
  );
}

function meta::nothing::andThem<S,T|n>(result: Result<S|*>[1], fn: FunctionDefinition<{S[*]->Result<T|n>[1]}>[1]): Result<T|n>[1] {
  if($result.errors->isEmpty(), 
    | $fn->eval($result.values->evaluateAndDeactivate()), 
    | ^Result<T|n>(errors=$result.errors)
  );
}

function meta::nothing::then<S,T|n>(result: Result<S|0..1>[1], fn: FunctionDefinition<{S[1]->T[n]}>[1]): Result<T|n>[1] {
  if($result.errors->isEmpty(), 
    | ^Result<T|n>(values=$fn->eval($result.values->evaluateAndDeactivate()->unwrap())), 
    | ^Result<T|n>(errors=$result.errors)
  );
}

function meta::nothing::them<S,T|n>(result: Result<S|*>[1], fn: FunctionDefinition<{S[*]->T[n]}>[1]): Result<T|n>[1] {
  if($result.errors->isEmpty(), 
    | ^Result<T|n>(values=$fn->eval($result.values->evaluateAndDeactivate())), 
    | ^Result<T|n>(errors=$result.errors)
  );
}

function meta::nothing::inspect<T|m>(result: Result<T|m>[1], fn: FunctionDefinition<{T[m]->Any[*]}>[1]): Result<T|m>[1] {
  if($result.errors->isEmpty(), 
    {| let x = $fn->eval($result.values->evaluateAndDeactivate()); $result;},
    | $result
  );
}

function meta::nothing::else<S|m>(result: Result<S|m>[1], else: FunctionDefinition<{Any[*]->Result<S|m>[1]}>[1]): Result<S|m>[1] {
  if($result.errors->isEmpty(), | $result, | $else->eval($result.errors));
}

function meta::nothing::unwrap<T>(result: Result<T|0..1>[1]): T[1] {
  if($result.errors->isEmpty(), 
    | $result.values->toOne(), 
    | fail(
        $result
          .errors
          ->map(e | $e->toString())
          ->joinStrings()
      )
  );
}

function meta::nothing::unwrapm<T>(result: Result<T|*>[1]): T[*] {
  if($result.errors->isEmpty(), 
    | $result.values, 
    | fail(
        $result
          .errors
          ->map(e | $e->toString())
          ->joinStrings()
      )
  );
}

function meta::nothing::isOk<T|n>(result: Result<T|n>[1]): Boolean[1] {
  $result.errors->isEmpty();
}

function meta::nothing::isErr<T|n>(result: Result<T|n>[1]): Boolean[1] {
  !$result->isOk();
}

function meta::nothing::collect<T>(results: Result<T|0..1>[*]): Result<T|*>[1] {
  $results->fold({r: Result<T|0..1>[1], accum: Result<T|*>[1] |
    if($r->isOk(), | 
      ^Result<T|*>(
        values=if ($accum.errors->isEmpty(), | $accum.values->concatenate($r.values), | []), 
        errors=$accum.errors
      ), 
    | 
      ^Result<T|*>(errors=$accum.errors->concatenate($r.errors), values=[])
    )
  }, okm([])->cast(@Result<T|*>));
}

function meta::nothing::expand<T>(result: Result<T|*>[1]): Result<T|0..1>[*] {
  if ($result.errors->isEmpty(),
    | $result.values->map(v | ok($v)),
    | $result.errors->map(e | error($e, @T))
  );
}

function meta::nothing::tryCast<T>(source: Any[1], object: T[1]): Result<T|0..1>[1] {
  if ($source->instanceOf($object->type()), 
    | ^Result<T|0..1>(values=$source->cast($object)), 
    {|
      let error = 'Cannot cast object of type ' +
        $source->type().name->orElse('<<unknown>>') + 
        ' into class ' + 
        $object->type().name->orElse('<<unknown>>');
      ^Result<T|0..1>(errors=[$error]);
    }
  );
}

function meta::nothing::tryCastm<T>(source: Any[*], object: T[1]): Result<T|*>[1] {
  $source
    ->map(o | $o->tryCast($object))
    ->collect();
}

function meta::nothing::tryCast<T>(result: Result<Any|0..1>[1], object: T[1]): Result<T|0..1>[1] {
  $result->andThen(inner: Any[1] | $inner->tryCast($object));
}

function meta::nothing::tryCastm<T>(result: Result<Any|*>[1], object: T[1]): Result<T|*>[1] {
  $result->andThem(inner: Any[*] | $inner->tryCastm($object));
}

function meta::nothing::test<T>(
  result: Result<T|0..1>[1], 
  check: FunctionDefinition<{T[1]->Boolean[1]}>[1],
  else: FunctionDefinition<{T[1]->Result<T|0..1>[1]}>[1]
): Result<T|0..1>[1] {
  $result->andThen({inner: T[1] | 
    if ($check->eval($inner), | $result, | $else->eval($inner));
  });
}

function meta::nothing::test<T>(
  object: T[1], 
  check: FunctionDefinition<{T[1]->Boolean[1]}>[1],
  else: FunctionDefinition<{T[1]->Result<T|0..1>[1]}>[1]
): Result<T|0..1>[1] {
  $object->ok()->test($check, $else);
}

function meta::nothing::testm<T>(
  result: Result<T|*>[1], 
  check: FunctionDefinition<{T[*]->Boolean[1]}>[1],
  else: FunctionDefinition<{->Result<T|*>[1]}>[1]
): Result<T|*>[1] {
  $result->andThem({inner: T[*] | 
    if ($check->eval($inner), | $result, | $else->eval());
  });
}

@gbrgr
Copy link
Contributor Author

gbrgr commented Sep 1, 2022

@aziemchawdhary-gs just pinging you again in case you have any additional insights there. Thanks.

@epsstan
Copy link
Contributor

epsstan commented Sep 10, 2022

Tagging @hardikmaheshwari @gs-ssh16 @kevin-m-knight-gs for more visibility.

@saraswat
Copy link

@hardikmaheshwari @gs-ssh16 @kevin-m-knight-gs -- any thoughts?

@hardikmaheshwari
Copy link
Contributor

Hey @gbrgr, I think I agree that ideally, your code should have compiled without changes suggested by @aziemchawdhary-gs, though based on the current state it seems those changes are required. We can discuss more on the correct fix on the platform side

In meantime, to get around the issue you facing can you try these changes

function meta::nothing::expand<T>(result: Result<T|*>[1]): Result<T|0..1>[*] {
  if ($result.errors->isEmpty(),
    | $result.values->map(v | ok($v)),
    | $result.errors->map(e | error($e)).   // Removed @T, not really best way we should instantiate objects in pure
  );
}

// Changed signature of the function to use Nil instead of generic. I believe this is fine as there is not much type compilation check needed here and not really possible
function meta::nothing::error(errors: Any[*]): Result<Nil|0..1>[1] { 
  ^Result<Nil|0..1>(errors=$errors);
}

You will still need fix suggested by aziem, as errors you see after those are unrelated to one you saw before

@gbrgr
Copy link
Contributor Author

gbrgr commented Sep 26, 2022

Thank you @hardikmaheshwari. That indeed works for this minimal example. On my side I have a bigger codebase where multiple such errors occur. Is there some documentation available on how the interpreted language differs from the compiled language or is there some other way how to figure out what works in one but not the other? Thanks!

@kevin-m-knight-gs
Copy link
Contributor

Hi @gbrgr. There should be no difference between interpreted and compiled modes in terms of compilation (except that compiled mode might take longer as it does more work). If the Pure compiler successfully compiles the Pure code, then the Java generator should generate valid Java code for it. Any discrepancy between the two is a bug.

The first case here (the Java compiler complaining that something "is not a statement") is clearly a bug in the Java generator. As @aziemchawdhary-gs noted earlier, there is a relatively easy workaround. Still, the workaround is clumsy and shouldn't be necessary. This is a bug and should be fixed.

In the second case, I think that the suggestion that @hardikmaheshwari made is actually a better way to implement that particular function, regardless of Java compilation errors. Nonetheless, if the Pure compiler is compiling the original code, then the Java generator should be generating valid Java code. So, again, I tend to think this is a bug in the Java generator. But unlike the first case, the workaround here is actually a genuine improvement. So I would recommend using Hardik's suggestion in any case.

These are two distinct bugs. I believe both bugs are actually in legend-pure rather than legend-engine. I don't think that either will be trivial to fix, but I also don't think either is intractable.

@gbrgr
Copy link
Contributor Author

gbrgr commented Oct 4, 2022

Thanks @kevin-m-knight-gs for your detailed answer. Unfortunately, I encounter multiple compilation issues when I try to compile our open sourced WIP extension of PURE that integrates our cloud-native knowledge graph management system. The extension code is here: https://github.com/RelationalAI/legend-engine/tree/master/legend-engine-xt-relationalai-pure

What is the best way to pin down the exact issues a given PURE code base has? For example, at Java generation, I am just receiving a NullpointerExc. at some point, and it is not really clear to me how to track down that error. Thanks!

@kevin-m-knight-gs
Copy link
Contributor

I've created a legend-pure issue for the first bug and committed a fix for the second. The fix was released in legend-pure 3.7.1.

A NullPointerException during Java generation is definitely a bug. And that's the kind of bug that can be very difficult to trace back to Pure code. If you have a stack trace you can include here, I'll see what I can figure out from it.

@finos-admin
Copy link
Member

This issue is stale because it has been open for 30 days with no activity. Please remove stale label or add any comment to keep this open. Otherwise this will be closed in 5 days.

@finos-admin
Copy link
Member

This issue was closed because it has been inactive for 35 days. Please re-open if this issue is still relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants