diff --git a/Makefile b/Makefile index b3f29e483f27..0df8465f1e71 100644 --- a/Makefile +++ b/Makefile @@ -28,7 +28,7 @@ static ?= ## Enable static linking O := .build SOURCES := $(shell find src -name '*.cr') SPEC_SOURCES := $(shell find spec -name '*.cr') -override FLAGS += $(if $(release),--release )$(if $(stats),--stats )$(if $(progress),--progress )$(if $(threads),--threads $(threads) )$(if $(debug),-d )$(if $(static),--static )$(if $(LDFLAGS),--link-flags="$(LDFLAGS)" )$(if $(target),--cross-compile --target $(target) ) +override FLAGS += -D preview_multi_assign $(if $(release),--release )$(if $(stats),--stats )$(if $(progress),--progress )$(if $(threads),--threads $(threads) )$(if $(debug),-d )$(if $(static),--static )$(if $(LDFLAGS),--link-flags="$(LDFLAGS)" )$(if $(target),--cross-compile --target $(target) ) SPEC_WARNINGS_OFF := --exclude-warnings spec/std --exclude-warnings spec/compiler --exclude-warnings spec/primitives SPEC_FLAGS := $(if $(verbose),-v )$(if $(junit_output),--junit_output $(junit_output) ) CRYSTAL_CONFIG_LIBRARY_PATH := '$$ORIGIN/../lib/crystal' diff --git a/spec/compiler/codegen/multi_assign_spec.cr b/spec/compiler/codegen/multi_assign_spec.cr index 7b479c427d9b..1f7b937f3a2d 100644 --- a/spec/compiler/codegen/multi_assign_spec.cr +++ b/spec/compiler/codegen/multi_assign_spec.cr @@ -21,6 +21,38 @@ describe "Code gen: multi assign" do CR end + context "without strict_multi_assign" do + it "doesn't raise if value size in 1 to n assignment doesn't match target count" do + run(<<-CR).to_i.should eq(4) + require "prelude" + + begin + a, b = [1, 2, 3] + 4 + rescue ex : Exception + raise ex unless ex.message == "Multiple assignment count mismatch" + 5 + end + CR + end + end + + context "strict_multi_assign" do + it "raises if value size in 1 to n assignment doesn't match target count" do + run(<<-CR, flags: %w(strict_multi_assign)).to_i.should eq(5) + require "prelude" + + begin + a, b = [1, 2, 3] + 4 + rescue ex : Exception + raise ex unless ex.message == "Multiple assignment count mismatch" + 5 + end + CR + end + end + it "supports m to n assignment, with splat on left-hand side (1)" do run(<<-CR).to_i.should eq(12345) #{tuple_new} diff --git a/spec/compiler/normalize/multi_assign_spec.cr b/spec/compiler/normalize/multi_assign_spec.cr index f8357d170367..08c8fb44f0ce 100644 --- a/spec/compiler/normalize/multi_assign_spec.cr +++ b/spec/compiler/normalize/multi_assign_spec.cr @@ -12,15 +12,6 @@ describe "Normalize: multi assign" do CR end - it "normalizes 1 to n" do - assert_expand_second "d = 1; a, b, c = d", <<-CR - __temp_1 = d - a = __temp_1[0] - b = __temp_1[1] - c = __temp_1[2] - CR - end - it "normalizes n to n with []" do assert_expand_third "a = 1; b = 2; a[0], b[1] = 2, 3", <<-CR __temp_1 = 2 @@ -30,14 +21,6 @@ describe "Normalize: multi assign" do CR end - it "normalizes 1 to n with []" do - assert_expand_third "a = 1; b = 2; a[0], b[1] = 2", <<-CR - __temp_1 = 2 - a[0] = __temp_1[0] - b[1] = __temp_1[1] - CR - end - it "normalizes n to n with call" do assert_expand_third "a = 1; b = 2; a.foo, b.bar = 2, 3", <<-CR __temp_1 = 2 @@ -47,12 +30,67 @@ describe "Normalize: multi assign" do CR end - it "normalizes 1 to n with call" do - assert_expand_third "a = 1; b = 2; a.foo, b.bar = 2", <<-CR - __temp_1 = 2 - a.foo = __temp_1[0] - b.bar = __temp_1[1] - CR + context "without strict_multi_assign" do + it "normalizes 1 to n" do + assert_expand_second "d = 1; a, b, c = d", <<-CR + __temp_1 = d + a = __temp_1[0] + b = __temp_1[1] + c = __temp_1[2] + CR + end + + it "normalizes 1 to n with []" do + assert_expand_third "a = 1; b = 2; a[0], b[1] = 2", <<-CR + __temp_1 = 2 + a[0] = __temp_1[0] + b[1] = __temp_1[1] + CR + end + + it "normalizes 1 to n with call" do + assert_expand_third "a = 1; b = 2; a.foo, b.bar = 2", <<-CR + __temp_1 = 2 + a.foo = __temp_1[0] + b.bar = __temp_1[1] + CR + end + end + + context "strict_multi_assign" do + it "normalizes 1 to n" do + assert_expand_second "d = 1; a, b, c = d", <<-CR, flags: "strict_multi_assign" + __temp_1 = d + if __temp_1.size != 3 + ::raise(::IndexError.new("Multiple assignment count mismatch")) + end + a = __temp_1[0] + b = __temp_1[1] + c = __temp_1[2] + CR + end + + it "normalizes 1 to n with []" do + assert_expand_third "a = 1; b = 2; a[0], b[1] = 2", <<-CR, flags: "strict_multi_assign" + __temp_1 = 2 + if __temp_1.size != 2 + ::raise(::IndexError.new("Multiple assignment count mismatch")) + end + a[0] = __temp_1[0] + b[1] = __temp_1[1] + CR + end + + it "normalizes 1 to n with call" do + assert_expand_third "a = 1; b = 2; a.foo, b.bar = 2", <<-CR, flags: "strict_multi_assign" + __temp_1 = 2 + if __temp_1.size != 2 + ::raise(::IndexError.new("Multiple assignment count mismatch")) + end + a.foo = __temp_1[0] + b.bar = __temp_1[1] + CR + end end it "normalizes m to n, with splat on left-hand side, splat is empty" do diff --git a/spec/compiler/semantic/multi_assign_spec.cr b/spec/compiler/semantic/multi_assign_spec.cr new file mode 100644 index 000000000000..c1bc1069e3d4 --- /dev/null +++ b/spec/compiler/semantic/multi_assign_spec.cr @@ -0,0 +1,64 @@ +require "../../spec_helper" + +describe "Semantic: multi assign" do + context "without strict_multi_assign" do + it "doesn't error if assigning tuple to fewer targets" do + assert_type(%( + require "prelude" + + x = {1, 2, ""} + a, b = x + {a, b} + )) { tuple_of [int32, int32] } + end + end + + context "strict_multi_assign" do + it "errors if assigning tuple to fewer targets" do + assert_error %( + require "prelude" + + x = {1, 2, ""} + a, b = x + ), "cannot assign Tuple(Int32, Int32, String) to 2 targets", flags: "strict_multi_assign" + end + + pending "errors if assigning tuple to more targets" do + assert_error %( + require "prelude" + + x = {1} + a, b = x + ), "cannot assign Tuple(Int32) to 2 targets", flags: "strict_multi_assign" + end + + it "errors if assigning union of tuples to fewer targets" do + assert_error %( + require "prelude" + + x = true ? {1, 2, 3} : {4, 5, 6, 7} + a, b = x + ), "cannot assign (Tuple(Int32, Int32, Int32) | Tuple(Int32, Int32, Int32, Int32)) to 2 targets", flags: "strict_multi_assign" + end + + it "doesn't error if some type in union matches target count" do + assert_type(%( + require "prelude" + + x = true ? {1, "", 3} : {4, 5} + a, b = x + {a, b} + ), flags: "strict_multi_assign") { tuple_of [int32, union_of(int32, string)] } + end + + it "doesn't error if some type in union has no constant size" do + assert_type(%( + require "prelude" + + x = true ? {1, "", 3} : [4, 5] + a, b = x + {a, b} + ), flags: "strict_multi_assign") { tuple_of [int32, union_of(int32, string)] } + end + end +end diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr index 14067b3e508a..d23e5dd49bd9 100644 --- a/spec/spec_helper.cr +++ b/spec/spec_helper.cr @@ -85,26 +85,28 @@ def assert_normalize(from, to, flags = nil, *, file = __FILE__, line = __LINE__) to_nodes end -def assert_expand(from : String, to, *, file = __FILE__, line = __LINE__) - assert_expand Parser.parse(from), to, file: file, line: line +def assert_expand(from : String, to, *, flags = nil, file = __FILE__, line = __LINE__) + assert_expand Parser.parse(from), to, flags: flags, file: file, line: line end -def assert_expand(from_nodes : ASTNode, to, *, file = __FILE__, line = __LINE__) - to_nodes = LiteralExpander.new(new_program).expand(from_nodes) +def assert_expand(from_nodes : ASTNode, to, *, flags = nil, file = __FILE__, line = __LINE__) + program = new_program + program.flags.concat(flags.split) if flags + to_nodes = LiteralExpander.new(program).expand(from_nodes) to_nodes.to_s.strip.should eq(to.strip), file: file, line: line end -def assert_expand_second(from : String, to, *, file = __FILE__, line = __LINE__) +def assert_expand_second(from : String, to, *, flags = nil, file = __FILE__, line = __LINE__) node = (Parser.parse(from).as(Expressions))[1] - assert_expand node, to, file: file, line: line + assert_expand node, to, flags: flags, file: file, line: line end -def assert_expand_third(from : String, to, *, file = __FILE__, line = __LINE__) +def assert_expand_third(from : String, to, *, flags = nil, file = __FILE__, line = __LINE__) node = (Parser.parse(from).as(Expressions))[2] - assert_expand node, to, file: file, line: line + assert_expand node, to, flags: flags, file: file, line: line end -def assert_error(str, message = nil, *, inject_primitives = false, file = __FILE__, line = __LINE__, flags = nil) +def assert_error(str, message = nil, *, inject_primitives = false, flags = nil, file = __FILE__, line = __LINE__) expect_raises TypeException, message, file, line do semantic str, inject_primitives: inject_primitives, flags: flags end diff --git a/spec/std/kernel_spec.cr b/spec/std/kernel_spec.cr index b31588d4f444..9842ec80283e 100644 --- a/spec/std/kernel_spec.cr +++ b/spec/std/kernel_spec.cr @@ -3,12 +3,12 @@ require "./spec_helper" describe "exit" do it "exits normally with status 0" do - status, _ = compile_and_run_source "exit" + status, _, _ = compile_and_run_source "exit" status.success?.should be_true end it "exits with given error code" do - status, _ = compile_and_run_source "exit 42" + status, _, _ = compile_and_run_source "exit 42" status.success?.should be_false status.exit_code.should eq(42) end @@ -16,7 +16,7 @@ end describe "at_exit" do it "runs handlers on normal program ending" do - status, output = compile_and_run_source <<-CODE + status, output, _ = compile_and_run_source <<-CODE at_exit do puts "handler code" end @@ -27,7 +27,7 @@ describe "at_exit" do end it "runs handlers on explicit program ending" do - status, output = compile_and_run_source <<-'CODE' + status, output, _ = compile_and_run_source <<-'CODE' at_exit do |exit_code| puts "handler code, exit code: #{exit_code}" end @@ -40,7 +40,7 @@ describe "at_exit" do end it "runs handlers in reverse order" do - status, output = compile_and_run_source <<-CODE + status, output, _ = compile_and_run_source <<-CODE at_exit do puts "first handler code" end @@ -59,7 +59,7 @@ describe "at_exit" do end it "runs all handlers maximum once" do - status, output = compile_and_run_source <<-CODE + status, output, _ = compile_and_run_source <<-CODE at_exit do puts "first handler code" end @@ -86,7 +86,7 @@ describe "at_exit" do end it "allows handlers to change the exit code with explicit `exit` call" do - status, output = compile_and_run_source <<-'CODE' + status, output, _ = compile_and_run_source <<-'CODE' at_exit do |exit_code| puts "first handler code, exit code: #{exit_code}" end @@ -114,7 +114,7 @@ describe "at_exit" do end it "allows handlers to change the exit code with explicit `exit` call (2)" do - status, output = compile_and_run_source <<-'CODE' + status, output, _ = compile_and_run_source <<-'CODE' at_exit do |exit_code| puts "first handler code, exit code: #{exit_code}" end @@ -210,7 +210,7 @@ describe "at_exit" do end it "allows at_exit inside at_exit" do - status, output = compile_and_run_source <<-CODE + status, output, _ = compile_and_run_source <<-CODE at_exit do puts "1" at_exit do diff --git a/spec/std/process_spec.cr b/spec/std/process_spec.cr index 4cda37bab562..c8e75b23103b 100644 --- a/spec/std/process_spec.cr +++ b/spec/std/process_spec.cr @@ -165,7 +165,7 @@ describe Process do end pending_win32 "chroot raises when unprivileged" do - status, output = compile_and_run_source <<-'CODE' + status, output, _ = compile_and_run_source <<-'CODE' begin Process.chroot("/usr") puts "FAIL" diff --git a/src/compiler/crystal/codegen/primitives.cr b/src/compiler/crystal/codegen/primitives.cr index 281b1374f813..306b83a1061c 100644 --- a/src/compiler/crystal/codegen/primitives.cr +++ b/src/compiler/crystal/codegen/primitives.cr @@ -1136,8 +1136,8 @@ class Crystal::CodeGenVisitor success_ordering = atomic_ordering_from_symbol_literal(call.args[-2]) failure_ordering = atomic_ordering_from_symbol_literal(call.args[-1]) - pointer, cmp, new = call_args - value = builder.cmpxchg(pointer, cmp, new, success_ordering, failure_ordering) + ptr, cmp, new, _, _ = call_args + value = builder.cmpxchg(ptr, cmp, new, success_ordering, failure_ordering) value_ptr = alloca llvm_type(node.type) store extract_value(value, 0), gep(value_ptr, 0, 0) store extract_value(value, 1), gep(value_ptr, 0, 1) @@ -1150,8 +1150,8 @@ class Crystal::CodeGenVisitor ordering = atomic_ordering_from_symbol_literal(call.args[-2]) singlethread = bool_from_bool_literal(call.args[-1]) - _, pointer, val = call_args - builder.atomicrmw(op, pointer, val, ordering, singlethread) + _, ptr, val, _, _ = call_args + builder.atomicrmw(op, ptr, val, ordering, singlethread) end def codegen_primitive_fence(call, node, target_def, call_args) @@ -1168,7 +1168,7 @@ class Crystal::CodeGenVisitor ordering = atomic_ordering_from_symbol_literal(call.args[-2]) volatile = bool_from_bool_literal(call.args[-1]) - ptr = call_args.first + ptr, _, _ = call_args inst = builder.load(ptr) inst.ordering = ordering @@ -1182,7 +1182,7 @@ class Crystal::CodeGenVisitor ordering = atomic_ordering_from_symbol_literal(call.args[-2]) volatile = bool_from_bool_literal(call.args[-1]) - ptr, value = call_args + ptr, value, _, _ = call_args inst = builder.store(value, ptr) inst.ordering = ordering diff --git a/src/compiler/crystal/semantic/cleanup_transformer.cr b/src/compiler/crystal/semantic/cleanup_transformer.cr index a316188b7f89..4db2f6cff86e 100644 --- a/src/compiler/crystal/semantic/cleanup_transformer.cr +++ b/src/compiler/crystal/semantic/cleanup_transformer.cr @@ -306,6 +306,45 @@ module Crystal node end + def transform(node : MultiAssign) + if @program.has_flag?("strict_multi_assign") + if node.values.size == 1 + # the expanded node always starts with `temp = {{ node.values[0] }}`; + # this is the whole Assign node and its deduced type is equal to the + # original RHS's type + temp_assign = node.expanded.as(Expressions).expressions.first + target_count = node.targets.size + + case type = temp_assign.type + when UnionType + sizes = type.union_types.map { |union_type| constant_size(union_type) } + if sizes.none? &.in?(target_count, nil) + node.values.first.raise "cannot assign #{type} to #{target_count} targets" + end + else + if size = constant_size(type) + unless size == target_count + node.values.first.raise "cannot assign #{type} to #{target_count} targets" + end + end + end + end + end + + if expanded = node.expanded + return expanded.transform self + end + + node + end + + private def constant_size(type) + case type + when TupleInstanceType + type.size + end + end + def transform(node : Path) # Some constants might not have been cleaned up at this point because # they don't have an explicit `Assign` node. One example is regex diff --git a/src/compiler/crystal/semantic/literal_expander.cr b/src/compiler/crystal/semantic/literal_expander.cr index dcd55481c07d..5ab558d1ea61 100644 --- a/src/compiler/crystal/semantic/literal_expander.cr +++ b/src/compiler/crystal/semantic/literal_expander.cr @@ -663,6 +663,14 @@ module Crystal # a = temp[0] # b = temp[1] # + # If the flag "strict_multi_assign" is present, requires `temp`'s size to + # match the number of assign targets exactly: (it must respond to `#size`) + # + # temp = [1, 2] + # raise ... if temp.size != 2 + # a = temp[0] + # b = temp[1] + # # From: # # a, *b, c, d = [1, 2] @@ -681,17 +689,22 @@ module Crystal if node.values.size == 1 value = node.values[0] middle_splat = splat_index && (0 < splat_index < node.targets.size - 1) + raise_on_count_mismatch = @program.has_flag?("strict_multi_assign") || middle_splat temp_var = new_temp_var # temp = ... - assigns = Array(ASTNode).new(node.targets.size + (splat_underscore ? 0 : 1) + (middle_splat ? 1 : 0)) + assigns = Array(ASTNode).new(node.targets.size + (splat_underscore ? 0 : 1) + (raise_on_count_mismatch ? 1 : 0)) assigns << Assign.new(temp_var.clone, value).at(value) # raise ... if temp.size < ... - if middle_splat + if raise_on_count_mismatch size_call = Call.new(temp_var.clone, "size").at(value) - size_comp = Call.new(size_call, "<", NumberLiteral.new(node.targets.size - 1)).at(value) + if middle_splat + size_comp = Call.new(size_call, "<", NumberLiteral.new(node.targets.size - 1)).at(value) + else + size_comp = Call.new(size_call, "!=", NumberLiteral.new(node.targets.size)).at(value) + end index_error = Call.new(Path.global("IndexError"), "new", StringLiteral.new("Multiple assignment count mismatch")).at(value) raise_call = Call.global("raise", index_error).at(value) assigns << If.new(size_comp, raise_call).at(value) diff --git a/src/compiler/crystal/tools/formatter.cr b/src/compiler/crystal/tools/formatter.cr index 59b7b4dd65fa..5d4fde772d4f 100644 --- a/src/compiler/crystal/tools/formatter.cr +++ b/src/compiler/crystal/tools/formatter.cr @@ -2678,7 +2678,7 @@ module Crystal write "(" has_parentheses = true - has_newlines, found_comment = format_call_args(node, true, base_indent) + has_newlines, found_comment, _ = format_call_args(node, true, base_indent) found_comment ||= skip_space if @token.type == :NEWLINE ends_with_newline = true @@ -2687,7 +2687,7 @@ module Crystal elsif has_args || node.block_arg write " " unless passed_backslash_newline skip_space - has_newlines, found_comment = format_call_args(node, false, base_indent) + has_newlines, found_comment, _ = format_call_args(node, false, base_indent) end if block = node.block diff --git a/src/humanize.cr b/src/humanize.cr index 15744c462667..14fe060fd52c 100644 --- a/src/humanize.cr +++ b/src/humanize.cr @@ -228,7 +228,8 @@ struct Number magnitude = 1 end - magnitude, unit = yield_result = yield magnitude, self.to_f + yield_result = yield magnitude, self.to_f + magnitude, unit = yield_result[0..1] decimal_places = precision if significant diff --git a/src/time.cr b/src/time.cr index d85fe3f3ab03..66f9989c714b 100644 --- a/src/time.cr +++ b/src/time.cr @@ -1356,7 +1356,7 @@ struct Time # Returns a copy of this `Time` representing the end of the semester. def at_end_of_semester : Time - year, month = year_month_day_day_year + year, month, _, _ = year_month_day_day_year if month <= 6 month, day = 6, 30 else @@ -1367,7 +1367,7 @@ struct Time # Returns a copy of this `Time` representing the end of the quarter. def at_end_of_quarter : Time - year, month = year_month_day_day_year + year, month, _, _ = year_month_day_day_year if month <= 3 month, day = 3, 31 elsif month <= 6 @@ -1404,7 +1404,7 @@ struct Time # Returns a copy of this `Time` representing midday (`12:00`) of the same day. def at_midday : Time - year, month, day = year_month_day_day_year + year, month, day, _ = year_month_day_day_year Time.local(year, month, day, 12, 0, 0, nanosecond: 0, location: location) end