<html>
    <head>
      <base href="https://bugs.webkit.org/" />
    </head>
    <body>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - [INTL] Implement Collator Compare Functions"
   href="https://bugs.webkit.org/show_bug.cgi?id=147604#c31">Comment # 31</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - [INTL] Implement Collator Compare Functions"
   href="https://bugs.webkit.org/show_bug.cgi?id=147604">bug 147604</a>
              from <span class="vcard"><a class="email" href="mailto:sukolsak&#64;gmail.com" title="Sukolsak Sakshuwong &lt;sukolsak&#64;gmail.com&gt;"> <span class="fn">Sukolsak Sakshuwong</span></a>
</span></b>
        <pre>Oops, sorry. I accidentally clicked Save Changes. I will fix the linking issue on Windows.

(In reply to <a href="show_bug.cgi?id=147604#c29">comment #29</a>)
<span class="quote">&gt; Seems a little silly to do this as two steps. It could just be:
&gt; 
&gt;     else if (sensitivityString.isNull() || sensitivityString == &quot;variant&quot;)
&gt;         m_sensitivity = Sensitivity::Variant;
&gt; 
&gt; Or even skip the ASSERT_NOT_REACHED thing and do this:
&gt; 
&gt;     else
&gt;         m_sensitivity = Sensitivity::Variant;</span >

Will do.

<span class="quote">&gt; &gt; Source/JavaScriptCore/runtime/IntlCollator.cpp:325
&gt; &gt; +void IntlCollator::createCollator(ExecState&amp; state)
&gt; &gt; +{
&gt; 
&gt; I think we could use an ASSERT(!m_collator) at the start of this function to
&gt; help make it clear locally in this function that we won’t leak m_collator by
&gt; overwriting it without closing it.</span >

Will do.

<span class="quote">&gt; &gt; Source/JavaScriptCore/runtime/IntlCollatorPrototype.cpp:93
&gt; &gt; +    // 1. Let collator be the this value.
&gt; &gt; +    IntlCollator* collator = jsDynamicCast&lt;IntlCollator*&gt;(state-&gt;thisValue());
&gt; &gt; +
&gt; &gt; +    // 2. Assert: Type(collator) is Object and collator has an [[initializedCollator]] internal slot whose value is true.
&gt; &gt; +    ASSERT(collator);
&gt; 
&gt; I’m really surprised that “Assert” in the specification really means what
&gt; ASSERT means in our code. I would have expected that an assertion in the
&gt; specification is more about conditions that would lead to an exception as
&gt; opposed to conditions that can’t possible be true because we are guaranteed
&gt; the caller won’t violate certain invariants.
&gt; 
&gt; Is it really true that no one can call this function with a thisValue that
&gt; is not an IntlCollator*? If so, then why do a jsDynamicCast rather than a
&gt; static_cast?</span >

It's true. Here's the definition of Intl.Collator.prototype.compare:

    &quot;the phrase 'this Collator object' refers to the object that is the this value for the invocation of the function; a TypeError exception is thrown if the this value is not an object or an object that does not have an [[initializedCollator]] internal slot with value true.&quot;

    &quot;1. Let collator be this Collator object.
    2. If collator.[[boundCompare]] is undefined, then
        a. Let F be a new built-in function object as defined in 11.3.4.
        b. The value of F’s length property is 2.
        c. Let bc be BoundFunctionCreate(F, «this value»).
        d. Set collator.[[boundCompare]] to bc.
    3. Return collator.[[boundCompare]].&quot;

&quot;F&quot; in Step 2 is our IntlCollatorFuncCompare function. Since it's bound to a value that we have already checked that it is a Collator, the spec can assert that.

We do have a test case &quot;new Intl.Collator().compare.call(5, 'a', 'b')&quot;. As expected, it does not throw.

I will change jsDynamicCast to jsCast.</pre>
        </div>
      </p>
      <hr>
      <span>You are receiving this mail because:</span>
      
      <ul>
          <li>You are the assignee for the bug.</li>
      </ul>
    </body>
</html>