Skip to content

[AutoDiff] Better diagnose non-differentiability for throwing functions #81669

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 21, 2025
Merged

[AutoDiff] Better diagnose non-differentiability for throwing functions #81669

merged 1 commit into from
May 21, 2025

Conversation

asl
Copy link
Contributor

@asl asl commented May 21, 2025

Fixes #81608

@asl asl requested review from rxwei and kovdan01 May 21, 2025 08:31
@asl asl requested a review from eeckstein as a code owner May 21, 2025 08:31
@asl
Copy link
Contributor Author

asl commented May 21, 2025

@swift-ci please test

@asl asl mentioned this pull request May 21, 2025
Closed
@asl asl added the AutoDiff label May 21, 2025
@asl asl changed the title Better diagnose non-differentiability for throwing functions [AutoDiff] Better diagnose non-differentiability for throwing functions May 21, 2025
kovdan01
kovdan01 approved these changes May 21, 2025
View reviewed changes
Copy link
Contributor

@kovdan01 kovdan01 left a comment

Choose a reason for hiding this comment

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

As far as I see, we now have 3 new code paths leading to non-differentiability diagnostics, while only 1 is covered by test. It's probably worth adding tests for all the 3 code paths (if that's not too challenging)

Except this, LGTM

@asl
Copy link
Contributor Author

asl commented May 21, 2025

As far as I see, we now have 3 new code paths leading to non-differentiability diagnostics, while only 1 is covered by test. It's probably worth adding tests for all the 3 code paths (if that's not too challenging)

Except this, LGTM

In fact two, as currently there is no way getSingleTerminator would return nullptr. Still, if new terminators would be added in the future it would be better not to crash here. And as for move_value we previously decided with you to ignore that case :)

@asl asl enabled auto-merge (squash) May 21, 2025 10:01
@asl asl merged commit bc94ac3 into main May 21, 2025
5 checks passed
@asl asl deleted the 81608-fix branch May 21, 2025 16:13
hamishknight added a commit to hamishknight/swift that referenced this pull request May 22, 2025
@hamishknight
Revert "[AutoDiff] Better diagnose non-differentiability for throwing…
11bd596 
… functions (swiftlang#81669)"

This reverts commit bc94ac3.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kovdan01 kovdan01 kovdan01 approved these changes

@rxwei rxwei Awaiting requested review from rxwei

@eeckstein eeckstein Awaiting requested review from eeckstein eeckstein is a code owner

Assignees
No one assigned
Labels
AutoDiff
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[AutoDiff] Fatal error when using regular non-differentiable Array map
2 participants
@asl @kovdan01