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 4 commits
Commits
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
1 change: 0 additions & 1 deletion samples/binary-trees.cr
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,4 @@ OptionParser.parse! do |opts|
end
end

in_filenames = ARGV

do_work ARGV, output_filename, ignore_case
6 changes: 1 addition & 5 deletions scripts/generate_unicode_data.cr
Original file line number Diff line number Diff line change
Expand Up @@ -204,15 +204,11 @@ body.each_line do |line|
end

downcase_ranges = case_ranges entries, &.downcase
downcase_one_ranges, downcase_ranges = downcase_ranges.partition { |r| r.delta == 1 }
downcase_one_ranges, _ = downcase_ranges.partition { |r| r.delta == 1 }

upcase_ranges = case_ranges entries, &.upcase
upcase_ranges.select! { |r| r.delta != -1 }

alternate_ranges = alternate_ranges(downcase_one_ranges)
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 used in the ECR template. That's why the tool doesn't see it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks! I knew I was missing something there...


casefold_ranges = case_ranges entries, &.casefold

all_strides = {} of String => Array(Stride)
categories = %w(Lu Ll Lt Mn Mc Me Nd Nl No Zs Zl Zp Cc Cf Cs Co Cn)

Expand Down
2 changes: 1 addition & 1 deletion spec/compiler/crystal/tools/context_spec.cr
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
2 changes: 1 addition & 1 deletion spec/spec_helper.cr
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ end
def assert_normalize(from, to, flags = nil)
program = Program.new
program.flags = flags if flags
normalizer = Normalizer.new(program)
# normalizer = Normalizer.new(program)
Copy link
Member

Choose a reason for hiding this comment

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

why is this not removed?

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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Loading