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

[enhance] 中置演算子式評価時の演算子優先度評価の実装 #135

Merged
merged 4 commits into from
Aug 12, 2021

Conversation

cwd-k2
Copy link
Contributor

@cwd-k2 cwd-k2 commented Aug 5, 2021

What

中置演算子式 (Infix) の演算子の優先度評価を実装した
優先度を評価し、木を作ることでネストする関数呼び出しに書き換えることで実現

Why

中置演算子式導入時に未実装であった優先度評価を実装した
より直感的に AiScript のコーディングが可能になると考えられる

Additional info (optional)

この PR では優先度や関数への置き換えをインタプリタで行っているが、将来的には意味解析などの段階で行うようにしたい (#124)

@codecov
Copy link

codecov bot commented Aug 5, 2021

Codecov Report

Merging #135 (8bebf1a) into master (3e81959) will increase coverage by 1.17%.
The diff coverage is 99.13%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #135      +/-   ##
==========================================
+ Coverage   70.94%   72.11%   +1.17%     
==========================================
  Files          15       16       +1     
  Lines        2024     2109      +85     
  Branches      301      304       +3     
==========================================
+ Hits         1436     1521      +85     
  Misses        586      586              
  Partials        2        2              
Impacted Files Coverage Δ
src/node.ts 0.00% <0.00%> (ø)
src/interpreter/index.ts 93.00% <100.00%> (-0.36%) ⬇️
src/interpreter/infix-to-fncall.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e81959...8bebf1a. Read the comment docs.

Comment on lines 70 to 72
for (let i = 2; i < nodes.length; i++) {
tree = insert(infos[i - 1], nodes[i], tree);
}
Copy link
Contributor

@marihachi marihachi Aug 8, 2021

Choose a reason for hiding this comment

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

オフセット計算でマイナスするよりは

for (let i = 1; i < infos.length; i++) {
  tree = insert(infos[i], tree, nodes[i + 1]);
}

の方が分かりやすい気がした

@marihachi
Copy link
Contributor

marihachi commented Aug 8, 2021

特に意図がなければ、関数名は動詞から始まるほうが良さそう
→ Tree, operatorInfo

@syuilo
Copy link
Collaborator

syuilo commented Aug 10, 2021

どの演算子がどういう優先度かっていうの暗記するの大変そうだなと思った

@cwd-k2
Copy link
Contributor Author

cwd-k2 commented Aug 10, 2021

優先度は相対的なものだから「AよりもBが優先される」程度のものでしかないよ

@cwd-k2
Copy link
Contributor Author

cwd-k2 commented Aug 10, 2021

優先度の振り方は大まかに
四則演算>比較>論理演算
で四則演算・論理演算はどちらも
積>和
になってるので他の言語と大差ないはず

@syuilo
Copy link
Collaborator

syuilo commented Aug 10, 2021

他の言語でいつも計算は括弧で括ってたから意識したことなかった🥴

@marihachi
Copy link
Contributor

marihachi commented Aug 10, 2021

優先度の違う演算子が式に出てきたら、括弧で明示しないといけないことにするとか (実装は結構ややこしそうだけど...)
意図しない計算結果になることは防げる気がする
でも、結局ユーザーに優先度を意識させることになるな...

@cwd-k2
Copy link
Contributor Author

cwd-k2 commented Aug 10, 2021

単にタイプ数が増えるだけだと思うんだけど
現時点で括る括らないはオプショナルなのでガイドライン的に優先度を示すので十分な気がする

ルールとして括弧を強制するほどのものかなあ
嫌なら明示してねってのが筋が通ってる気がするんだけど

そのルールにするならやるけど

@cwd-k2
Copy link
Contributor Author

cwd-k2 commented Aug 10, 2021

方針を決めるならばアンケートとった方がいいかも

@syuilo
Copy link
Collaborator

syuilo commented Aug 11, 2021

ルールとして括弧を強制するほどのものかなあ

優先度について知らない人が括弧が無い計算式を見たら意図が把握できなくなりそう

まあ私がイレギュラーなだけかもしれないんで無視してもらってもOK

@syuilo syuilo merged commit c77ebdf into aiscript-dev:master Aug 12, 2021
@syuilo
Copy link
Collaborator

syuilo commented Aug 12, 2021

🙏

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

Successfully merging this pull request may close these issues.

3 participants