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

Lexer: Inline else binds to wrong if/switch #2342

Closed
satyr opened this issue May 17, 2012 · 6 comments
Closed

Lexer: Inline else binds to wrong if/switch #2342

satyr opened this issue May 17, 2012 · 6 comments
Labels

Comments

@satyr
Copy link
Collaborator

satyr commented May 17, 2012

(Migrating from satyr/coco#131)

$ coffee -bcs
  if a then if b then c else d
  switch e
    when f then if g then h else i

// Generated by CoffeeScript 1.3.3

if (a) {
  if (b) {
    c;

  }
} else {
  d;

}

switch (e) {
  case f:
    if (g) {
      h;

    }
    break;
  default:
    i;

}

Expected compilation:

if (a) {
  if (b) {
    c;
  } else {
    d;
  }
}

switch (e) {
  case f:
    if (g) {
      h;
    } else {
      i;
    }
}
@nadinengland
Copy link

This is a common problem in generating AST for languages, even JavaScript exhibits this issue. Here are two ways of interpreting the example:

      if (a) if (b) return c; else return d;
Outer ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Inner        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

      if (a) if (b) return c; else return d;
Outer ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Inner        ~~~~~~~~~~~~~~~~

This is normally solved is picking a convention and sticking to it, based on the token's greediness. This gist demonstrates how both CoffeeScript and JavaScript deals with the ambiguity; it appears that CoffeeScript is sticking to JavaScript semantics.

Seeing as though CoffeeScript can only portray meaning (or rather scoping, I don't mean variable scoping however) through whitespace, may I suggest being explicit:

# Ambiguous
if a then if b then c else d
# Explicit: A
if a
  if b then c else d
# Explicit: B
if a
  c if b
else d

@epidemian
Copy link
Contributor

Seeing as though CoffeeScript can only portray meaning (or rather scoping, I don't mean variable scoping however) through whitespace, may I suggest being explicit:

Parentheses can also be used to make it explicit:

# Explicit: A'
if a then (if b then c else d)
# Explicit: B'
if a then (if b then c) else d

That being said, i agree with @satyr: the current behaviour in the ambiguous case looks quite confusing to me; especially the swtich/if/else case.

@nadinengland
Copy link

Entirely forgot about parentheses :< they work perfectly, how foolish of me.

This appears to be fixed in 1.6.2, how come this issue is still open? @Nami-Doc is correct I don't know what lead me to believe it was looking back at my tests.

@vendethiel
Copy link
Collaborator

Doesn't seem to be fixed here

@nadinengland
Copy link

I retract my previous statement that CoffeeScript was sticking to JavaScript semantics, I was wrong. The gist has been updated to properlly illustrate the bug using @epidemian's parentheses suggestion.

The rewriter, when resolving some LARL issues, appears to be replacing THEN tokens with an INDENT that only effects the SINGLE_LINER's block. Having little experience with the CS complier I'm not confident about this but it seems that the ELSE branch is left at the same indentation as the outer IF when this occurs.

@vendethiel
Copy link
Collaborator

see satyr/coco#127

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants