Skip to content

Refactor symbols for primitive number kinds to enums #11621

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 4 commits into from
Feb 3, 2022
Merged

Refactor symbols for primitive number kinds to enums #11621

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d9ea7d6
Use enums instead of symbols for primitive number kinds
HertzDevil Dec 19, 2021
b58456a
Merge remote-tracking branch 'upstream/master' into refactor/number-k…
HertzDevil Dec 21, 2021
54c0ba7
Merge remote-tracking branch 'upstream/master' into refactor/number-k…
HertzDevil Jan 23, 2022
11d99b2
Merge remote-tracking branch 'upstream/master' into refactor/number-k…
HertzDevil Jan 29, 2022
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
10 changes: 5 additions & 5 deletions spec/compiler/lexer/lexer_spec.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ private def it_lexes(string, type, value)
end
end

private def it_lexes(string, type, value, number_kind)
private def it_lexes(string, type, value, number_kind : NumberKind)
it "lexes #{string.inspect}" do
lexer = Lexer.new string
token = lexer.next_token
Expand Down Expand Up @@ -72,11 +72,11 @@ private def it_lexes_f64(values)
values.each { |value| it_lexes_number :f64, value }
end

private def it_lexes_number(number_kind, value : Array)
private def it_lexes_number(number_kind : NumberKind, value : Array)
it_lexes value[0], :NUMBER, value[1], number_kind
end

private def it_lexes_number(number_kind, value : String)
private def it_lexes_number(number_kind : NumberKind, value : String)
it_lexes value, :NUMBER, value, number_kind
end

Expand Down Expand Up @@ -557,11 +557,11 @@ describe "Lexer" do

it "lexes float then zero (bug)" do
lexer = Lexer.new "2.5 0"
lexer.next_token.number_kind.should eq(:f64)
lexer.next_token.number_kind.should eq(NumberKind::F64)
lexer.next_token.type.should eq(:SPACE)
token = lexer.next_token
token.type.should eq(:NUMBER)
token.number_kind.should eq(:i32)
token.number_kind.should eq(NumberKind::I32)
end

it "lexes symbol with quote" do
Expand Down
25 changes: 0 additions & 25 deletions src/compiler/crystal/codegen/ast.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,31 +5,6 @@ module Crystal
def no_returns?
type?.try &.no_return?
end

def zero?
false
end

def false?
false
end
end

class BoolLiteral
def false?
!value
end
end

class NumberLiteral
def zero?
case :kind
when :f32, :f64
value == "0.0"
else
value == "0"
end
end
end

class Def
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/crystal/codegen/call.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,10 +229,10 @@ class Crystal::CodeGenVisitor
# If we are passing variadic arguments there are some special rules
if i >= target_def_args_size
arg_type = arg.type.remove_indirection
if arg_type.is_a?(FloatType) && arg_type.kind == :f32
if arg_type.is_a?(FloatType) && arg_type.kind.bytesize < 64
# Floats must be passed as doubles (there are no float varargs)
call_arg = extend_float @program.float64, call_arg
elsif arg_type.is_a?(IntegerType) && arg_type.kind.in?(:i8, :u8, :i16, :u16)
elsif arg_type.is_a?(IntegerType) && arg_type.kind.bytesize < 32
# Integer with a size less that `int` must be converted to `int`
call_arg = extend_int arg_type, @program.int32, call_arg
end
Expand Down
26 changes: 12 additions & 14 deletions src/compiler/crystal/codegen/codegen.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,32 +458,30 @@ module Crystal

def visit(node : NumberLiteral)
case node.kind
when :i8
in .i8?
@last = int8(node.value.to_i8)
when :u8
in .u8?
@last = int8(node.value.to_u8)
when :i16
in .i16?
@last = int16(node.value.to_i16)
when :u16
in .u16?
@last = int16(node.value.to_u16)
when :i32
in .i32?
@last = int32(node.value.to_i32)
when :u32
in .u32?
@last = int32(node.value.to_u32)
when :i64
in .i64?
@last = int64(node.value.to_i64)
when :u64
in .u64?
@last = int64(node.value.to_u64)
when :i128
in .i128?
@last = int128(node.value.to_i128)
when :u128
in .u128?
@last = int128(node.value.to_u128)
when :f32
in .f32?
@last = float32(node.value)
when :f64
in .f64?
@last = float64(node.value)
else
node.raise "Bug: unhandled number kind: #{node.kind}"
end
end

Expand Down
2 changes: 1 addition & 1 deletion src/compiler/crystal/codegen/const.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class Crystal::CodeGenVisitor
# TODO: there's an LLVM bug that prevents us from having internal globals of type i128 or u128:
# https://bugs.llvm.org/show_bug.cgi?id=42932
# so we just use global.
if @single_module && !(type.is_a?(IntegerType) && (type.kind == :i128 || type.kind == :u128))
if @single_module && !(type.is_a?(IntegerType) && (type.kind.i128? || type.kind.u128?))
global.linkage = LLVM::Linkage::Internal if @single_module
end

