Skip to content

Report syntax error for too-long bin/hex/oct integer literals #11447

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.

Sign up for GitHub

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

Merged
straight-shoota merged 6 commits into from
Nov 29, 2021
Merged

Report syntax error for too-long bin/hex/oct integer literals #11447

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ea43502
Report syntax error for too-long bin/hex/oct integer literals
oprypin Nov 12, 2021
85409b5
Merge remote-tracking branch 'origin/master' into parsnu
oprypin Nov 19, 2021
a8da279
Take the size suffix into account
oprypin Nov 19, 2021
4883093
Octal digits go only up to 7...
oprypin Nov 19, 2021
e28fb57
Preserve the original literal, with suffix
oprypin Nov 20, 2021
157bf29
Refactor the check for first digit
oprypin Nov 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 37 additions & 3 deletions spec/compiler/lexer/lexer_spec.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,13 +311,47 @@ describe "Lexer" do
assert_syntax_error "18446744073709551616_u128", "18446744073709551616 doesn't fit in an UInt64. UInt128 literals that don't fit in an UInt64 are currently not supported"
assert_syntax_error "-1_u128", "Invalid negative value -1 for UInt128"

assert_syntax_error "0123", "octal constants should be prefixed with 0o"
assert_syntax_error "00", "octal constants should be prefixed with 0o"
assert_syntax_error "01_i64", "octal constants should be prefixed with 0o"

assert_syntax_error "0xFF_i8", "255 doesn't fit in an Int8"
assert_syntax_error "0o200_i8", "128 doesn't fit in an Int8"
assert_syntax_error "0b10000000_i8", "128 doesn't fit in an Int8"

assert_syntax_error "0123", "octal constants should be prefixed with 0o"
assert_syntax_error "00", "octal constants should be prefixed with 0o"
assert_syntax_error "01_i64", "octal constants should be prefixed with 0o"
# 2**31 - 1
it_lexes_i32 [["0x7fffffff", "2147483647"], ["0o17777777777", "2147483647"], ["0b1111111111111111111111111111111", "2147483647"]]
it_lexes_i32 [["0x7fffffff_i32", "2147483647"], ["0o17777777777_i32", "2147483647"], ["0b1111111111111111111111111111111_i32", "2147483647"]]
# 2**32 - 1
it_lexes_i64 [["0xffffffff", "4294967295"], ["0o37777777777", "4294967295"], ["0b11111111111111111111111111111111", "4294967295"]]
# 2**32
it_lexes_i64 [["0x100000000", "4294967296"], ["0o40000000000", "4294967296"], ["0b100000000000000000000000000000000", "4294967296"]]
assert_syntax_error "0x100000000i32", "4294967296 doesn't fit in an Int32"
assert_syntax_error "0o40000000000i32", "4294967296 doesn't fit in an Int32"
assert_syntax_error "0b100000000000000000000000000000000i32", "4294967296 doesn't fit in an Int32"
# 2**63 - 1
it_lexes_i64 [["0x7fffffffffffffff", "9223372036854775807"], ["0o777777777777777777777", "9223372036854775807"], ["0b111111111111111111111111111111111111111111111111111111111111111", "9223372036854775807"]]
# 2**63
it_lexes_u64 [["0x8000000000000000", "9223372036854775808"], ["0o1000000000000000000000", "9223372036854775808"], ["0b1000000000000000000000000000000000000000000000000000000000000000", "9223372036854775808"]]
assert_syntax_error "0x8000000000000000i64", "9223372036854775808 doesn't fit in an Int64"
assert_syntax_error "0o1000000000000000000000i64", "9223372036854775808 doesn't fit in an Int64"
assert_syntax_error "0b1000000000000000000000000000000000000000000000000000000000000000i64", "9223372036854775808 doesn't fit in an Int64"
# 2**64 - 1
it_lexes_u64 [["0xffff_ffff_ffff_ffff", "18446744073709551615"], ["0o177777_77777777_77777777", "18446744073709551615"], ["0b11111111_11111111_11111111_11111111_11111111_11111111_11111111_11111111", "18446744073709551615"]]
it_lexes_u64 [["0x00ffffffffffffffff", "18446744073709551615"], ["0o001777777777777777777777", "18446744073709551615"], ["0b001111111111111111111111111111111111111111111111111111111111111111", "18446744073709551615"]]
# 2**64
assert_syntax_error "0x10000_0000_0000_0000", "0x10000_0000_0000_0000 doesn't fit in an UInt64"
assert_syntax_error "0o200000_00000000_00000000", "0o200000_00000000_00000000 doesn't fit in an UInt64"
assert_syntax_error "0b100000000_00000000_00000000_00000000_00000000_00000000_00000000_00000000", "0b100000000_00000000_00000000_00000000_00000000_00000000_00000000_00000000 doesn't fit in an UInt64"
# Very large
assert_syntax_error "0x1afafafafafafafafafafaf", "0x1afafafafafafafafafafaf doesn't fit in an UInt64"
assert_syntax_error "0x1afafafafafafafafafafafi32", "0x1afafafafafafafafafafafi32 doesn't fit in an Int32"
assert_syntax_error "0o1234567123456712345671234567", "0o1234567123456712345671234567 doesn't fit in an UInt64"
assert_syntax_error "0o12345671234567_12345671234567_i8", "0o12345671234567_12345671234567_i8 doesn't fit in an Int8"
assert_syntax_error "0b100000000000000000000000000000000000000000000000000000000000000000", "0b100000000000000000000000000000000000000000000000000000000000000000 doesn't fit in an UInt64"

