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

Useless assigns #6055

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion samples/binary-trees.cr
Expand Up @@ -31,7 +31,6 @@ stretch_depth = max_depth + 1
stretch_tree = bottom_up_tree(0, stretch_depth)

puts "stretch tree of depth #{stretch_depth}\t check: #{item_check(stretch_tree)}"
stretch_tree = nil

long_lived_tree = bottom_up_tree(0, max_depth)

Expand Down
3 changes: 0 additions & 3 deletions samples/red_black_tree.cr
Expand Up @@ -328,10 +328,7 @@ class RedBlackTreeRunner
def initialize(n = 10_000)
@n = n

random = Random.new(1234) # repeatable random seq
@a1 = Array(Int32).new(n) { rand(99_999) }

random = Random.new(4321) # repeatable random seq
Copy link
Member

Choose a reason for hiding this comment

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

Not so repeatable huh

@a2 = Array(Int32).new(n) { rand(99_999) }
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this code instead be fixed to do what it was intended to do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should, but not as a part of this PR.


@tree = RedBlackTree.new
Expand Down
2 changes: 0 additions & 2 deletions samples/wordcount.cr
Expand Up @@ -48,6 +48,4 @@ OptionParser.parse! do |opts|
end
end

in_filenames = ARGV

do_work ARGV, output_filename, ignore_case
2 changes: 1 addition & 1 deletion spec/compiler/crystal/tools/context_spec.cr
Expand Up @@ -23,7 +23,7 @@ private def run_context_tool(code)
code = code.gsub('‸', "")

if cursor_location
visitor, result = processed_context_visitor(code, cursor_location)
_, result = processed_context_visitor(code, cursor_location)

yield result
else
Expand Down
2 changes: 1 addition & 1 deletion spec/compiler/crystal/tools/expand_spec.cr
Expand Up @@ -25,7 +25,7 @@ private def run_expand_tool(code)
code = code.gsub('‸', "")

if cursor_location
visitor, result = processed_expand_visitor(code, cursor_location)
_, result = processed_expand_visitor(code, cursor_location)

yield result
else
Expand Down
6 changes: 3 additions & 3 deletions spec/compiler/crystal/tools/implementations_spec.cr
Expand Up @@ -28,7 +28,7 @@ private def assert_implementations(code)
code = code.gsub('‸', "").gsub('༓', "")

if cursor_location
visitor, result = processed_implementation_visitor(code, cursor_location)
_, result = processed_implementation_visitor(code, cursor_location)

result_location = result.implementations.not_nil!.map { |e| Location.new(e.filename.not_nil!, e.line.not_nil!, e.column.not_nil!).to_s }.sort

Expand Down Expand Up @@ -173,7 +173,7 @@ describe "implementations" do
end

