Bug #19167: Object#inspect does not correctly show NilClass TrueClass and FalseClass stored in instance variables

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: ruby 3.1.0p0 (2021-12-25 revision fb4df44d16) [x86_64-darwin20]
• Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN

[tompng (tomoya ishida)]

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]>

[eightbitraptor (Matthew Valentine-House)]

• File 0001-Fix-Object-inspect-with-NilClass-as-an-ivar.patch 0001-Fix-Object-inspect-with-NilClass-as-an-ivar.patch added

tompng (tomoya ishida) wrote:

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.

[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 4191a6b90d3eeb63a31609dba29a1904efee3738:

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?

[eightbitraptor (Matthew Valentine-House)]

It looks like this behaviour was codified in a spec since the commit that @alanwu (Alan Wu) mentioned was committed.

This 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.

[eightbitraptor (Matthew Valentine-House)]

I've pushed a fix and a regression test up to PR #6872

[nobu (Nobuyoshi Nakada)]

I'll change the flag.

[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.

[nobu (Nobuyoshi Nakada)]

• Status changed from Open to Closed

Applied in changeset git|11acb7f7bcf6e80e03cf83bba863b9b3f980fdca.

---

[Bug #19167] Remove useless conversion of classes for special const