Skip to content

Commit 7f2e6d4

Browse files
committed
Rewrite SimpleDidSetRequest to not require a typechecked body
SimpleDidSetRequest's implementation was depending on type-checking the body of the observer, meaning that computing the interface type of a `didSet` would require type-checking the body first. This triggers some request cycles, and is unnecessarily complicated. Rewrite the implementation to work on a non-type-checked body, by using name lookup to establish when the `oldValue` parameter is referenced from the body.
1 parent 9ffed75 commit 7f2e6d4

File tree

2 files changed

+18
-9
lines changed

2 files changed

+18
-9
lines changed

lib/AST/Decl.cpp

-8
Original file line numberDiff line numberDiff line change
@@ -957,14 +957,6 @@ Type AbstractFunctionDecl::getThrownInterfaceType() const {
957957

958958
llvm::Optional<Type>
959959
AbstractFunctionDecl::getEffectiveThrownErrorType() const {
960-
// FIXME: Only getters can have thrown error types right now, and DidSet
961-
// has a cyclic reference if we try to get its interface type here. Find a
962-
// better way to express this.
963-
if (auto accessor = dyn_cast<AccessorDecl>(this)) {
964-
if (accessor->getAccessorKind() != AccessorKind::Get)
965-
return llvm::None;
966-
}
967-
968960
Type interfaceType = getInterfaceType();
969961
if (hasImplicitSelfDecl()) {
970962
if (auto fnType = interfaceType->getAs<AnyFunctionType>())

lib/Sema/TypeCheckStorage.cpp

+18-1
Original file line numberDiff line numberDiff line change
@@ -3797,6 +3797,9 @@ bool SimpleDidSetRequest::evaluate(Evaluator &evaluator,
37973797
virtual PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
37983798
if (!E)
37993799
return Action::Continue(E);
3800+
3801+
// If we have an already-resolved reference to the parameter in question,
3802+
// we're done.
38003803
if (auto DRE = dyn_cast<DeclRefExpr>(E)) {
38013804
if (auto decl = DRE->getDecl()) {
38023805
if (decl == OldValueParam) {
@@ -3806,6 +3809,20 @@ bool SimpleDidSetRequest::evaluate(Evaluator &evaluator,
38063809
}
38073810
}
38083811

3812+
// If we have an unresolved reference with the same name as the parameter,
3813+
// perform name lookup to determine whether it refers to this parameter.
3814+
if (auto UDRE = dyn_cast<UnresolvedDeclRefExpr>(E)) {
3815+
auto loc = UDRE->getLoc();
3816+
auto sf = OldValueParam->getDeclContext()
3817+
->getOutermostParentSourceFile();
3818+
auto name = UDRE->getName().getFullName();
3819+
if (name == OldValueParam->getName() &&
3820+
ASTScope::lookupSingleLocalDecl(sf, name, loc) == OldValueParam) {
3821+
foundOldValueRef = true;
3822+
return Action::Stop();
3823+
}
3824+
}
3825+
38093826
return Action::Continue(E);
38103827
}
38113828

@@ -3838,7 +3855,7 @@ bool SimpleDidSetRequest::evaluate(Evaluator &evaluator,
38383855
// If we find a reference to the implicit 'oldValue' parameter, then it is
38393856
// not a "simple" didSet because we need to fetch it.
38403857
auto walker = OldValueFinder(param);
3841-
if (auto *body = decl->getTypecheckedBody())
3858+
if (auto *body = decl->getBody())
38423859
body->walk(walker);
38433860
auto hasOldValueRef = walker.didFindOldValueRef();
38443861
if (!hasOldValueRef) {

0 commit comments

Comments
 (0)