[ruby-core:111135] [Ruby master Bug#19167] Object#inspect does not correctly show NilClass TrueClass and FalseClass stored in instance variables

Issue #19167 has been reported by tompng (tomoya ishida). ---------------------------------------- Bug #19167: Object#inspect does not correctly show NilClass TrueClass and FalseClass stored in instance variables https://bugs.ruby-lang.org/issues/19167 * Author: tompng (tomoya ishida) * Status: Open * Priority: Normal * ruby -v: ruby 3.1.0p0 (2021-12-25 revision fb4df44d16) [x86_64-darwin20] * Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN ---------------------------------------- ~~~ruby Object.new.instance_eval do @a = nil @b = NilClass @c = true @d = TrueClass @e = [nil, NilClass, true, TrueClass] puts self.inspect end # actual # => #<Object:0x2f15e3c8 @a=nil, @b=nil, @c=true, @d=true, @e=[nil, NilClass, true, TrueClass]> # expected # => #<Object:0x2f15e3c8 @a=nil, @b=NilClass, @c=true, @d=TrueClass, @e=[nil, NilClass, true, TrueClass]> ~~~ -- https://bugs.ruby-lang.org/

Issue #19167 has been updated by eightbitraptor (Matthew Valentine-House). File 0001-Fix-Object-inspect-with-NilClass-as-an-ivar.patch added tompng (tomoya ishida) wrote:
~~~ruby Object.new.instance_eval do @a = nil @b = NilClass @c = true @d = TrueClass @e = [nil, NilClass, true, TrueClass] puts self.inspect end # actual # => #<Object:0x2f15e3c8 @a=nil, @b=nil, @c=true, @d=true, @e=[nil, NilClass, true, TrueClass]> # expected # => #<Object:0x2f15e3c8 @a=nil, @b=NilClass, @c=true, @d=TrueClass, @e=[nil, NilClass, true, TrueClass]> ~~~
This is exhibiting the same behaviour on the `master` branch. This looks to be happening because `inspect_i` from `object.c` gets called for each instance variable, and that is using the `PRIsVALUE` macro to print out the values, whereas when we inspect the array in `@e` we use `rb_ary_inspect` which prints each element using `rb_inspect`, and the behaviour of these two differs when being used on `NilClass` and `TrueClass` I agree that this seems like unintended behaviour, and I've attached a possible patch. I haven't yet investigated fully how `PRIsVALUE` works - so my solution may not be the correct one, nor have I investigated whether there is any performance overhead to using `rb_inspect` in the `inspect_i` case. ---------------------------------------- Bug #19167: Object#inspect does not correctly show NilClass TrueClass and FalseClass stored in instance variables https://bugs.ruby-lang.org/issues/19167#change-100512 * Author: tompng (tomoya ishida) * Status: Open * Priority: Normal * ruby -v: ruby 3.1.0p0 (2021-12-25 revision fb4df44d16) [x86_64-darwin20] * Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN ---------------------------------------- ~~~ruby Object.new.instance_eval do @a = nil @b = NilClass @c = true @d = TrueClass @e = [nil, NilClass, true, TrueClass] puts self.inspect end # actual # => #<Object:0x2f15e3c8 @a=nil, @b=nil, @c=true, @d=true, @e=[nil, NilClass, true, TrueClass]> # expected # => #<Object:0x2f15e3c8 @a=nil, @b=NilClass, @c=true, @d=TrueClass, @e=[nil, NilClass, true, TrueClass]> ~~~ ---Files-------------------------------- 0001-Fix-Object-inspect-with-NilClass-as-an-ivar.patch (751 Bytes) -- https://bugs.ruby-lang.org/

Issue #19167 has been updated by alanwu (Alan Wu). Currently it's using a format string with `"%+"PRIsVALUE`, which calls rb_inspect() for most values, but with special-case handling for a NilClass and friends. The special handling was introduced in commit:4191a6b90d3eeb63a31609dba29a1904efee3738: ```diff diff --git a/sprintf.c b/sprintf.c index 4549791d2029e..afe27c2d63ae3 100644 --- a/sprintf.c +++ b/sprintf.c @@ -1338,6 +1338,26 @@ ruby__sfvextra(rb_printf_buffer *fp, size_t valsize, void *valp, long *sz, int s rb_raise(rb_eRuntimeError, "rb_vsprintf reentered"); } if (sign == '+') { + if (RB_TYPE_P(value, T_CLASS)) { +# define LITERAL(str) (*sz = rb_strlen_lit(str), str) + + if (value == rb_cNilClass) { + return LITERAL("nil"); + } + else if (value == rb_cFixnum) { + return LITERAL("Fixnum"); + } + else if (value == rb_cSymbol) { + return LITERAL("Symbol"); + } + else if (value == rb_cTrueClass) { + return LITERAL("true"); + } + else if (value == rb_cFalseClass) { + return LITERAL("false"); + } +# undef LITERAL + } value = rb_inspect(value); ``` The commit is titled "preserve encodings in error messages", and none of the other changes use the `+` modifier. Maybe the special handling was added by accident? ---------------------------------------- Bug #19167: Object#inspect does not correctly show NilClass TrueClass and FalseClass stored in instance variables https://bugs.ruby-lang.org/issues/19167#change-100514 * Author: tompng (tomoya ishida) * Status: Open * Priority: Normal * ruby -v: ruby 3.1.0p0 (2021-12-25 revision fb4df44d16) [x86_64-darwin20] * Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN ---------------------------------------- ~~~ruby Object.new.instance_eval do @a = nil @b = NilClass @c = true @d = TrueClass @e = [nil, NilClass, true, TrueClass] puts self.inspect end # actual # => #<Object:0x2f15e3c8 @a=nil, @b=nil, @c=true, @d=true, @e=[nil, NilClass, true, TrueClass]> # expected # => #<Object:0x2f15e3c8 @a=nil, @b=NilClass, @c=true, @d=TrueClass, @e=[nil, NilClass, true, TrueClass]> ~~~ ---Files-------------------------------- 0001-Fix-Object-inspect-with-NilClass-as-an-ivar.patch (751 Bytes) -- https://bugs.ruby-lang.org/

Issue #19167 has been updated by eightbitraptor (Matthew Valentine-House). It looks like this behaviour was codified in a spec since the commit that @alanwu mentioned was committed. [This commit (a28aa80c739a1d169649a4da833ef48cfb3465b3)](https://github.com/ruby/ruby/commit/a28aa80c739a1d169649a4da833ef48cfb3465b3) introduces the following specs ``` + it "formats a TrueClass VALUE as `TrueClass` if sign not specified in format" do + s = 'Result: TrueClass.' + @s.rb_sprintf3(true.class).should == s + end + + it "formats a TrueClass VALUE as 'true' if sign specified in format" do + s = 'Result: true.' + @s.rb_sprintf4(true.class).should == s + end end ``` This would indicate that the behaviour of "+PRIsVALUE" in `rb_sprintf` is working as intended, and that we shouldn't remove the special casing from `ruby__sfvextra`. ---------------------------------------- Bug #19167: Object#inspect does not correctly show NilClass TrueClass and FalseClass stored in instance variables https://bugs.ruby-lang.org/issues/19167#change-100521 * Author: tompng (tomoya ishida) * Status: Open * Priority: Normal * ruby -v: ruby 3.1.0p0 (2021-12-25 revision fb4df44d16) [x86_64-darwin20] * Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN ---------------------------------------- ~~~ruby Object.new.instance_eval do @a = nil @b = NilClass @c = true @d = TrueClass @e = [nil, NilClass, true, TrueClass] puts self.inspect end # actual # => #<Object:0x2f15e3c8 @a=nil, @b=nil, @c=true, @d=true, @e=[nil, NilClass, true, TrueClass]> # expected # => #<Object:0x2f15e3c8 @a=nil, @b=NilClass, @c=true, @d=TrueClass, @e=[nil, NilClass, true, TrueClass]> ~~~ ---Files-------------------------------- 0001-Fix-Object-inspect-with-NilClass-as-an-ivar.patch (751 Bytes) -- https://bugs.ruby-lang.org/

Issue #19167 has been updated by eightbitraptor (Matthew Valentine-House). I've pushed a fix and a regression test up to [PR #6872](https://github.com/ruby/ruby/pull/6872) ---------------------------------------- Bug #19167: Object#inspect does not correctly show NilClass TrueClass and FalseClass stored in instance variables https://bugs.ruby-lang.org/issues/19167#change-100522 * Author: tompng (tomoya ishida) * Status: Open * Priority: Normal * ruby -v: ruby 3.1.0p0 (2021-12-25 revision fb4df44d16) [x86_64-darwin20] * Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN ---------------------------------------- ~~~ruby Object.new.instance_eval do @a = nil @b = NilClass @c = true @d = TrueClass @e = [nil, NilClass, true, TrueClass] puts self.inspect end # actual # => #<Object:0x2f15e3c8 @a=nil, @b=nil, @c=true, @d=true, @e=[nil, NilClass, true, TrueClass]> # expected # => #<Object:0x2f15e3c8 @a=nil, @b=NilClass, @c=true, @d=TrueClass, @e=[nil, NilClass, true, TrueClass]> ~~~ ---Files-------------------------------- 0001-Fix-Object-inspect-with-NilClass-as-an-ivar.patch (751 Bytes) -- https://bugs.ruby-lang.org/

Issue #19167 has been updated by nobu (Nobuyoshi Nakada). I'll change the flag. ---------------------------------------- Bug #19167: Object#inspect does not correctly show NilClass TrueClass and FalseClass stored in instance variables https://bugs.ruby-lang.org/issues/19167#change-100523 * Author: tompng (tomoya ishida) * Status: Open * Priority: Normal * ruby -v: ruby 3.1.0p0 (2021-12-25 revision fb4df44d16) [x86_64-darwin20] * Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN ---------------------------------------- ~~~ruby Object.new.instance_eval do @a = nil @b = NilClass @c = true @d = TrueClass @e = [nil, NilClass, true, TrueClass] puts self.inspect end # actual # => #<Object:0x2f15e3c8 @a=nil, @b=nil, @c=true, @d=true, @e=[nil, NilClass, true, TrueClass]> # expected # => #<Object:0x2f15e3c8 @a=nil, @b=NilClass, @c=true, @d=TrueClass, @e=[nil, NilClass, true, TrueClass]> ~~~ ---Files-------------------------------- 0001-Fix-Object-inspect-with-NilClass-as-an-ivar.patch (751 Bytes) -- https://bugs.ruby-lang.org/

Issue #19167 has been updated by nobu (Nobuyoshi Nakada). Essentially, this behavior is an "unofficial" feature for internal use only (not expected to be documented), so it has not been described in doc/extension.rdoc.
Note: In the format string, "%"PRIsVALUE can be used for Object#to_s (or Object#inspect if '+' flag is set) output (and related argument must be a VALUE). Since it conflicts with "%i", for integers in format strings, use "%d".
And I inadvertently overlooked this side-effect. As the conclusion, I'll remove this behavior and use another way for the error messages which used this behavior. And I'll mark the code in ruby-spec with `ruby_bug`. ---------------------------------------- Bug #19167: Object#inspect does not correctly show NilClass TrueClass and FalseClass stored in instance variables https://bugs.ruby-lang.org/issues/19167#change-100530 * Author: tompng (tomoya ishida) * Status: Open * Priority: Normal * ruby -v: ruby 3.1.0p0 (2021-12-25 revision fb4df44d16) [x86_64-darwin20] * Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN ---------------------------------------- ~~~ruby Object.new.instance_eval do @a = nil @b = NilClass @c = true @d = TrueClass @e = [nil, NilClass, true, TrueClass] puts self.inspect end # actual # => #<Object:0x2f15e3c8 @a=nil, @b=nil, @c=true, @d=true, @e=[nil, NilClass, true, TrueClass]> # expected # => #<Object:0x2f15e3c8 @a=nil, @b=NilClass, @c=true, @d=TrueClass, @e=[nil, NilClass, true, TrueClass]> ~~~ ---Files-------------------------------- 0001-Fix-Object-inspect-with-NilClass-as-an-ivar.patch (751 Bytes) -- https://bugs.ruby-lang.org/
participants (4)
-
alanwu (Alan Wu)
-
eightbitraptor (Matthew Valentine-House)
-
nobu (Nobuyoshi Nakada)
-
tompng (tomoya ishida)