summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--NEWS.md7
-rw-r--r--array.rb2
-rw-r--r--compile.c14
-rw-r--r--dir.rb1
-rw-r--r--gems/bundled_gems2
-rw-r--r--iseq.c6
-rw-r--r--lib/optparse.rb4
-rw-r--r--lib/rdoc/context.rb2
-rw-r--r--pack.rb1
-rw-r--r--rjit_c.rb1
-rw-r--r--test/ruby/test_method.rb59
-rw-r--r--test/ruby/test_optimization.rb2
-rw-r--r--tool/mk_builtin_loader.rb2
-rw-r--r--tool/test/testunit/test_parallel.rb2
-rw-r--r--trace_point.rb5
-rw-r--r--vm_backtrace.c10
-rw-r--r--vm_core.h1
-rw-r--r--vm_insnhelper.c57
18 files changed, 165 insertions, 13 deletions
diff --git a/NEWS.md b/NEWS.md
index bdb4fa1eb1..ff08c4e566 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -111,7 +111,14 @@ See GitHub releases like [GitHub Releases of Logger](https://github.com/ruby/log
## JIT
+## Miscellaneous changes
+
+* Passing a block to a method which doesn't use the passed block will show
+ a warning on verbose mode (`-w`).
+ [[Feature #15554]]
+
[Feature #13557]: https://bugs.ruby-lang.org/issues/13557
+[Feature #15554]: https://bugs.ruby-lang.org/issues/15554
[Feature #16495]: https://bugs.ruby-lang.org/issues/16495
[Feature #18290]: https://bugs.ruby-lang.org/issues/18290
[Feature #18980]: https://bugs.ruby-lang.org/issues/18980
diff --git a/array.rb b/array.rb
index 8e809b35c9..f63ff00056 100644
--- a/array.rb
+++ b/array.rb
@@ -43,6 +43,8 @@ class Array
# Related: #each_index, #reverse_each.
def each
Primitive.attr! :inline_block
+ Primitive.attr! :use_block
+
unless defined?(yield)
return Primitive.cexpr! 'SIZED_ENUMERATOR(self, 0, 0, ary_enum_length)'
end
diff --git a/compile.c b/compile.c
index 3fb1566ad4..2a7bdc2c99 100644
--- a/compile.c
+++ b/compile.c
@@ -2098,6 +2098,7 @@ iseq_set_arguments(rb_iseq_t *iseq, LINK_ANCHOR *const optargs, const NODE *cons
if (block_id) {
body->param.block_start = arg_size++;
body->param.flags.has_block = TRUE;
+ body->param.flags.use_block = 1;
}
iseq_calc_param_size(iseq);
@@ -5918,6 +5919,7 @@ defined_expr0(rb_iseq_t *iseq, LINK_ANCHOR *const ret,
ADD_INSN(ret, line_node, putnil);
ADD_INSN3(ret, line_node, defined, INT2FIX(DEFINED_YIELD), 0,
PUSH_VAL(DEFINED_YIELD));
+ ISEQ_BODY(ISEQ_BODY(iseq)->local_iseq)->param.flags.use_block = 1;
return;
case NODE_BACK_REF:
@@ -8628,6 +8630,9 @@ compile_builtin_attr(rb_iseq_t *iseq, const NODE *node)
else if (strcmp(RSTRING_PTR(string), "inline_block") == 0) {
ISEQ_BODY(iseq)->builtin_attrs |= BUILTIN_ATTR_INLINE_BLOCK;
}
+ else if (strcmp(RSTRING_PTR(string), "use_block") == 0) {
+ ISEQ_BODY(iseq)->param.flags.use_block = 1;
+ }
else {
goto unknown_arg;
}
@@ -9377,6 +9382,8 @@ compile_super(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *const node, i
}
else {
/* NODE_ZSUPER */
+ ISEQ_BODY(ISEQ_BODY(iseq)->local_iseq)->param.flags.use_block = 1;
+
int i;
const rb_iseq_t *liseq = body->local_iseq;
const struct rb_iseq_constant_body *const local_body = ISEQ_BODY(liseq);
@@ -9510,6 +9517,7 @@ compile_yield(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *const node, i
ADD_SEQ(ret, args);
ADD_INSN1(ret, node, invokeblock, new_callinfo(iseq, 0, FIX2INT(argc), flag, keywords, FALSE));
+ ISEQ_BODY(ISEQ_BODY(iseq)->local_iseq)->param.flags.use_block = 1;
if (popped) {
ADD_INSN(ret, node, pop);
@@ -12935,7 +12943,10 @@ ibf_dump_iseq_each(struct ibf_dump *dump, const rb_iseq_t *iseq)
(body->param.flags.has_block << 6) |
(body->param.flags.ambiguous_param0 << 7) |
(body->param.flags.accepts_no_kwarg << 8) |
- (body->param.flags.ruby2_keywords << 9);
+ (body->param.flags.ruby2_keywords << 9) |
+ (body->param.flags.anon_rest << 10) |
+ (body->param.flags.anon_kwrest << 11) |
+ (body->param.flags.use_block << 12);
#if IBF_ISEQ_ENABLE_LOCAL_BUFFER
# define IBF_BODY_OFFSET(x) (x)
@@ -13151,6 +13162,7 @@ ibf_load_iseq_each(struct ibf_load *load, rb_iseq_t *iseq, ibf_offset_t offset)
load_body->param.flags.ruby2_keywords = (param_flags >> 9) & 1;
load_body->param.flags.anon_rest = (param_flags >> 10) & 1;
load_body->param.flags.anon_kwrest = (param_flags >> 11) & 1;
+ load_body->param.flags.use_block = (param_flags >> 12) & 1;
load_body->param.size = param_size;
load_body->param.lead_num = param_lead_num;
load_body->param.opt_num = param_opt_num;
diff --git a/dir.rb b/dir.rb
index 632c49eee9..42b475ab2c 100644
--- a/dir.rb
+++ b/dir.rb
@@ -408,6 +408,7 @@ class Dir
# specifies that patterns may match short names if they exist; Windows only.
#
def self.glob(pattern, _flags = 0, flags: _flags, base: nil, sort: true)
+ Primitive.attr! :use_block
Primitive.dir_s_glob(pattern, flags, base, sort)
end
end
diff --git a/gems/bundled_gems b/gems/bundled_gems
index 842fbe0ce3..ad979c1c5c 100644
--- a/gems/bundled_gems
+++ b/gems/bundled_gems
@@ -6,7 +6,7 @@
# - revision: revision in repository-url to test
# if `revision` is not given, "v"+`version` or `version` will be used.
-minitest 5.22.3 https://github.com/minitest/minitest 287b35d60c8e19c11ba090efc6eeb225325a8520
+minitest 5.22.3 https://github.com/minitest/minitest ea9caafc0754b1d6236a490d59e624b53209734a
power_assert 2.0.3 https://github.com/ruby/power_assert 84e85124c5014a139af39161d484156cfe87a9ed
rake 13.2.1 https://github.com/ruby/rake
test-unit 3.6.2 https://github.com/test-unit/test-unit
diff --git a/iseq.c b/iseq.c
index 3659dab2f5..4d4006777e 100644
--- a/iseq.c
+++ b/iseq.c
@@ -538,6 +538,11 @@ iseq_location_setup(rb_iseq_t *iseq, VALUE name, VALUE path, VALUE realpath, int
RB_OBJ_WRITE(iseq, &loc->label, name);
RB_OBJ_WRITE(iseq, &loc->base_label, name);
loc->first_lineno = first_lineno;
+
+ if (ISEQ_BODY(iseq)->local_iseq == iseq && strcmp(RSTRING_PTR(name), "initialize") == 0) {
+ ISEQ_BODY(iseq)->param.flags.use_block = 1;
+ }
+
if (code_location) {
loc->node_id = node_id;
loc->code_location = *code_location;
@@ -1011,6 +1016,7 @@ pm_iseq_new_with_opt(pm_scope_node_t *node, VALUE name, VALUE path, VALUE realpa
{
rb_iseq_t *iseq = iseq_alloc();
ISEQ_BODY(iseq)->prism = true;
+ ISEQ_BODY(iseq)->param.flags.use_block = true; // unused block warning is not supported yet
if (!option) option = &COMPILE_OPTION_DEFAULT;
diff --git a/lib/optparse.rb b/lib/optparse.rb
index 76ff38aca3..1dec3fa124 100644
--- a/lib/optparse.rb
+++ b/lib/optparse.rb
@@ -461,7 +461,7 @@ class OptionParser
candidates
end
- def candidate(key, icase = false, pat = nil)
+ def candidate(key, icase = false, pat = nil, &_)
Completion.candidate(key, icase, pat, &method(:each))
end
@@ -739,7 +739,7 @@ class OptionParser
#
# Raises an exception if argument is not present.
#
- def parse(arg, argv)
+ def parse(arg, argv, &_)
unless arg
raise MissingArgument if argv.empty?
arg = argv.shift
diff --git a/lib/rdoc/context.rb b/lib/rdoc/context.rb
index c6edfb473c..c688d562c3 100644
--- a/lib/rdoc/context.rb
+++ b/lib/rdoc/context.rb
@@ -710,7 +710,7 @@ class RDoc::Context < RDoc::CodeObject
# This method exists to make it easy to work with Context subclasses that
# aren't part of RDoc.
- def each_ancestor # :nodoc:
+ def each_ancestor(&_) # :nodoc:
end
##
diff --git a/pack.rb b/pack.rb
index d505eaee35..b6e29c3eab 100644
--- a/pack.rb
+++ b/pack.rb
@@ -17,6 +17,7 @@ class String
# returns that array.
# See {Packed Data}[rdoc-ref:packed_data.rdoc].
def unpack(fmt, offset: 0)
+ Primitive.attr! :use_block
Primitive.pack_unpack(fmt, offset)
end
diff --git a/rjit_c.rb b/rjit_c.rb
index 0ba33b40bf..b9a5bb0b55 100644
--- a/rjit_c.rb
+++ b/rjit_c.rb
@@ -1093,6 +1093,7 @@ module RubyVM::RJIT # :nodoc: all
ruby2_keywords: [CType::BitField.new(1, 1), 9],
anon_rest: [CType::BitField.new(1, 2), 10],
anon_kwrest: [CType::BitField.new(1, 3), 11],
+ use_block: [CType::BitField.new(1, 4), 12],
), Primitive.cexpr!("OFFSETOF(((struct rb_iseq_constant_body *)NULL)->param, flags)")],
size: [CType::Immediate.parse("unsigned int"), Primitive.cexpr!("OFFSETOF(((struct rb_iseq_constant_body *)NULL)->param, size)")],
lead_num: [CType::Immediate.parse("int"), Primitive.cexpr!("OFFSETOF(((struct rb_iseq_constant_body *)NULL)->param, lead_num)")],
diff --git a/test/ruby/test_method.rb b/test/ruby/test_method.rb
index ec06f4c50a..67b3e03e10 100644
--- a/test/ruby/test_method.rb
+++ b/test/ruby/test_method.rb
@@ -1623,4 +1623,63 @@ class TestMethod < Test::Unit::TestCase
end
RUBY
end
+
+ def test_warn_unused_block
+ assert_in_out_err '-w', <<-'RUBY' do |_out, err, _status|
+ def foo = nil
+ foo{} # warn
+ send(:foo){} # warn
+ b = Proc.new{}
+ foo(&b) # warn
+ RUBY
+ assert_equal 3, err.size
+ err = err.join
+ assert_match(/-:2: warning/, err)
+ assert_match(/-:3: warning/, err)
+ assert_match(/-:5: warning/, err)
+ end
+
+ assert_in_out_err '-w', <<-'RUBY' do |_out, err, _status|
+ def foo = nil
+ 10.times{foo{}} # warn once
+ RUBY
+ assert_equal 1, err.size
+ end
+
+ assert_in_out_err '-w', <<-'RUBY' do |_out, err, _status|
+ def foo = nil; b = nil
+ foo(&b) # no warning
+ 1.object_id{} # no warning because it is written in C
+
+ class C
+ def initialize
+ end
+ end
+ C.new{} # no warning
+
+ RUBY
+ assert_equal 0, err.size
+ end
+
+ assert_in_out_err '-w', <<-'RUBY' do |_out, err, _status|
+ class C0
+ def foo = nil
+ def bar = nil
+ def baz = nil
+ end
+
+ class C1 < C0
+ def foo = super
+ def bar = super()
+ def baz(&_) = super(&_)
+ end
+
+ C1.new.foo{} # no warning
+ C1.new.bar{} # warning
+ C1.new.baz{} # no warning
+ RUBY
+ assert_equal 1, err.size
+ assert_match(/-:14: warning.+bar/, err.join)
+ end
+ end
end
diff --git a/test/ruby/test_optimization.rb b/test/ruby/test_optimization.rb
index 70b6bde6ed..44ec615405 100644
--- a/test/ruby/test_optimization.rb
+++ b/test/ruby/test_optimization.rb
@@ -577,7 +577,7 @@ class TestRubyOptimization < Test::Unit::TestCase
begin;
class String
undef freeze
- def freeze
+ def freeze(&)
block_given?
end
end
diff --git a/tool/mk_builtin_loader.rb b/tool/mk_builtin_loader.rb
index 07c8291249..4abd497f0e 100644
--- a/tool/mk_builtin_loader.rb
+++ b/tool/mk_builtin_loader.rb
@@ -6,7 +6,7 @@ require_relative 'ruby_vm/helpers/c_escape'
SUBLIBS = {}
REQUIRED = {}
-BUILTIN_ATTRS = %w[leaf inline_block]
+BUILTIN_ATTRS = %w[leaf inline_block use_block]
def string_literal(lit, str = [])
while lit
diff --git a/tool/test/testunit/test_parallel.rb b/tool/test/testunit/test_parallel.rb
index f79c3a1d80..6882fd6c5f 100644
--- a/tool/test/testunit/test_parallel.rb
+++ b/tool/test/testunit/test_parallel.rb
@@ -119,7 +119,7 @@ module TestParallel
result = Marshal.load($1.chomp.unpack1("m"))
assert_equal(5, result[0])
- pend "TODO: result[1] returns 17. We should investigate it" do
+ pend "TODO: result[1] returns 17. We should investigate it" do # TODO: misusage of pend (pend doens't use given block)
assert_equal(12, result[1])
end
assert_kind_of(Array,result[2])
diff --git a/trace_point.rb b/trace_point.rb
index e61b4b6802..b136e9b399 100644
--- a/trace_point.rb
+++ b/trace_point.rb
@@ -94,6 +94,7 @@ class TracePoint
# Access from other threads is also forbidden.
#
def self.new(*events)
+ Primitive.attr! :use_block
Primitive.tracepoint_new_s(events)
end
@@ -131,6 +132,7 @@ class TracePoint
# trace.enabled? #=> true
#
def self.trace(*events)
+ Primitive.attr! :use_block
Primitive.tracepoint_trace_s(events)
end
@@ -196,6 +198,7 @@ class TracePoint
# out calls by itself from :line handler, otherwise it will call itself infinitely).
#
def self.allow_reentry
+ Primitive.attr! :use_block
Primitive.tracepoint_allow_reentry
end
@@ -258,6 +261,7 @@ class TracePoint
# #=> RuntimeError: access from outside
#
def enable(target: nil, target_line: nil, target_thread: :default)
+ Primitive.attr! :use_block
Primitive.tracepoint_enable_m(target, target_line, target_thread)
end
@@ -294,6 +298,7 @@ class TracePoint
# trace.disable { p tp.lineno }
# #=> RuntimeError: access from outside
def disable
+ Primitive.attr! :use_block
Primitive.tracepoint_disable_m
end
diff --git a/vm_backtrace.c b/vm_backtrace.c
index 6f4379ad23..3fe816930d 100644
--- a/vm_backtrace.c
+++ b/vm_backtrace.c
@@ -201,8 +201,8 @@ location_lineno_m(VALUE self)
VALUE rb_mod_name0(VALUE klass, bool *permanent);
-static VALUE
-gen_method_name(VALUE owner, VALUE name)
+VALUE
+rb_gen_method_name(VALUE owner, VALUE name)
{
bool permanent;
if (RB_TYPE_P(owner, T_CLASS) || RB_TYPE_P(owner, T_MODULE)) {
@@ -235,7 +235,7 @@ retry:
case ISEQ_TYPE_MAIN:
return ISEQ_BODY(iseq)->location.label;
case ISEQ_TYPE_METHOD:
- return gen_method_name(owner, ISEQ_BODY(iseq)->location.label);
+ return rb_gen_method_name(owner, ISEQ_BODY(iseq)->location.label);
case ISEQ_TYPE_BLOCK:
case ISEQ_TYPE_PLAIN: {
int level = 0;
@@ -269,7 +269,7 @@ static VALUE
location_label(rb_backtrace_location_t *loc)
{
if (loc->cme && loc->cme->def->type == VM_METHOD_TYPE_CFUNC) {
- return gen_method_name(loc->cme->owner, rb_id2str(loc->cme->def->original_id));
+ return rb_gen_method_name(loc->cme->owner, rb_id2str(loc->cme->def->original_id));
}
else {
VALUE owner = Qnil;
@@ -457,7 +457,7 @@ location_to_str(rb_backtrace_location_t *loc)
file = GET_VM()->progname;
lineno = 0;
}
- name = gen_method_name(loc->cme->owner, rb_id2str(loc->cme->def->original_id));
+ name = rb_gen_method_name(loc->cme->owner, rb_id2str(loc->cme->def->original_id));
}
else {
file = rb_iseq_path(loc->iseq);
diff --git a/vm_core.h b/vm_core.h
index 2a9d5f906f..57d90b343f 100644
--- a/vm_core.h
+++ b/vm_core.h
@@ -418,6 +418,7 @@ struct rb_iseq_constant_body {
unsigned int ruby2_keywords: 1;
unsigned int anon_rest: 1;
unsigned int anon_kwrest: 1;
+ unsigned int use_block: 1;
} flags;
unsigned int size;
diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index fe038e8618..68a74a3e33 100644
--- a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@@ -2966,6 +2966,57 @@ vm_call_single_noarg_leaf_builtin(rb_execution_context_t *ec, rb_control_frame_t
return builtin_invoker0(ec, calling->recv, NULL, (rb_insn_func_t)bf->func_ptr);
}
+VALUE rb_gen_method_name(VALUE owner, VALUE name); // in vm_backtrace.c
+
+static void
+warn_unused_block(const rb_callable_method_entry_t *cme, const rb_iseq_t *iseq, void *pc)
+{
+ static st_table *dup_check_table = NULL;
+
+ st_data_t key = 0;
+ union {
+ VALUE v;
+ unsigned char b[SIZEOF_VALUE];
+ } k1 = {
+ .v = (VALUE)pc,
+ }, k2 = {
+ .v = (VALUE)cme->def,
+ };
+
+ // make unique key from pc and me->def pointer
+ for (int i=0; i<SIZEOF_VALUE; i++) {
+ // fprintf(stderr, "k1:%3d k2:%3d\n", k1.b[i], k2.b[SIZEOF_VALUE-1-i]);
+ key |= (st_data_t)(k1.b[i] ^ k2.b[SIZEOF_VALUE-1-i]) << (8 * i);
+ }
+
+ if (0) {
+ fprintf(stderr, "SIZEOF_VALUE:%d\n", SIZEOF_VALUE);
+ fprintf(stderr, "pc:%p def:%p\n", pc, cme->def);
+ fprintf(stderr, "key:%p\n", (void *)key);
+ }
+
+ if (!dup_check_table) {
+ dup_check_table = st_init_numtable();
+ }
+
+ // duplication check
+ if (st_insert(dup_check_table, key, 1)) {
+ // already shown
+ }
+ else {
+ VALUE m_loc = rb_method_entry_location((const rb_method_entry_t *)cme);
+ VALUE name = rb_gen_method_name(cme->defined_class, ISEQ_BODY(iseq)->location.base_label);
+
+ if (!NIL_P(m_loc)) {
+ rb_warning("the passed block for '%"PRIsVALUE"' defined at %"PRIsVALUE":%"PRIsVALUE" may be ignored",
+ name, RARRAY_AREF(m_loc, 0), RARRAY_AREF(m_loc, 1));
+ }
+ else {
+ rb_warning("the block may be ignored because '%"PRIsVALUE"' does not use a block", name);
+ }
+ }
+}
+
static inline int
vm_callee_setup_arg(rb_execution_context_t *ec, struct rb_calling_info *calling,
const rb_iseq_t *iseq, VALUE *argv, int param_size, int local_size)
@@ -2974,6 +3025,12 @@ vm_callee_setup_arg(rb_execution_context_t *ec, struct rb_calling_info *calling,
const struct rb_callcache *cc = calling->cc;
bool cacheable_ci = vm_ci_markable(ci);
+ if (UNLIKELY(!ISEQ_BODY(iseq)->param.flags.use_block &&
+ calling->block_handler != VM_BLOCK_HANDLER_NONE &&
+ !(vm_ci_flag(calling->cd->ci) & VM_CALL_SUPER))) {
+ warn_unused_block(vm_cc_cme(cc), iseq, (void *)ec->cfp->pc);
+ }
+
if (LIKELY(!(vm_ci_flag(ci) & VM_CALL_KW_SPLAT))) {
if (LIKELY(rb_simple_iseq_p(iseq))) {
rb_control_frame_t *cfp = ec->cfp;