Project

General

Profile

Bug #7522

Non-core "Type()" Kernel methods return new objects

Added by jballanc (Joshua Ballanco) almost 7 years ago. Updated 23 days ago.

Status:
Closed
Priority:
Normal
Target version:
-
ruby -v:
2.0.0-preview1
[ruby-core:50596]

Description

The methods Array(), String(), Float(), Integer(), Hash(), and Rational() all return their argument when the argument is already an instance of the type in question. For example:

a = []
a.equal? Array(a) #=> true

However, the similar methods Pathname(), BigDecimal(), and Complex() do not do this:

p = Pathname.new('/tmp')
p.equal? Pathname(p) #=> false

I had the impression that the "Type()" methods were intended as "safe" coercion methods. That is, if no type conversion is required, then the system is left unchanged (and no new objects are created). The attached patch fixes the three methods mentioned above to adhere to this same invariant.


Files

kernel_methods.diff (2.97 KB) kernel_methods.diff jballanc (Joshua Ballanco), 12/06/2012 06:17 AM
kernel_methods.diff (3.05 KB) kernel_methods.diff jballanc (Joshua Ballanco), 12/10/2012 04:24 AM
kernel-pathname-bigdecimal-complex-return-arg-7522.patch (3.78 KB) kernel-pathname-bigdecimal-complex-return-arg-7522.patch jeremyevans0 (Jeremy Evans), 08/07/2019 04:11 PM

Associated revisions

Revision 2e551356
Added by jeremyevans (Jeremy Evans) 23 days ago

Make Kernel#{Pathname,BigDecimal,Complex} return argument if given correct type

This is how Kernel#{Array,String,Float,Integer,Hash,Rational} work.
BigDecimal and Complex instances are always frozen, so this should
not cause backwards compatibility issues for those. Pathname
instances are not frozen, so potentially this could cause backwards
compatibility issues by not returning a new object.

Based on a patch from Joshua Ballanco, some minor changes by me.

Fixes [Bug #7522]

History

Updated by charliesome (Charlie Somerville) almost 7 years ago

=begin
Your change to ext/bigdecimal/bigdecimal.c will cause a compiler warning:

compiling bigdecimal.c
bigdecimal.c: In function ‘BigDecimal_global_new’:
bigdecimal.c:2414: warning: ISO C90 forbids mixed declarations and code

You should move the declaration of (({pv})) above the your if statement, but leave the assignment where it is.
=end

Updated by jballanc (Joshua Ballanco) almost 7 years ago

Ah, thanks for that catch!

Updated patch is attached.

#3

Updated by ko1 (Koichi Sasada) over 6 years ago

  • Category set to core
  • Assignee set to mame (Yusuke Endoh)
  • Target version set to 2.0.0

I'm not sure who should take this issue.

Updated by mame (Yusuke Endoh) over 6 years ago

  • Status changed from Open to Assigned
  • Assignee changed from mame (Yusuke Endoh) to matz (Yukihiro Matsumoto)
  • Target version changed from 2.0.0 to 2.6

Sorry but let me postpone this to the next minor.
I'm afraid if the existing code depends on the traditional behavior.
It should have been included in rc1, which is my fault. Sorry, blame me.

--
Yusuke Endoh mame@tsg.ne.jp

Updated by jballanc (Joshua Ballanco) over 6 years ago

No apology necessary. Thanks for the help!

#6

Updated by naruse (Yui NARUSE) almost 2 years ago

  • Target version deleted (2.6)

Updated by jeremyevans0 (Jeremy Evans) 2 months ago

This issue still exists in the master branch, and while it isn't a bug, I think we should make the change. This change should cause no issues for BigDecimal and Complex, since those instances are already frozen. It could potentially cause issues for Pathname, since Pathname instances are not frozen by default, but considering Kernel#{Hash,Array,String} already return the receiver, it makes sense for Kernel#Pathname to do so as well.

Attached is an updated patch that applies against the master branch.

#8

Updated by nobu (Nobuyoshi Nakada) 2 months ago

  • Has duplicate Feature #16095: 2 Features: remove (simplify) 'new' keyword and Property Shorthand added
#9

Updated by nobu (Nobuyoshi Nakada) 2 months ago

  • Has duplicate deleted (Feature #16095: 2 Features: remove (simplify) 'new' keyword and Property Shorthand)

Updated by matz (Yukihiro Matsumoto) 25 days ago

Accepted.

Matz.

#11

Updated by jeremyevans (Jeremy Evans) 23 days ago

  • Status changed from Assigned to Closed

Applied in changeset git|2e551356a7a6e74ba07283e000ff16f5d1ea6506.


Make Kernel#{Pathname,BigDecimal,Complex} return argument if given correct type

This is how Kernel#{Array,String,Float,Integer,Hash,Rational} work.
BigDecimal and Complex instances are always frozen, so this should
not cause backwards compatibility issues for those. Pathname
instances are not frozen, so potentially this could cause backwards
compatibility issues by not returning a new object.

Based on a patch from Joshua Ballanco, some minor changes by me.

Fixes [Bug #7522]

Also available in: Atom PDF