Skip to content
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

Existence checks are missing in switch/pattern matching when using `that` #743

Closed
ruby-random opened this issue Jun 28, 2015 · 1 comment
Closed
Labels
bug

Comments

@ruby-random
Copy link

@ruby-random ruby-random commented Jun 28, 2015

This code:

-> | foo? => 42

Produces this compiled output:

// Generated by LiveScript 1.4.0
(function(){
  switch (false) {
  case typeof foo == 'undefined' || foo === null:
    return 42;
  }
});

However, if you mention that in the return clause:

-> | foo? => that

It will compile to this:

// Generated by LiveScript 1.4.0
(function(){
  var that;
  switch (false) {
  case (that = foo) == null:
    return that;
  }
});

Note missing checks for foo existence, so this code will die with ReferenceError when foo is not defined, and it shouldn't happen.

@vendethiel vendethiel added the bug label Jun 28, 2015
@misterfish misterfish mentioned this issue Jun 4, 2016
@misterfish
Copy link
Contributor

@misterfish misterfish commented Jun 4, 2016

Indeed this can be seen even more easily like this:

if a?
    console.log that # => ReferenceError

PR #895 might fix this.


if a?
    console.log that
var that;
if (typeof a != 'undefined' && a !== null && (that = a, true)) {
  console.log(that);
}

if not a?
    console.log that
var that;
if ((that = undefined) || typeof a == 'undefined' || a === null) {
  console.log(that);
}

a = 10
if a?
    console.log that
var a;
a = 10;
if ((that = a) != null) {
  console.log(that);
}

a = 10
if not a?
    console.log that
var a;
a = 10;
if ((that = a) == null) {
  console.log(that);
}

The switch and while variants should work as well, and forms such as

if a? and not b?
    that

work the same as before I believe.

Any other cases to think about? :)

@rhendric rhendric closed this May 17, 2017
This was referenced Jan 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants