Problem:
- Access to the underlying type is not provided. This limits
metaprogramming and usage in function templates.
Solution:
- Provide public access to the underlying type.
- Add test to ensure the underlying type is accessible.
Problem:
- Raw loops are often written to validate that any values in a
container meet a predicate, but raw loops are not as expressive as
functions implementing well-named algorithms and are error-prone.
Solution:
- Implement a very generic form of `any_of`.
This API was a mostly gratuitous deviation from POSIX that gave up some
portability in exchange for avoiding the occasional strlen().
I don't think that was actually achieving anything valuable, so let's
just chill out and have the same open() API as everyone else. :^)
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.
Problem:
- The implementation of `find` is coupled to the implementation of
`SinglyLinkedList`.
Solution:
- Decouple the implementation of `find` from the class by using a
generic `find` algorithm.
Problem:
- The implementation of `find` is coupled to the implementation of
`DoublyLinkedList`.
- `append` and `prepend` are implemented multiple times so that
r-value references can be moved from into the new node. This is
probably not called very often because a pr-value or x-value needs
to be used here.
Solution:
- Decouple the implementation of `find` from the class by using a
generic `find` algorithm.
- Make `append` and `prepend` be function templates so that they can
have binding references which can be forwarded.
Problem:
- The implementation of `find` is coupled to the implementation of `Vector`.
- `Vector::find` takes the predicate by value which might be expensive.
Solution:
- Decouple the implementation of `find` from `Vector` by using a
generic `find` algorithm.
- Change the name of `find` with a predicate to `find_if` so that a
binding reference can be used and the predicate can be forwarded to
avoid copies.
- Change all the `find(pred)` call sites to use `find_if`.
Problem:
- `find` is implemented inside of each container. This coupling
requires that each container needs to individually provide `find`.
Solution:
- Decouple the `find` functionality from the container. This allows
provides a `find` algorithm which can work with all
containers. Containers can still provide their own `find` in the
case where it can be optimized.
- This also allows for searching sub-ranges of a container rather than
the entire container as some of the container-specific member
functions enforced.
Note:
- @davidstone's talk from 2015 C++Now conference entitled "Functions
Want to be Free" encourages this style:
(https://www.youtube.com/watch?v=_lVlC0xzXDc), but it does come at
the cost of composability.
- A logical follow-on to this is to provide a mechanism to use a
short-hand function which automatically searches the entire
container. This could automatically use the container-provided
version if available so that functions which provide their own
optimized version get the benefit.
Problem:
- Clang ToT fails to build `AK/Tests/TestTypeTraits.cpp` because
`nullptr_t` is missing the `std` namespace qualifier.
Solution:
- Prepend the namespace qualifier.
Add requires clauses to constraints on InputStream and OutputStream
operator<< / operator>>. Make the constraint on String::number a
requires clause instead of SFINAE. Also, fix some unecessary IsSame in
Trie where specialized traits exist for the given use cases.
Use TypeLists to add test for IsIntegral, IsFloatingPoint, IsVoid,
IsNullPointer, IsArithmetic, IsFundamental, and AddConst type traits.
More can "easily" be added once the TypeList and macro magic is squinted
at for long enough :).
I ran into this exact but at least twenty times in Serenity alone. The
C++ Standard dictates that 'unsigned long' and 'unsigned long long' are
distinct types even though on most platforms they are usually both 64
bit integers.
Also it wasn't possible to evaluate IsIntegral<T> for types that were
not integers since it used MakeUnsigned<T> internally.
Problem:
- C functions with no arguments require a single `void` in the argument list.
Solution:
- Put the `void` in the argument list of functions in C header files.
Problem:
- File globbing is performed at the time of build system
generation. Any files which are not there at that time are not
included. So, when a new file is added it is not built unless the
build system is recreated.
Solution:
- Remove globbing from AK/Tests directory in favor of explicitly
listing the files.
Problem:
- `(void)` simply casts the expression to void. This is understood to
indicate that it is ignored, but this is really a compiler trick to
get the compiler to not generate a warning.
Solution:
- Use the `[[maybe_unused]]` attribute to indicate the value is unused.
Note:
- Functions taking a `(void)` argument list have also been changed to
`()` because this is not needed and shows up in the same grep
command.
This uses the KMP algorithm to implement the search.
Also replaces the slow route of the normal memmem() with KMP, which
should be fairly faster (O(n + m) as opposed to O(n * m)) :^)
Fixes a regression introduced by 5c1b3ce. The commit description there
asserts that the changes allow calling will_be_destroyed and
one_ref_left, which are not required to be const qualified. The
implementation in fact does require the methods to be const qualified,
because we forgot to add the const_cast inside the decltypes :^)
Problem:
- It is difficult to refactor because there are no tests to bind the
functionality.
- Arguments are not forwarded correctly to the constructor.
Solution:
- Add tests.
- Change constructor to take forwarding references.
Problem:
- Raw loops are often written to validate that all values in a
container meet a predicate, but raw loops are not as expressive as
functions implementing well-named algorithms and are error-prone.
Solution:
- Implement a very generic form of `all_of`.
Problem:
- C-style arrays do not automatically provide bounds checking and are
less type safe overall.
- `__builtin_memcmp` is not a constant expression in the current gcc.
Solution:
- Change private m_data to be AK::Array.
- Eliminate constructor from C-style array.
- Change users of the C-style array constructor to use the default
constructor.
- Change `operator==()` to be a hand-written comparison loop and let
the optimizer figure out to use `memcmp`.
Problem:
- `MACAddress` class is not usable in a compile-time context.
- `__builtin_memcpy` is not constexpr in gcc.
Solution:
- Decorate functions with `constexpr` keyword.
- Use default constructors and destructors.
- Change `__builtin_memcpy` to a hand-written `for` loop and let the
compiler's optimizer take care of it.
- Add tests to ensure compile-time capabilities.
This fixes an OOB access when the last read/written chunk is empty (as we _just_
started on a new chunk).
Also adds a test case to TestMemoryStream.
Found via human fuzzing in the shell:
```sh
for $(cat /dev/urandom) {
clear
match $it {
?* as (x) {
echo $x
sleep 1
}
}
}
```
would assert at some point.
Personally I found this unintuitive at first, but it is in line with
strtol(), Python's int() or JavaScript's parseInt(), so I guess it makes
sense.
Fixes#4097.
Problem:
- There are no unit tests for `MACAddress` class. This makes it
difficult to refactor and ensure the same behavior.
- `m_data` private member variable is uninitialized leading to undefined
behavior of `is_zero()`.
Solution:
- Add unit tests to cover basic functionality.
- Initialize `m_data`.
Problem:
- C++20 changes the way equality operators are generated. This results
in overload ambiguity as reported by clang.
Solution:
- Remove `AK::Vector::operator!=` because it will be automatically
generated in terms of `AK::Vector::operator==`.
- Change `AK::Vector::operator==` to be a function template so that
overload resolution is not confused about `a == b` vs `b == a`.
- Add tests to ensure the behavior works.
Notes:
- There is more info available at
https://brevzin.github.io/c++/2019/07/28/comparisons-cpp20/ for
deeper discussion about overload resolution, operator rewriting, and
generated functions.
Problem:
- `typedef` is a keyword which comes from C and carries with it old
syntax that is hard to read.
- Creating type aliases with the `using` keyword allows for easier
future maintenance because it supports template syntax.
- There is inconsistent use of `typedef` vs `using`.
Solution:
- Use `clang-tidy`'s checker called `modernize-use-using` to update
the syntax to use the newer syntax.
- Remove unused functions to make `clang-tidy` happy.
- This results in consistency within the codebase.
Problem:
- IPv4Address class cannot be used in a compile-time context.
- A union is used by initializing one of the members and reading the
non-active member. This is undefined behavior and not permitted in a
`constexpr` context.
Solution:
- Eliminate undefined behavior by changing to a simple `u32` for
storage instead of the union and performing mask/shift calculations
for obtaining the individual octets.
- Decorate functions with `constexpr` where possible. Currently string
formatting and optionals are not `constexpr`-capable so functions
using those are left out.
- Modify tests to validate functionality in a `constexpr` context in
addition to the run-time tests already being run. This ensures that
functionality is the same in both contexts.
This makes most operations thread safe, especially so that they
can safely be used in the Kernel. This includes obtaining a strong
reference from a weak reference, which now requires an explicit
call to WeakPtr::strong_ref(). Another major change is that
Weakable::make_weak_ref() may require the explicit target type.
Previously we used reinterpret_cast in WeakPtr, assuming that it
can be properly converted. But WeakPtr does not necessarily have
the knowledge to be able to do this. Instead, we now ask the class
itself to deliver a WeakPtr to the type that we want.
Also, WeakLink is no longer specific to a target type. The reason
for this is that we want to be able to safely convert e.g. WeakPtr<T>
to WeakPtr<U>, and before this we just reinterpret_cast the internal
WeakLink<T> to WeakLink<U>, which is a bold assumption that it would
actually produce the correct code. Instead, WeakLink now operates
on just a raw pointer and we only make those constructors/operators
available if we can verify that it can be safely cast.
In order to guarantee thread safety, we now use the least significant
bit in the pointer for locking purposes. This also means that only
properly aligned pointers can be used.
