Skip to content

Conversation

@vkuncak
Copy link
Collaborator

@vkuncak vkuncak commented May 29, 2021

Currently --eval does not helpfully show the bindings of variables when we encounter assertion failure. This PR makes a small change to RecursiveEvaluator to show the values of free variables of assertion failing predicate. It also displays the type of evaluator (recursive or codegen) when evaluating each function.

Given this input:

object VarDefault {
  case class AA(var x: Int = 555, var y: Int = 222)
  def addMe: Unit = {
    val aa = AA(2147483647, 1073741823)
    assert(0 <= aa.x && 0 <= aa.y, "both positive")
    val x = aa.x
    val y = aa.y
    val z = x + y
    assert(0 <= x + aa.y, "sum positive")
  }
}

the output with --eval --no-colors is:

Evaluating addMe
using default recursive evaluator
VarDefault.scala:9:12:  assertion failure of sum positive: 0 <= x + aa.y
Relevant variables at assertion failure point:
x -> 2147483647
aa -> AA(2147483647, 1073741823)
Result for addMe @3:3:
warning:  => CRASHED
warning:   sum positive
  
 stainless summary 
                                                                   
VarDefault.scala:3:3:                             addMe        crashed          0.0    
........................................................................................
total: 1    valid: 0    (0 from cache) invalid: 1    unknown: 0    time:     0.0       

@vkuncak vkuncak requested a review from jad-hamza May 29, 2021 20:32
Copy link
Contributor

@jad-hamza jad-hamza left a comment

Choose a reason for hiding this comment

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

Nice! I was missing that (similar) feature from leon-web :)

object Evaluator {
val RecursiveEvaluatorName: String = "default recursive evaluator"
val CodeGenEvaluatorName: String = "codegen evaluator"
var kind: String = RecursiveEvaluatorName
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we include the names into the evaluators instead of using the global variable kind? I'm afraid this requires changing Inox though to add a name field.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I did not want to chage Inox this time. Maybe later.

Copy link
Member

Choose a reason for hiding this comment

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

Instead of storing this in a global var, you could also compute the evaluator name to output inside the EvaluatorComponent based on ctx.options.findOptionOrDefault(optCodeGen) (especially since the property isn't really global).

Copy link
Collaborator Author

@vkuncak vkuncak May 31, 2021

Choose a reason for hiding this comment

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

I don't like checking the option since it's not telling me what the evaluator actually is, but what it should have been set to and there is complex initialization logic that determines that. Unfortunately I cannot use reflection and print dynamic class name e.g.with .getClass.getSimpleName() because somewhere in the code an anonymous class is created and the actual class name is lost.

Another question: how do I make --check-models benefit from this?

Copy link
Member

Choose a reason for hiding this comment

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

Reflection should work, I think there's something like getClass.getSimpleName which isn't too mangled.

Model checking currently takes place in Inox. The easiest way to get it to benefit from the new behavior would be to always disable optCheckModels in Inox (like here) and check models inside Stainless. The logic on the Inox side is here. Model checking in Stainless could actually be more powerful because we have access to the type checker, but you might need to be careful about running into loops.

val msg = (err match {
case Some(m) => m.toString + ": "
case None => ""
}) + pred.toString
Copy link
Contributor

Choose a reason for hiding this comment

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

We generally use asString instead of toString, so that printing respects options such as --print-ids.

reporter.info("Relevant variables at assertion failure point:")
val m = rctx.mappings
val fvs = exprOps.variablesOf(pred)
val fvDump: String = fvs.map(v => v.id.toString + " -> " + m.get(v.toVal).getOrElse("?").toString)
Copy link
Contributor

Choose a reason for hiding this comment

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

toString here too

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Replaced toString with asString in two places.

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