By golly, this is a lot more spec comments than I originally thought
I would need to do! This has exposed some bugs in the implementation,
as well as a whole lot of things which we are yet to implement.
No functional changes intended in this commit (already pretty large
as is!).
ECMA-262 implies that `MIN_VALUE` should be a denormalized value if
denormal arithmetic is supported. This is the case on x86-64 and AArch64
using standard GCC/Clang compilation settings.
test262 checks whether `Number.MIN_VALUE / 2.0` is equal to 0, which
only holds if `MIN_VALUE` is the smallest denormalized value.
This commit renames the existing `NumericLimits<FloatingPoint>::min()`
to `min_normal()` and adds a `min_denormal()` method to force users to
explicitly think about which one is appropriate for their use case. We
shouldn't follow the STL's confusingly designed interface in this
regard.
Instead of checking the address of a temporary, grab the address of the
current frame pointer to determine how much memory is left on the stack.
This better communicates to the compiler what we're trying to do, and
resolves some crashes with ASAN in test-js while the option
detect_stack_use_after_return is turned on.
This was missed in 02b74e5a70
We need to disable consteval in AK::String as well as AK::StringView,
and we need to disable it when building both the tools build and the
fuzzer build.
These recursive templates have a measurable impact on the compile speed
of Variant-heavy code like LibWeb. Using these builtins leads to a 2.5%
speedup for the measured compilation units.
oss-fuzz ships a pre-release commit of clang-15 for all of their build
bots. Until they update to a release of clang-15 that includes the fix
for this bug, or a later release, we need to keep the workaround in
place.
The Windows CRT definition of assert() is not noreturn, and causes
compile errors when using it as the backing for VERIFY() in debug
configurations of applications like the Jakt compiler.
Apple Clang 14.0.3 (Xcode 14.3) miscompiles this builtin on AArch64,
causing the borrow flag to be set incorrectly. I have added a detailed
writeup on Qemu's issue tracker, where the same issue led to a hang when
emulating x86:
https://gitlab.com/qemu-project/qemu/-/issues/1659#note_1408275831
I don't know of any specific issue caused by this on Lagom, but better
safe than sorry.
GCC 14 (https://gcc.gnu.org/g:2b4e0415ad664cdb3ce87d1f7eee5ca26911a05b)
has added support for the previously Clang-specific add/subtract with
borrow builtins. Let's use `__has_builtin` to detect them instead of
assuming that only Clang has them. We should prefer these to the
x86-specific ones as architecture-independent middle-end optimizations
might deal with them better.
As an added bonus, this significantly improves codegen on AArch64
compared to the fallback implementation that uses
`__builtin_{add,sub}_overflow`.
For now, the code path with the x86-specific intrinsics stays so as to
avoid regressing the performance of builds with GCC 12 and 13.
The previous version had a sequence of calls that are likely not
optimized out, while this version is strictly a sequence of static type
conversion which are always fully optimized out.
The previous alignment would always resolve to 8-bytes, which is below
the required alignments of types that could exist in userspace (long
double, 128-bit integers, SSE, etc).
The FileSlash state was erroneously copying the base URL host, instead
of the base URL path excluding the last path component. This resulted in
invalid file URLs.
Calling `from_utf8` with a DeprecatedString will hide the fact that we
have a DeprecatedString, while using `from_deprecated_string` with a
StringView will silently and needlessly allocate a DeprecatedString,
so let's forbid that.
This does a few things:
- The decoder uses a 32- or 64-bit integer as a reservoir of the data
being decoded, rather than one single byte as it was previously.
- `read_bool()` only refills the reservoir (value) when the size drops
below one byte. Previously, it would read out a bit-sized range from
the data to completely refill the 8-bit value, doing much more work
than necessary for each individual read.
- VP9-specific code for reading the marker bit was moved to its own
function in Context.h.
- A debug flag `VPX_DEBUG` was added to optionally enable checking of
the final bits in a VPX ranged arithmetic decode and ensure that it
contains all zeroes. These zeroes are a bitstream requirement for
VP9, and are also present for all our lossy WebP test inputs
currently. This can be useful to test whether all the data present in
the range has been consumed.
A lot of the size of this diff comes from the removal of error handling
from all the range decoder reads in LibVideo/VP9 and LibGfx/WebP (VP8),
since it is now checked only at the end of the range.
In a benchmark decoding `Tests/LibGfx/test-inputs/4.webp`, decode times
are improved by about 22.8%, reducing average runtime from 35.5ms±1.1ms
down to 27.4±1.1ms.
This should cause no behavioral changes.
Change the name and return type of
`IPv6Address::to_deprecated_string()` to `IPv6Address::to_string()`
with return type `ErrorOr<String>`.
It will now propagate errors that occur when writing to the
StringBuilder.
There are two users of `to_deprecated_string()` that now use
`to_string()`:
1. `Formatted<IPv6Address>`: it now propagates errors.
2. `inet_ntop`: it now sets errno to ENOMEM and returns.
This has KString, KBuffer, DoubleBuffer, KBufferBuilder, IOWindow,
UserOrKernelBuffer and ScopedCritical classes being moved to the
Kernel/Library subdirectory.
Also, move the panic and assertions handling code to that directory.
This partially implements CSS-Animations-1 (though there are references
to CSS-Animations-2).
Current limitations:
- Multi-selector keyframes are not supported.
- Most animation properties are ignored.
- Timing functions are not applied.
- Non-absolute values are not interpolated unless the target is also of
the same non-absolute type (e.g. 10% -> 25%, but not 10% -> 20px).
- The JavaScript interface is left as an exercise for the next poor soul
looking at this code.
With those said, this commit implements:
- Interpolation for most common types
- Proper keyframe resolution (including the synthetic from-keyframe
containing the initial state)
- Properly driven animations, and proper style invalidation
Co-Authored-By: Andreas Kling <kling@serenityos.org>
This class takes on the duties of CLOCK_MONOTONIC, a time without a
defined reference point that always increases. This informs some
important design decisions about the class API: MonotonicTime cannot be
constructed from external time data, except as a computation based on
other monotonic time, or the current monotonic time. Importantly, there
is no default constructor, since the reference point of monotonic time
is unspecified and therefore without meaning as a default.
The current use of monotonic time (via Duration) includes some potential
problems that may be caught when we move most to all code to
MonotonicTime in the next commit.
The API restrictions have one important relaxation:
Kernel::TimeManagement is allowed to exchange raw time data within
MonotonicTime freely. This is required for the clock-agnostic time
accessors for timeouts and syscalls, as well as creating monotonic time
data from hardware in the first place.
"Wherever applicable" = most places, actually :^), especially for
networking and filesystem timestamps.
This includes changes to unzip, which uses DOSPackedTime, since that is
changed for the FAT file systems.
This is a generic wrapper for a time instant relative to the unix epoch,
and does not account for leap seconds. It should be used in place of
Duration in most current cases.
This is a trivial change, and since this batch of commits will make a
large-scale rebuild necessary anyways, it seems sensible. The feature is
useful for e.g. building compound constant durations at compile time in
a readable way.
That's what this class really is; in fact that's what the first line of
the comment says it is.
This commit does not rename the main files, since those will contain
other time-related classes in a little bit.
This attribute is used for functions in the kernel that are entirely
written in assembly, yet defined in C++ source files.
Without `__attribute__((naked))`, Clang might decide to inline these
functions, making any `ret` instructions within them actually exit the
caller, or discard argument values as they appear "dead". This issue
caused a kernel panic when using the `execve` syscall in AArch64
SerenityOS built by Clang.
While the empty definition so far appears to work fine with GCC, simpler
test cases do similarly suffer from unintended inlining, so define
`NAKED` as a synonym of `NEVER_INLINE` to avert future issues.
Perhaps we should move users of `NAKED` to plain assembly files?
This makes aarch64Clang builds boot :^)
This underlines that we still copy-construct and copy-assign HashMaps.
Primarily, this makes it easier to develop towards OOM-safe(r) internal
data structures, by providing a reminder (the FIXME) and an easy error-
checking switch (just change it to "delete" to see some of the errors).
See identical code in LittleEndianBitStream; even in the bytewise
reading BigEndianBitStream an offset of 8 is not inconsistent state and
handled just fine by read_bits.
Rather than tracking our position in the bit buffer, we can simply shift
away the bits that we read. This is mostly for simplicity, but also does
help performance a bit.
Using the "enwik8" file as a test (100MB uncompressed, commonly used in
benchmarks: https://www.mattmahoney.net/dc/enwik8.zip), compression time
decreases from:
3.96s to 3.79s on Serenity (cold)
1.08s to 1.04s on Serenity (warm)
0.83s to 0.82s on Linux
We have outln() and out(), warnln() and warn(),
now we have dbgln() and dbg().
This is useful for printing arrays element-by-element while still
only printing one line per array.
This class, in a similar fashion to what has been done with
`InputBufferedStream`, postpones write to the stream until an internal
buffer is full.
This patch also adds the `OutputBufferedFile` alias.
In a similar fashion to what have been done with `fill_from_stream`,
this new method allows to write CircularBuffer's data to a Stream
without additional copies.
Due to 582c55a, both `must_set()` and `set()` should be providing the
same behavior. Not only is that a reason to remove `must_set()`, but
it also performs erroneous behavior since it inserts an element at
a specified index instead of modifying an element at that index.
"The official project language is American English […]."
5d2e915623/CONTRIBUTING.md (L30)
Here's a short statistic of the occurrences of the word "behavio(u)r":
$ git grep -IPioh 'behaviou?r' | sort | uniq -c | sort -n
2 BEHAVIOR
24 Behaviour
32 behaviour
407 Behavior
992 behavior
Therefore, it is clear that "behaviour" (56 occurrences) should be
regarded a typo, and "behavior" (1401 occurrences) should be preferred.
Note that The occurrences in LibJS are intentionally NOT changed,
because there are taken verbatim from the specification. Hence:
$ git grep -IPioh 'behaviou?r' | sort | uniq -c | sort -n
2 BEHAVIOR
10 behaviour
24 Behaviour
407 Behavior
1014 behavior
Our current `peek_bits` function allows retrieving more bits than we can
actually provide, so whenever someone discards the requested bit count
afterwards we were underflowing the value instead.
Resolves#18618.
8134dcc changed `JsonArray::set()` to insert elements at an index
instead of changing existing elements in-place. Since no behavior
such as `Vector::try_at()` exists yet, it returns nothing.
GCC 13 was released on 2023-04-26. This commit fixes Lagom build errors
when using an updated host toolchain:
- Adds a workaround for a bug in constraint handling, which made LibJS
fail to compile: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109683
- Silences the new `-Wdangling-reference` diagnostic globally. It
produces multiple false positives with no clear way to silence them
without `#pragmas`.
- Silences `-Wself-move` in `RefPtr` tests as GCC 13 adds this
previously Clang-exclusive warning.
This further optimizes floating point parsing (specifically with a large
amount of digits). The commit shaves additional 20% of the run time for
750-digit numbers. No performance degradation is noticeable for small
numbers.
Although it might seem like we've switched to more generic functions,
which must run slower, it is not the case. The time required to parse
"1", for example, decreased by 1%. For numbers with more digits, the
effect is more noticeable: 8-digit numbers are parsed ~5% faster; for
gigantic 750-digit numbers, parsing is 2 times faster.
The later result is achieved by using UFixedBigInt<64>::wide_multiply
instead of u128::operator*(u128).
Additionally, split it into two versions (for IsIntegral<T> -- asking
to place value into register and for !IsIntegral<T> -- asking to place
value into memory with memory clobber), so that Clang is no more
completely confused about `taint_for_optimizer(AK::StringView&)`.
There's no real reason to make this a debug-only formatter, on top of
that, jakt has a optional formatter that prints None/foo instead of
OptionalNone/Optional(foo), which is more concise anyway, so switch to
that.
I found this handy for debugging, and so might others.
This now also adds a formatter for TimeZone::TimeZone. This is needed
for FormatIfSupported<Optional<TimeZone::TimeZone>> to compile. As
FormatIfSupported sees a formatter for Optional exists, but not that
there's not one for TimeZone::TimeZone.
While swap() is available in the global namespace in normal conditions,
!USING_AK_GLOBALLY will make this name unavailable in the global
namespace, making these calls fail to compile.
That pattern seems to show up a lot in code written by people that
aren't intimately familiar with the lifetime model of Error and Strings.
This commit makes the compiler detect it and present a more helpful
diagnostic than "garbage string at runtime".
This brings the function name in line with how we usually name those
functions, which is with a `read_` or `write_` prefix depending on what
they do.
While at it, make the internal `_impl` function private and not-virtual,
since there is no good reason to ever override that function.
