New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix optimized `compare` when first arg is Nil #975

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
1 participant
@brian-carroll

brian-carroll commented Aug 25, 2018

Fix a "truthiness" bug in the PROD version of _Utils_cmp

This line wants to check if the first argument is a Tuple. In PROD mode it uses if (!x.$) to check for falsiness, since tuples have no $ property in PROD mode.

Unfortunately _List_Nil can also be passed in here, which is represented as { $: 0 }. When that happens, x.$ exists, but since it is falsy it passes the condition and is misidentified as a tuple! The tuple-related code then runs even though the inputs are lists. This can produce incorrect results, such as in the SSCCE below.

This PR fixes it by using hasOwnProperty instead of a truthiness check.

(There are, of course, lots of ways to check for missing properties. Not sure what your preference is. I figured that checking the existence of a property should be more efficient than doing anything with its value. Dunno if that's right or not.)

module Main exposing (main)

import Browser
import Html exposing (Html, text)

main = Browser.sandbox
    { init = ()
    , view = view
    , update = \() () -> ()
    }

view : () -> Html msg
view () =
    if [] < [0] then
        text "Default compile shows this"
    else
        text "--optimize shows this"

@brian-carroll brian-carroll changed the title from Prevent _List_Nil from being misidentified as a Tuple in _Utils_cmp to Fix optimized `compare` when first arg is Nil Aug 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment