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

Fix #3524 #3525

Merged
merged 10 commits into from
Nov 25, 2024
Merged

Fix #3524 #3525

merged 10 commits into from
Nov 25, 2024

Conversation

Drodt
Copy link
Member

@Drodt Drodt commented Oct 30, 2024

Related Issue

This pull request addresses #3524.

Intended Change

For all loopScopeInv rules: Declares for each local variable i a new PV i_before, stores the value of i before the loop in i_before, and applies {i := i_before} to the frame condition.

Type of pull request

  • Bug fix: Breaking-Change (existing proofs with loops might no longer load)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • There are changes to the (Java) code
  • There are changes to the taclet rule bas

Ensuring quality

  • I made sure that introduced/changed code is well documented (javadoc and inline comments).
  • I added new test case(s) for new functionality.
  • I have tested the feature as follows: Some local tests, including the example of Soundness bug: Assignable treatment is wrong #3524.
  • I have checked that runtime performance has not deteriorated.

Additional information and contact(s)

Thanks @unp1!

The contributions within this pull request are licensed under GPLv2 (only) for inclusion in KeY.

@unp1 unp1 linked an issue Oct 30, 2024 that may be closed by this pull request
@mattulbrich
Copy link
Member

Guys, ... you are quick! 👏 @Drodt @unp1

@wadoon wadoon requested a review from mattulbrich October 31, 2024 08:53
@wadoon wadoon added this to the v2.14.4 milestone Oct 31, 2024
Copy link

github-actions bot commented Oct 31, 2024

Qodana Community for JVM

It seems all right 👌

No new problems were found according to the checks applied

💡 Qodana analysis was run in the pull request mode: only the changed files were checked

View the detailed Qodana report

To be able to view the detailed Qodana report, you can either:

To get *.log files or any other Qodana artifacts, run the action with upload-result option set to true,
so that the action will upload the files as the job artifacts:

      - name: 'Qodana Scan'
        uses: JetBrains/[email protected]
        with:
          upload-result: true
Contact Qodana team

Contact us at [email protected]

Copy link
Member

@mattulbrich mattulbrich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that the fix requires a larger change on the loop rule. There seem to be new "_Before" variables now. But ... are there not already _Before-Variables available on the sequent?

@Drodt
Copy link
Member Author

Drodt commented Nov 4, 2024

It seems that the fix requires a larger change on the loop rule. There seem to be new "_Before" variables now. But ... are there not already _Before-Variables available on the sequent?

Not necessarily for possibly modified local variables wrt. the loop scope rule. The old loop rule did add those variables, the loop scope rule does not.

@Drodt Drodt added the Review Request Waiting for review label Nov 5, 2024
Copy link
Member

@mattulbrich mattulbrich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Loooks a lot cleaner without the maps in the schema variable. Thanks

@unp1 unp1 added this pull request to the merge queue Nov 25, 2024
Merged via the queue into main with commit 9ef68b3 Nov 25, 2024
15 checks passed
@unp1 unp1 deleted the fix-2524 branch November 25, 2024 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Soundness bug: Assignable treatment is wrong
4 participants