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

Add more theorems and constants #1375

Merged
merged 19 commits into from
Dec 18, 2024
Merged

Add more theorems and constants #1375

merged 19 commits into from
Dec 18, 2024

Conversation

xrchz
Copy link
Member

@xrchz xrchz commented Dec 15, 2024

At a high level, this PR includes:

  • bool_to_bit
  • theorems about chunks including a tail-recursive version
  • theorems about dropWhile
  • theorems about word_to_bin_list/word_from_bin_list -- operating on words as a list of bits
  • some theorems about BITWISE etc. mostly in service of the above

These theorems were produced as part of an upcoming improvement to cv evaluation of Keccak_256

@xrchz xrchz requested a review from mn200 December 15, 2024 14:05
@xrchz xrchz changed the title Add some theorems about chunks Add more theorems and constants Dec 15, 2024
@xrchz xrchz marked this pull request as draft December 15, 2024 14:28
@xrchz xrchz marked this pull request as ready for review December 15, 2024 18:51
@binghe
Copy link
Member

binghe commented Dec 16, 2024

Hi,

You should first fix CI tests of other builds and handle OT builds at last. From the latest CI test results I think your removal of previous assumption 32 is not right:

32 |- !f g M N. M = N /\ (!x. x = N ==> f x = g x) ==> LET f M = LET g N

Besides, even the goal doesn't show up as part of the generated goals, in principle we should NOT delete any proof from prove_base_assumsScript.sml. Instead, we should still keep this proof under a different name (LET_CONG in this case), at the end of this file, just like other existing proofs without a th* name.

My understanding is that, these extra proofs are those needed by core theories beyond boss. Missing any of these proofs will cause leaking assumptions later, e.g. when hol-real is built and installed.

@xrchz
Copy link
Member Author

xrchz commented Dec 17, 2024

You should first fix CI tests of other builds and handle OT builds at last. From the latest CI test results I think your removal of previous assumption 32 is not right:

I've reverted that change. How would you recommend to proceed now?

@xrchz
Copy link
Member Author

xrchz commented Dec 17, 2024

Maybe we're done? all the CI checks pass

Copy link
Member

@mn200 mn200 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. Please think about suggestions.

src/list/src/numposrepScript.sml Show resolved Hide resolved
src/list/src/rich_listScript.sml Outdated Show resolved Hide resolved
@binghe
Copy link
Member

binghe commented Dec 18, 2024

You should first fix CI tests of other builds and handle OT builds at last. From the latest CI test results I think your removal of previous assumption 32 is not right:

I've reverted that change. How would you recommend to proceed now?

The fix is good. But it seems that the newly added th61 is nothing but the existing BOOL_EQ_DISTINCT appeared later:

(* |- (T <=/=> F) /\ (F <=/=> T) *)
val BOOL_EQ_DISTINCT = store_thm("BOOL_EQ_DISTINCT", concl boolTheory.BOOL_EQ_DISTINCT,
  PURE_REWRITE_TAC[iff_F,not_not,iff_T,not_F,and_T]);

I suggest removing the above redundant theorem (maybe you can consider using its shorter proof for th61) and put val BOOL_EQ_DISTINCT = th61; after th61 for backward compatibility. Both actions are optional, because redundant theorems create at most warnings when installing OT articles, and the name BOOL_EQ_DISTINCT is actually not used later in this same file.

P. S. previous this bridging theorem didn't show up in the generated goals because it's not used in any proof of core theories before boss, but it will be used later. Now because your new theorems directly use it, so it's moved into the generated goals. I don't have a good solution to prevent future OT builds broken for this kind of issues.

@xrchz
Copy link
Member Author

xrchz commented Dec 18, 2024

I suggest removing the above redundant theorem (maybe you can consider using its shorter proof for th61) and put val BOOL_EQ_DISTINCT = th61; after th61 for backward compatibility.

Thanks for the feedback! I followed this suggestion.

@xrchz xrchz merged commit 5bbb8a2 into develop Dec 18, 2024
3 of 4 checks passed
@mn200 mn200 deleted the upstreaming branch December 18, 2024 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants