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

feat: Tighten specifications on Strings standard library #4868

Merged

Conversation

robin-aws
Copy link
Member

@robin-aws robin-aws commented Dec 12, 2023

Description

As follow-up from the initial review, this adds a bodiless CharsConsistent lemma to the abstract Std.Strings.ParametricConversion module that ensures chars and charsToDigits are consistent, which is necessary but not sufficient to prove that forall n :: ToInt(OfInt(n)) == n.

Also makes some documentation and naming improvements, adding an IsDigitChar predicate for better readability than c in charsToDigit (which is slightly different from c in chars) and merges the two confusingly-named ToNumberStr and OfNumberStr predicate into one IsNumberStr (with the definition of the former, weaker predicate).

How has this been tested?

Existing tests + verification.

By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.

@@ -172,6 +178,9 @@ module {:disableNonlinearArithmetic} Std.Strings {
'a' := 0xA, 'b' := 0xB, 'c' := 0xC, 'd' := 0xD, 'e' := 0xE, 'f' := 0xF,
'A' := 0xA, 'B' := 0xB, 'C' := 0xC, 'D' := 0xD, 'E' := 0xE, 'F' := 0xF
]
// The size of the map makes this impractical to verify easily.
Copy link
Member Author

Choose a reason for hiding this comment

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

I find this fascinating: I couldn't find an {:rlimit} value high enough to keep this from timing out without hitting an overflow exception in Boogie. :) And yet it's totally tractable to just statically evaluate the ensures clause, since there are so few values to test, and this is arguably just as sound a proof as verification!

@robin-aws robin-aws enabled auto-merge (squash) December 12, 2023 01:24
Copy link
Contributor

@alex-chew alex-chew left a comment

Choose a reason for hiding this comment

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

LGTM, though it's not immediately obvious that removing the < base conditions is safe since nothing states that the elements of chars are unique.

@robin-aws robin-aws merged commit 2cadde8 into dafny-lang:master Dec 12, 2023
20 checks passed
@robin-aws
Copy link
Member Author

Thanks Alex! I neglected to call removing the < base conditions in the PR description: they were totally redundant because those were only ever applied to values of type digit, which was already a subtype with that condition.

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.

None yet

2 participants