From 1571454e9c80ad751e5b52a756e8e839c31fa904 Mon Sep 17 00:00:00 2001 From: Andrew Konchin Date: Thu, 7 Nov 2024 16:09:11 +0200 Subject: [PATCH 1/2] Implement rb_category_warn C function --- CHANGELOG.md | 1 + lib/cext/include/ruby/internal/error.h | 16 +++++++++ .../truffleruby/truffleruby-abi-version.h | 2 +- lib/truffle/truffle/cext.rb | 6 ++++ spec/ruby/optional/capi/ext/kernel_spec.c | 13 ++++++- spec/ruby/optional/capi/kernel_spec.rb | 34 +++++++++++++++++++ spec/truffle/capi/error_spec.rb | 1 + src/main/c/cext/error.c | 20 +++++++++-- 8 files changed, 89 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f7d66c734e53..c78155aead1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ Compatibility: * Implement `rb_enc_interned_str()` (#3703, @Th3-M4jor). * Implement `rb_hash_bulk_insert()` (#3705, @Th3-M4jor). * Remove deprecated `Pathname#{taint,untaint}` methods (#3681, @andrykonchin). +* Add `rb_category_warn` function (#3710, @andrykonchin). Performance: diff --git a/lib/cext/include/ruby/internal/error.h b/lib/cext/include/ruby/internal/error.h index bbd852833902..e842d6b5bcfe 100644 --- a/lib/cext/include/ruby/internal/error.h +++ b/lib/cext/include/ruby/internal/error.h @@ -607,6 +607,11 @@ static inline void rb_warn(const char *fmt, ...) { void rb_warn(const char *fmt, ...); #endif +#ifdef TRUFFLERUBY +bool rb_warning_category_enabled_p(rb_warning_category_t category); +void rb_tr_category_warn_va_list(rb_warning_category_t category, const char *fmt, va_list args); +#endif + RBIMPL_ATTR_COLD() RBIMPL_ATTR_NONNULL((2)) RBIMPL_ATTR_FORMAT(RBIMPL_PRINTF_FORMAT, 2, 3) @@ -616,7 +621,18 @@ RBIMPL_ATTR_FORMAT(RBIMPL_PRINTF_FORMAT, 2, 3) * @param[in] cat Category e.g. deprecated. * @param[in] fmt Format specifier string compatible with rb_sprintf(). */ +#ifdef TRUFFLERUBY +static inline void rb_category_warn(rb_warning_category_t category, const char *fmt, ...) { + if (!NIL_P(ruby_verbose) && rb_warning_category_enabled_p(category)) { + va_list args; + va_start(args, fmt); + rb_tr_category_warn_va_list(category, fmt, args); + va_end(args); + } +} +#else void rb_category_warn(rb_warning_category_t cat, const char *fmt, ...); +#endif RBIMPL_ATTR_NONNULL((1, 3)) RBIMPL_ATTR_FORMAT(RBIMPL_PRINTF_FORMAT, 3, 4) diff --git a/lib/cext/include/truffleruby/truffleruby-abi-version.h b/lib/cext/include/truffleruby/truffleruby-abi-version.h index 97938b5ccff2..8298adea4d1f 100644 --- a/lib/cext/include/truffleruby/truffleruby-abi-version.h +++ b/lib/cext/include/truffleruby/truffleruby-abi-version.h @@ -20,6 +20,6 @@ // $RUBY_VERSION must be the same as TruffleRuby.LANGUAGE_VERSION. // $ABI_NUMBER starts at 1 and is incremented for every ABI-incompatible change. -#define TRUFFLERUBY_ABI_VERSION "3.3.5.1" +#define TRUFFLERUBY_ABI_VERSION "3.3.5.2" #endif diff --git a/lib/truffle/truffle/cext.rb b/lib/truffle/truffle/cext.rb index 59a28ce02821..125431fa1380 100644 --- a/lib/truffle/truffle/cext.rb +++ b/lib/truffle/truffle/cext.rb @@ -2323,6 +2323,12 @@ def rb_warning_category_enabled_p(category) Warning[category] end + def rb_tr_warn_category(message, category) + location = caller_locations(1, 1)[0] + message_with_prefix = location.label + ': warning: ' + message + Warning.warn(message_with_prefix, category: category) + end + def rb_tr_flags(object) Truffle::CExt::RBasic.new(object).compute_flags end diff --git a/spec/ruby/optional/capi/ext/kernel_spec.c b/spec/ruby/optional/capi/ext/kernel_spec.c index e6bf4870ec19..56448010d8aa 100644 --- a/spec/ruby/optional/capi/ext/kernel_spec.c +++ b/spec/ruby/optional/capi/ext/kernel_spec.c @@ -67,11 +67,20 @@ VALUE kernel_spec_rb_block_call_no_func(VALUE self, VALUE ary) { return rb_block_call(ary, rb_intern("map"), 0, NULL, (rb_block_call_func_t)NULL, Qnil); } - VALUE kernel_spec_rb_frame_this_func(VALUE self) { return ID2SYM(rb_frame_this_func()); } +VALUE kernel_spec_rb_category_warn_deprecated(VALUE self) { + rb_category_warn(RB_WARN_CATEGORY_DEPRECATED, "foo"); + return Qnil; +} + +VALUE kernel_spec_rb_category_warn_deprecated_with_integer_extra_value(VALUE self, VALUE value) { + rb_category_warn(RB_WARN_CATEGORY_DEPRECATED, "foo %d", FIX2INT(value)); + return Qnil; +} + VALUE kernel_spec_rb_ensure(VALUE self, VALUE main_proc, VALUE arg, VALUE ensure_proc, VALUE arg2) { VALUE main_array, ensure_array; @@ -381,6 +390,8 @@ void Init_kernel_spec(void) { rb_define_method(cls, "rb_block_lambda", kernel_spec_rb_block_lambda, 0); rb_define_method(cls, "rb_frame_this_func_test", kernel_spec_rb_frame_this_func, 0); rb_define_method(cls, "rb_frame_this_func_test_again", kernel_spec_rb_frame_this_func, 0); + rb_define_method(cls, "rb_category_warn_deprecated", kernel_spec_rb_category_warn_deprecated, 0); + rb_define_method(cls, "rb_category_warn_deprecated_with_integer_extra_value", kernel_spec_rb_category_warn_deprecated_with_integer_extra_value, 1); rb_define_method(cls, "rb_ensure", kernel_spec_rb_ensure, 4); rb_define_method(cls, "rb_eval_string", kernel_spec_rb_eval_string, 1); rb_define_method(cls, "rb_eval_cmd_kw", kernel_spec_rb_eval_cmd_kw, 3); diff --git a/spec/ruby/optional/capi/kernel_spec.rb b/spec/ruby/optional/capi/kernel_spec.rb index a169813cd536..07dcc96b41ad 100644 --- a/spec/ruby/optional/capi/kernel_spec.rb +++ b/spec/ruby/optional/capi/kernel_spec.rb @@ -530,6 +530,40 @@ def proc_caller end end + describe "rb_category_warn" do + it "emits a warning into stderr" do + Warning[:deprecated] = true + + -> { + @s.rb_category_warn_deprecated + }.should complain(/warning: foo/, verbose: true) + end + + it "supports printf format modifiers" do + Warning[:deprecated] = true + + -> { + @s.rb_category_warn_deprecated_with_integer_extra_value(42) + }.should complain(/warning: foo 42/, verbose: true) + end + + it "does not emits a warning when a category is disabled" do + Warning[:deprecated] = false + + -> { + @s.rb_category_warn_deprecated + }.should_not complain(verbose: true) + end + + it "does not emits a warning when $VERBOSE is nil" do + Warning[:deprecated] = true + + -> { + @s.rb_category_warn_deprecated + }.should_not complain(verbose: nil) + end + end + describe "rb_ensure" do it "executes passed function and returns its value" do proc = -> x { x } diff --git a/spec/truffle/capi/error_spec.rb b/spec/truffle/capi/error_spec.rb index a08a68911f53..ec9ac5fb8379 100644 --- a/spec/truffle/capi/error_spec.rb +++ b/spec/truffle/capi/error_spec.rb @@ -10,6 +10,7 @@ load_extension("error") +# C functions defined in CRuby internal headers, so they don't belong to the public API. describe "C-API error functions" do before :each do @e = CApiErrorSpecs.new diff --git a/src/main/c/cext/error.c b/src/main/c/cext/error.c index 9141f2034ad7..e2eb838e9e80 100644 --- a/src/main/c/cext/error.c +++ b/src/main/c/cext/error.c @@ -9,7 +9,7 @@ */ #include -bool rb_warning_category_enabled_p(rb_warning_category_t category) { +VALUE rb_tr_warning_category_to_name(rb_warning_category_t category) { VALUE category_name; switch (category) { @@ -23,9 +23,25 @@ bool rb_warning_category_enabled_p(rb_warning_category_t category) { category_name = Qnil; } + return category_name; +} + +void rb_tr_category_warn_va_list(rb_warning_category_t category, const char *fmt, va_list args) { + VALUE message, category_name; + + message = rb_vsprintf(fmt, args); + category_name = rb_tr_warning_category_to_name(category); + RUBY_CEXT_INVOKE("rb_tr_warn_category", message, category_name); +} + +bool rb_warning_category_enabled_p(rb_warning_category_t category) { + VALUE category_name; + + category_name = rb_tr_warning_category_to_name(category); + if (category_name != Qnil) { return polyglot_as_boolean(RUBY_CEXT_INVOKE_NO_WRAP("rb_warning_category_enabled_p", category_name)); } else { - return Qtrue; + return true; } } From 95e39c8322ba5be2b34c563a7a1596477ba8c457 Mon Sep 17 00:00:00 2001 From: Andrew Konchin Date: Wed, 13 Nov 2024 20:21:08 +0200 Subject: [PATCH 2/2] Fix rb_warn and add location and "warning: " prefix --- lib/cext/ABI_check.txt | 2 +- lib/truffle/truffle/cext.rb | 6 ++++++ spec/ruby/optional/capi/kernel_spec.rb | 22 ++++++---------------- src/main/c/cext/truffleruby.c | 2 +- 4 files changed, 14 insertions(+), 18 deletions(-) diff --git a/lib/cext/ABI_check.txt b/lib/cext/ABI_check.txt index 0cfbf08886fc..00750edc07d6 100644 --- a/lib/cext/ABI_check.txt +++ b/lib/cext/ABI_check.txt @@ -1 +1 @@ -2 +3 diff --git a/lib/truffle/truffle/cext.rb b/lib/truffle/truffle/cext.rb index 125431fa1380..4c7554fd3722 100644 --- a/lib/truffle/truffle/cext.rb +++ b/lib/truffle/truffle/cext.rb @@ -2319,6 +2319,12 @@ def rb_eval_cmd_kw(cmd, args, kw_splat) end end + def rb_tr_warn(message) + location = caller_locations(1, 1)[0] + message_with_prefix = location.label + ': warning: ' + message + Warning.warn(message_with_prefix) + end + def rb_warning_category_enabled_p(category) Warning[category] end diff --git a/spec/ruby/optional/capi/kernel_spec.rb b/spec/ruby/optional/capi/kernel_spec.rb index 07dcc96b41ad..50e8b2f3fdc7 100644 --- a/spec/ruby/optional/capi/kernel_spec.rb +++ b/spec/ruby/optional/capi/kernel_spec.rb @@ -156,26 +156,16 @@ class CApiKernelSpecs::Exc < StandardError end describe "rb_warn" do - before :each do - @stderr, $stderr = $stderr, IOStub.new - @verbose = $VERBOSE - end - - after :each do - $stderr = @stderr - $VERBOSE = @verbose - end - it "prints a message to $stderr if $VERBOSE evaluates to true" do - $VERBOSE = true - @s.rb_warn("This is a warning") - $stderr.should =~ /This is a warning/ + -> { + @s.rb_warn("This is a warning") + }.should complain(/warning: This is a warning/, verbose: true) end it "prints a message to $stderr if $VERBOSE evaluates to false" do - $VERBOSE = false - @s.rb_warn("This is a warning") - $stderr.should =~ /This is a warning/ + -> { + @s.rb_warn("This is a warning") + }.should complain(/warning: This is a warning/, verbose: false) end end diff --git a/src/main/c/cext/truffleruby.c b/src/main/c/cext/truffleruby.c index 79cd4adc419b..a898ca8ae4d5 100644 --- a/src/main/c/cext/truffleruby.c +++ b/src/main/c/cext/truffleruby.c @@ -35,7 +35,7 @@ int rb_tr_obj_equal(VALUE first, VALUE second) { } void rb_tr_warn_va_list(const char *fmt, va_list args) { - RUBY_INVOKE(rb_mKernel, "warn", rb_vsprintf(fmt, args)); + RUBY_CEXT_INVOKE("rb_tr_warn", rb_vsprintf(fmt, args)); } VALUE rb_tr_zlib_crc_table(void) {