Quite a few includes aren't directly used anywhere within the header
itself. We can remove these to avoid unintentionally propagating them
into other source files.
Avoids reusing the same variable for two different areas of behavior and
uses separately named variables. This can help debugging if the first
condition isn't true, but the tool run condition is true, as it provides
an error count to compare against.
Factors out the ToolPassInfo construction to its own function keep the
code isolated from everything else. Also simplifies the assignment in
main().
We can also migrate the global cwdbuf into this function to narrow its
scope down.
Isolates the tool construction code and also makes the assignment within
main more straightforward. While we're at it, we can also make use of
std::make_unique within the relocated function.
Type punning like this isn't actually well-defined behavior. Only
casting to char/unsigned char is well defined. This presents a problem
when using FourCC instances in constexpr contexts, given constexpr
contexts are forbidden from having undefined behavior in them.
Because of that, any attempt to use the const char* constructor within a
constexpr context would always result in a compilation error.
We can utilize std::char_traits to generically handle the defined
character type. Since C++17, std::char_traits' length() function is
constexpr, so we can also make StrLen constexpr.
Same behavior, but more idiomatic. While we're at it, we can make said
constructor and the conversion operator explicit to make the class a
little less error-prone.
We already construct a std::string instance, so we can just append to
it instead of creating another temporary with std::string's operator+.
We also change this to append using the string view getter functions, as
this allows the appending process to do less work. When a pointer is
passed in, a strlen call would need to be performed in order to
determine the total characters to append. However, we already know the
size (via the string view).
We don't really need to call out to the C functions to perform the
comparison behavior when the views already have a comparison function
as part of their interface.
std::string_view instances can contain character values that lie outside
the range of an unsigned char (negative values). If such a value is
passed into std::isspace, then the behavior of the function is
undefined. To avoid this, we add these casts.
std::string_view instances aren't guaranteed to be null-terminated, so
we shouldn't be treating them as if they are in these functions, and
should instead use a bounded comparison based off their sizes.
This way we prevent an edge-case from ever becoming a problem and also
remove an ifdef, making the code uniform across all implementations.
Phillip Stephens
Lioncash