[webkit-reviews] review denied: [Bug 103374] Remove AtomicString(AtomicStringImpl*) constructor. : [Attachment 176193] Remove a constructor no one uses.

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied  review:
Bug 103374: Remove AtomicString(AtomicStringImpl*) constructor.
https://bugs.webkit.org/show_bug.cgi?id=103374

Attachment 176193: Remove a constructor no one uses.
https://bugs.webkit.org/attachment.cgi?id=176193&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=176193&action=review


> Source/WTF/ChangeLog:9
> +	   No one uses this constructor. It seems harmful to have a such an
> +	   implicit conversion constructor.

How did you determine that no one uses this? If you simply removed it and
recompiled, then that did not prove it was unused. Since AtomicStringImpl is
derived from StringImpl, callers will start using AtomicString(StringImpl*)
constructor instead. That will cause the code to do the same thing, but slower
by doing a hash table lookup.

This constructor is an optimization, and you are removing it. Which is only OK
if the optimization is not effective.