@Kha @dselsam:
This hack was preventing us from making `Expr` a "real" Lean type.
This was bad for a few reasons:
- It was hard to extend/modify `Expr` in Lean since we would also have
to modify the C++ code that creates the `Expr` objects with the hidden
fields.
- `Expr.lam` and `Expr.forallE` were not following the Lean layout
standard where we sort fields by size. @Kha: recall we used that to
avoid a UB. The issue with `Expr.lam` and `Expr.forallE` is that they
have a "visible" field (`BinderInfo`), which is smaller than
hidden fields such as hash code.
- `Expr.fvar` had only one field at `Expr.lean,` but four behind the
scenes.
I added a new constructor `Local` that is only accessible from C++.
It is only used in legacy code we inherited from Lean2.
We will eventually delete it.
This refactoring was quite painful since many parts of the codebase
were mixing the new `Expr.fvar` with the old `Expr.local`.
I doubt I would be able to do it without the new staging framework
@Kha built.
BTW, some of the patches are horrible. I didn't care much since we
are going to deleted the super ugly files. That being said,
you should expect new weird bevaior due to `Expr.fvar` vs `Expr.local`.
Next step: use the new `ExprCachedData` to make all `Expr` hidden visibles
accessible from Lean.
checkpoint
@kha @dselsam: I added a small repro for the bug reported by Daniel on
Zulip. The current fix is not polished at all since we will replace
the equation compiler with one implemented in Lean. The bug is once
again on the code that handle nested `match`-expressions containing
recursive calls. We had problems in this module before, and the
current compilation strategy using auxiliary `*._match_<id>` functions
is also very inconvenient for users. They are often puzzled when they
see these auxiliary functions appearing in proof goals after unfolding
and/or simplification. They usually don't know what to do with these
auxiliary definitions, and have no idea how they were defined and what
they correspond to if the function has several nested
`match`-expression. Right now, the best option is to use `#print
<fun-name>._match_<id>` which is far from ideal.
@kha: @dselsam and I discussed an alternative approach where we do not
create the auxiliary definitions, annotate the generated `cases_on`
applications with meta-data indicating they correspond to a nested
match, and modify the pretty printer to display these annotated
`cases_on` applications using the `match` syntax. With these
modifications, the behavior will be similar to the one in Coq where
complex `match`-expressions are reduced to atomic ones. The only
difference is that we represent these "atomic" `match`-expressions
using `cases_on` applications.
This commit uses a simpler version of this approach where we do not
create auxiliary `*._match_<id>` functions, and more importantly do
not use the dreadful `pull_nested_rec_fn` code.
The idea is to mimic the behavior of meta definitions.
This is a temporary fix. The new equation compiler will have to provide
a better solution for this problem.
- Lean strings (like std::string) may contain null characters. The
codebase was ignoring this issue.
- We now have a wrapper `string_ref` for wrapping Lean string objects in
C++. This wrapper also implements correctly the coercions std::string <-> string_ref.
Remark: I also found a few places where the code relies on the
following property which is not true
Forall s : std::string, std::string(s.c_str()) == s
- `name` object wrapper was assuming that all numerals were small
`nat` values. This is true in most cases, but the system would
crash when processing if it is a big number.
- The commit tries to make sure runtime/util/kernel are correct.
Modules that will be deleted contain many `TODO` comments
indicating they may crash and/or produce incorrect results
when strings contain null characters and numerals are big.
cc @kha
@kha: I thought about using `string` instead of `string_ref`.
We consistently use `std::string`. So, it should be fine, but I
was concerned about code readability.
After we bootstrap Lean4, we will be able to delete `lean::list`
template, and rename `lean::list_ref` to `lean::list`.
I am going to add `pair_ref` for wrapping Lean pair objects.
If we use `lean::string` instead of `lean::string_ref`, then
we should also use `lean::pair` instead of `lean::pair_ref`.
But, there is a problem in this case since we have
https://github.com/leanprover/lean4/blob/master/src/util/pair.h#L13
:(
Motivations:
- Clear execution cost semantics for recursive functions.
- Auxiliary meta definition may assist recursive definition unfolding in the type_context object.
Next step: use meta auxiliary definition at code generation.
Fixes#1363
After error recovery has been implemented in the elaborator, a few
assumptions made in the type context are not valid anymore since we may
be recovering from errors, and the local and metavariable contexts may
be invalid.
I used the approach used in the class environment.
- find* methods return optional<...>
- get* methods throw exception for unknown elements
Remarks:
I preserved code patterns such as
optional<local_decl> d = lctx.find_local_decl(...)
lean_assert(d)
and did not convert them into
local_decl d = lctx.get_local_decl(...)
Reason: the intention is clear that the local must be defined there.
If it is not we should analyze the problem and decide whether we should
throw an exception or not.
However, I converted code patterns such as
local_decl d = *lctx.find_local_decl(...)
into
local_decl d = lctx.get_local_decl(...)
Disclaimer: this change fixes issue #1363, but it may obfuscate other bugs.
