Skip to content

Conversation

@dan-zheng
Copy link
Contributor

Fix IterableDeclContext::loadAllMembers for structs/enums generated during differentiation transform.

Previously, 7e89bee disabled loadAllMembers for AD-generated structs/enums.

This caused a SIL verification failure for test/AutoDiff/autodiff_generated_decl_member_loading.swift because enum members were not loaded.

Now, loadAllMembers is re-enabled. Other necessary fixes:

  • Generated structs/enums are set to implicit.
    • Since implicit declarations are not printed via -emit-sil, data structure generation tests now rely on -Xllvm -debug-only=differentiation.
  • Remove calls to NominalTypeDecl::setBraces.
  • Strategically set SourceLoc for EnumElementDecl and EnumCaseDecl to prevent duplicate enum cases from printing.

Resolves TF-805.

Fix `IterableDeclContext::loadAllMembers` for structs/enums generated during
differentiation transform.

Previously, this patch disabled `loadAllMembers` for AD-generated structs/enums:
swiftlang@7e89bee

This caused a SIL verification failure for
test/AutoDiff/autodiff_generated_decl_member_loading.swift because enum members
were not loaded.

Now, `loadAllMembers` is re-enabled. Other necessary fixes:
- Generated structs/enums are set to implicit.
  - Since implicit declarations are not printed via `-emit-sil`, data structure
    generation tests now rely on `-Xllvm -debug-only=differentiation`.
- Remove calls to `NominalTypeDecl::setBraces`.
- Strategically set `SourceLoc` for `EnumElementDecl` and `EnumCaseDecl` to
  prevent duplicate enum cases from printing.

Resolves TF-805.
@dan-zheng dan-zheng added the tensorflow This is for "tensorflow" branch PRs. label Sep 20, 2019
@dan-zheng dan-zheng requested review from marcrasi and rxwei September 20, 2019 00:29
@dan-zheng
Copy link
Contributor Author

@swift-ci Please test tensorflow

@marcrasi
Copy link

Did you forget to git add the regression test?

@dan-zheng
Copy link
Contributor Author

dan-zheng commented Sep 20, 2019

Did you forget to git add the regression test?

Yes, thanks for the catch! Added in 2b8e854.

@dan-zheng
Copy link
Contributor Author

@swift-ci Please test tensorflow

@dan-zheng
Copy link
Contributor Author

@swift-ci Please test tensorflow macOS

@ematejska ematejska merged commit de6b75a into swiftlang:tensorflow Sep 20, 2019
@dan-zheng dan-zheng deleted the TF-805 branch September 20, 2019 02:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tensorflow This is for "tensorflow" branch PRs.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants