IETS3 / iets3.opensource

Open Source Parts of IETS3
Apache License 2.0
44 stars 22 forks source link

Synchronize test.in.os.m1 Java Generator and Interpreter tests #163

Closed lhartl closed 5 years ago

lhartl commented 5 years ago

Java Generator and Interpreter should behave in the same way. Therefore the same tests should be executed for both. To achieve this, the java generator has to be completed. First step is to do this for the model test.in.os.m1

lhartl commented 5 years ago

In some interpreterTestSuites, error checks are used. This prevents errors from being shown, but they exist and generation fails. How can I deal with this properly? example: screen shot 2019-01-02 at 14 38 46 branch: master Node URL: http://127.0.0.1:63320/node?ref=r%3Acd8f023a-94dd-4e45-bbf0-a419cfafa530%28test.in.expr.os.m1%40tests%29%2F7061117989424255977&project=org.iets3.opensource

edit: done

markusvoelter commented 5 years ago

You mean: in executable tests, not in typesystem tests? I think this is a bug.

lhartl commented 5 years ago

Yes, the checks are within the executable interpreter tests. I'll move them to the typesystem tests then.

markusvoelter commented 5 years ago

Ok.

lhartl commented 5 years ago

screen shot 2019-01-02 at 14 41 57 In this example, a problem shows up. In KernelF, it is possible to write ValRefs (here: brothers) without assigning a value to them or using them. In Java the corresponding code is not valid. Example code is inside a function. The brothers; statement does not compile.

PCollection<Person_contracts> brothers = TreePVector.from(p.getsiblings().stream().filter(new Predicate<Person_contracts>() {
          public boolean test(Person_contracts o) {
            return new Function<ParameterSetWrapper, Boolean>() {
              public Boolean apply(ParameterSetWrapper param) {
                Person_contracts it = (Person_contracts) param.parameters.get(0);
                return new _FunctionTypes._return_P0_E0<Boolean>() {
                  public Boolean invoke() {
                    Boolean b = (it.getgender() == Generated_contracts_Test.Gender.male);
                    if (it.getgender() == null) {
                      return null;
                    }
                    return b;
                  }
                }.invoke();
              }
            }.apply(new ParameterSetWrapper(o));
          }
        }).collect(Collectors.toList()));
        brothers;

The problem here is, that in some cases, it is ok to write something like the problematic brothers in KernelF, for example as the last expression within a function body. In this case, it would be wrapped in a return statement in the Java code. Any suggestions how to deal with this?

markusvoelter commented 5 years ago

Right now, there is a warning (I think) if this expression has no effect, because then it is really pointless. If it has an effect then it is ok (but then we also need it in Java).

I am happy to make this warning an error and thus prevent this case, solving the Java problem.

lhartl commented 5 years ago

There is no warning and in my opinion it has no effect. branch: master node URL: http://127.0.0.1:63320/node?ref=r%3Acd8f023a-94dd-4e45-bbf0-a419cfafa530%28test.in.expr.os.m1%40tests%29%2F6095949300274625231&project=org.iets3.opensource

markusvoelter commented 5 years ago

I won't open MPS before Monday :-)

This is in a test suite, right? I think test suites inibit "trivial" errors like this. There should be checking rule that outputs an error unless its under some kind of "DontShowTrivialStuff" node.

lhartl commented 5 years ago

Ok.

You do not have to open MPS. I'm taking notes here, so I don't forget to ask when you are back from holidays :) .

markusvoelter commented 5 years ago

:-)

lhartl commented 5 years ago

Yes, the stuff is in a TestSuite

lhartl commented 5 years ago

Within a Library, there is an error for this problem. One should consider to also show this error in TestSuites since it breaks Java generation.

Edit: done

markusvoelter commented 5 years ago

Agreed.

lhartl commented 5 years ago

AssignmentExpr confuses me a little. Since this exists now, what is the point of having mutables?

markusvoelter commented 5 years ago

Explain?

lhartl commented 5 years ago

All standard stuff in KernelF was immutable, as far as I knew. Meaning assignment only during declaration. To have mutable stuff (e.g. reassign a variable) one had to use mutables. The AssignmentExpr allows assignment to immutables after declaration, which breaks the concept in my opinion. I don't really get how this fits in.

markusvoelter commented 5 years ago

Let's talk on Monday. It fits :-)

On Thu, Jan 3, 2019 at 10:50 AM lhartl notifications@github.com wrote:

All standard stuff in KernelF was immutable, as far as I knew. Meaning assignment only during declaration. To have mutable stuff (e.g. reassign a variable) one had to use mutables. The AssignmentExpr allows assignment to immutables after declaration, which breaks the concept in my opinion. I don't really get how this fits in.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/IETS3/iets3.opensource/issues/163#issuecomment-451096616, or mute the thread https://github.com/notifications/unsubscribe-auth/AAkJysSgI_Kw8PnrMjwQqI0iB2_PLXhLks5u_dJOgaJpZM4ZmcGm .

-- Dr. Markus Völter

voelter - ingenieurbüro für softwaretechnologie voelter@acm.org | +49 (0) 171 / 86 01 869

http://voelter.de -- personal website incl. papers, talks, books http://voelter.de/essays -- writings on language engineering http://omegataupodcast.net -- science and engineering podcast

lhartl commented 5 years ago

Since the elseSection of the IfStatement is now optional, the Java generator of the if statement needs to be rewritten (see varLib).

markusvoelter commented 5 years ago

(just for clarification: it is only optional if the then part has a modify-Effect).

lhartl commented 5 years ago

Concept for dealing with FailExpr in the Java Generator is needed.

markusvoelter commented 5 years ago

FailException extends RuntimeException

lhartl commented 5 years ago

Yes, but it's not that easy in the generator because just reducing the FailExpr to throw new FailException() can lead to Java Code like this: return throw new FailException();

Edit: done

markusvoelter commented 5 years ago

I see. Ok.

szarnekow commented 5 years ago

Something like return FailException.fail() with public static <T> T fail() { throw new FailException(); } could work.