Issue #18007 has been updated by naruse (Yui NARUSE).


I want Ruby 3.1 to be compatible with 3.0. Therefore even if it is accepted=
, you can merge this in 3.2.

----------------------------------------
Feature #18007: Help developers of C extensions meet requirements in "doc/e=
xtension.rdoc"
https://bugs.ruby-lang.org/issues/18007#change-92907

* Author: mdalessio (Mike Dalessio)
* Status: Open
* Priority: Normal
----------------------------------------
A pull request for this feature has been submitted at https://github.com/ru=
by/ruby/pull/4604

## Problem being solved

This option is intended to help developers of C extensions to check if thei=
r code meets the requirements explained in "doc/extension.rdoc". Specifical=
ly, I want to ensure that `T_DATA` object classes undefine or redefine the =
`allocate` method.

There is currently no easy way for an author of a C extension to easily see=
 where they have made the mistake of letting the default `allocate` class m=
ethod remain.

## Description of the solution

Compiled with this option, Ruby will warn when a `T_DATA` object is created=
 whose class has not undefined or redefined the `allocate` method.

A new function is defined, `rb_data_object_check`. That function is called =
from `rb_data_object_wrap()` and
`rb_data_typed_object_wrap()` (which implement the `Data_Wrap_Struct` famil=
y of macros).

The warning, when emitted, looks like this:

```
warning: T_DATA class Nokogiri::XML::Document should undefine or redefine t=
he allocate method, please see doc/extension.rdoc
```

## Examples of this problem in the wild

Using this option, I found that [many of Nokogiri's classes needed to undef=
ine `allocate`](https://github.com/sparklemotion/nokogiri/commit/c5ba3a5).

This PR also updates these core Ruby classes by undefining `allocate`:

- `ObjectSpace::InternalObjectWrapper`
- `Socket::Ifaddr`

## Questions for reviewers

__Does this check really need to be behind a configuration option?__ Perfor=
mance impact is very small (see benchmarks below), but I put it behind a fl=
ag because I am worried that there may be a many C extensions that would em=
it warnings at runtime, and the users of those extensions cannot fix the pr=
oblem and so would mostly just be annoyed.

__Should this warning be emitted with the `deprecated` category?__

## Benchmarking

I benchmarked this code by allocating `Nokogiri::XML::NodeSet`s in a loop. =
This is a class with a [relatively simple `allocate` function](https://gith=
ub.com/sparklemotion/nokogiri/blob/6d688d8c0f3351797e9576d3710acf458582bb30=
/ext/nokogiri/xml_node_set.c#L441-L464).

The runs cover the four combinations of enabled/disabled, and warnings/no-w=
arnings.

```
ruby 3.1.0dev (2021-06-25T04:02:18Z flavorjones-extens.. de943189aa) [x86_6=
4-linux]
Warming up --------------------------------------
disabled, warn=3Dfalse   490.143k i/100ms
Calculating -------------------------------------
disabled, warn=3Dfalse      4.863M (=B1 1.5%) i/s -     49.014M in  10.0811=
77s

ruby 3.1.0dev (2021-06-25T04:02:18Z flavorjones-extens.. de943189aa) [x86_6=
4-linux]
Warming up --------------------------------------
 disabled, warn=3Dtrue   483.070k i/100ms
Calculating -------------------------------------
 disabled, warn=3Dtrue      4.839M (=B1 1.4%) i/s -     48.790M in  10.0838=
99s

Comparison:
disabled, warn=3Dfalse:  4863064.0 i/s
 disabled, warn=3Dtrue:  4839310.1 i/s - same-ish: difference falls within =
error


ruby 3.1.0dev (2021-06-25T04:02:18Z flavorjones-extens.. de943189aa) [x86_6=
4-linux]
Warming up --------------------------------------
 enabled, warn=3Dfalse   484.398k i/100ms
Calculating -------------------------------------
 enabled, warn=3Dfalse      4.840M (=B1 1.9%) i/s -     48.440M in  10.0118=
54s

Comparison:
disabled, warn=3Dfalse:  4863064.0 i/s
 enabled, warn=3Dfalse:  4840123.2 i/s - same-ish: difference falls within =
error
 disabled, warn=3Dtrue:  4839310.1 i/s - same-ish: difference falls within =
error


ruby 3.1.0dev (2021-06-25T04:02:18Z flavorjones-extens.. de943189aa) [x86_6=
4-linux]
Warming up --------------------------------------
  enabled, warn=3Dtrue   492.200k i/100ms
Calculating -------------------------------------
  enabled, warn=3Dtrue      4.866M (=B1 2.1%) i/s -     48.728M in  10.0174=
55s

Comparison:
  enabled, warn=3Dtrue:  4866434.8 i/s
disabled, warn=3Dfalse:  4863064.0 i/s - same-ish: difference falls within =
error
 enabled, warn=3Dfalse:  4840123.2 i/s - same-ish: difference falls within =
error
 disabled, warn=3Dtrue:  4839310.1 i/s - same-ish: difference falls within =
error
```

My conclusion is that the performance impact is very small, and we could om=
it the option if the Ruby core maintainers decide this behavior should be o=
n by default.



-- =

https://bugs.ruby-lang.org/

Unsubscribe: <mailto:ruby-core-request / ruby-lang.org?subject=3Dunsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>