[ruby-core:118279] [Ruby master Bug#20573] Warning.warn shouldn't be called for disabled warnings

[ruby-core:118279] [Ruby master Bug#20573] Warning.warn shouldn't be called for disabled warnings

[tenderlovemaking (Aaron Patterson) via ruby-core]

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/

[ruby-core:118280] [Ruby master Bug#20573] Warning.warn shouldn't be called for disabled warnings

[tenderlovemaking (Aaron Patterson) via ruby-core]

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/

[ruby-core:118285] [Ruby master Bug#20573] Warning.warn shouldn't be called for disabled warnings

[byroot (Jean Boussier) via ruby-core]

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/

[ruby-core:118286] [Ruby master Bug#20573] Warning.warn shouldn't be called for disabled warnings

[byroot (Jean Boussier) via ruby-core]

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/

[ruby-core:118287] [Ruby master Bug#20573] Warning.warn shouldn't be called for disabled warnings

[byroot (Jean Boussier) via ruby-core]

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/

[ruby-core:118291] [Ruby master Bug#20573] Warning.warn shouldn't be called for disabled warnings

[tenderlovemaking (Aaron Patterson) via ruby-core]

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/

[ruby-core:118300] [Ruby master Bug#20573] Warning.warn shouldn't be called for disabled warnings

[byroot (Jean Boussier) via ruby-core]

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/

[ruby-core:118414] [Ruby master Bug#20573] Warning.warn shouldn't be called for disabled warnings

[Eregon (Benoit Daloze) via ruby-core]

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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:118505] [Ruby master Bug#20573] Warning.warn shouldn't be called for disabled warnings

[k0kubun (Takashi Kokubun) via ruby-core]

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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:118645] [Ruby master Bug#20573] Warning.warn shouldn't be called for disabled warnings

[nagachika (Tomoyuki Chikanaga) via ruby-core]

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/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/