Skip to content

Comments

Obtaining the old Hoare triple from the new#55

Merged
didriklundberg merged 10 commits intokth-step:masterfrom
didriklundberg:dev_assume
May 16, 2019
Merged

Obtaining the old Hoare triple from the new#55
didriklundberg merged 10 commits intokth-step:masterfrom
didriklundberg:dev_assume

Conversation

@didriklundberg
Copy link
Member

@didriklundberg didriklundberg commented May 14, 2019

I added new theorems and definitions to obtain an AssumptionViolated-free Hoare triple from the regular one, provided the program contains no Assume statements. Also, corrected typo in exec_to_labels HT.

The terminology is a little rough around the edges. I will likely make some clean-up commit that renames and moves some lemmata around, and makes the proofs nicer. Please provide feedback so that everything can be sorted out in one go.

I am also working on a proof procedure to obtain the result of an Assume-free BIR program (the definitions should be very straightforward to use, however).

EDIT: Added commit to resolve #53.

…ee Hoare triple from the regular one, provided the program contains no Assume statements. Also, corrected typo in exec_to_labels HT.
Copy link
Member

@totorigolo totorigolo left a comment

Choose a reason for hiding this comment

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

I'm just commenting because I want to let @andreaslindner review, since it's his tool :)

Some points:

  • Why did you create a new theory about Hoare triple, instead of adding these theorems in bir_wpTheory?
  • Why did you add those TODOs instead of putting the theorems directly where they should be?

Otherwise, that looks really useful 👍

@andreaslindner
Copy link
Member

I'm just commenting because I want to let @andreaslindner review, since it's his tool :)

Thanks for thinking about me, but we're working all together on this :)

@totorigolo
Copy link
Member

I'm just commenting because I want to let @andreaslindner review, since it's his tool :)

Thanks for thinking about me, but we're working all together on this :)

It's also because I'm not really used to writing proofs, so I think that you review is way more valuable ;)

@didriklundberg
Copy link
Member Author

* Why did you create a new theory about Hoare triple, instead of adding these theorems in `bir_wpTheory`?

Because bir_wpScript.sml is already of humongous size. I like to maintain some degree of modularity and not keep all theorems in a single file. But of course, this subdivision can always be more precise. The intent of bir_htTheory is to contain HT manipulation theorems not directly related to the WP propagation mechanisms.

* Why did you add those `TODO`s instead of putting the theorems directly where they should be?

Not really for any strong reason, but I figured I could just as well receive comments first. I don't want to clutter existing theories with new theorems without making sure they go in the right place. Some of these theorems contain comments on where I intend to put them (so that we don't mess up commit history by moving them back and forth at PR review). So if @andreaslindner doesn't have any strong opinions I will just spread them out myself in a new commit.

@totorigolo
Copy link
Member

Btw, I focused on style and readability for a HolBA user in my review. Please ask if you have a specific question/concern :)

totorigolo
totorigolo previously approved these changes May 15, 2019
Copy link
Member

@totorigolo totorigolo left a comment

Choose a reason for hiding this comment

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

I'm happy with the code now 🙂 Sorry for this lengthy review ^^'

EDIT: I leave you decide if you want to wait for Andreas' review :)

@totorigolo totorigolo mentioned this pull request May 15, 2019
@andreaslindner
Copy link
Member

andreaslindner commented May 15, 2019

The intent of bir_htTheory is to contain HT manipulation theorems not directly related to the WP propagation mechanisms.

I thought the same when I saw this pull request the first time. It makes totally sense to divide hoare triples from wp. One is the formalization of pre and post-conditions and the other is a mechanism making use of the formalizations.

Summing up: I like this idea.

Copy link
Member

@andreaslindner andreaslindner left a comment

Choose a reason for hiding this comment

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

Looks good! After fixing the issue that Thomas pointed out and maybe changing the definition like I suggested if you like, you can merge it from my perspective. Or add more first if you want to add more of course

@andreaslindner
Copy link
Member

Oh, maybe a little test script for the new library could be useful.

andreaslindner
andreaslindner previously approved these changes May 16, 2019
Copy link
Member

@andreaslindner andreaslindner left a comment

Choose a reason for hiding this comment

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

Looks still good.

@didriklundberg
Copy link
Member Author

didriklundberg commented May 16, 2019

Oh, maybe a little test script for the new library could be useful.

I'll see if I can get time to fix that later today. Otherwise I'll just rebase and merge and do it in a later PR. :)

Copy link
Member

@andreaslindner andreaslindner left a comment

Choose a reason for hiding this comment

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

That's a funny one :)

@didriklundberg didriklundberg merged commit 2ccec3a into kth-step:master May 16, 2019
@didriklundberg didriklundberg deleted the dev_assume branch September 18, 2019 08:50
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.

Duplicate theorems in bir_program_blocksTheory

3 participants