Merge pull request #79573 from DougGregor/unsafe-for-in-loop

DougGregor · web-flow · commit 115abcb63702 · 2025-02-24T14:49:45.000-08:00
[SE-0458] Implement "unsafe" effect for the for-in loop
diff --git a/include/swift/AST/ASTBridging.h b/include/swift/AST/ASTBridging.h
@@ -2290,11 +2290,12 @@ BridgedFallthroughStmt_createParsed(BridgedSourceLoc cLoc,
                                     BridgedDeclContext cDC);
 
 SWIFT_NAME("BridgedForEachStmt.createParsed(_:labelInfo:forLoc:tryLoc:awaitLoc:"
-           "pattern:inLoc:sequence:whereLoc:whereExpr:body:)")
+           "unsafeLoc:pattern:inLoc:sequence:whereLoc:whereExpr:body:)")
 BridgedForEachStmt BridgedForEachStmt_createParsed(
     BridgedASTContext cContext, BridgedLabeledStmtInfo cLabelInfo,
     BridgedSourceLoc cForLoc, BridgedSourceLoc cTryLoc,
-    BridgedSourceLoc cAwaitLoc, BridgedPattern cPat, BridgedSourceLoc cInLoc,
+    BridgedSourceLoc cAwaitLoc, BridgedSourceLoc cUnsafeLoc,
+    BridgedPattern cPat, BridgedSourceLoc cInLoc,
     BridgedExpr cSequence, BridgedSourceLoc cWhereLoc,
     BridgedNullableExpr cWhereExpr, BridgedBraceStmt cBody);
 
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -8218,8 +8218,12 @@ GROUPED_WARNING(preconcurrency_import_unsafe,Unsafe,none,
       "introduce data races", ())
 GROUPED_WARNING(unsafe_without_unsafe,Unsafe,none,
       "expression uses unsafe constructs but is not marked with 'unsafe'", ())
+GROUPED_WARNING(for_unsafe_without_unsafe,Unsafe,none,
+      "for-in loop uses unsafe constructs but is not marked with 'unsafe'", ())
 WARNING(no_unsafe_in_unsafe,none,
         "no unsafe operations occur within 'unsafe' expression", ())
+WARNING(no_unsafe_in_unsafe_for,none,
+        "no unsafe operations occur within 'unsafe' for-in loop", ())
 NOTE(make_subclass_unsafe,none,
      "make class %0 @unsafe to allow unsafe overrides of safe superclass methods",
      (DeclName))