it_lexes_i64 [["0o700000000000000000000", "8070450532247928832"]]
it_lexes_u64 [["0o1000000000000000000000", "9223372036854775808"]]

assert_syntax_error "4f33", "invalid float suffix"
assert_syntax_error "4f65", "invalid float suffix"
Expand Down
52 changes: 46 additions & 6 deletions src/compiler/crystal/syntax/lexer.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1780,61 +1780,101 @@ module Crystal
next_char

num = 0_u64
num_size = 0
while true
case next_char
when '0'
num *= 2
num = num << 1
if num_size > 0
num_size += 1
end
when '1'
num = num * 2 + 1
num = (num << 1) + 1
num_size += 1
when '_'
# Nothing
else
break
end
end

num = nil if num_size > 64
finish_scan_prefixed_number num, negative, start
end

def scan_octal_number(start, negative)
next_char

num = 0_u64

num_size = first_digit = 0
while true
char = next_char
if '0' <= char <= '7'
num = num * 8 + (char - '0')
num = (num << 3) | (char - '0')
if num_size == 0
first_digit = num
num_size += 1 if char != '0'
else
num_size += 1
end
elsif char == '_'
else
break
end
end

# 0o177777_77777777_77777777 is the largest UInt64.
num = nil if {num_size, first_digit} > {22, 0o1}
Copy link
Copy Markdown
Member

@straight-shoota straight-shoota Nov 20, 2021

Choose a reason for hiding this comment

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

This condition looks unconventional and I find it a little harder to read. Maybe this would be a better way to write this?

Suggested change
num = nil if {num_size, first_digit} > {22, 0o1}
num = nil if num_size > 22 || (num_size == 22 && first_digit == 1)

It's fine by me to keep it that way, though.

Copy link
Copy Markdown
Member Author

@oprypin oprypin Nov 20, 2021

Choose a reason for hiding this comment

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

Well because it's the latest idea of mine, obviously I prefer to keep it 😀
Not a big deal though, others can weigh in as well.

Copy link
Copy Markdown
Contributor

@Sija Sija Nov 26, 2021

Choose a reason for hiding this comment

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

Looks dandy, but to be honest I can't say for sure what's happening under the hood without consulting docs on Tuple#<=>, not to mention redundant Tuple allocation. IMO it would be more lightweight and understandable if it was written using the more explicit version :)

finish_scan_prefixed_number num, negative, start
end

def scan_hex_number(start, negative = false)
next_char

num = 0_u64
num_size = 0
while true
char = next_char
if char == '_'
else
hex_value = char_to_hex(char) { nil }
if hex_value
num = num * 16 + hex_value
num = (num << 4) | hex_value
if num_size > 0 || char != '0'
num_size += 1
end
else
break
end
end
end

# 0xFFFF_FFFF_FFFF_FFFF is the longest UInt64.
num = nil if num_size > 16
finish_scan_prefixed_number num, negative, start
end

def finish_scan_prefixed_number(num, negative, start)
def finish_scan_prefixed_number(num : Int?, negative : Bool, start : Int32)
if num.nil? # Doesn't even fit in UInt64
case current_char
when 'i'
consume_int_suffix
when 'u'
consume_uint_suffix
else
@token.number_kind = :u64
end
case @token.number_kind
when :i8, :i16, :i32, :i64, :i128
type_name = "Int" + @token.number_kind.to_s[1..]
when :u8, :u16, :u32, :u64, :u128
type_name = "UInt" + @token.number_kind.to_s[1..]
else
raise "BUG: Expecting an integer token, got #{@token.number_kind}"
end
raise_value_doesnt_fit_in type_name, string_range_from_pool(start), start
end

if negative
string_value = (num.to_i64 * -1).to_s
else
Expand Down