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

Milnes strings #3576

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

briangmilnes
Copy link
Contributor

I have 'thickened' strings quite a bit.
There are additions to the end of FStar.String.fst that define a partial string equality
and string equality and provide a string difference (for upcoming Final testing framework). The added functions are in FStar.String.Base and quite a few
proofs in FStar.String.Properties. As well as as a decidable equality class on streq.

FStar.String.Properties has 36 proofs showing the relationships between
streq, streq' (it's boolean equivalent), lengths, indexes and the string differencer
first_diff.

This, as expected, does not seem to increase proof times.

I did not include Base and Properties into FStar.String.fsti, so it's questionable
if I have the right form here. And is it right to name the property streq and the
boolean streq'?

I also added a tests/validate-time/Test.FStar.String.fst to test this.

And adjusted fstar.mk to allow overwriting of WARN_ERROR so I could cut down some warnings in validate-time tests.

…alidation-time with a Test.LexemeFL.fst that checks __FL__
…) added a validation-time test for it 3) make WARN_ERROR?= in ulib/gmake/fstar.mk so I could suppress some warnings in test
@briangmilnes
Copy link
Contributor Author

Still have the github writing to the slack channel problem, so my fix did not take.

@nikswamy
Copy link
Collaborator

Hi Brian, Thanks for this PR. A few comments:

  1. I don't think we need streq and the equality class for strings. Strings already have decidable equality (they are eqtype) and an equality instance is given here https://github.com/FStarLang/FStar/blob/master/ulib/FStar.Class.Eq.fst#L43

  2. I would rename FStar.String.Base to FStar.String.FirstDiff and remove the changes to FStar.String.fsti. By declaring these in FStar.String.fsti you are effectively assuming them as primitives in FStar.String, although you have given implementations in FStar.String.Base

  3. I'll leave some other more localized comments


/// Return the first difference position as an option for the whole string.
val first_diff (s1 s2: string) :
Tot (o : (option (pos: nat{pos <= (min (strlen s1) (strlen s2))})) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is usually easier to work with o:option pos { Some? o ==> Some?.v o ==> min (strlen s1) (strlen s2) } that with the type you've written

(((Some?.v o) = strlen s1 \/ (Some?.v o) = strlen s2) /\ strlen s1 <> strlen s2)
\/
(((Some?.v o) < strlen s1 /\ (Some?.v o) < strlen s2) /\
(index s1 (Some?.v o) <> (index s2 (Some?.v o)))))
Copy link
Collaborator

Choose a reason for hiding this comment

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

This big refinement formula here is a duplicate of the formula in first_diff. Refactoring them into a single definition is better than copying it

@briangmilnes
Copy link
Contributor Author

briangmilnes commented Oct 22, 2024 via email

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.

2 participants