Expand Down
4 changes: 2 additions & 2 deletions src/compiler/crystal/codegen/llvm_builder_helper.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ module Crystal

def float(value, type)
case type.kind
when :f32
when .f32?
float32(value.to_f32)
when :f64
when .f64?
float64(value.to_f64)
else
raise "Unsupported float type"
Expand Down
8 changes: 4 additions & 4 deletions src/compiler/crystal/codegen/primitives.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,9 +262,9 @@ class Crystal::CodeGenVisitor
private def codegen_out_of_range(target_type : IntegerType, arg_type : FloatType, arg)
min_value, max_value = target_type.range
max_value = case arg_type.kind
when :f32
when .f32?
float32_upper_bound(max_value)
when :f64
when .f64?
float64_upper_bound(max_value)
else
raise "BUG: unknown float type"
Expand Down Expand Up @@ -307,7 +307,7 @@ class Crystal::CodeGenVisitor
end

private def codegen_out_of_range(target_type : FloatType, arg_type : IntegerType, arg)
if arg_type.kind == :u128 && target_type.kind == :f32
if arg_type.kind.u128? && target_type.kind.f32?
# since Float32::MAX < UInt128::MAX
# the value will be outside of the float range if
# arg > Float32::MAX
Expand Down Expand Up @@ -668,7 +668,7 @@ class Crystal::CodeGenVisitor

def codegen_convert(from_type : IntegerType, to_type : FloatType, arg, *, checked : Bool)
if checked
if from_type.kind == :u128 && to_type.kind == :f32
if from_type.kind.u128? && to_type.kind.f32?
overflow = codegen_out_of_range(to_type, from_type, arg)
codegen_raise_overflow_cond(overflow)
end
Expand Down
22 changes: 10 additions & 12 deletions src/compiler/crystal/interpreter/c.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,14 @@ module Crystal
class IntegerType
def ffi_type : FFI::Type
case kind
when :i8 then FFI::Type.sint8
when :u8 then FFI::Type.uint8
when :i16 then FFI::Type.sint16
when :u16 then FFI::Type.uint16
when :i32 then FFI::Type.sint32
when :u32 then FFI::Type.uint32
when :i64 then FFI::Type.sint64
when :u64 then FFI::Type.uint64
when :f32 then FFI::Type.float
when :f64 then FFI::Type.double
when .i8? then FFI::Type.sint8
when .u8? then FFI::Type.uint8
when .i16? then FFI::Type.sint16
when .u16? then FFI::Type.uint16
when .i32? then FFI::Type.sint32
when .u32? then FFI::Type.uint32
when .i64? then FFI::Type.sint64
when .u64? then FFI::Type.uint64
else
raise "BUG: missing ffi_type for #{self} (#{self.class})"
Comment on lines +30 to 39
Copy link
Member

@straight-shoota straight-shoota Dec 29, 2021

Choose a reason for hiding this comment

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

I suppose this could already be exhaustive with replacing else by an explicit in .i128?, .u128? branch?

Copy link
Contributor Author

@HertzDevil HertzDevil Jan 3, 2022

Choose a reason for hiding this comment

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

Not true, there are also .f32? and .f64? (and the opposite in Crystal::FloatType#ffi_type would need all 10 integer checks)

end
Expand All @@ -46,8 +44,8 @@ module Crystal
class FloatType
def ffi_type : FFI::Type
case kind
when :f32 then FFI::Type.float
when :f64 then FFI::Type.double
when .f32? then FFI::Type.float
when .f64? then FFI::Type.double
else
raise "BUG: missing ffi_type for #{self} (#{self.class})"
end
Expand Down
26 changes: 12 additions & 14 deletions src/compiler/crystal/interpreter/compiler.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,34 +317,32 @@ class Crystal::Repl::Compiler < Crystal::Visitor

private def compile_number(node, kind, value)
case kind
when :i8
in .i8?
put_i8 value.to_i8, node: node
when :u8
in .u8?
put_u8 value.to_u8, node: node
when :i16
in .i16?
put_i16 value.to_i16, node: node
when :u16
in .u16?
put_u16 value.to_u16, node: node
when :i32
in .i32?
put_i32 value.to_i32, node: node
when :u32
in .u32?
put_u32 value.to_u32, node: node
when :i64
in .i64?
put_i64 value.to_i64, node: node
when :u64
in .u64?
put_u64 value.to_u64, node: node
when :i128
in .i128?
# TODO: implement String#to_i128 and use it
put_i128 value.to_i64.to_i128!, node: node
when :u128
in .u128?
# TODO: implement String#to_i128 and use it
put_u128 value.to_u64.to_u128!, node: node
when :f32
in .f32?
put_i32 value.to_f32.unsafe_as(Int32), node: node
when :f64
in .f64?
put_i64 value.to_f64.unsafe_as(Int64), node: node
else
node.raise "BUG: missing interpret for NumberLiteral with kind #{kind}"
end
end

Expand Down
Loading