Skip to content

Inherit parent generic context#120

Merged
omochi merged 10 commits intomainfrom
parent_generics
Apr 23, 2024
Merged

Inherit parent generic context#120
omochi merged 10 commits intomainfrom
parent_generics

Conversation

@sidepelican
Copy link
Collaborator

TypeConverterがジェネリック型パラメータを調べるときに、親コンテキストの型をたどって再帰的に全てのジェネリックパラメータを集めるようにします。


private func genericParams(stype: any SType) throws -> [any TypeConverter] {
let parentParams = if let parent = stype.asNominal?.parent ?? stype.asTypeAlias?.parent,
parent.typeDecl !== stype.typeDecl {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

根っこのSTypeはparentとして自身を持っているみたいで、ループしてしまうのでこのようなガードが入っています。
根っこのSTypeはparentを持たないほうが自然に思いましたが、少しコードを追ったくらいではよくわからなかったので一旦こうしています。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

やり方を変えたら不要にできました

@sidepelican sidepelican requested a review from omochi April 19, 2024 08:12
@omochi
Copy link
Owner

omochi commented Apr 19, 2024

ありがとうございます。
テストケースにおいて、
期待したようなトランスパイルがされていそうなのですが、
C2TSの機能セットとしてはさらに踏み込んだ動作も必要です。

以下に説明します。


既存の実装において、以下のように特殊化された型(S<Int>)をメンバに持つ型(K)がある時

struct S<T> {
    var a: T
}

struct K {
    var k: S<Int>
}

Kのデコードにおいては、
特殊化を考慮してジェネリックなデコーダを使ったコンクリートなデコーダを生成します。

k = S_decode(json, identity)

↑うろ覚えですが、ジェネリックなデコーダに対して、
Int をデコードするための identity 関数がセットされています。

もちろん enum E { case a } があって S<E> だったら E_decode が渡されます。


この特殊化の挙動が、
今回のネストした型に対しても正しく動く必要があります。
変換後の型のジェネリックパラメータは、
変換元の親の型のジェネリックパラメータの分が増えているので、
特殊化の処理における埋めるパラメータのマッチングも同様にオフセットする必要があります。

例えば以下のような入力に対して

struct S<T> {
    struct G {
         var b: T
    }
}

struct K {
    var k: S<Int>.G
}

kのデコードにおいては、

k = S_G_decode(json, identity)

となる必要があります。

また、さらに難しいケースで、内側の型もパラメータを持っている場合は

struct S<T> {
    struct G<U> {
         var b: T
         var d: U
    }
}

enum E { case o }
case V { case o }

struct K {
    var k: S<E>.G<V>
}

のような入力に対して

k = S_G_decode(json, E_decode, V_decode)

と、なる必要があります。

また、この S_G のパラメータを解決する際に、
それがジェネリックコンテキストからくる場合もあります。

struct S<T> {
    struct G<U> {
         var b: T
         var d: U
    }
}

struct K<X, Y> {
    var k: S<X>.G<Y>
}

こういった入力のテストケースの追加と対応をお願いします。

Copy link
Collaborator Author

@sidepelican sidepelican left a comment

Choose a reason for hiding this comment

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

レビューありがとうございます。

確かにencode/decodeの際に正しくジェネリックパラメータが扱われていない問題があったので、修正箇所を広げました。

対応の途中で、CodecPresenceの結果が常に2値しか取り得ないことに気が付きました。
既存の.conditionalのケースは型がジェネリックだった場合に判断を遅延させるために機能していたようですが、実際はジェネリックな場合は常にencode/decodeを生成するので.required相当でした(特に操作がない場合はコーディング関数にidentityを渡す)。
既存テストケースで.conditionalを検査している部分はあくまで内部動作の途中状態を検査していて、.conditionalをなくしても既存のコード生成部分のテストケースに影響がありませんでした。

そして、CodecPresenceが2値しか取り得ない以上、hasDecodehasEncodeと機能が全く同じとなったため、それらに統合しました。

具体的な理由を忘れてしまったのですが、.conditionalな状態がなくなったことで親スコープの型パラに対する考慮が容易になりました


extension SType {
internal var typeDecl: (any TypeDecl)? {
internal var tsGenericArgs: [any SType] {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

このPRの主要な変更である、「親スコープのジェネリック引数を引き継ぐ」という部分に対応する変更はここともう1箇所だけです。
このジェネリック引数は親スコープのものを含んでいるTS専用の表現であるため、頭にtsをつけて専用感を出しました

Copy link
Owner

Choose a reason for hiding this comment

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

ts 側の構造を意識したprefix良いと思います。

実装は直感的にこんな感じで良さそうです。


public func genericParams() throws -> [any TypeConverter] {
guard let decl = self.swiftType.typeDecl,
return try genericParams(stype: self.swiftType)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

主要な変更のもう一箇所がここです。
argとparamsそれぞれで親スコープをたどるよう変更した形です

@sidepelican
Copy link
Collaborator Author

@omochi 再レビューお願いします

}

extension SType {
internal var typeDecl: (any TypeDecl)? {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

これはSwiftTypeReader側に全く同じ定義があって、シャドウしているのとあちらのほうがメンテされていてケースが多いのでこっちは消しました

Copy link
Owner

@omochi omochi left a comment

Choose a reason for hiding this comment

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

ありがとうございます。

テストケースを見た感じうまくいってそうです。
内部もうろ覚えですが実装はこんな感じになるはずなのであってそうです。

小さな変更で対応できてるのもいいですね。
既存の設計とうまく噛み合っているんでしょうね。

そして、CodecPresenceが2値しか取り得ない以上、hasDecode、hasEncodeと機能が全く同じとなったため、それらに統合しました。

なんか理由があって導入したと思うんですが・・・
まあテストが通ってるなら消しても大丈夫そうですね。


try assertGenerate(
source: """
struct S<T> {
Copy link
Owner

Choose a reason for hiding this comment

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

素朴な入力に見えますが追加する必要ありますか?誤植?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

これJSON型が生成されないことの検査がないと思って追加してました。
Tを持ってるけど、プロパティには使われてないので変換がシンプルであることの検査のつもりでした。

Copy link
Owner

Choose a reason for hiding this comment

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

あ〜なるほど。unexpectedsがポイントだったんですね。okです。


extension SType {
internal var typeDecl: (any TypeDecl)? {
internal var tsGenericArgs: [any SType] {
Copy link
Owner

Choose a reason for hiding this comment

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

ts 側の構造を意識したprefix良いと思います。

実装は直感的にこんな感じで良さそうです。

return parentParams
}
return try genericContext.genericParams.items.map { (param) in
return parentParams + (try genericContext.genericParams.items.map { (param) in
Copy link
Owner

Choose a reason for hiding this comment

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

良さそうな雰囲気。

export function U_decode(json: U_JSON): U {
const k = S_K_decode<E, E_JSON>(json.k, E_decode);
const k2 = S_K2_decode<E, E_JSON>(json.k2, E_decode);
const k3 = json.k3 as S_K<number>;
Copy link
Owner

Choose a reason for hiding this comment

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

お〜いい感じですね

E_JSON,
T,
T_JSON
>(json.k, E_decode, T_decode);
Copy link
Owner

Choose a reason for hiding this comment

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

お〜


func testRecursive() throws {
if true || true {
throw XCTSkip("unsupported")
Copy link
Owner

Choose a reason for hiding this comment

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

@omochi omochi merged commit f088d13 into main Apr 23, 2024
@omochi omochi deleted the parent_generics branch April 23, 2024 07:20
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