By doing the offset calculation in {get,put}_by_index() we would
delegate these operations to Object for any index >= (array length -
byte offset). By doing the offset calculation in data() instead, we can
just use the unaltered property index for indexing the returned Span.
In other words: data()[0] now returns the same value as indexing the
TypedArray at index 0 in JS.
This also fixes a bug in the js REPL which would not consider the byte
offset and subsequently access the underlying ArrayBuffer data with a
wrong index.
Problem:
- `static` variables consume memory and sometimes are less
optimizable.
- `static const` variables can be `constexpr`, usually.
- `static` function-local variables require an initialization check
every time the function is run.
Solution:
- If a global `static` variable is only used in a single function then
move it into the function and make it non-`static` and `constexpr`.
- Make all global `static` variables `constexpr` instead of `const`.
- Change function-local `static const[expr]` variables to be just
`constexpr`.
We already do this for the SimpleIndexedPropertyStorage, so for indexed
properties with GenericIndexedPropertyStorage this would previously
crash. Since overwriting the array-like size with a larger value won't
magically insert values at previously unset indices, we need to handle
such an out of bounds access gracefully and just return an empty value.
Fixes#7043.
This was a bit hard to find as a local variable - rename it to uppercase
LENGTH_SETTER_GENERIC_STORAGE_THRESHOLD and move it to the top (next to
SPARSE_ARRAY_HOLE_THRESHOLD) for good visibility.
Before this patch, every shape would permanently remember every other
shape it had ever transitioned to. This could lead to pathological
accumulation of unused shape objects in some cases.
Fix this by using WeakPtr instead of a strongly visited Shape* in the
the forward transition chain map. This means that we will now miss out
on some shape sharing opportunities, but since this is not required
for correctness it doesn't matter.
Note that the backward transition chain is still strongly cached,
as it's necessary for the reification of property tables.
An interesting future optimization could be to allow property tables
to get garbage collected (by detaching them from the shape object)
and then reconstituted from the backwards transition chain (if needed.)
If we're able to allocate cells from a freelist, we should always
prefer that over the lazy freelist, since this may further defer
faulting in additional memory for the HeapBlock.
Thanks to @gunnarbeutner for pointing this out. :^)
HeapBlock now implements the same lazy freelist as LibC malloc() does,
where new blocks start out in a "bump allocator" mode that gets used
until we've bump-allocated all the way to the end of the block.
Then we fall back to the old freelist style as before.
This means we don't have to pre-initialize the freelist on HeapBlock
construction. This defers page faults and reduces memory usage for
blocks where all cells don't get used. :^)
This had very bad interactions with ccache, often leading to rebuilds
with 100% cache misses, etc. Ali says it wasn't that big of a speedup
in the end anyway, so let's not bother with it.
We can always bring it back in the future if it seems like a good idea.
Absolutely massive allocations > 1024 bytes would go into the size
class which was 3172 bytes. 3172 happens to not be 8 byte aligned, and
so made UBSAN very sad on x86_64. Change the largest allocator to be
3072 bytes, which is in fact a multiple of 8 :^)
When using VM::set_variable() to put the created ScriptFunction onto a
ScopeObject, we would previously unexpectedly reach the global object as
set_variable() checks each traversed scope for an existing Variable with
the given name - which would cause a leak of the inner function past the
outer function (we even had a test expecting that behaviour!). Now we
first declare functions (as DeclarationKind::Var) before setting them.
This will need some more work to make hoisting across non-lexical scopes
work, but it fixes this specific issue for now.
Fixes#6766.
The TryStatement handler execution creates a new LexicalEnvironment
without a current function set, which we were not accounting for when
trying to get the super constructor while executing a SuperExpression.
This makes it work but isn't pretty - this needs some refactoring to be
close to the spec for that to happen.
Fixes#7045.
This is a partial revert of commit 60064e2, which removed the validation
of RegExp flags during runtime and expected the parser to do that
exclusively - however this was not taking into account the RegExp()
constructor, which was subsequently crashing on invalid flags.
Also adds test for these constructor error cases, which were obviously
missing before.
Fixes#7042.
This patch changes the validation of RegExp flags (checking for
invalid and duplicate values) from a SyntaxError at runtime to a
SyntaxError at parse time - it's not something that's supposed to be
catchable.
As a nice side effect, this simplifies the RegExpObject constructor a
bit, as it can no longer throw an exception and doesn't have to validate
the flags itself.
This is how it's stored internally - even though we still only construct
from i32. I had the compiler yell at me while trying something with this
and didn't want to add yet another cast, so let's quickly fix this.
In HackStudio's Debugger a custom GlobalObject is used to reflect
debugger variables into the JS scope by overriding GlobalObject's
get method. However, when throwing a custom error during that lookup
it was replaced with the generic "not found" js exception. This patch
makes it instead pass along the custom error.
LibWeb is now responsible for logging unhandled exceptions itself,
which means set_should_log_exceptions() is no longer used and can be
removed. It turned out to be not the best option for web page exception
logging, as we would have no indication regarding whether the exception
was later handled of not.
Instead of having to run queued promise jobs in LibWeb in various
places, this allows us to consolidate that into one function - this is
very close to how the spec describes it as well ("at some future point
in time, when there is no running execution context and the execution
context stack is empty, the implementation must [...]").
Eventually this will also be used to log unhandled exceptions, and
possibly other actions that require JS execution to have ended.
Instead of storing the function names (in a badly named Vector<String>)
and source ranges separately, consolidate them into a new struct:
TracebackFrame. This makes it both easier to use now and easier to
extend in the future.
Unlike before we now keep each call frame's current node source range
in the traceback frame next to the function name, meaning we can display
line and column numbers outside of the VM and after the call stack is
emptied.
This would return an empty value once it hits an exception check
otherwise. Considering that this mostly is used in situations where we
already *do* have an exception (traceback printing, for example), let's
make this easier for ourselves to use.
This is very similar to AK::TemporaryChange (and in fact replaces one
use of it), but since we can't directly set VM's m_exception from
outside of the VM, we need something more sophisticated.
Sometimes we need to temporarily remove the stored exception for some
other operation to succeed (e.g. anything that uses call(), as well as
get_without_side_effects()) and later restore it - the boilerplate code
required for this is annoying enough to justify a helper.
The native C++ < and > operators won't handle this correctly, so the
result was different depending on the order of arguments. This is now
fixed by explicitly checking for positive and negative zero values.
Fixes#6589.