KeYProject / key

KeY Theorem Prover for Deductive Java Verification
https://key-project.org
Other
41 stars 24 forks source link

Immutable and valid JML set statement #3400

Closed wadoon closed 4 months ago

wadoon commented 6 months ago

Version of #3195 but with an immutable Java AST.

This PR implements JML set-statement in a non-Java-like fashion (no reduction to copy assignment) preserving the semantics of Java.

Translation of JML goes directly from JML to KeY terms lazily inside the built-in rule SetStatementRule.

TODO

Type of pull request

Ensuring quality

mattulbrich commented 6 months ago

This version verifies the following program:

class A {

    //@ ensures true;
    void m() {

        //@ ghost \bigint x = 0;

        int i = 0;
        while(i < 2) {
            //@ ghost \bigint y = 1;
            //@ set x = x + y;
            i++;
        }
        //@ assert x == 0;
        // but should be x == 2;
    }
}

which is not correct at all. The problem is that in every loop iteration y becomes a different symbol. However, in x+y the y remains always the original variable and is not updated to the respective instances. That is the reason why in assumptions and assertions the copy is kept and actually modified (breaking immutability).

mattulbrich commented 6 months ago

Even simpler: The following can be proved:

    void m() {
        //@ ghost \bigint x = 0;
        //@ set x = x + 1;
        //@ assert x == 0;
    }

The x in the set statement does not refer to the correct variable (neither when reading, nor when writing)

wadoon commented 6 months ago

Then I would say, the simple does not work and we need to go back to the drawing board.

@unp1 Do you have insides into the mechanism of variable renaming? Is there a chance, to get around the ProgVarReplaceVisitor completely? Why do we not book this information along a goal and/or a modality?

wadoon commented 5 months ago

Everything is terrible. Current test program:

class Test {
    //@ ghost int x;

    int a;

    //@ requires true; ensures x==a;
    void foo() {
        //@set x = 0;
        a = 0;
        //@set x = x + 1;
        a = a + 1;
        //@set x = x + 1;
        a = a + 1;
        //@set x = x + 1;
        a = a + 1;
        //@set x = x + 1;
        a = a + 1;
        //@set x = x + 1;
        a = a + 1;
    }

    //@ ghost int rec;
    int cer;

    //@ requires a >= 0; ensures rec == cer; measured_by a;
    int voo(int a) {
        if (a == 0) {
            //@ set rec = 0;
            cer = 0;
            return 0;
        } else {
            int r = voo(a - 1) + 1;
            //@ set rec = r;
            cer = r;
            //@ assert r >=0;
            return r;
        }
    }

}

There are only two possibilities:

wadoon commented 5 months ago

@mattulbrich The mutable information of SetStatements moved to SpecificationRepository. Hence, the AST is immutable again. If the change suits you, we need to adopt the JmlAssert also.

The example above is proofable except for the faulty assert statement.

There are currently two construction sites:

  1. PrettyPrinter can not show the current expression, as it has no access to services or the spec repo.
  2. It is unclear whether a copy of the statement is necessary in ProgVarReplacer or IntroAtPreDefOps.
wadoon commented 5 months ago

I would also now transform JMLAssert to SpecificationRepository.

wadoon commented 5 months ago

Added a new highlighting for JML statement:

image

The old highlighting was green, and was inherited from comments.

wadoon commented 5 months ago

State not so bad:

Unit-Tests:

image

RAP:

image

image

mattulbrich commented 5 months ago

Added a new highlighting for JML statement:

Would it perhaps make sense to deliberately deviate from JML like syntax here to avoid confusion on the user's side?

like //@ set a := 42 for instance or //@ set heap := heap[o.ghst := 42] which is more faithful to the JavaDL side.

Perhaps even use //# not //@ or something else to avoid confusion.

codecov[bot] commented 5 months ago

Codecov Report

Attention: Patch coverage is 55.33333% with 268 lines in your changes are missing coverage. Please review.

Project coverage is 37.87%. Comparing base (a5b90ec) to head (45435fb).

Files Patch % Lines
...rc/main/java/de/uka/ilkd/key/pp/PrettyPrinter.java 61.88% 117 Missing :warning:
...in/java/de/uka/ilkd/key/rule/SetStatementRule.java 25.58% 31 Missing and 1 partial :warning:
...ka/ilkd/key/proof/mgt/SpecificationRepository.java 13.04% 17 Missing and 3 partials :warning:
...a/ilkd/key/java/visitor/ProgVarReplaceVisitor.java 11.11% 16 Missing :warning:
...lkd/key/java/visitor/ProgramVariableCollector.java 0.00% 11 Missing :warning:
...d/key/speclang/jml/translation/JMLSpecFactory.java 80.76% 6 Missing and 4 partials :warning:
.../ilkd/key/rule/metaconstruct/IntroAtPreDefsOp.java 0.00% 9 Missing :warning:
.../main/java/de/uka/ilkd/key/rule/JmlAssertRule.java 0.00% 8 Missing :warning:
.../uka/ilkd/key/rule/SetStatementBuiltInRuleApp.java 0.00% 8 Missing :warning:
...java/de/uka/ilkd/key/java/statement/JmlAssert.java 14.28% 6 Missing :warning:
... and 10 more
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #3400 +/- ## ============================================ + Coverage 37.85% 37.87% +0.01% - Complexity 17042 17079 +37 ============================================ Files 2082 2086 +4 Lines 127290 127465 +175 Branches 21441 21460 +19 ============================================ + Hits 48183 48272 +89 - Misses 73194 73280 +86 Partials 5913 5913 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.