Skip to content

Commit

Permalink
Everywhere: Replace ctype.h to avoid narrowing conversions
Browse files Browse the repository at this point in the history
This replaces ctype.h with CharacterType.h everywhere I could find
issues with narrowing conversions. While using it will probably make
sense almost everywhere in the future, the most critical places should
have been addressed.
  • Loading branch information
MaxWipfli authored and awesomekling committed Jun 3, 2021
1 parent 1c9d87c commit bc8d16a
Show file tree
Hide file tree
Showing 16 changed files with 153 additions and 266 deletions.
36 changes: 3 additions & 33 deletions AK/URL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* SPDX-License-Identifier: BSD-2-Clause
*/

#include <AK/CharacterTypes.h>
#include <AK/Debug.h>
#include <AK/LexicalPath.h>
#include <AK/StringBuilder.h>
Expand All @@ -14,26 +15,6 @@

namespace AK {

constexpr bool is_ascii_alpha(u32 code_point)
{
return ('a' <= code_point && code_point <= 'z') || ('A' <= code_point && code_point <= 'Z');
}

constexpr bool is_ascii_digit(u32 code_point)
{
return '0' <= code_point && code_point <= '9';
}

constexpr bool is_ascii_alphanumeric(u32 code_point)
{
return is_ascii_alpha(code_point) || is_ascii_digit(code_point);
}

constexpr bool is_ascii_hex_digit(u32 code_point)
{
return is_ascii_digit(code_point) || (code_point >= 'a' && code_point <= 'f') || (code_point >= 'A' && code_point <= 'F');
}

// FIXME: It could make sense to force users of URL to use URLParser::parse() explicitly instead of using a constructor.
URL::URL(StringView const& string)
: URL(URLParser::parse({}, string))
Expand Down Expand Up @@ -403,17 +384,6 @@ String URL::percent_encode(StringView const& input, URL::PercentEncodeSet set)
return builder.to_string();
}

constexpr u8 parse_hex_digit(u8 digit)
{
if (digit >= '0' && digit <= '9')
return digit - '0';
if (digit >= 'a' && digit <= 'f')
return digit - 'a' + 10;
if (digit >= 'A' && digit <= 'F')
return digit - 'A' + 10;
VERIFY_NOT_REACHED();
}

String URL::percent_decode(StringView const& input)
{
if (!input.contains('%'))
Expand All @@ -427,9 +397,9 @@ String URL::percent_decode(StringView const& input)
builder.append_code_point(*it);
} else {
++it;
u8 byte = parse_hex_digit(*it) << 4;
u8 byte = parse_ascii_hex_digit(*it) << 4;
++it;
byte += parse_hex_digit(*it);
byte += parse_ascii_hex_digit(*it);
builder.append(byte);
}
}
Expand Down
21 changes: 1 addition & 20 deletions AK/URLParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* SPDX-License-Identifier: BSD-2-Clause
*/

#include <AK/CharacterTypes.h>
#include <AK/Debug.h>
#include <AK/Optional.h>
#include <AK/SourceLocation.h>
Expand All @@ -15,26 +16,6 @@

