Merge pull request swiftlang#81419 from DougGregor/lookupexpr-effects-cleanup

[Effects] Simplify handling of LookupExpr in effects checking

Author: DougGregor

lib/Sema/TypeCheckEffects.cpp

@@ -1398,7 +1398,8 @@ class ApplyClassifier {
     if (isa<SubscriptDecl>(cdr.getDecl()))
       return PotentialEffectReason::forSubscriptAccess();
 
-    assert(isa<VarDecl>(cdr.getDecl()));
+    // Note that declarations other than properties end up with
+    // "property access" effect reasons for historical... reasons.
     return PotentialEffectReason::forPropertyAccess();
   }
 
@@ -1537,18 +1538,18 @@ class ApplyClassifier {
     if (!member)
       return Classification::forInvalidCode();
 
-    PotentialEffectReason reason =
-        PotentialEffectReason::forPropertyAccess();
-    Classification classification;
+    ConcreteDeclRef declToClassify;
     if (auto getter = getEffectfulGetOnlyAccessor(member)) {
-      reason = getKindOfEffectfulProp(member);
-      classification = Classification::forDeclRef(
-          getter, ConditionalEffectKind::Always, reason, E->getLoc(),
-          assumedSafeArguments && assumedSafeArguments->contains(E));
-    } else if (isa<SubscriptExpr>(E) || isa<DynamicSubscriptExpr>(E)) {
-      reason = PotentialEffectReason::forSubscriptAccess();
+      declToClassify = getter;
+    } else {
+      declToClassify = member;
     }
 
+    PotentialEffectReason reason = getKindOfEffectfulProp(member);
+    Classification classification = Classification::forDeclRef(
+          declToClassify, ConditionalEffectKind::Always, reason, E->getLoc(),
+          assumedSafeArguments && assumedSafeArguments->contains(E));
+
     classification.setDowngradeToWarning(
         downgradeAsyncAccessToWarning(member.getDecl()));
 
@@ -1557,14 +1558,6 @@ class ApplyClassifier {
         E->isImplicitlyAsync().has_value(), E->isImplicitlyThrows(),
         reason);
 
-    if (!classification.hasUnsafe()) {
-      classification.merge(
-          Classification::forDeclRef(
-            member, ConditionalEffectKind::Always, reason, E->getLoc(),
-            assumedSafeArguments && assumedSafeArguments->contains(E),
-            EffectKind::Unsafe));
-    }
-
     if (assumedSafeArguments) {
       // If the declaration we're referencing is explicitly @safe, mark arguments
       // as potentially being safe.
@@ -1646,9 +1639,10 @@ class ApplyClassifier {
         PotentialEffectReason::forApply());
   }
 
-  /// Check to see if the given function application throws or is async.
+  /// Check to see if the given function application throws, is async, or
+  /// involves unsafe behavior.
   Classification classifyApply(
-      ApplyExpr *E, 
+      ApplyExpr *E,
       llvm::DenseSet<const Expr *> *assumedSafeArguments
   ) {
     // An apply expression is a potential throw site if the function throws.
@@ -1664,9 +1658,28 @@ class ApplyClassifier {
     if (!type) return Classification::forInvalidCode();
     auto fnType = type->getAs<AnyFunctionType>();
     if (!fnType) return Classification::forInvalidCode();
-
     Expr *calleeFn = nullptr;
     auto fnRef = AbstractFunction::getAppliedFn(E, &calleeFn);
+
+    auto *args = E->getArgs();
+    return classifyApply(
+        E, fnRef, calleeFn, fnType, args,
+        E->isImplicitlyAsync().has_value(), E->implicitlyThrows(),
+        assumedSafeArguments);
+  }
+
+  /// Check to see if the given function application throws, is async, or
+  /// involves unsafe behavior.
+  Classification classifyApply(
+      Expr *call,
+      const AbstractFunction &fnRef,
+      Expr *calleeFn,
+      const AnyFunctionType *fnType,
+      ArgumentList *args,
+      bool implicitlyAsync,
+      bool implicitlyThrows,
+      llvm::DenseSet<const Expr *> *assumedSafeArguments
+  ) {
     auto substitutions = fnRef.getSubstitutions();
     const bool hasAnyConformances =
         llvm::any_of(substitutions.getConformances(),
@@ -1692,26 +1705,23 @@ class ApplyClassifier {
       // If this is calling a @safe function, treat local variables as being
       // safe.
       if (fnRef.getExplicitSafety() == ExplicitSafety::Safe) {
-        markArgumentsSafe(E->getArgs(), *assumedSafeArguments);
+        markArgumentsSafe(args, *assumedSafeArguments);
       }
     }
 
-    ASTContext &ctx = type->getASTContext();
+    ASTContext &ctx = fnType->getASTContext();
 
     // If the function doesn't have any effects or conformances, we're done
     // here.
     if (!fnType->isThrowing() &&
-        !E->implicitlyThrows() &&
+        !implicitlyThrows &&
         !fnType->isAsync() &&
-        !E->isImplicitlyAsync() &&
+        !implicitlyAsync &&
         !hasAnyConformances &&
         fnRef.getExplicitSafety() == ExplicitSafety::Safe) {
       return Classification();
     }
 
-    // Decompose the application.
-    auto *args = E->getArgs();
-
     // If any of the arguments didn't type check, fail.
     for (auto arg : *args) {
       auto *argExpr = arg.getExpr();
@@ -1721,7 +1731,7 @@ class ApplyClassifier {
 
     Classification result;
     result.mergeImplicitEffects(
-        ctx, E->isImplicitlyAsync().has_value(), E->implicitlyThrows(),
+        ctx, implicitlyAsync, implicitlyThrows,
         PotentialEffectReason::forApply());
 
     // Downgrade missing 'await' errors for preconcurrency references.
@@ -1732,8 +1742,8 @@ class ApplyClassifier {
     auto classifyApplyEffect = [&](EffectKind kind) {
       if (kind != EffectKind::Unsafe &&
           !fnType->hasEffect(kind) &&
-          !(kind == EffectKind::Async && E->isImplicitlyAsync()) &&
-          !(kind == EffectKind::Throws && E->implicitlyThrows())) {
+          !(kind == EffectKind::Async && implicitlyAsync) &&
+          !(kind == EffectKind::Throws && implicitlyThrows)) {
         return;
       }
 
@@ -1767,7 +1777,7 @@ class ApplyClassifier {
         } else if (kind == EffectKind::Unsafe) {
           // For explicitly @unsafe functions, the entire call is considered
           // unsafe.
-          AbstractFunctionDecl *calleeFn =
+          AbstractFunctionDecl *calleeDecl =
             fnRef.getKind() == AbstractFunction::Function
               ? fnRef.getFunction()
               : nullptr;
@@ -1776,7 +1786,7 @@ class ApplyClassifier {
               Classification::forUnsafe(
                 {
                   UnsafeUse::forReferenceToUnsafe(
-                    calleeFn, true, fnRef.getType(), E->getLoc())
+                    calleeDecl, true, fnRef.getType(), call->getLoc())
                 }
               ));
           return;
@@ -1807,7 +1817,7 @@ class ApplyClassifier {
           // to fix their code.
           if (kind == EffectKind::Async &&
               fnRef.getKind() == AbstractFunction::Function) {
-            if (auto *ctor = dyn_cast<ConstructorRefCallExpr>(E->getFn())) {
+            if (auto *ctor = dyn_cast<ConstructorRefCallExpr>(calleeFn)) {
               if (ctor->getFn()->isImplicit() && args->isUnlabeledUnary())
                 result.setDowngradeToWarning(true);
             }
@@ -1848,15 +1858,15 @@ class ApplyClassifier {
           return;
         }
 
-        AbstractFunctionDecl *calleeFn =
+        AbstractFunctionDecl *calleeDecl =
           fnRef.getKind() == AbstractFunction::Function
             ? fnRef.getFunction()
             : nullptr;
 
         for (unsigned i = 0, e = args->size(); i < e; ++i) {
           Type origParamType = fnRef.getOrigParamInterfaceType(i);
           auto argClassification = classifyArgument(
-              E, calleeFn, args->getLabel(i), i,
+              call, calleeDecl, args->getLabel(i), i,
               args->getExpr(i), origParamType, fnRef.getSubstitutions(),
               kind);
 
@@ -1874,7 +1884,7 @@ class ApplyClassifier {
           if (auto appliedSelf = fnRef.getAppliedSelf()) {
             Type origParamType = fnRef.getOrigParamSelfType();
             auto selfClassification = classifyArgument(
-                E, calleeFn, ctx.Id_self, 0, appliedSelf->getBase(),
+                call, calleeDecl, ctx.Id_self, 0, appliedSelf->getBase(),
                 origParamType, fnRef.getSubstitutions(), kind);
             result.merge(selfClassification);
           }
@@ -1924,7 +1934,7 @@ class ApplyClassifier {
     // If the safety of the callee is unspecified, check the safety of the
     // arguments specifically.
     if (hasUnspecifiedSafety &&
-        !(assumedSafeArguments && assumedSafeArguments->contains(E))) {
+        !(assumedSafeArguments && assumedSafeArguments->contains(call))) {
       classifyApplyEffect(EffectKind::Unsafe);
     }
 
@@ -2611,7 +2621,7 @@ class ApplyClassifier {
 
   /// Classify an argument being passed to a rethrows/reasync function.
   Classification classifyArgument(
-      ApplyExpr *call, const Decl* calleeDecl, Identifier argName,
+      Expr *call, const Decl* calleeDecl, Identifier argName,
       unsigned argIndex, Expr *arg, Type paramType, SubstitutionMap subs,
       EffectKind kind) {
     // Check for an unsafe argument.
@@ -2718,7 +2728,7 @@ class ApplyClassifier {
   }
 
   /// Classify an argument to a rethrows/reasync function that's a tuple literal.
-  Classification classifyTupleArgument(ApplyExpr *call,
+  Classification classifyTupleArgument(Expr *call,
                                        const Decl* calleeDecl,
                                        Identifier argName,
                                        unsigned argIndex,