Skip to content

同じソースファイルが複数回生成されてしまうことがある問題の修正#128

Merged
omochi merged 3 commits intomainfrom
fix_dup
Dec 17, 2024
Merged

同じソースファイルが複数回生成されてしまうことがある問題の修正#128
omochi merged 3 commits intomainfrom
fix_dup

Conversation

@sidepelican
Copy link
Collaborator

@sidepelican sidepelican commented Dec 17, 2024

課題

PackageGeneratorにおいて、特定の状況において同じソースファイルが複数回生成されるという問題がありました。

以下の状況で再現します。

  • AとBのファイルがあり、どちらも生成対象として指定されている
  • BはAに依存している
  • 生成処理がBから開始される(生成処理は入力とは逆順に行われるため、A, Bの順で入力された場合)

同じソースファイルが複数回処理された場合、didConvertSourceがその回数分呼び出されてしまうため、外部から別途TSコードをPackageEntryに注入していた場合、そのコードが重複してしまいます。
(PackageEntryのもつsourceが参照型であることと、シンボルテーブル生成時の結果を使い回す実装が組み合わさって起こる)

原因

現在のPackageGeneratorの処理は以下のようになっています。

  1. 読み込んだすべてのSwiftコードに関してTSコードを生成し、シンボルテーブルを作成する
  2. 生成対象に指定されたファイルをすべて生成対象スタックに積む
  3. 生成対象スタックから1つ取り出してTSコードを生成(1で生成したキャッシュを使う
    )し、importされているシンボルを調べる。その中に未生成のものがあればそれを生成対象スタックに追加
  4. 生成対象スタックがなくなるまで繰り替えす

この際、3で未生成のものかどうかをgeneratedSourcesという集合で管理していましたが、generatedSourcesに含まれていないけど生成対象スタックに含まている、というケースを見逃していました。
そのケースの場合、生成対象スタックには同じソースファイルが複数積まれることが生じえます。

例えば上のA,Bのファイルのケースの場合の生成対象スタックの変化は以下の形になります。

STEP 生成対象スタックの状態 状況
1 [A.swift, B.swift] 初期状態
2 [A.swift] 末尾から処理。Bを処理中
3 [A.swift, A.swift] Bの依存にAが含まれ、かつgeneratedSourcesにAは含まれないので、Aを追加

A.swiftが2回処理されることになりました。

修正方針

スタックから値を取り出した際、それがgeneratedSourcesに含まれている場合はスキップできるようにします。

@sidepelican

This comment was marked as resolved.

Comment on lines +90 to +92
guard generatedSources.insert(source).inserted else {
continue
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

本質的な変更はここだけです

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.

なるほど
ありがとうございます

@omochi omochi merged commit e9b7d27 into main Dec 17, 2024
1 check passed
@omochi omochi deleted the fix_dup branch December 17, 2024 08:06
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