This variant returns ErrorOr<NonnullOwnPtr<T>> instead of KResultOr.
Eventually the KResultOr variant should go away once the kernel adopts
Error and ErrorOr<T>.
While I was working on LibWeb, I got a page fault at 0xe0e0e0e4.
This indicates a destroyed RefPtr if compiled with SANITIZE_PTRS
defined. However, the page fault handler didn't print out this
indication.
This makes the page fault handler print out a note if the faulting
address looks like a recently destroyed RefPtr, OwnPtr, NonnullRefPtr,
NonnullOwnPtr, ThreadSafeRefPtr or ThreadSafeNonnullRefPtr. It will
only do this if SANITIZE_PTRS is defined, as smart pointers don't get
scrubbed without it being defined.
This commit moves the KResult and KResultOr objects to Kernel/API to
signify that they may now be freely used by userspace code at points
where a syscall-related error result is to be expected. It also exposes
KResult and KResultOr to the global namespace to make it nicer to use
for userspace code.
This is basically a complement to adopt_nonnull_ref_or_enomem, and
simplifies boilerplate for try_create functions which just return ENOMEM
or the object based on whether it was able to allocate.
Aggregate initialization with brace-enclosed parameters is a
[C++20 feature][1] not yet implemented by Clang. This caused compile
errors if we tried to use the factory functions to create smart pointers
to aggregates.
As a (temporary) fix, [the LWG's previously proposed solution][2] is
implemented by this commit.
Now, wherever it's not possible to direct-initialize, aggregate
initialization is performed.
[1]:
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p0960r3.html
[2]: http://www.open-std.org/jtc1/sc22/wg21/docs/lwg-active.html#2089
These functions abstract away the need to call the proper new operator
("throwing" or "non-throwing") and manually adopt the resulting raw
pointer. Modelled after the existing `NonnullOwnPtr<T> make()`
functions, these forward their parameters to the object's constructor.
Note: These can't be used in the common "factory method" idiom, as
private constructors can't be called from a standalone function.
The naming is consistent with AK's and Shell's previous implementation
of these:
- `make` creates a `NonnullOwnPtr<T>` and aborts if the allocation could
not be performed.
- `try_make` creates an `OwnPtr<T>`, which may be null if the allocation
failed.
- `create` creates a `NonnullRefPtr<T>`, and aborts on allocation
failure.
- `try_create` creates a `RefPtr<T>`, which may be null if the
allocation was not successful.
Unfortunately adopt_own requires a reference, which obviously does not
work well with when attempting to harden against allocation failure.
The adopt_own_if_nonnull() variant will allow you to avoid using bare
pointers, while still allowing you to handle allocation failure.
Also, the PeekType of smart pointers is now T* instead of const T*.
Note: This commit doesn't compile, it breaks HashMap::get() for some
types. Fixed in the next commit.
SPDX License Identifiers are a more compact / standardized
way of representing file license information.
See: https://spdx.dev/resources/use/#identifiers
This was done with the `ambr` search and replace tool.
ambr --no-parent-ignore --key-from-file --rep-from-file key.txt rep.txt *
(...and ASSERT_NOT_REACHED => VERIFY_NOT_REACHED)
Since all of these checks are done in release builds as well,
let's rename them to VERIFY to prevent confusion, as everyone is
used to assertions being compiled out in release.
We can introduce a new ASSERT macro that is specifically for debug
checks, but I'm doing this wholesale conversion first since we've
accumulated thousands of these already, and it's not immediately
obvious which ones are suitable for ASSERT.
Problem:
- Many constructors are defined as `{}` rather than using the ` =
default` compiler-provided constructor.
- Some types provide an implicit conversion operator from `nullptr_t`
instead of requiring the caller to default construct. This violates
the C++ Core Guidelines suggestion to declare single-argument
constructors explicit
(https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c46-by-default-declare-single-argument-constructors-explicit).
Solution:
- Change default constructors to use the compiler-provided default
constructor.
- Remove implicit conversion operators from `nullptr_t` and change
usage to enforce type consistency without conversion.
This looks at three things:
- if the type has a typedef `AllowOwnPtr', respect that
- if not, disallow construction if both of `ref()' and `unref()' are
present.
Note that in the second case, if a type only defines `ref()' or only
defines `unref()', an OwnPtr can be created, as a RefPtr of that type
would be ill-formed.
Also marks a `Performance' to explicitly allow OwnPtrs.
We should always leak to an observed variable, otherwise
it's an actual leak. This is similar to AK::RefPtr::leak_ref()
which is also marked as [[nodiscard]].
Same issue here as we had with RefPtr and NonnullRefPtr.
Since we can't make copies of an owning pointer, we don't get quite the
same static_ptr_cast<T> here. Instead I've only added a new templated
version of OwnPtr::release_nonnull() in this patch, to solve the only
issue that popped up.
I'm not sure what the best solution here is, but this works for now.
This was only used by HashTable::dump() which I used when doing the
first HashTable implementation. Removing this allows us to also remove
most includes of <AK/kstdio.h>.
As suggested by Joshua, this commit adds the 2-clause BSD license as a
comment block to the top of every source file.
For the first pass, I've just added myself for simplicity. I encourage
everyone to add themselves as copyright holders of any file they've
added or modified in some significant way. If I've added myself in
error somewhere, feel free to replace it with the appropriate copyright
holder instead.
Going forward, all new source files should include a license header.
Add the concept of a PeekType to Traits<T>. This is the type we'll
return (wrapped in an Optional) from HashMap::get().
The PeekType for OwnPtr<T> and NonnullOwnPtr<T> is const T*,
which means that HashMap::get() will return an Optional<const T*> for
maps-of-those.
Okay, so, OwnPtr<T>::release_nonnull() returns a NonnullOwnPtr<T>.
It assumes that the OwnPtr is non-null to begin with.
Note that this removes the value from the OwnPtr, as there can only be
a single owner.
This is just like OwnPtr (also single-owner), except it cannot be null.
NonnullOwnPtr is perfect as the return type of functions that never need to
return nullptr.
It's also useful as an argument type to encode the fact that the argument
must not be nullptr.
The make<Foo>() helper is changed to return NonnullOwnPtr<Foo>.
Note: You can move() out of a NonnullOwnPtr, and after that the object is
in an invalid state. Internally it will be a nullptr at this point, so we'll
still catch misuse, but the only thing that should be done in this state
is running the destructor. I've used consumable annotations to generate some
warnings when using a NonnullOwnPtr after moving from it, but these only
work when compiling with clang, so be aware of that.
We shouldn't allow constructing e.g an OwnPtr from a RefPtr, and similar
conversions. Instead just delete those functions so the compiler whines
loudly if you try to use them.
This patch also deletes constructing OwnPtr from a WeakPtr, even though
that *may* be a valid thing to do, it's sufficiently weird that we can
make the client jump through some hoops if he really wants it. :^)
This is prep work for supporting HashMap with NonnullRefPtr<T> as values.
It's currently not possible because many HashTable functions require being
able to default-construct the value type.
Also run it across the whole tree to get everything using the One True Style.
We don't yet run this in an automated fashion as it's a little slow, but
there is a snippet to do so in makeall.sh.
