[ruby-core:118279] [Ruby master Bug#20573] Warning.warn shouldn't be called for disabled warnings
Issue #20573 has been reported by tenderlovemaking (Aaron Patterson). ---------------------------------------- Bug #20573: Warning.warn shouldn't be called for disabled warnings https://bugs.ruby-lang.org/issues/20573 * Author: tenderlovemaking (Aaron Patterson) * Status: Open * Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- Currently `Warning.warn` will be called for all warnings, even if that particular category is disabled. For example ```ruby module Warning def warn(message, category:) p message => category end end def get_var $= end p Warning[:deprecated] get_var ``` I think that internally we should _not_ call `Warning.warn` unless the category is enabled. I've sent a PR here: https://github.com/ruby/ruby/pull/10960 -- https://bugs.ruby-lang.org/
Issue #20573 has been updated by tenderlovemaking (Aaron Patterson). Oops, I send this before pasting the output of the script: ``` $ ./miniruby test.rb false {"test.rb:8: warning: variable $= is no longer effective\n"=>:deprecated} ``` You can see that "deprecated" warnings are not enabled, but `Warning.warn` is still called. ---------------------------------------- Bug #20573: Warning.warn shouldn't be called for disabled warnings https://bugs.ruby-lang.org/issues/20573#change-108773 * Author: tenderlovemaking (Aaron Patterson) * Status: Open * Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- Currently `Warning.warn` will be called for all warnings, even if that particular category is disabled. For example ```ruby module Warning def warn(message, category:) p message => category end end def get_var $= end p Warning[:deprecated] get_var ``` I think that internally we should _not_ call `Warning.warn` unless the category is enabled. I've sent a PR here: https://github.com/ruby/ruby/pull/10960 -- https://bugs.ruby-lang.org/
Issue #20573 has been updated by byroot (Jean Boussier). Agreed, `Warning.warn` patches shouldn't be called for disabled categories. I was actually 100% convinced this was the behavior, and this fix is consistent with `Warning.warn` not being invoked for verbose warnings when `$VERBOSE = false`. ---------------------------------------- Bug #20573: Warning.warn shouldn't be called for disabled warnings https://bugs.ruby-lang.org/issues/20573#change-108780 * Author: tenderlovemaking (Aaron Patterson) * Status: Open * Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- Currently `Warning.warn` will be called for all warnings, even if that particular category is disabled. For example ```ruby module Warning def warn(message, category:) p message => category end end def get_var $= end p Warning[:deprecated] get_var ``` I think that internally we should _not_ call `Warning.warn` unless the category is enabled. I've sent a PR here: https://github.com/ruby/ruby/pull/10960 -- https://bugs.ruby-lang.org/
Issue #20573 has been updated by byroot (Jean Boussier). In [Feature #16345] it was stated:
We chose Warning.disable(:deprecated) instead of re-defining Warning.warn in order to avoid string object generation.
The intent was clearly for `Warning.warn` not to be called. ---------------------------------------- Bug #20573: Warning.warn shouldn't be called for disabled warnings https://bugs.ruby-lang.org/issues/20573#change-108787 * Author: tenderlovemaking (Aaron Patterson) * Status: Open * Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- Currently `Warning.warn` will be called for all warnings, even if that particular category is disabled. For example ```ruby module Warning def warn(message, category:) p message => category end end def get_var $= end p Warning[:deprecated] get_var ``` I think that internally we should _not_ call `Warning.warn` unless the category is enabled. I've sent a PR here: https://github.com/ruby/ruby/pull/10960 -- https://bugs.ruby-lang.org/
Issue #20573 has been updated by byroot (Jean Boussier). On Ruby 2.7.8 when `Warning[:deprecated] = false` was introduced: ```ruby def Warning.warn(message, **) p [:warn, message] end def foo(a, **b) [a, b] end hash = {bar: 2} foo(1, hash) ``` Produces no output, `Warning.warn` isn't called. I think this was the intent all along. ---------------------------------------- Bug #20573: Warning.warn shouldn't be called for disabled warnings https://bugs.ruby-lang.org/issues/20573#change-108789 * Author: tenderlovemaking (Aaron Patterson) * Status: Open * Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- Currently `Warning.warn` will be called for all warnings, even if that particular category is disabled. For example ```ruby module Warning def warn(message, category:) p message => category end end def get_var $= end p Warning[:deprecated] get_var ``` I think that internally we should _not_ call `Warning.warn` unless the category is enabled. I've sent a PR here: https://github.com/ruby/ruby/pull/10960 -- https://bugs.ruby-lang.org/
Issue #20573 has been updated by tenderlovemaking (Aaron Patterson). byroot (Jean Boussier) wrote in #note-6:
In [Feature #16345] it was stated:
We chose Warning.disable(:deprecated) instead of re-defining Warning.warn in order to avoid string object generation.
The intent was clearly for `Warning.warn` not to be called.
I'm reading the ticket the same way. It sounds like this is a bug. ---------------------------------------- Bug #20573: Warning.warn shouldn't be called for disabled warnings https://bugs.ruby-lang.org/issues/20573#change-108795 * Author: tenderlovemaking (Aaron Patterson) * Status: Open * Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN ---------------------------------------- Currently `Warning.warn` will be called for all warnings, even if that particular category is disabled. For example ```ruby module Warning def warn(message, category:) p message => category end end def get_var $= end p Warning[:deprecated] get_var ``` I think that internally we should _not_ call `Warning.warn` unless the category is enabled. I've sent a PR here: https://github.com/ruby/ruby/pull/10960 -- https://bugs.ruby-lang.org/
Issue #20573 has been updated by byroot (Jean Boussier). Backport changed from 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN to 3.1: WONTFIX, 3.2: REQUIRED, 3.3: REQUIRED Given it's a bug fix I think we should mark it for backport. But of course it's up to branch maintainers to decide whether the fix is worth backporting. ---------------------------------------- Bug #20573: Warning.warn shouldn't be called for disabled warnings https://bugs.ruby-lang.org/issues/20573#change-108806 * Author: tenderlovemaking (Aaron Patterson) * Status: Closed * Backport: 3.1: WONTFIX, 3.2: REQUIRED, 3.3: REQUIRED ---------------------------------------- Currently `Warning.warn` will be called for all warnings, even if that particular category is disabled. For example ```ruby module Warning def warn(message, category:) p message => category end end def get_var $= end p Warning[:deprecated] get_var ``` I think that internally we should _not_ call `Warning.warn` unless the category is enabled. I've sent a PR here: https://github.com/ruby/ruby/pull/10960 -- https://bugs.ruby-lang.org/
Issue #20573 has been updated by Eregon (Benoit Daloze). As a small note on this, it's typically better to check `$VERBOSE` level and if the category is enabled before even generating/formatting the message String, for performance reasons. So if that's always done the check in Warning.warn would be basically redundant. But I agree it makes sense conceptually to check and also in some cases where e.g. the message String is static + frozen and then there is no cost for that message String. ---------------------------------------- Bug #20573: Warning.warn shouldn't be called for disabled warnings https://bugs.ruby-lang.org/issues/20573#change-108930 * Author: tenderlovemaking (Aaron Patterson) * Status: Closed * Backport: 3.1: WONTFIX, 3.2: REQUIRED, 3.3: REQUIRED ---------------------------------------- Currently `Warning.warn` will be called for all warnings, even if that particular category is disabled. For example ```ruby module Warning def warn(message, category:) p message => category end end def get_var $= end p Warning[:deprecated] get_var ``` I think that internally we should _not_ call `Warning.warn` unless the category is enabled. I've sent a PR here: https://github.com/ruby/ruby/pull/10960 -- https://bugs.ruby-lang.org/
Issue #20573 has been updated by k0kubun (Takashi Kokubun). Backport changed from 3.1: WONTFIX, 3.2: REQUIRED, 3.3: REQUIRED to 3.1: WONTFIX, 3.2: REQUIRED, 3.3: DONE ruby_3_3 commit:a3eb5e5c70eaee12964cdd807b8f19950003141f. ---------------------------------------- Bug #20573: Warning.warn shouldn't be called for disabled warnings https://bugs.ruby-lang.org/issues/20573#change-109027 * Author: tenderlovemaking (Aaron Patterson) * Status: Closed * Backport: 3.1: WONTFIX, 3.2: REQUIRED, 3.3: DONE ---------------------------------------- Currently `Warning.warn` will be called for all warnings, even if that particular category is disabled. For example ```ruby module Warning def warn(message, category:) p message => category end end def get_var $= end p Warning[:deprecated] get_var ``` I think that internally we should _not_ call `Warning.warn` unless the category is enabled. I've sent a PR here: https://github.com/ruby/ruby/pull/10960 -- https://bugs.ruby-lang.org/
Issue #20573 has been updated by nagachika (Tomoyuki Chikanaga). Backport changed from 3.1: WONTFIX, 3.2: REQUIRED, 3.3: DONE to 3.1: WONTFIX, 3.2: DONE, 3.3: DONE ruby_3_2 commit:4f1e047f86b159528055d37ee0da2ad6e5a38c23 merged revision(s) commit:a3eb5e5c70eaee12964cdd807b8f19950003141f. ---------------------------------------- Bug #20573: Warning.warn shouldn't be called for disabled warnings https://bugs.ruby-lang.org/issues/20573#change-109173 * Author: tenderlovemaking (Aaron Patterson) * Status: Closed * Backport: 3.1: WONTFIX, 3.2: DONE, 3.3: DONE ---------------------------------------- Currently `Warning.warn` will be called for all warnings, even if that particular category is disabled. For example ```ruby module Warning def warn(message, category:) p message => category end end def get_var $= end p Warning[:deprecated] get_var ``` I think that internally we should _not_ call `Warning.warn` unless the category is enabled. I've sent a PR here: https://github.com/ruby/ruby/pull/10960 -- https://bugs.ruby-lang.org/
participants (5)
-
byroot (Jean Boussier) -
Eregon (Benoit Daloze)
-
k0kubun (Takashi Kokubun)
-
nagachika (Tomoyuki Chikanaga)
-
tenderlovemaking (Aaron Patterson)