Skip to content

[AutoDiff upstream] forbid @derivative of protocol req#29031

Merged
swift-ci merged 2 commits into
swiftlang:masterfrom
marcrasi:forbid-protocol-req
Jan 7, 2020
Merged

[AutoDiff upstream] forbid @derivative of protocol req#29031
swift-ci merged 2 commits into
swiftlang:masterfrom
marcrasi:forbid-protocol-req

Conversation

@marcrasi
Copy link
Copy Markdown

@marcrasi marcrasi commented Jan 6, 2020

Forbids @derivatives of protocol requirements, because they do not do anything until TF-982 is implemented.

This is the master branch version of #28890.

@marcrasi marcrasi requested a review from dan-zheng January 6, 2020 23:34
Comment thread lib/Sema/TypeCheckAttr.cpp Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I feel we need an error message stating clearly that defining derivatives for protocol requirements is not yet supported.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Customized diagnostics may require extra support in findAbstractFunctionDecl in TypeCheckAttr.cpp. Currently, when std::function<bool(AbstractFunctionDecl *)> &isValidCandidate returns false for all candidates, a single generic error is emitted (std::function<void()> &noneValidDiagnostic).

We could change isValidCandidate emit diagnostics as a side effect, but this requires clever diagnostic cancellation (via DiagnosticTransaction) in case a valid candidate is found.

Alternatively, we could keep track of all invalid candidates and change noneValidDiagnostic to std::function<void(AbstractFunctionDecl *)> &notValidDiagnostic, so customized diagnostics can be emitted for every invalid candidate.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This seems like more work than it's worth, especially since it's a temporary limitation, so I won't do this in this PR.

@marcrasi marcrasi force-pushed the forbid-protocol-req branch from 7561235 to d69e892 Compare January 6, 2020 23:58
@marcrasi
Copy link
Copy Markdown
Author

marcrasi commented Jan 7, 2020

@swift-ci please test and merge

Comment thread test/AutoDiff/Sema/derivative_attr_type_checking.swift
@dan-zheng
Copy link
Copy Markdown
Contributor

@swift-ci Please test

@swift-ci
Copy link
Copy Markdown
Contributor

swift-ci commented Jan 7, 2020

Build failed
Swift Test OS X Platform
Git Sha - d69e892

@swift-ci
Copy link
Copy Markdown
Contributor

swift-ci commented Jan 7, 2020

Build failed
Swift Test Linux Platform
Git Sha - d69e892

@swift-ci swift-ci merged commit b6e0ffd into swiftlang:master Jan 7, 2020
@marcrasi marcrasi deleted the forbid-protocol-req branch January 7, 2020 19:14
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.

4 participants