This has been a long-standing problem, but we didn't use to call the
destructors of items on the stack unless we explicitly `pop()` or
`discard()` them.
When interpretation was interrupted midway-through (because something
failed), we left `Pointer`s on the stack. Since all `Block`s track what
`Pointer`s point to them (via a doubly-linked list in the `Pointer`),
that meant we potentially leave deallocated pointers in that list. We
used to work around this by removing the `Pointer` from the list before
deallocating the block.
However, we now want to track pointers to global blocks as well, which
poses a problem since the blocks are never deallocated and thus those
pointers are always left dangling.
I've tried a few different approaches to fixing this but in the end I
just gave up on the idea of never knowing what items are in the stack.
We already have an `ItemTypes` vector that we use for debugging
assertions. This patch simply enables this vector unconditionally and
uses it in the abort case to properly `discard()` all elements from the
stack. That's a little sad IMO but I don't know of another way of
solving this problem.
As expected, this is a slight hit to compile times:
https://llvm-compile-time-tracker.com/compare.php?from=574d0a92060bf4808776b7a0239ffe91a092b15d&to=0317105f559093cfb909bfb01857a6b837991940&stat=instructions:u
Rename isConstexpr to isValid, the former was always a bad name. Save a
constexpr bit in Function so we don't have to access the decl in
CheckCallable.
Create the Function* handles for all functions we see, but delay the
actual compilation until we really call the function. This speeds up
compile times with the new interpreter a bit.
Some function types are special to us, so add an enum and determinte the
function kind once when creating the function, instead of looking at the
Decl every time we need the information.
Note that PointerUnion::{is,get,dyn_cast} have been soft deprecated in
PointerUnion.h:
// FIXME: Replace the uses of is(), get() and dyn_cast() with
// isa<T>, cast<T> and the llvm::dyn_cast<T>
FunctionDecl::getBuiltinID() is surprisingly slow and we tend to call it
quite a bit, especially when interpreting builtin functions. Caching the
BuiltinID here reduces the time I need to compile the
floating_comparison namespace from builtin-functions.cpp from 7.2s to
6.3s locally.
I started out by adding a new pointer type for blocks, and I was fully
prepared to compile their AST to bytecode and later call them.
... then I found out that the current interpreter doesn't support
calling blocks at all. So we reuse `Function` to support sources other
than `FunctionDecl`s and classify `BlockPointerType` as `PT_FnPtr`.
