[AutoDiff] factor derivative typechecking helper out of AttributeChecker #28879
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
marcrasi
force-pushed
the
less-mutating-typechecking
branch
from
b5e4b60 to
7592fbd
Compare
dan-zheng
approved these changes
Dec 19, 2019
marcrasi
force-pushed
the
less-mutating-typechecking
branch
from
7592fbd to
962c3d8
Compare
|
@swift-ci please test and merge |
1 similar comment
|
@swift-ci please test and merge |
marcrasi
force-pushed
the
less-mutating-typechecking
branch
from
962c3d8 to
637314a
Compare
|
@swift-ci please test and merge |
1 similar comment
|
@swift-ci please test and merge |
dan-zheng
added a commit
to dan-zheng/swift
that referenced
this pull request
Dec 20, 2019
Fix build error introduced in swiftlang#28892. Caused by not re-running CI after swiftlang#28879 was merged.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In order to allow a
@derivativein one file to define the derivative of a function in another file, we nned to calculate the derivative configurations of all the@derivativeattributes in the module, not just those in the primary file (see #28790 for a work in progress of this).AttributeChecker::visitDerivativeAttris the function that does the necessary calculation, so this PR factors it into a separate function that others can call to do the calculation. This PR makes a few changes to the logic:ASTContext::DerivativeAttrsnow stores a set of all attributes per configuration, because otherwise you get buggy order-dependent behavior in the duplicate attr check when you calltypeCheckDerivativeAttrmultiple times per attribute.typeCheckDerivativeAttrreturns a boolean indicating error instead of callingattr->setInvalid()so that non-AttributeCheckercallers can leaveattr->setInvalid()to theAttributeChecker.Note: This PR makes the
AttributeCheckerno longer usediagnoseAndRemoveAttr(), so you don't get a "fixItRemove" on the diagnostics any more. Keeping it would make the refactoring a bit more complicated, and I don't think it's that valuable to keep, because in many of the error cases removing the attribute isn't really the right way to fix things.