Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix uppercase B and X parsing in the integer literals. #102400

Merged
merged 1 commit into from
Feb 5, 2025

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented Feb 4, 2025

Fixes literals with upper case prefix, like 0XA or 0B1001 in the GDScript, Expression and font import dialog (Unicode ranges).

@bruvzg bruvzg added this to the 4.4 milestone Feb 4, 2025
@bruvzg bruvzg requested review from a team as code owners February 4, 2025 07:59
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good to me.

I suggest adding unit tests for this, e.g. here:

TEST_CASE("[String] String to integer") {
static const char *nums[14] = { "1237461283", "- 22", "0", " - 1123412", "", "10_000_000", "-1_2_3_4", "10__000", " 1 2 34 ", "-0", "007", "--45", "---46", "-7-2" };
static const int num[14] = { 1237461283, -22, 0, -1123412, 0, 10000000, -1234, 10000, 1234, 0, 7, 45, -46, -72 };
for (int i = 0; i < 14; i++) {
CHECK(String(nums[i]).to_int() == num[i]);
}
CHECK(String("0b1011").to_int() == 1011); // Looks like a binary number, but to_int() handles this as a base-10 number, "b" is just ignored.
CHECK(String("0x1012").to_int() == 1012); // Looks like a hexadecimal number, but to_int() handles this as a base-10 number, "x" is just ignored.
ERR_PRINT_OFF
CHECK(String("999999999999999999999999999999999999999999999999999999999").to_int() == INT64_MAX); // Too large, largest possible is returned.
CHECK(String("-999999999999999999999999999999999999999999999999999999999").to_int() == INT64_MIN); // Too small, smallest possible is returned.
ERR_PRINT_ON
}
TEST_CASE("[String] Hex to integer") {
static const char *nums[12] = { "0xFFAE", "22", "0", "AADDAD", "0x7FFFFFFFFFFFFFFF", "-0xf", "", "000", "000f", "0xaA", "-ff", "-" };
static const int64_t num[12] = { 0xFFAE, 0x22, 0, 0xAADDAD, 0x7FFFFFFFFFFFFFFF, -0xf, 0, 0, 0xf, 0xaa, -0xff, 0x0 };
for (int i = 0; i < 12; i++) {
CHECK(String(nums[i]).hex_to_int() == num[i]);
}
// Invalid hex strings should return 0.
static const char *invalid_nums[15] = { "qwerty", "QWERTY", "0xqwerty", "0x00qwerty", "qwerty00", "0x", "0x__", "__", "x12", "+", " ff", "ff ", "f f", "+ff", "--0x78" };
ERR_PRINT_OFF
for (int i = 0; i < 15; i++) {
CHECK(String(invalid_nums[i]).hex_to_int() == 0);
}
CHECK(String("0xFFFFFFFFFFFFFFFFFFFFFFF").hex_to_int() == INT64_MAX); // Too large, largest possible is returned.
CHECK(String("-0xFFFFFFFFFFFFFFFFFFFFFFF").hex_to_int() == INT64_MIN); // Too small, smallest possible is returned.
ERR_PRINT_ON
}
TEST_CASE("[String] Bin to integer") {
static const char *nums[10] = { "", "0", "0b0", "0b1", "0b", "1", "0b1010", "-0b11", "-1010", "0b0111111111111111111111111111111111111111111111111111111111111111" };
static const int64_t num[10] = { 0, 0, 0, 1, 0, 1, 10, -3, -10, 0x7FFFFFFFFFFFFFFF };
for (int i = 0; i < 10; i++) {
CHECK(String(nums[i]).bin_to_int() == num[i]);
}
// Invalid bin strings should return 0. The long "0x11...11" is just too long for a 64 bit int.
static const char *invalid_nums[16] = { "qwerty", "QWERTY", "0bqwerty", "0b00qwerty", "qwerty00", "0x__", "0b__", "__", "b12", "+", "-", "0x12ab", " 11", "11 ", "1 1", "--0b11" };
for (int i = 0; i < 16; i++) {
CHECK(String(invalid_nums[i]).bin_to_int() == 0);
}
ERR_PRINT_OFF
CHECK(String("0b111111111111111111111111111111111111111111111111111111111111111111111111111111111").bin_to_int() == INT64_MAX); // Too large, largest possible is returned.
CHECK(String("-0b111111111111111111111111111111111111111111111111111111111111111111111111111111111").bin_to_int() == INT64_MIN); // Too small, smallest possible is returned.
ERR_PRINT_ON
}

And here (also add a 0b test there):

TEST_CASE("[Expression] Underscored numeric literals") {
Expression expression;
CHECK_MESSAGE(
expression.parse("1_000_000") == OK,
"The expression should parse successfully.");
CHECK_MESSAGE(
expression.parse("1_000.000") == OK,
"The expression should parse successfully.");
CHECK_MESSAGE(
expression.parse("0xff_99_00") == OK,
"The expression should parse successfully.");
}

@bruvzg bruvzg requested a review from a team as a code owner February 4, 2025 12:01
@Repiteo Repiteo merged commit f899138 into godotengine:master Feb 5, 2025
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Feb 5, 2025

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants