[Concurrency] Downgrade missing 'await' error to a warning for synchronous

access to isolated global and static variables that are lazily initialized.

(cherry picked from commit 61b43e6)

Author: hborla

lib/Sema/TypeCheckConcurrency.cpp

@@ -537,12 +537,18 @@ static bool varIsSafeAcrossActors(const ModuleDecl *fromModule,
 }
 
 bool swift::isLetAccessibleAnywhere(const ModuleDecl *fromModule,
-                                    VarDecl *let) {
+                                    VarDecl *let,
+                                    ActorReferenceResult::Options &options) {
   auto isolation = getActorIsolation(let);
-  ActorReferenceResult::Options options = llvm::None;
   return varIsSafeAcrossActors(fromModule, let, isolation, options);
 }
 
+bool swift::isLetAccessibleAnywhere(const ModuleDecl *fromModule,
+                                    VarDecl *let) {
+  ActorReferenceResult::Options options = llvm::None;
+  return isLetAccessibleAnywhere(fromModule, let, options);
+}
+
 namespace {
   /// Describes the important parts of a partial apply thunk.
   struct PartialApplyThunkInfo {

lib/Sema/TypeCheckConcurrency.h

@@ -550,6 +550,13 @@ bool isAccessibleAcrossActors(
     const DeclContext *fromDC,
     llvm::Optional<ReferencedActor> actorInstance = llvm::None);
 
+/// Determines if the 'let' can be read from anywhere within the given module,
+/// regardless of the isolation or async-ness of the context in which
+/// the var is read.
+bool isLetAccessibleAnywhere(const ModuleDecl *fromModule,
+                             VarDecl *let,
+                             ActorReferenceResult::Options &options);
+
 /// Check whether given variable references to a potentially
 /// isolated actor.
 bool isPotentiallyIsolatedActor(

lib/Sema/TypeCheckEffects.cpp

@@ -611,6 +611,8 @@ static void simple_display(llvm::raw_ostream &out, ConditionalEffectKind kind) {
 class Classification {
   bool IsInvalid = false;  // The AST is malformed.  Don't diagnose.
 
+  bool downgradeToWarning = false;
+
   ConditionalEffectKind ThrowKind = ConditionalEffectKind::None;
   llvm::Optional<PotentialEffectReason> ThrowReason;
 
@@ -703,6 +705,14 @@ class Classification {
   }
 
   bool isInvalid() const { return IsInvalid; }
+
+  bool shouldDowngradeToWarning() const {
+    return downgradeToWarning;
+  }
+  void setDowngradeToWarning(bool downgrade) {
+    downgradeToWarning = downgrade;
+  }
+
   ConditionalEffectKind getConditionalKind(EffectKind kind) const {
     switch (kind) {
     case EffectKind::Throws: return ThrowKind;
@@ -2087,15 +2097,20 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
 
   struct DiagnosticInfo {
     DiagnosticInfo(Expr &failingExpr,
-                   PotentialEffectReason reason) :
+                   PotentialEffectReason reason,
+                   bool downgradeToWarning) :
       reason(reason),
-      expr(failingExpr) {}
+      expr(failingExpr),
+      downgradeToWarning(downgradeToWarning) {}
 
     /// Reason for throwing
     PotentialEffectReason reason;
 
     /// Failing expression
     Expr &expr;
+
+    /// Whether the error should be downgraded to a warning.
+    bool downgradeToWarning;
   };
 
   SmallVector<Expr *, 4> errorOrder;
@@ -2504,6 +2519,23 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
     return PotentialEffectReason::forPropertyAccess();
   }
 
+  /// Whether a missing 'await' error on accessing an async var should be
+  /// downgraded to a warning. This is only the case for synchronous access
+  /// to isolated global or static 'let' variables.
+  bool downgradeAsyncAccessToWarning(Decl *decl) {
+    if (auto *var = dyn_cast<VarDecl>(decl)) {
+      ActorReferenceResult::Options options = llvm::None;
+      // The newly-diagnosed cases are invalid regardless of the module context
+      // of the caller, i.e. isolated static and global 'let' variables.
+      auto *module = var->getDeclContext()->getParentModule();
+      if (!isLetAccessibleAnywhere(module, var, options)) {
+        return options.contains(ActorReferenceResult::Flags::Preconcurrency);
+      }
+    }
+
+    return false;
+  }
+
   ShouldRecurse_t checkLookup(LookupExpr *E) {
     auto member = E->getMember();
     if (auto getter = getEffectfulGetOnlyAccessor(member)) {
@@ -2536,10 +2568,13 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
       }
 
       if (!effects.empty()) {
-        checkThrowAsyncSite(E, requiresTry,
-                            Classification::forEffect(effects,
-                                                      ConditionalEffectKind::Always,
-                                                      getKindOfEffectfulProp(member)));
+        auto classification = Classification::forEffect(
+            effects, ConditionalEffectKind::Always,
+            getKindOfEffectfulProp(member));
+        classification.setDowngradeToWarning(
+            downgradeAsyncAccessToWarning(member.getDecl()));
+
+        checkThrowAsyncSite(E, requiresTry, classification);
       }
     }
 
@@ -2564,9 +2599,13 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
                                   PotentialEffectReason::forPropertyAccess()));
 
     } else if (E->isImplicitlyAsync()) {
+      auto classification =  Classification::forUnconditional(
+          EffectKind::Async, PotentialEffectReason::forPropertyAccess());
+      classification.setDowngradeToWarning(
+          downgradeAsyncAccessToWarning(E->getDecl()));
+
       checkThrowAsyncSite(E, /*requiresTry=*/E->isImplicitlyThrows(),
-            Classification::forUnconditional(EffectKind::Async,
-                                   PotentialEffectReason::forPropertyAccess()));
+                          classification);
 
     } else if (auto decl = E->getDecl()) {
       if (auto var = dyn_cast<VarDecl>(decl)) {
@@ -2701,8 +2740,9 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
                                     CurContext.isWithinInterpolatedString());
         if (uncoveredAsync.find(anchor) == uncoveredAsync.end())
           errorOrder.push_back(anchor);
-        uncoveredAsync[anchor].emplace_back(*expr,
-                                            classification.getAsyncReason());
+        uncoveredAsync[anchor].emplace_back(
+            *expr, classification.getAsyncReason(),
+            classification.shouldDowngradeToWarning());
       }
     }
 
