Skip to content

Commit ff830dd

Browse files
committed
[Effects] Simplify handling of LookupExpr in effects checking
Our logic for doing the "declaration reference" classification was unnecessarily convoluted, and did "unsafe" classification twice for properties and subscripts that have other effects (throws/async) on their getters. Simplify it.
1 parent 78234e2 commit ff830dd

File tree

1 file changed

+11
-18
lines changed

1 file changed

+11
-18
lines changed

lib/Sema/TypeCheckEffects.cpp

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1398,7 +1398,8 @@ class ApplyClassifier {
13981398
if (isa<SubscriptDecl>(cdr.getDecl()))
13991399
return PotentialEffectReason::forSubscriptAccess();
14001400

1401-
assert(isa<VarDecl>(cdr.getDecl()));
1401+
// Note that declarations other than properties end up with
1402+
// "property access" effect reasons for historical... reasons.
14021403
return PotentialEffectReason::forPropertyAccess();
14031404
}
14041405

@@ -1537,18 +1538,18 @@ class ApplyClassifier {
15371538
if (!member)
15381539
return Classification::forInvalidCode();
15391540

1540-
PotentialEffectReason reason =
1541-
PotentialEffectReason::forPropertyAccess();
1542-
Classification classification;
1541+
ConcreteDeclRef declToClassify;
15431542
if (auto getter = getEffectfulGetOnlyAccessor(member)) {
1544-
reason = getKindOfEffectfulProp(member);
1545-
classification = Classification::forDeclRef(
1546-
getter, ConditionalEffectKind::Always, reason, E->getLoc(),
1547-
assumedSafeArguments && assumedSafeArguments->contains(E));
1548-
} else if (isa<SubscriptExpr>(E) || isa<DynamicSubscriptExpr>(E)) {
1549-
reason = PotentialEffectReason::forSubscriptAccess();
1543+
declToClassify = getter;
1544+
} else {
1545+
declToClassify = member;
15501546
}
15511547

1548+
PotentialEffectReason reason = getKindOfEffectfulProp(member);
1549+
Classification classification = Classification::forDeclRef(
1550+
declToClassify, ConditionalEffectKind::Always, reason, E->getLoc(),
1551+
assumedSafeArguments && assumedSafeArguments->contains(E));
1552+
15521553
classification.setDowngradeToWarning(
15531554
downgradeAsyncAccessToWarning(member.getDecl()));
15541555

@@ -1557,14 +1558,6 @@ class ApplyClassifier {
15571558
E->isImplicitlyAsync().has_value(), E->isImplicitlyThrows(),
15581559
reason);
15591560

1560-
if (!classification.hasUnsafe()) {
1561-
classification.merge(
1562-
Classification::forDeclRef(
1563-
member, ConditionalEffectKind::Always, reason, E->getLoc(),
1564-
assumedSafeArguments && assumedSafeArguments->contains(E),
1565-
EffectKind::Unsafe));
1566-
}
1567-
15681561
if (assumedSafeArguments) {
15691562
// If the declaration we're referencing is explicitly @safe, mark arguments
15701563
// as potentially being safe.

0 commit comments

Comments
 (0)