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

inconsistent implicit multiplication #463

Closed
julinas opened this Issue Sep 10, 2015 · 3 comments

Comments

Projects
None yet
3 participants
@julinas

julinas commented Sep 10, 2015

This is on the released version... 2 * 3 3 3 is interpreted as syntax error, but 2 * 3 3 is parsed as 2 * 3 * 3. 2 + 2 3 also gives a syntax error instead of 2 + 2 * 3 as expected. Maybe it's no longer a problem in develop?

@FSMaxB

This comment has been minimized.

Collaborator

FSMaxB commented Sep 12, 2015

I didn't even know something like 2 * 3 3 was even possible. But looking at the source code of the parser I found this:

// parse implicit multiplication
if ((token_type == TOKENTYPE.SYMBOL) ||
(token == 'in' && (node && node.isConstantNode)) ||
(token_type == TOKENTYPE.NUMBER && !(node && node.isConstantNode)) ||
(token == '(' || token == '[')) {
// symbol: implicit multiplication like '2a', '(2+3)a', 'a b'
// number: implicit multiplication like '(2+3)2'
// Note: we don't allow implicit multiplication between numbers,
// like '2 3'. I'm not sure whether that is a good idea.
// parenthesis: implicit multiplication like '2(3+4)', '(3+4)(1+2)', '2[1,2,3]'
node = new OperatorNode('*', 'multiply', [node, parseMultiplyDivide()]);
}

    // parse implicit multiplication
    if ((token_type == TOKENTYPE.SYMBOL) ||
        (token == 'in' && (node && node.isConstantNode)) ||
        (token_type == TOKENTYPE.NUMBER && !(node && node.isConstantNode)) ||
        (token == '(' || token == '[')) {
      // symbol:      implicit multiplication like '2a', '(2+3)a', 'a b'
      // number:      implicit multiplication like '(2+3)2'
      //              Note: we don't allow implicit multiplication between numbers,
      //              like '2 3'. I'm not sure whether that is a good idea.
      // parenthesis: implicit multiplication like '2(3+4)', '(3+4)(1+2)', '2[1,2,3]'
      node = new OperatorNode('*', 'multiply', [node, parseMultiplyDivide()]);
    }

The comments suggest that implicit multiplication between numbers wasn't intended at all.

In my understanding this means one of two things:

  1. It is a bug that 2 * 3 3 works, according to the comments it shouldn't.
  2. It is a bug that 2 * 3 3 3 doesn't work and that 2 3 doesn't work.

But I don't quite understand the parser at this point.

@josdejong: What is your take on this.

@FSMaxB FSMaxB changed the title from 2 * 3 3 3 interpreted as syntax error to inconsistent implicit multiplication Sep 12, 2015

@FSMaxB FSMaxB added the bug label Sep 12, 2015

@josdejong

This comment has been minimized.

Owner

josdejong commented Sep 12, 2015

Thanks for bringing this up.

The comments suggest that implicit multiplication between numbers wasn't intended at all.

That's correct. Implicit multiplication between two numbers should not be supported, so "2 * 3 3" should throw an error instead of evaluating.

Reason for not allowing implicit multiplication between values was that I thought it could easily lead to misreading expressions, you can easily confuse it with a list of values (how do you think [1 2 3] would evaluate? as [1, 2, 3] or as [1 * 2 * 3]?).

@josdejong

This comment has been minimized.

Owner

josdejong commented Sep 13, 2015

This inconsistent behavior is fixed now in the develop branch.

@julinas julinas closed this Sep 17, 2015

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