(...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.
Arbitrarily split up to make git bisect easier.
These unnecessary #include's were found by combining an automated tool (which
determined likely candidates) and some brain power (which decided whether
the #include is also semantically superfluous).
My favorite #include:
#include "Applications/Piano/Music.h" // You can't have too much music in life!
Formatter<char> internally uses Formatter<StringView> when in
Mode::Character, but that would only accept Mode::{Default,String} and
ASSERT_NOT_REACHED() otherwise, causing String::formatted("{:c}", 'a')
to crash
This commit also removes a few functions like raw_out and vwarn. If we
want to write raw output, we can do this as follows:
out("{}", "Hello, World!");
The vout stuff isn't really public API anyways, so no need for another
vwarn.
We are adding the process name as prefix and a newline as suffix to any
message written to debug. Thus, the following doesn't make any sense:
for (u8 byte : bytes)
dbg("{:02x} ", byte);
dbgln();
Which function call would put the prefix? This doesn't make any sense,
thus these functions must go.
The example above could be converted to:
StringBuilder builder;
for (u8 byte : bytes)
builder.appendff("{:02x} ", byte);
dbgln("{}", builder.build());
Formatter is specialized in the header file. The definition in the
implementation file is extraneous and has no effect. Simply removing
it so that there is no confusion.
When we write the format specifier '{:#08x}' we are asking for eight
significant digits, zero padding and the prefix '0x'.
However, previously we got only six significant digits because the
prefix counted towards the width. (The number '8' here is the total
width and not the number of significant digits.)
Both fmtlib and printf shared this behaviour. However, I am introducing
a special case here because when we do zero padding we really only care
about the digits and not the width.
Notice that zero padding is a special case anyways, because zero padding
goes after the prefix as opposed to any other padding which goes before
it.
In the future all (normal) output should be written by any of the
following functions:
out (currently called new_out)
outln
dbg (currently called new_dbg)
dbgln
warn (currently called new_warn)
warnln
However, there are still a ton of uses of the old out/warn/dbg in the
code base so the new functions are called new_out/new_warn/new_dbg. I am
going to rename them as soon as all the other usages are gone (this
might take a while.)
I also added raw_out/raw_dbg/raw_warn which don't do any escaping,
this should be useful if no formatting is required and if the input
contains tons of curly braces. (I am not entirely sure if this function
will stay, but I am adding it for now.)
It's now save to pass a signed integer as parameter and then use it as
replacement field (previously, this would just cast it to size_t which
would be bad.)
This finally takes care of the kind-of excessive boilerplate code that were the
ctype adapters. On the other hand, I had to link `LibC/ctype.cpp` to the Kernel
(for `AK/JsonParser.cpp` and `AK/Format.cpp`). The previous commit actually makes
sense now: the `string.h` includes in `ctype.{h,cpp}` would require to link more LibC
stuff to the Kernel when it only needs the `_ctype_` array of `ctype.cpp`, and there
wasn't any string stuff used in ctype.
Instead of all this I could have put static derivatives of `is_any_of()` in the
concerned AK files, however that would have meant more boilerplate and workarounds;
so I went for the Kernel approach.
Instead of just implementing format specifiers ad-hog this commit
implements the exact syntax std::format uses.
There are still a ton of features that are not supported by this
implementation, however, the format specifiers should be parsed
correctly.
In some cases however, the format specifiers aren't quite parsed
correctly, for example:
String::formatted("{:{}}", 42, 4)
should produce the string " 42" however an (unrelated) assertion fails.
This is because vformat doesn't consider nested parentheses. I have to
spend some time coming up with a simple way of doing this, I don't feel
like doing that right now.
The fundamental code for this already exists, by limiting the number of
format arguments (arbitrarily) to 256 large widths are used to encode
that these should be taken from other format parameters.
With this commit, <AK/Format.h> has a more supportive role and isn't
used directly.
Essentially, there now is a public 'vformat' function ('v' for vector)
which takes already type erased parameters. The name is choosen to
indicate that this function behaves similar to C-style functions taking
a va_list equivalent.
The interface for frontend users are now 'String::formatted' and
'StringBuilder::appendff'.
Two things I hate about C++:
1. 'int', 'signed int' and 'unsigned int' are two distinct types while
'char, 'signed char' and 'unsigned char' are *three* distinct types.
This is because 'signed int' is an alias for 'int' but 'signed char'
can't be an alias for 'char' because on some weird systems 'char' is
unsigned.
One might think why not do it the other way around, make 'int' an
alias for 'signed int' and 'char' an alias for whatever that is on
the platform, or make 'char' signed on all platforms. But who am I
to ask?
2. 'unsigned long' and 'unsigned long long' are always different types,
even if both are 64 bit numbers.
This commit fixes a few bugs that coming from this.
See Also: 1b3169f405.
This function is not avaliable in the kernel.
In the future it would be nice to have some sort of <charconv> header
that does this for all integer types and then call it in strtoull and et
cetera.
The difference would be that this function say 'from_chars' would return
an Optional and not just interpret anything invalid as zero.