it "find full trace for macro expansions" do
visitor, result = processed_implementation_visitor(%(
_, result = processed_implementation_visitor(%(
macro foo
def bar
end
Expand Down Expand Up @@ -211,7 +211,7 @@ describe "implementations" do
end

it "can display text output" do
visitor, result = processed_implementation_visitor(%(
_, result = processed_implementation_visitor(%(
macro foo
def bar
end
Expand Down
2 changes: 1 addition & 1 deletion spec/compiler/crystal/tools/playground_spec.cr
Expand Up @@ -581,7 +581,7 @@ private def assert_compile(source)
sources = Playground::Session.instrument_and_prelude("", "", 0, source, Logger.new(nil))
compiler = Compiler.new
compiler.no_codegen = true
result = compiler.compile sources, "fake-no-build"
compiler.compile sources, "fake-no-build"
end

describe Playground::Session do
Expand Down
2 changes: 1 addition & 1 deletion spec/compiler/crystal_path/crystal_path_spec.cr
Expand Up @@ -98,7 +98,7 @@ describe Crystal::CrystalPath do

it "prints an explanatory message for non-relative requires" do
crystal_path = Crystal::CrystalPath.new(__DIR__)
ex = expect_raises Exception, /If you're trying to require a shard/ do
expect_raises Exception, /If you're trying to require a shard/ do
crystal_path.find "non_existent", relative_to: __DIR__
end
end
Expand Down
4 changes: 2 additions & 2 deletions spec/compiler/semantic/class_spec.cr
Expand Up @@ -35,7 +35,7 @@ describe "Semantic: class" do
end

it "types generic of generic type" do
result = assert_type("
assert_type("
class Foo(T)
def set
@coco = 2
Expand All @@ -48,7 +48,7 @@ describe "Semantic: class" do
") do
foo = types["Foo"].as(GenericClassType)
foo_i32 = foo.instantiate([int32] of TypeVar)
foo_foo_i32 = foo.instantiate([foo_i32] of TypeVar)
foo.instantiate([foo_i32] of TypeVar)
end
end

Expand Down
1 change: 0 additions & 1 deletion spec/compiler/semantic/did_you_mean_spec.cr
Expand Up @@ -237,7 +237,6 @@ describe "Semantic: did you mean" do
end

it "suggests a better alternative to logical operators (#2715)" do
message = "undefined method 'and'"
message = " (did you mean '&&'?)".colorize.yellow.bold.to_s
assert_error %(
def rand(x : Int32)
Expand Down
4 changes: 2 additions & 2 deletions spec/compiler/semantic/instance_var_spec.cr
Expand Up @@ -4154,7 +4154,7 @@ describe "Semantic: instance var" do
end

it "declares instance var of generic class" do
result = assert_type("
assert_type("
class Foo(T)
@x : T

Expand All @@ -4172,7 +4172,7 @@ describe "Semantic: instance var" do
end

it "declares instance var of generic class after reopen" do
result = assert_type("
assert_type("
class Foo(T)
end

Expand Down
2 changes: 1 addition & 1 deletion spec/compiler/semantic/pointer_spec.cr
Expand Up @@ -72,7 +72,7 @@ describe "Semantic: pointer" do
end

it "types pointer of constant" do
result = assert_type("
assert_type("
FOO = 1
pointerof(FOO)
") { pointer_of(int32) }
Expand Down
8 changes: 4 additions & 4 deletions spec/compiler/semantic/virtual_spec.cr
Expand Up @@ -107,7 +107,7 @@ describe "Semantic: virtual" do
x.foo
")
result = semantic nodes
mod, nodes = result.program, result.node.as(Expressions)
_, nodes = result.program, result.node.as(Expressions)
nodes.last.as(Call).target_defs.not_nil!.size.should eq(1)
end

Expand All @@ -130,7 +130,7 @@ describe "Semantic: virtual" do
x.foo
")
result = semantic nodes
mod, nodes = result.program, result.node.as(Expressions)
_, nodes = result.program, result.node.as(Expressions)
nodes.last.as(Call).target_defs.not_nil!.size.should eq(2)
end

Expand Down Expand Up @@ -608,7 +608,7 @@ describe "Semantic: virtual" do
end

it "uses virtual type as generic type if class is abstract" do
result = assert_type("
assert_type("
abstract class Foo
end

Expand All @@ -620,7 +620,7 @@ describe "Semantic: virtual" do
end

it "uses virtual type as generic type if class is abstract even in union" do
result = assert_type("
assert_type("
abstract class Foo
end

Expand Down
1 change: 0 additions & 1 deletion spec/spec_helper.cr
Expand Up @@ -68,7 +68,6 @@ end
def assert_normalize(from, to, flags = nil)
program = Program.new
program.flags = flags if flags
normalizer = Normalizer.new(program)
from_nodes = Parser.parse(from)
to_nodes = program.normalize(from_nodes)
to_nodes.to_s.strip.should eq(to.strip)
Expand Down
2 changes: 1 addition & 1 deletion spec/std/array_spec.cr
Expand Up @@ -350,7 +350,7 @@ describe "Array" do

it "does compact" do
a = [1, nil, 2, nil, 3]
b = a.compact.should eq([1, 2, 3])
a.compact.should eq([1, 2, 3])
a.should eq([1, nil, 2, nil, 3])
end

Expand Down
2 changes: 1 addition & 1 deletion spec/std/concurrent_spec.cr
Expand Up @@ -28,7 +28,7 @@ describe "concurrent" do

it "re-raises errors from Fibers as ConcurrentExecutionException" do
exception = expect_raises(ConcurrentExecutionException) do
a, b = parallel(raising_job, raising_job)
Copy link
Member

Choose a reason for hiding this comment

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

This is not useless per-se. It does matter that the return type is a tuple. But it is tested in the following spec.

Copy link
Contributor Author

@veelenga veelenga Jun 6, 2018

Choose a reason for hiding this comment

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

So, this spec is not good then. parallel(raising_job, raising_job) raises an error and never assigns to a, b. If you are talking about compile-time checking, check out one test below.

parallel(raising_job, raising_job)
end

exception.cause.should be_a(SomeParallelJobException)
Expand Down
1 change: 0 additions & 1 deletion spec/std/deque_spec.cr
Expand Up @@ -611,7 +611,6 @@ describe "Deque" do
it "works while modifying deque" do
a = Deque{1, 2, 3}
count = 0
it = a.each_index
a.each_index.each do
count += 1
a.clear
Expand Down
2 changes: 1 addition & 1 deletion spec/std/dir_spec.cr
Expand Up @@ -404,7 +404,7 @@ describe "Dir" do
end

it "double close doesn't error" do
dir = Dir.open(__DIR__) do |dir|
Dir.open(__DIR__) do |dir|
dir.close
dir.close
end
Expand Down
2 changes: 0 additions & 2 deletions spec/std/file_spec.cr
Expand Up @@ -122,7 +122,6 @@ describe "File" do
end

it "raises an error when a component of the path is a file" do
filename = "#{__DIR__}/data/non_existing_file.txt"
ex = expect_raises(Errno, /Error determining size/) do
File.empty?("#{__FILE__}/")
end
Expand Down Expand Up @@ -462,7 +461,6 @@ describe "File" do
end

it "raises an error when a component of the path is a file" do
filename = "#{__DIR__}/data/non_existing_file.txt"
ex = expect_raises(Errno, /Error determining size/) do
File.size("#{__FILE__}/")
end
Expand Down
2 changes: 1 addition & 1 deletion spec/std/float_printer/grisu3_spec.cr
Expand Up @@ -90,7 +90,7 @@ describe "grisu3" do
it "failure case" do
# grisu should not be able to do this number
# this number is reused to ensure the fallback works
status, point, str = test_grisu 3.5844466002796428e+298
status, _, str = test_grisu 3.5844466002796428e+298
status.should eq false
str.should_not eq "35844466002796428"
end
Expand Down
7 changes: 3 additions & 4 deletions spec/std/hash_spec.cr
Expand Up @@ -672,7 +672,6 @@ describe "Hash" do

it "reduces the hash to the accumulated value of memo" do
hash = {:a => 'b', :c => 'd', :e => 'f'}
result = nil
result = hash.each_with_object({} of Char => Symbol) do |(k, v), memo|
memo[v] = k
end
Expand Down Expand Up @@ -756,7 +755,7 @@ describe "Hash" do

it "reduces the hash to the accumulated value of memo" do
hash = {:a => 'b', :c => 'd', :e => 'f'}
result = hash.reduce("") do |memo, (k, v)|
result = hash.reduce("") do |memo, (_, v)|
memo + v
end
result.should eq("bdf")
Expand All @@ -769,7 +768,7 @@ describe "Hash" do
it { {:a => 2, :b => 3}.reject([:b, :a]).should eq({} of Symbol => Int32) }
it "does not change currrent hash" do
h = {:a => 3, :b => 6, :c => 9}
h2 = h.reject(:b, :c)
h.reject(:b, :c)
h.should eq({:a => 3, :b => 6, :c => 9})
end
end
Expand All @@ -792,7 +791,7 @@ describe "Hash" do
it { {:a => 2, :b => 3}.select([:b, :a]).should eq({:a => 2, :b => 3}) }
it "does not change currrent hash" do
h = {:a => 3, :b => 6, :c => 9}
h2 = h.select(:b, :c)
h.select(:b, :c)
h.should eq({:a => 3, :b => 6, :c => 9})
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/std/http/headers_spec.cr
Expand Up @@ -20,7 +20,7 @@ describe HTTP::Headers do

it "raises an error if header value contains invalid character" do
expect_raises ArgumentError do
headers = HTTP::Headers{"invalid-header" => "\r\nLocation: http://example.com"}
HTTP::Headers{"invalid-header" => "\r\nLocation: http://example.com"}
end
end

Expand Down
1 change: 0 additions & 1 deletion spec/std/http/request_spec.cr
Expand Up @@ -330,7 +330,6 @@ module HTTP

it "is affected when #query is modified" do
request = Request.from_io(IO::Memory.new("GET /api/v3/some/resource?foo=bar&foo=baz&baz=qux HTTP/1.1\r\n\r\n")).as(Request)
params = request.query_params

new_query = "foo=not-bar&foo=not-baz&not-baz=hello&name=world"
request.query = new_query
Expand Down
1 change: 0 additions & 1 deletion spec/std/http/server/handlers/log_handler_spec.cr
Expand Up @@ -23,7 +23,6 @@ describe HTTP::LogHandler do
response = HTTP::Server::Response.new(io)
context = HTTP::Server::Context.new(request, response)

called = false
log_io = IO::Memory.new
handler = HTTP::LogHandler.new(log_io)
handler.next = ->(ctx : HTTP::Server::Context) { raise "foo" }
Expand Down
2 changes: 1 addition & 1 deletion spec/std/kernel_spec.cr
Expand Up @@ -175,7 +175,7 @@ describe "at_exit" do
end

it "errors when used in an at_exit handler" do
status, output, error = build_and_run <<-CODE
status, _, error = build_and_run <<-CODE
at_exit do
at_exit {}
end
Expand Down
2 changes: 1 addition & 1 deletion spec/std/named_tuple_spec.cr
Expand Up @@ -310,7 +310,7 @@ describe "NamedTuple" do
it "merges with other named tuple" do
a = {one: 1, two: 2, three: 3, four: 4, five: 5, "im \"string": "works"}
b = {two: "Two", three: true, "new one": "ok"}
c = a.merge(b).merge(four: "Four").should eq({one: 1, two: "Two", three: true, four: "Four", five: 5, "new one": "ok", "im \"string": "works"})
a.merge(b).merge(four: "Four").should eq({one: 1, two: "Two", three: true, four: "Four", five: 5, "new one": "ok", "im \"string": "works"})
end

it "does types" do
Expand Down
2 changes: 0 additions & 2 deletions spec/std/proc_spec.cr
Expand Up @@ -17,13 +17,11 @@ describe "Proc" do
end

it "does to_s" do
str = IO::Memory.new
f = ->(x : Int32) { x.to_f }
f.to_s.should eq("#<Proc(Int32, Float64):0x#{f.pointer.address.to_s(16)}>")
end

it "does to_s when closured" do
str = IO::Memory.new
a = 1.5
f = ->(x : Int32) { x + a }
f.to_s.should eq("#<Proc(Int32, Float64):0x#{f.pointer.address.to_s(16)}:closure>")
Expand Down
2 changes: 1 addition & 1 deletion spec/std/socket_spec.cr
Expand Up @@ -595,7 +595,7 @@ describe UDPSocket do

client.send("message less than buffer", server.local_address)

bytes_read, client_addr = server.receive(buffer.to_slice)
bytes_read, _ = server.receive(buffer.to_slice)
message = String.new(buffer.to_slice[0, bytes_read])
message.should eq("message less than buffer")

Expand Down
3 changes: 0 additions & 3 deletions src/compiler/crystal/codegen/fun.cr
Expand Up @@ -38,8 +38,6 @@ class Crystal::CodeGenVisitor
func.varargs?
)

p2 = new_fun.params.to_a

func.params.to_a.each_with_index do |p1, index|
attrs = new_fun.attributes(index + 1)
new_fun.add_attribute(attrs, index + 1) unless attrs.value == 0
Expand Down Expand Up @@ -298,7 +296,6 @@ class Crystal::CodeGenVisitor
when LLVM::ABI::ArgKind::Direct
llvm_return_type = (ret_type.cast || ret_type.type)
when LLVM::ABI::ArgKind::Indirect
sret = true
offset += 1
llvm_args_types.insert 0, ret_type.type.pointer
llvm_return_type = llvm_context.void
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/crystal/codegen/primitives.cr
Expand Up @@ -425,7 +425,7 @@ class Crystal::CodeGenVisitor
llvm_type = llvm_embedded_type(type.element_type)

old_debug_location = @current_debug_location
if @debug.line_numbers? && (location = node.location)
if @debug.line_numbers? && node.location
set_current_debug_location(node.location)
end

Expand Down
4 changes: 2 additions & 2 deletions src/compiler/crystal/command.cr
Expand Up @@ -180,15 +180,15 @@ class Crystal::Command

output_filename = Crystal.tempfile(config.output_filename)

result = config.compile output_filename
config.compile output_filename

unless config.compiler.no_codegen?
execute output_filename, config.arguments, config.compiler
end
end

private def types
config, result = compile_no_codegen "tool types"
_, result = compile_no_codegen "tool types"
@progress_tracker.stage("Tool (types)") do
Crystal.print_types result.node
end
Expand Down