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

Variableに型とattributeの情報を付与 #370

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

FineArchs
Copy link
Member

  • type.tsを整理
  • 使用されていなかった型の情報をVariableに付与
  • Valueに付いていたattributeの情報をVariableに付け替え

@codecov
Copy link

codecov bot commented Sep 21, 2023

Codecov Report

Patch coverage: 63.01% and project coverage change: +0.26% 🎉

Comparison is base (d4071dd) 88.47% compared to head (bb4ae63) 88.73%.
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #370      +/-   ##
==========================================
+ Coverage   88.47%   88.73%   +0.26%     
==========================================
  Files          21       21              
  Lines        3452     3461       +9     
  Branches      572      573       +1     
==========================================
+ Hits         3054     3071      +17     
+ Misses        386      378       -8     
  Partials       12       12              
Files Changed Coverage Δ
src/type.ts 56.05% <47.91%> (+4.47%) ⬆️
src/interpreter/index.ts 93.39% <87.50%> (+0.21%) ⬆️
src/interpreter/variable.ts 86.48% <92.85%> (+1.87%) ⬆️
src/index.ts 100.00% <100.00%> (ø)
src/interpreter/value.ts 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@marihachi
Copy link
Contributor

isCompatibleType()がちょっと変更しづらそうです
短くはなっていますが...

@FineArchs
Copy link
Member Author

今の&&を繋げる形式だと、例えば後で例外を投げる処理を入れたくなった時とかに拡張しづらいみたいな感じですか?

@marihachi
Copy link
Contributor

そのあたりはより複雑な処理に変わる可能性が高いので、文を入れ込めるようにしたほうが良さそうです

@FineArchs
Copy link
Member Author

修正しました。問題ないでしょうか?

@FineArchs
Copy link
Member Author

そういえばなんですが、isCompatibleType()は元々どのような目的で作られたものなんでしょうか?
「型bの値は型aに代入可能である」ことをチェックするものなら、
https://github.com/syuilo/aiscript/blob/242731801efc3d9129accc20dfeb1fd5614a62d3/src/type.ts#L56
ここは修正する必要があります。

@marihachi
Copy link
Contributor

元々はインタプリタの実行時エラーだけあって、Type系は静的に型チェックも任意でできるようにしようっていう試みで追加しました。
現状は明示的に型を付けた場合だけ、型チェックが走ったはずです。それ以外の場合はデフォルトでany型として処理されます。

@marihachi
Copy link
Contributor

型は全部付ける、推論もできるようにする、っていう方向で変えていくのであればanyを除去することもできるかもしれません。大変かも...

@FineArchs
Copy link
Member Author

anyを除去するつもりはないですが、型推論に使用するのであれば
let a:any = <num型>は許容すべきですが、
let a:num= <any型>は禁止すべきであるため、
isCompatible()がそのチェックの関数であれば修正すべきです。
ですが、そうでないなら本来の用途を知りたいです。

@marihachi
Copy link
Contributor

marihachi commented Sep 22, 2023

代入ができるかどうかの判定、比較の判定で使うつもりで書きました
これまだどこからも使ってないですね...

@marihachi
Copy link
Contributor

型チェックは機能していないので、ある程度しっかりしたものにするには仕様検討から進めることになると思います。

@FineArchs
Copy link
Member Author

代入可能判定で使えるように
https://github.com/syuilo/aiscript/blob/242731801efc3d9129accc20dfeb1fd5614a62d3/src/type.ts#L56

https://github.com/syuilo/aiscript/blob/bb4ae63c7d34e87a6da300ca12904fe1dd2b0ea8/src/type.ts#L60-L61
に修正しました。

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.

2 participants