namespace AK {

constexpr bool is_ascii_alpha(u32 code_point)
{
return ('a' <= code_point && code_point <= 'z') || ('A' <= code_point && code_point <= 'Z');
}

constexpr bool is_ascii_digit(u32 code_point)
{
return '0' <= code_point && code_point <= '9';
}

constexpr bool is_ascii_alphanumeric(u32 code_point)
{
return is_ascii_alpha(code_point) || is_ascii_digit(code_point);
}

constexpr bool is_ascii_hex_digit(u32 code_point)
{
return is_ascii_digit(code_point) || (code_point >= 'a' && code_point <= 'f') || (code_point >= 'A' && code_point <= 'F');
}

constexpr bool is_url_code_point(u32 code_point)
{
// FIXME: [...] and code points in the range U+00A0 to U+10FFFD, inclusive, excluding surrogates and noncharacters.
Expand Down
75 changes: 27 additions & 48 deletions Userland/Libraries/LibGUI/EditingEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,23 @@
* SPDX-License-Identifier: BSD-2-Clause
*/

#include <AK/CharacterTypes.h>
#include <LibGUI/EditingEngine.h>
#include <LibGUI/Event.h>
#include <LibGUI/TextEditor.h>

namespace GUI {

constexpr bool is_vim_alphanumeric(u32 code_point)
{
return is_ascii_alphanumeric(code_point) || code_point == '_';
}

constexpr bool is_vim_punctuation(u32 code_point)
{
return is_ascii_punctuation(code_point) && code_point != '_';
}

EditingEngine::~EditingEngine()
{
}
Expand Down Expand Up @@ -379,15 +390,7 @@ TextPosition EditingEngine::find_beginning_of_next_word()
* If the end of the input is reached, jump there
*/

auto vim_isalnum = [](int c) {
return c == '_' || isalnum(c);
};

auto vim_ispunct = [](int c) {
return c != '_' && ispunct(c);
};

bool started_on_punct = vim_ispunct(m_editor->current_line().to_utf8().characters()[m_editor->cursor().column()]);
bool started_on_punct = is_vim_punctuation(m_editor->current_line().to_utf8().characters()[m_editor->cursor().column()]);
bool has_seen_whitespace = false;
bool is_first_line = true;
auto& lines = m_editor->lines();
Expand All @@ -409,18 +412,18 @@ TextPosition EditingEngine::find_beginning_of_next_word()
const u32* line_chars = line.view().code_points();
const u32 current_char = line_chars[column_index];

if (started_on_punct && vim_isalnum(current_char)) {
if (started_on_punct && is_vim_alphanumeric(current_char)) {
return { line_index, column_index };
}

if (vim_ispunct(current_char) && !started_on_punct) {
if (is_vim_punctuation(current_char) && !started_on_punct) {
return { line_index, column_index };
}

if (isspace(current_char))
if (is_ascii_space(current_char))
has_seen_whitespace = true;

if (has_seen_whitespace && (vim_isalnum(current_char) || vim_ispunct(current_char))) {
if (has_seen_whitespace && (is_vim_alphanumeric(current_char) || is_vim_punctuation(current_char))) {
return { line_index, column_index };
}

Expand Down Expand Up @@ -449,14 +452,6 @@ TextPosition EditingEngine::find_end_of_next_word()
* If the end of the input is reached, jump there
*/

auto vim_isalnum = [](int c) {
return c == '_' || isalnum(c);
};

auto vim_ispunct = [](int c) {
return c != '_' && ispunct(c);
};

bool is_first_line = true;
bool is_first_iteration = true;
auto& lines = m_editor->lines();
Expand All @@ -481,7 +476,7 @@ TextPosition EditingEngine::find_end_of_next_word()
const u32* line_chars = line.view().code_points();
const u32 current_char = line_chars[column_index];

if (column_index == lines.at(line_index).length() - 1 && !is_first_iteration && (vim_isalnum(current_char) || vim_ispunct(current_char)))
if (column_index == lines.at(line_index).length() - 1 && !is_first_iteration && (is_vim_alphanumeric(current_char) || is_vim_punctuation(current_char)))
return { line_index, column_index };
else if (column_index == lines.at(line_index).length() - 1) {
is_first_iteration = false;
Expand All @@ -490,10 +485,10 @@ TextPosition EditingEngine::find_end_of_next_word()

const u32 next_char = line_chars[column_index + 1];

if (!is_first_iteration && vim_isalnum(current_char) && (isspace(next_char) || vim_ispunct(next_char)))
if (!is_first_iteration && is_vim_alphanumeric(current_char) && (is_ascii_space(next_char) || is_vim_punctuation(next_char)))
return { line_index, column_index };

if (!is_first_iteration && vim_ispunct(current_char) && (isspace(next_char) || vim_isalnum(next_char)))
if (!is_first_iteration && is_vim_punctuation(current_char) && (is_ascii_space(next_char) || is_vim_alphanumeric(next_char)))
return { line_index, column_index };

if (line_index == lines.size() - 1 && column_index == line.length() - 1) {
Expand All @@ -513,15 +508,7 @@ void EditingEngine::move_to_end_of_next_word()

TextPosition EditingEngine::find_end_of_previous_word()
{
auto vim_isalnum = [](int c) {
return c == '_' || isalnum(c);
};

auto vim_ispunct = [](int c) {
return c != '_' && ispunct(c);
};

bool started_on_punct = vim_ispunct(m_editor->current_line().to_utf8().characters()[m_editor->cursor().column()]);
bool started_on_punct = is_vim_punctuation(m_editor->current_line().to_utf8().characters()[m_editor->cursor().column()]);
bool is_first_line = true;
bool has_seen_whitespace = false;
auto& lines = m_editor->lines();
Expand All @@ -545,19 +532,19 @@ TextPosition EditingEngine::find_end_of_previous_word()
const u32* line_chars = line.view().code_points();
const u32 current_char = line_chars[column_index];

if (started_on_punct && vim_isalnum(current_char)) {
if (started_on_punct && is_vim_alphanumeric(current_char)) {
return { line_index, column_index };
}

if (vim_ispunct(current_char) && !started_on_punct) {
if (is_vim_punctuation(current_char) && !started_on_punct) {
return { line_index, column_index };
}

if (isspace(current_char)) {
if (is_ascii_space(current_char)) {
has_seen_whitespace = true;
}

if (has_seen_whitespace && (vim_isalnum(current_char) || vim_ispunct(current_char))) {
if (has_seen_whitespace && (is_vim_alphanumeric(current_char) || is_vim_punctuation(current_char))) {
return { line_index, column_index };
}

Expand All @@ -580,14 +567,6 @@ void EditingEngine::move_to_end_of_previous_word()

TextPosition EditingEngine::find_beginning_of_previous_word()
{
auto vim_isalnum = [](int c) {
return c == '_' || isalnum(c);
};

auto vim_ispunct = [](int c) {
return c != '_' && ispunct(c);
};

bool is_first_iterated_line = true;
bool is_first_iteration = true;
auto& lines = m_editor->lines();
Expand Down Expand Up @@ -618,7 +597,7 @@ TextPosition EditingEngine::find_beginning_of_previous_word()
const u32* line_chars = line.view().code_points();
const u32 current_char = line_chars[column_index];

if (column_index == 0 && !is_first_iteration && (vim_isalnum(current_char) || vim_ispunct(current_char))) {
if (column_index == 0 && !is_first_iteration && (is_vim_alphanumeric(current_char) || is_vim_punctuation(current_char))) {
return { line_index, column_index };
} else if (line_index == 0 && column_index == 0) {
return { line_index, column_index };
Expand All @@ -629,10 +608,10 @@ TextPosition EditingEngine::find_beginning_of_previous_word()

const u32 next_char = line_chars[column_index - 1];

if (!is_first_iteration && vim_isalnum(current_char) && (isspace(next_char) || vim_ispunct(next_char)))
if (!is_first_iteration && is_vim_alphanumeric(current_char) && (is_ascii_space(next_char) || is_vim_punctuation(next_char)))
return { line_index, column_index };

if (!is_first_iteration && vim_ispunct(current_char) && (isspace(next_char) || vim_isalnum(next_char)))
if (!is_first_iteration && is_vim_punctuation(current_char) && (is_ascii_space(next_char) || is_vim_alphanumeric(next_char)))
return { line_index, column_index };

is_first_iteration = false;
Expand Down
16 changes: 8 additions & 8 deletions Userland/Libraries/LibGUI/TextDocument.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@
*/

#include <AK/Badge.h>
#include <AK/CharacterTypes.h>
#include <AK/ScopeGuard.h>
#include <AK/StringBuilder.h>
#include <AK/Utf8View.h>
#include <LibCore/Timer.h>
#include <LibGUI/TextDocument.h>
#include <LibGUI/TextEditor.h>
#include <LibRegex/Regex.h>
#include <ctype.h>

namespace GUI {

Expand Down Expand Up @@ -104,7 +104,7 @@ size_t TextDocumentLine::first_non_whitespace_column() const
{
for (size_t i = 0; i < length(); ++i) {
auto code_point = code_points()[i];
if (!isspace(code_point))
if (!is_ascii_space(code_point))
return i;
}
return length();
Expand All @@ -114,7 +114,7 @@ Optional<size_t> TextDocumentLine::last_non_whitespace_column() const
{
for (ssize_t i = length() - 1; i >= 0; --i) {
auto code_point = code_points()[i];
if (!isspace(code_point))
if (!is_ascii_space(code_point))
return i;
}
return {};
Expand All @@ -124,7 +124,7 @@ bool TextDocumentLine::ends_in_whitespace() const
{
if (!length())
return false;
return isspace(code_points()[length() - 1]);
return is_ascii_space(code_points()[length() - 1]);
}

bool TextDocumentLine::can_select() const
Expand Down Expand Up @@ -638,11 +638,11 @@ TextPosition TextDocument::first_word_break_before(const TextPosition& position,
if (target.column() == line.length())
modifier = 1;

auto is_start_alphanumeric = isalnum(line.code_points()[target.column() - modifier]);
auto is_start_alphanumeric = is_ascii_alphanumeric(line.code_points()[target.column() - modifier]);

while (target.column() > 0) {
auto prev_code_point = line.code_points()[target.column() - 1];
if ((is_start_alphanumeric && !isalnum(prev_code_point)) || (!is_start_alphanumeric && isalnum(prev_code_point)))
if ((is_start_alphanumeric && !is_ascii_alphanumeric(prev_code_point)) || (!is_start_alphanumeric && is_ascii_alphanumeric(prev_code_point)))
break;
target.set_column(target.column() - 1);
}
Expand All @@ -662,11 +662,11 @@ TextPosition TextDocument::first_word_break_after(const TextPosition& position)
return TextPosition(position.line() + 1, 0);
}

auto is_start_alphanumeric = isalnum(line.code_points()[target.column()]);
auto is_start_alphanumeric = is_ascii_alphanumeric(line.code_points()[target.column()]);

while (target.column() < line.length()) {
auto next_code_point = line.code_points()[target.column()];
if ((is_start_alphanumeric && !isalnum(next_code_point)) || (!is_start_alphanumeric && isalnum(next_code_point)))
if ((is_start_alphanumeric && !is_ascii_alphanumeric(next_code_point)) || (!is_start_alphanumeric && is_ascii_alphanumeric(next_code_point)))
break;
target.set_column(target.column() + 1);
}
Expand Down
8 changes: 4 additions & 4 deletions Userland/Libraries/LibGUI/TextEditor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* SPDX-License-Identifier: BSD-2-Clause
*/

#include <AK/CharacterTypes.h>
#include <AK/Debug.h>
#include <AK/ScopeGuard.h>
#include <AK/StringBuilder.h>
Expand All @@ -25,7 +26,6 @@
#include <LibGfx/FontDatabase.h>
#include <LibGfx/Palette.h>
#include <LibSyntax/Highlighter.h>
#include <ctype.h>
#include <fcntl.h>
#include <math.h>
#include <stdio.h>
Expand Down Expand Up @@ -1232,12 +1232,12 @@ size_t TextEditor::number_of_selected_words() const
bool in_word = false;
auto selected_text = this->selected_text();
for (char c : selected_text) {
if (in_word && isspace(c)) {
if (in_word && is_ascii_space(c)) {
in_word = false;
word_count++;
continue;
}
if (!in_word && !isspace(c))
if (!in_word && !is_ascii_space(c))
in_word = true;
}
if (in_word)
Expand Down Expand Up @@ -1561,7 +1561,7 @@ void TextEditor::recompute_visual_lines(size_t line_index)
auto glyph_spacing = font().glyph_spacing();
for (size_t i = 0; i < line.length(); ++i) {
auto code_point = line.code_points()[i];
if (isspace(code_point)) {
if (is_ascii_space(code_point)) {
last_whitespace_index = i;
line_width_since_last_whitespace = 0;
}
Expand Down
Loading

0 comments on commit bc8d16a

Please sign in to comment.