diff --git a/include/swift/AST/Stmt.h b/include/swift/AST/Stmt.h
@@ -1003,6 +1003,7 @@ class ForEachStmt : public LabeledStmt {
   SourceLoc ForLoc;
   SourceLoc TryLoc;
   SourceLoc AwaitLoc;
+  SourceLoc UnsafeLoc;
   Pattern *Pat;
   SourceLoc InLoc;
   Expr *Sequence;
@@ -1020,13 +1021,14 @@ class ForEachStmt : public LabeledStmt {
 
 public:
   ForEachStmt(LabeledStmtInfo LabelInfo, SourceLoc ForLoc, SourceLoc TryLoc,
-              SourceLoc AwaitLoc, Pattern *Pat, SourceLoc InLoc, Expr *Sequence,
+              SourceLoc AwaitLoc, SourceLoc UnsafeLoc, Pattern *Pat,
+              SourceLoc InLoc, Expr *Sequence,
               SourceLoc WhereLoc, Expr *WhereExpr, BraceStmt *Body,
               std::optional<bool> implicit = std::nullopt)
       : LabeledStmt(StmtKind::ForEach, getDefaultImplicitFlag(implicit, ForLoc),
                     LabelInfo),
-        ForLoc(ForLoc), TryLoc(TryLoc), AwaitLoc(AwaitLoc), Pat(nullptr),
-        InLoc(InLoc), Sequence(Sequence), WhereLoc(WhereLoc),
+        ForLoc(ForLoc), TryLoc(TryLoc), AwaitLoc(AwaitLoc), UnsafeLoc(UnsafeLoc),
+        Pat(nullptr), InLoc(InLoc), Sequence(Sequence), WhereLoc(WhereLoc),
         WhereExpr(WhereExpr), Body(Body) {
     setPattern(Pat);
   }
@@ -1064,6 +1066,7 @@ class ForEachStmt : public LabeledStmt {
 
   SourceLoc getAwaitLoc() const { return AwaitLoc; }
   SourceLoc getTryLoc() const { return TryLoc; }
+  SourceLoc getUnsafeLoc() const { return UnsafeLoc; }
   
   /// getPattern - Retrieve the pattern describing the iteration variables.
   /// These variables will only be visible within the body of the loop.
diff --git a/include/swift/Parse/IDEInspectionCallbacks.h b/include/swift/Parse/IDEInspectionCallbacks.h
@@ -287,7 +287,8 @@ class CodeCompletionCallbacks {
   virtual void completeStmtLabel(StmtKind ParentKind) {};
 
   virtual
-  void completeForEachPatternBeginning(bool hasTry, bool hasAwait) {};
+  void completeForEachPatternBeginning(
+      bool hasTry, bool hasAwait, bool hasUnsafe) {};
 
   virtual void completeTypeAttrBeginning() {};
 
diff --git a/lib/AST/Bridging/StmtBridging.cpp b/lib/AST/Bridging/StmtBridging.cpp
@@ -191,14 +191,15 @@ BridgedFallthroughStmt_createParsed(BridgedSourceLoc cLoc,
 BridgedForEachStmt BridgedForEachStmt_createParsed(
     BridgedASTContext cContext, BridgedLabeledStmtInfo cLabelInfo,
     BridgedSourceLoc cForLoc, BridgedSourceLoc cTryLoc,
-    BridgedSourceLoc cAwaitLoc, BridgedPattern cPat, BridgedSourceLoc cInLoc,
+    BridgedSourceLoc cAwaitLoc, BridgedSourceLoc cUnsafeLoc,
+    BridgedPattern cPat, BridgedSourceLoc cInLoc,
     BridgedExpr cSequence, BridgedSourceLoc cWhereLoc,
     BridgedNullableExpr cWhereExpr, BridgedBraceStmt cBody) {
   return new (cContext.unbridged()) ForEachStmt(
       cLabelInfo.unbridged(), cForLoc.unbridged(), cTryLoc.unbridged(),
-      cAwaitLoc.unbridged(), cPat.unbridged(), cInLoc.unbridged(),
-      cSequence.unbridged(), cWhereLoc.unbridged(), cWhereExpr.unbridged(),
-      cBody.unbridged());
+      cAwaitLoc.unbridged(), cUnsafeLoc.unbridged(), cPat.unbridged(),
+      cInLoc.unbridged(), cSequence.unbridged(), cWhereLoc.unbridged(),
+      cWhereExpr.unbridged(), cBody.unbridged());
 }
 
 BridgedGuardStmt BridgedGuardStmt_createParsed(BridgedASTContext cContext,
diff --git a/lib/ASTGen/Sources/ASTGen/Stmts.swift b/lib/ASTGen/Sources/ASTGen/Stmts.swift
@@ -330,6 +330,7 @@ extension ASTGenVisitor {
       forLoc: self.generateSourceLoc(node.forKeyword),
       tryLoc: self.generateSourceLoc(node.tryKeyword),
       awaitLoc: self.generateSourceLoc(node.awaitKeyword),
+      unsafeLoc: self.generateSourceLoc(node.unsafeKeyword),
       // NOTE: The pattern can be either a refutable pattern after `case` or a
       // normal binding pattern. ASTGen doesn't care because it should be handled
       // by the parser.
diff --git a/lib/IDE/CodeCompletion.cpp b/lib/IDE/CodeCompletion.cpp
@@ -302,7 +302,8 @@ class CodeCompletionCallbacksImpl : public CodeCompletionCallbacks,
   void completeGenericRequirement() override;
   void completeAfterIfStmtElse() override;
   void completeStmtLabel(StmtKind ParentKind) override;
-  void completeForEachPatternBeginning(bool hasTry, bool hasAwait) override;
+  void completeForEachPatternBeginning(
+      bool hasTry, bool hasAwait, bool hasUnsafe) override;
   void completeTypeAttrBeginning() override;
   void completeTypeAttrInheritanceBeginning() override;
   void completeOptionalBinding() override;
@@ -636,14 +637,16 @@ void CodeCompletionCallbacksImpl::completeStmtLabel(StmtKind ParentKind) {
 }
 
 void CodeCompletionCallbacksImpl::completeForEachPatternBeginning(
-    bool hasTry, bool hasAwait) {
+    bool hasTry, bool hasAwait, bool hasUnsafe) {
   CurDeclContext = P.CurDeclContext;
   Kind = CompletionKind::ForEachPatternBeginning;
   ParsedKeywords.clear();
   if (hasTry)
     ParsedKeywords.emplace_back("try");
   if (hasAwait)
     ParsedKeywords.emplace_back("await");
+  if (hasUnsafe)
+    ParsedKeywords.emplace_back("unsafe");
 }
 
 void CodeCompletionCallbacksImpl::completeOptionalBinding() {
diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp
@@ -2368,6 +2368,7 @@ ParserResult<Stmt> Parser::parseStmtForEach(LabeledStmtInfo LabelInfo) {
   auto StartOfControl = Tok.getLoc();
   SourceLoc AwaitLoc;
   SourceLoc TryLoc;
+  SourceLoc UnsafeLoc;
 
   if (Tok.isContextualKeyword("await")) {
     AwaitLoc = consumeToken();
@@ -2378,10 +2379,15 @@ ParserResult<Stmt> Parser::parseStmtForEach(LabeledStmtInfo LabelInfo) {
     }
   }
 
+  if (Context.LangOpts.hasFeature(Feature::WarnUnsafe) &&
+      Tok.isContextualKeyword("unsafe")) {
+    UnsafeLoc = consumeToken();
+  }
+  
   if (Tok.is(tok::code_complete)) {
     if (CodeCompletionCallbacks) {
       CodeCompletionCallbacks->completeForEachPatternBeginning(
-          TryLoc.isValid(), AwaitLoc.isValid());
+          TryLoc.isValid(), AwaitLoc.isValid(), UnsafeLoc.isValid());
     }
     consumeToken(tok::code_complete);
     // Since 'completeForeachPatternBeginning' is a keyword only completion,
@@ -2495,7 +2501,8 @@ ParserResult<Stmt> Parser::parseStmtForEach(LabeledStmtInfo LabelInfo) {
 
   return makeParserResult(
       Status,
-      new (Context) ForEachStmt(LabelInfo, ForLoc, TryLoc, AwaitLoc, pattern.get(), InLoc,
+      new (Context) ForEachStmt(LabelInfo, ForLoc, TryLoc, AwaitLoc, UnsafeLoc,
+                                pattern.get(), InLoc,
                                 Container.get(), WhereLoc, Where.getPtrOrNull(),
                                 Body.get()));
 }
diff --git a/lib/Sema/BuilderTransform.cpp b/lib/Sema/BuilderTransform.cpp
@@ -726,6 +726,7 @@ class ResultBuilderTransform
     auto *newForEach = new (ctx)
         ForEachStmt(forEachStmt->getLabelInfo(), forEachStmt->getForLoc(),
                     forEachStmt->getTryLoc(), forEachStmt->getAwaitLoc(),
+                    forEachStmt->getUnsafeLoc(),
                     forEachStmt->getPattern(), forEachStmt->getInLoc(),
                     forEachStmt->getParsedSequence(),
                     forEachStmt->getWhereLoc(), forEachStmt->getWhere(),
diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp
@@ -4632,10 +4632,11 @@ generateForEachStmtConstraints(ConstraintSystem &cs, DeclContext *dc,
           AwaitExpr::createImplicit(ctx, nextCall->getLoc(), nextCall);
     }
 
-    // Wrap the 'next' call in 'unsafe', if there is one.
-    if (unsafeExpr) {
-      nextCall = new (ctx) UnsafeExpr(unsafeExpr->getLoc(), nextCall, Type(),
-                                      /*implicit=*/true);
+    // Wrap the 'next' call in 'unsafe', if the for..in loop has that
+    // effect.
+    if (stmt->getUnsafeLoc().isValid()) {
+      nextCall = new (ctx) UnsafeExpr(
+          stmt->getUnsafeLoc(), nextCall, Type(), /*implicit=*/true);
     }
 
     // The iterator type must conform to IteratorProtocol.
diff --git a/lib/Sema/TypeCheckEffects.cpp b/lib/Sema/TypeCheckEffects.cpp
diff --git a/test/Unsafe/safe.swift b/test/Unsafe/safe.swift
diff --git a/test/Unsafe/unsafe-suppression.swift b/test/Unsafe/unsafe-suppression.swift

Original file line number	Diff line number	Diff line change
`@@ -4632,10 +4632,11 @@ generateForEachStmtConstraints(ConstraintSystem &cs, DeclContext *dc,`
`4632`	`4632`	`AwaitExpr::createImplicit(ctx, nextCall->getLoc(), nextCall);`
`4633`	`4633`	`}`
`4634`	`4634`
`4635`		`- // Wrap the 'next' call in 'unsafe', if there is one.`
`4636`		`- if (unsafeExpr) {`
`4637`		`- nextCall = new (ctx) UnsafeExpr(unsafeExpr->getLoc(), nextCall, Type(),`
`4638`		`- /implicit=/true);`
	`4635`	`+ // Wrap the 'next' call in 'unsafe', if the for..in loop has that`
	`4636`	`+ // effect.`
	`4637`	`+ if (stmt->getUnsafeLoc().isValid()) {`
	`4638`	`+ nextCall = new (ctx) UnsafeExpr(`
	`4639`	`+ stmt->getUnsafeLoc(), nextCall, Type(), /implicit=/true);`
`4639`	`4640`	`}`
`4640`	`4641`
`4641`	`4642`	`// The iterator type must conform to IteratorProtocol.`