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

`String.toLower` - permanent breakage #246

Closed
srid opened this Issue May 15, 2015 · 4 comments

Comments

Projects
None yet
4 participants
@srid

srid commented May 15, 2015

I believe I found an odd bug in String.toLower:

> import String
> String.toLower
<function: toLower> : String -> String
> String.toLower "Hello"
"hello" : String
> a = "Hello"
"Hello" : String
> String.toLower a
"hello" : String
> a = String.toLower a
TypeError: Cannot read property 'toLowerCase' of undefined
> String.toLower "Broken for ever"
TypeError: Cannot read property 'toLowerCase' of undefined
>
@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz May 15, 2015

Member

I believe the real problem is that a = String.toLower a defines a self-recursive value. The a being defined talks about itself, not the previous definition. There is a known bug where elm-repl will not recover well from certain kinds of errors, so I suspect you can get this behavior with many functions. Can you look on the elm-repl repo for relevant issues to confirm?

Member

evancz commented May 15, 2015

I believe the real problem is that a = String.toLower a defines a self-recursive value. The a being defined talks about itself, not the previous definition. There is a known bug where elm-repl will not recover well from certain kinds of errors, so I suspect you can get this behavior with many functions. Can you look on the elm-repl repo for relevant issues to confirm?

@srid

This comment has been minimized.

Show comment
Hide comment
@srid

srid May 15, 2015

I first caught this during runtime of my app, and used elm-repl here only to create a simpler repro. Curiously I cannot reproduce this using a simple example below and elm-make:

I tried this simple example:

import Graphics.Element exposing (..)
import String

main : Element
main =
  let a = "Hello, world"
      a = String.toLower a
  in show a

elm-make reported: Name Collision: There can only be one definition of 'a'..

However, in my relatively complex program elm-make succeeded, while the browser threw the same error as shown in this screenshot:
screen shot 2015-05-15 at 1 57 56 pm

You can find the specific change which (still) triggers this bug here: srid/chronicle@611cbad

My workaround, as can be seen from this commit, is to create a new variable instead of assigning back. If the form let a = f a is not recommended, shouldn't the compiler fail under all cases?

srid commented May 15, 2015

I first caught this during runtime of my app, and used elm-repl here only to create a simpler repro. Curiously I cannot reproduce this using a simple example below and elm-make:

I tried this simple example:

import Graphics.Element exposing (..)
import String

main : Element
main =
  let a = "Hello, world"
      a = String.toLower a
  in show a

elm-make reported: Name Collision: There can only be one definition of 'a'..

However, in my relatively complex program elm-make succeeded, while the browser threw the same error as shown in this screenshot:
screen shot 2015-05-15 at 1 57 56 pm

You can find the specific change which (still) triggers this bug here: srid/chronicle@611cbad

My workaround, as can be seen from this commit, is to create a new variable instead of assigning back. If the form let a = f a is not recommended, shouldn't the compiler fail under all cases?

@justinmanley

This comment has been minimized.

Show comment
Hide comment
@justinmanley

justinmanley Jun 3, 2015

@srid -

I believe that the reason that elm-make fails in the above example is because multiple definitions of the same name in the same scope are not allowed. The two definitions for a below are in the same let-expression, and thus in the same scope. That's why this is rejected by the compiler.

let a = "Hello, world"
    a = String.toLower a
in show a

When you have

f a = 
    let a = String.toLower a
    in a

the let-binding introduces a new scope in which a new binding for a is introduced which shadows the previous binding. The reason that this version throws a compiler error is because all let-bindings are recursive in Elm (that is, there is no special letrec construct), so names are actually brought into scope in the body of the expression that they refer to. This means that the a on the right side of the let-expression actually refers to the a on the left side of the let-expression, not to the variable bound by the definition of f.

This may not be what you are used to if you are coming from a language where a special keyword is required to create a recursive definition.

justinmanley commented Jun 3, 2015

@srid -

I believe that the reason that elm-make fails in the above example is because multiple definitions of the same name in the same scope are not allowed. The two definitions for a below are in the same let-expression, and thus in the same scope. That's why this is rejected by the compiler.

let a = "Hello, world"
    a = String.toLower a
in show a

When you have

f a = 
    let a = String.toLower a
    in a

the let-binding introduces a new scope in which a new binding for a is introduced which shadows the previous binding. The reason that this version throws a compiler error is because all let-bindings are recursive in Elm (that is, there is no special letrec construct), so names are actually brought into scope in the body of the expression that they refer to. This means that the a on the right side of the let-expression actually refers to the a on the left side of the let-expression, not to the variable bound by the definition of f.

This may not be what you are used to if you are coming from a language where a special keyword is required to create a recursive definition.

@jvoigtlaender

This comment has been minimized.

Show comment
Hide comment
@jvoigtlaender

jvoigtlaender Jan 19, 2016

Contributor

I'm closing this. It does not appear to be an issue of core, but rather an instance of elm/compiler#873.

Contributor

jvoigtlaender commented Jan 19, 2016

I'm closing this. It does not appear to be an issue of core, but rather an instance of elm/compiler#873.

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