Skip to content
This repository has been archived by the owner on Sep 20, 2021. It is now read-only.

Add ^ operator for power #26

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion Arithmetic.pp
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
%token minus \-|−
%token times \*|×
%token div /|÷
%token pow \^
%token constant [A-Z_]+[A-Z0-9_]+
%token id \w+

Expand All @@ -65,7 +66,10 @@
ternary() ( ::times:: #multiplication expression() )?

ternary:
term() ( ::div:: #division expression() )?
quaternary() ( ::div:: #division expression() )?

quaternary:
term() ( ::pow:: #power expression() )?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure that ^ has not the same priority (precedence) than an other existing operator?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to many sources found on Google (!!), ^ priority is lower than multiplication and division operators.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since PHP5.6, we have the ** operator with the following precedence: http://php.net/manual/en/language.operators.precedence.php. It is a right associative operator. Is it the case here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As said here,

  • the common ^ operator is left associative (according to my mind and my calculator (2^3^2=64))
  • the PHP ** operator is right associative (2 ** 3 ** 2 == 512).

What about Hoa\Math ^ operator ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's right associative here (2^3^2 == 512) like the ** operator.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, it seem my mind and my college calculator were wrong ;)
https://en.wikipedia.org/wiki/Order_of_operations#Special_cases

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, do we have the same associativity than in PHP? If yes, why not using **? We will be able to fix test suites directly too.


term:
( ::bracket_:: expression() ::_bracket:: #group )
Expand Down
6 changes: 5 additions & 1 deletion Test/Unit/Arithmetic.pp
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
%token minus \-
%token times \*
%token div /
%token pow \^
%token constant [A-Z_]+[A-Z0-9_]+
%token id \w+

Expand All @@ -64,7 +65,10 @@
ternary() ( ::times:: #multiplication expression() )?

ternary:
term() ( ::div:: #division expression() )?
quaternary() ( ::div:: #division expression() )?

quaternary:
term() ( ::pow:: #power expression() )?

term:
( ::bracket_:: expression() ::_bracket:: #group )
Expand Down
10 changes: 10 additions & 0 deletions Visitor/Arithmetic.php
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,16 @@ public function visit(

break;

case '#power':
$children[0]->accept($this, $a, $eldnah);

$acc = function ($b) use ($a, $acc) {
return $acc(pow($a(), $b));
};

$children[1]->accept($this, $acc, $eldnah);
break;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CS: An empty new line is missing before the break statement.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain this part please?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Short answer, no. Longer, I have basically copy/pasted what is done for #multiplication and updated the operation ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Are tests green?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Funny: 147a38d.


case '#fakegroup':
case '#group':
$children[0]->accept($this, $a, $eldnah);
Expand Down