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

Run SMT solver on requested queries #303

Merged
merged 1 commit into from
Dec 3, 2018
Merged

Run SMT solver on requested queries #303

merged 1 commit into from
Dec 3, 2018

Conversation

leonardoalt
Copy link
Member

@leonardoalt leonardoalt commented Nov 30, 2018

and rerun compiler with given responses

@axic
Copy link
Member

axic commented Nov 30, 2018

There are 3 PRs fixing this and this doesn't work on Node 4 :)

@leonardoalt
Copy link
Member Author

What's the workaround then?

@axic
Copy link
Member

axic commented Nov 30, 2018

#105 is the real solution. A quick intermediate solutions it dropping Node 4 support.

#127 and #246 do the same change as you.

@coveralls
Copy link

coveralls commented Nov 30, 2018

Coverage Status

Coverage remained the same at 56.982% when pulling 1aaffe1 on smt_wrapper into 1cee2ae on master.

@axic
Copy link
Member

axic commented Nov 30, 2018

It seems however that your change is somewhat a 3rd option which indeed works.

@axic axic reopened this Nov 30, 2018
@axic
Copy link
Member

axic commented Nov 30, 2018

Oh works because ti doesn't have the test! :)

@axic
Copy link
Member

axic commented Nov 30, 2018

Can you try adding your version to #127 (just push a commit) ?

@leonardoalt
Copy link
Member Author

It should be the same, shouldn't it?

@leonardoalt
Copy link
Member Author

Pushed there

@leonardoalt leonardoalt changed the title Fix bug when reading from stdin Run SMT solver on requested queries Nov 30, 2018
solcjs Outdated Show resolved Hide resolved
SMTChecker.js Outdated Show resolved Hide resolved
SMTChecker.js Outdated Show resolved Hide resolved
SMTChecker.js Outdated Show resolved Hide resolved
SMTChecker.js Outdated Show resolved Hide resolved
SMTChecker.js Outdated Show resolved Hide resolved
SMTChecker.js Outdated Show resolved Hide resolved
@axic
Copy link
Member

axic commented Nov 30, 2018

This is my proposal:

// Returns `null` if nothing is to be solved or a new `input` if it needs to be run again.
function handleSMTQueries(input, output, solver) {
  if (!output.auxiliaryInputRequested)
    return null

  // Could also just throw an exception here?
  if (!solverCallback)
    return null

  let queries = output.auxiliaryInputRequested.smtlib2queries
  var responses = {}
  for (var queryKey in queries) {
    responses[queryKey] = solver(queries[queryKey])
  }

  input.auxiliaryInput.smtlib2esponses = responses
  return input
}

@leonardoalt
Copy link
Member Author

input.auxiliaryInput.smtlib2esponses = responses
doesn't work because input.auxiliaryInput is still undefined at that point

@axic
Copy link
Member

axic commented Nov 30, 2018

It is the high level design :)

@leonardoalt
Copy link
Member Author

Updated extracting the solve part into a sync function

@leonardoalt leonardoalt force-pushed the smt_wrapper branch 2 times, most recently from 05f5c8d to 1e435d2 Compare November 30, 2018 16:05
@leonardoalt
Copy link
Member Author

not sure about the changes to solcjs, it's gotten quite long with the error handling

smtchecker.js Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
@axic
Copy link
Member

axic commented Dec 3, 2018

Reordered some things and fixed lint.

@leonardoalt can you double check it works?

Can you create an issue for adding tests? That will need a manual installation of cvc4/z3.

@leonardoalt
Copy link
Member Author

Yes, it works!

@axic axic merged commit 0022a7c into master Dec 3, 2018
@axic axic deleted the smt_wrapper branch December 3, 2018 14:31
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.

3 participants