Commit graph

1 commit

Author SHA1 Message Date
Leonardo de Moura
7a58da1181 fix(library/tactic/user_attribute): nasty interaction between eval_expr and attribute_manager
eval_expr creates auxiliary definitions in the VM. These auxiliary
definitions are gone after the VM finishes.

We store vm_obj's in the attribute_manager.

Before this commit, Lean was crashing in the following scenario:

1- A new caching_user_attribute is defined, and the user data structure
contains closures.

2- The closures are created using eval_expr.

3- When reusing the cached values, the system crashes when trying
to apply a closure created using eval_expr. The closure points to
an auxiliary definition that has already been deleted.

The new test exposes the problem. This is not a hypothetical scenario,
the new test is based on the Lean - Mathematica integration being
developed by @rlewis1988.

The fix consists in making sure we do not cache anything if
the VM environment has been updated by eval_expr.

I believe this is acceptable behavior. eval_expr is a very low level
tactic, and I don't see a good motivation for invoking it when
constructing the cache.

BTW, the test can be relaxed if the vm_attr does not contain closures.
However, it doesn't seem to pay off.

Another potential fix would be to propagate the definitions created
by eval_expr to the main environment. However, I think this is not
acceptable.
We will be flooding the main environment with useless temporary definitions
created by `eval_expr`.

This commit also stores the environment at caching time, and make
sure the cache is only reused if the current environment is a descendant
of the the one at caching time. This is fixing a different potential
bug.
2017-02-10 15:24:01 -08:00