@@ -2888,7 +2928,13 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
         awaitInsertLoc = tryExpr->getSubExpr()->getStartLoc();
     }
 
+    bool downgradeToWarning = llvm::all_of(errors,
+        [&](DiagnosticInfo diag) -> bool {
+          return diag.downgradeToWarning;
+        });
+
     Ctx.Diags.diagnose(anchor->getStartLoc(), diag::async_expr_without_await)
+      .warnUntilSwiftVersionIf(downgradeToWarning, 6)
       .fixItInsert(awaitInsertLoc, "await ")
       .highlight(anchor->getSourceRange());
 

test/Concurrency/concurrent_value_checking.swift

@@ -4,8 +4,8 @@
 // REQUIRES: concurrency
 // REQUIRES: asserts
 
-class NotConcurrent { } // expected-note 16{{class 'NotConcurrent' does not conform to the 'Sendable' protocol}}
-// expected-complete-note @-1 11{{class 'NotConcurrent' does not conform to the 'Sendable' protocol}}
+class NotConcurrent { } // expected-note 18{{class 'NotConcurrent' does not conform to the 'Sendable' protocol}}
+// expected-complete-note @-1 12{{class 'NotConcurrent' does not conform to the 'Sendable' protocol}}
 
 // ----------------------------------------------------------------------
 // Sendable restriction on actor operations
@@ -115,12 +115,27 @@ func globalAsync(_: NotConcurrent?) async {
   globalSync(nil)
 }
 
+enum E {
+  @SomeGlobalActor static let notSafe: NotConcurrent? = nil
+}
+
 func globalTest() async {
-  // expected-error@+2 {{expression is 'async' but is not marked with 'await'}}
+  // expected-warning@+2 {{expression is 'async' but is not marked with 'await'}}
   // expected-note@+1 {{property access is 'async'}}
   let a = globalValue // expected-warning{{non-sendable type 'NotConcurrent?' in implicitly asynchronous access to global actor 'SomeGlobalActor'-isolated let 'globalValue' cannot cross actor boundary}}
   await globalAsync(a) // expected-complete-warning{{passing argument of non-sendable type 'NotConcurrent?' into global actor 'SomeGlobalActor'-isolated context may introduce data races}}
   await globalSync(a)  // expected-complete-warning{{passing argument of non-sendable type 'NotConcurrent?' into global actor 'SomeGlobalActor'-isolated context may introduce data races}}
+
+  // expected-warning@+2 {{expression is 'async' but is not marked with 'await'}}
+  // expected-note@+1 {{property access is 'async'}}
+  let _ = E.notSafe // expected-warning{{non-sendable type 'NotConcurrent?' in implicitly asynchronous access to global actor 'SomeGlobalActor'-isolated static property 'notSafe' cannot cross actor boundary}}
+
+  // expected-error@+3 {{expression is 'async' but is not marked with 'await'}}
+  // expected-note@+2 {{call is 'async'}}
+  // expected-note@+1 {{property access is 'async'}}
+  globalAsync(E.notSafe)
+  // expected-complete-warning@-1 {{passing argument of non-sendable type 'NotConcurrent?' into global actor 'SomeGlobalActor'-isolated context may introduce data races}}
+  // expected-warning@-2 {{non-sendable type 'NotConcurrent?' in implicitly asynchronous access to global actor 'SomeGlobalActor'-isolated static property 'notSafe' cannot cross actor boundary}}
 }
 
 struct HasSubscript {
@@ -139,7 +154,7 @@ class ClassWithGlobalActorInits { // expected-note 2{{class 'ClassWithGlobalActo
 
 @MainActor
 func globalTestMain(nc: NotConcurrent) async {
-  // expected-error@+2 {{expression is 'async' but is not marked with 'await'}}
+  // expected-warning@+2 {{expression is 'async' but is not marked with 'await'}}
   // expected-note@+1 {{property access is 'async'}}
   let a = globalValue // expected-warning{{non-sendable type 'NotConcurrent?' in implicitly asynchronous access to global actor 'SomeGlobalActor'-isolated let 'globalValue' cannot cross actor boundary}}
   await globalAsync(a) // expected-complete-warning{{passing argument of non-sendable type 'NotConcurrent?' into global actor 'SomeGlobalActor'-isolated context may introduce data races}}