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

Better error message for when type variable instances are not found for :checking #1357

Merged
merged 3 commits into from
Jun 3, 2022

Conversation

rybla
Copy link
Collaborator

@rybla rybla commented May 20, 2022

Addresses #1345

The problem: due to nondeterminism in the SMT solver, using :check on a property with numeric constraints can sometimes find and sometimes not find concrete instances for some numeric type variables. If the instance is not found, then the user is presented with the error message Cannot evaluate polymorphic value. which is misleading because, in fact, another run of :check might find an instance and to testing just fine.

In Cryptol.REPL.Monad, this PR introduces a new constructor to REPLException,

data REPLException =
  ...
  | InstantiationsNotFound Schema
  ...

which is very similar to EvalPolyError, which is the misleading exception raised in #1345. The exception InstantiationsNotFound is only thrown when the user :checks a property, and defaultReplExpr' fails to find a model (assignment of type variables) that satisfies the type constraints (list of Props). This failure mode is now reflected in the type of replEvalCheckedExpr which now returns REPL (Maybe ...), which allows the different call sites of replEvalCheckedExpr to handle the solver failure differently.

Now, the feedback a user gets they try the example in #1345 is:

Main> :check
property NTTandNTT'areInverses 
Cannot find instantiations for some type variables.
Type: {N, k, g, n}
  (prime N, N == 1 + k * n, fin k, g ^^ (N - 1) % N == 1, N > g,
   fin n) =>
  [n](Z N) -> Bit
Main> :check
property NTTandNTT'areInverses Showing a specific instance of polymorphic result:
  * Using '2' for signature variable 'N'
  * Using '1' for signature variable 'k'
  * Using '1' for signature variable 'g'
  * Using '1' for signature variable 'n'
Using exhaustive testing.
Passed 2 tests.
Q.E.D.

However, further work needs to be done to properly support testing for polymorphic properties. This PR merely makes the lacking capability less confusing for the user.

Copy link
Contributor

@robdockins robdockins left a comment

Choose a reason for hiding this comment

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

Overall, looks good to me, with a minor style nitpick.

src/Cryptol/REPL/Command.hs Show resolved Hide resolved
@rybla rybla merged commit bdbe080 into master Jun 3, 2022
@rybla rybla deleted the issue-1345-2 branch June 3, 2022 16:52
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