[Strict memory safety] Eliminate spurious warnings with synthesized Codable

When synthesizing code for Codable conformances involving unsafe types,
make sure to wrap the resulting expressions in "unsafe" when strict memory safety is enabled.

Tweak the warning-emission logic to suppress warnings about spurious
"unsafe" expressions when the compiler generated the "unsafe" itself,
so we don't spam the developer with warnings they can't fix. Also make
the checking for other suppression considerations safe when there are
no source locations, eliminating a potential assertion.

Fixes rdar://153665692.

Author: DougGregor

lib/Sema/CSGen.cpp

@@ -4734,9 +4734,10 @@ generateForEachStmtConstraints(ConstraintSystem &cs, DeclContext *dc,
         (isAsync &&
          ctx.LangOpts.StrictConcurrencyLevel == StrictConcurrency::Complete)) {
       SourceLoc loc = stmt->getUnsafeLoc();
+      bool implicit = stmt->getUnsafeLoc().isInvalid();
       if (loc.isInvalid())
         loc = stmt->getForLoc();
-      nextCall = new (ctx) UnsafeExpr(loc, nextCall, Type(), /*implicit=*/true);
+      nextCall = new (ctx) UnsafeExpr(loc, nextCall, Type(), implicit);
     }
 
     // The iterator type must conform to IteratorProtocol.

lib/Sema/DerivedConformance/DerivedConformanceCodable.cpp

@@ -772,10 +772,20 @@ lookupVarDeclForCodingKeysCase(DeclContext *conformanceDC,
   llvm_unreachable("Should have found at least 1 var decl");
 }
 
-static TryExpr *createEncodeCall(ASTContext &C, Type codingKeysType,
-                                 EnumElementDecl *codingKey,
-                                 Expr *containerExpr, Expr *varExpr,
-                                 bool useIfPresentVariant) {
+/// If strict memory safety checking is enabled, wrap the expression in an
+/// implicit "unsafe".
+static Expr *wrapInUnsafeIfNeeded(ASTContext &ctx, Expr *expr) {
+  if (ctx.LangOpts.hasFeature(Feature::StrictMemorySafety,
+                              /*allowMigration=*/true))
+    return UnsafeExpr::createImplicit(ctx, expr->getStartLoc(), expr);
+
+  return expr;
+}
+
+static Expr *createEncodeCall(ASTContext &C, Type codingKeysType,
+                              EnumElementDecl *codingKey,
+                              Expr *containerExpr, Expr *varExpr,
+                              bool useIfPresentVariant) {
   // CodingKeys.x
   auto *metaTyRef = TypeExpr::createImplicit(codingKeysType, C);
   auto *keyExpr = new (C) MemberRefExpr(metaTyRef, SourceLoc(), codingKey,
@@ -794,7 +804,7 @@ static TryExpr *createEncodeCall(ASTContext &C, Type codingKeysType,
   // try container.encode(x, forKey: CodingKeys.x)
   auto *tryExpr = new (C) TryExpr(SourceLoc(), callExpr, Type(),
                                   /*Implicit=*/true);
-  return tryExpr;
+  return wrapInUnsafeIfNeeded(C, tryExpr);
 }
 
 /// Synthesizes the body for `func encode(to encoder: Encoder) throws`.
@@ -929,7 +939,7 @@ deriveBodyEncodable_encode(AbstractFunctionDecl *encodeDecl, void *) {
     // try super.encode(to: container.superEncoder())
     auto *tryExpr = new (C) TryExpr(SourceLoc(), callExpr, Type(),
                                     /*Implicit=*/true);
-    statements.push_back(tryExpr);
+    statements.push_back(wrapInUnsafeIfNeeded(C, tryExpr));
   }
 
   auto *body = BraceStmt::create(C, SourceLoc(), statements, SourceLoc(),
@@ -1112,8 +1122,10 @@ deriveBodyEncodable_enum_encode(AbstractFunctionDecl *encodeDecl, void *) {
 
   // generate: switch self { }
   auto enumRef =
-      new (C) DeclRefExpr(ConcreteDeclRef(selfRef), DeclNameLoc(),
-                          /*implicit*/ true, AccessSemantics::Ordinary);
+      wrapInUnsafeIfNeeded(
+        C,
+        new (C) DeclRefExpr(ConcreteDeclRef(selfRef), DeclNameLoc(),
+                            /*implicit*/ true, AccessSemantics::Ordinary));
   auto switchStmt = createEnumSwitch(
       C, funcDC, enumRef, enumDecl, codingKeysEnum,
       /*createSubpattern*/ true,
@@ -1276,11 +1288,11 @@ static FuncDecl *deriveEncodable_encode(DerivedConformance &derived) {
   return encodeDecl;
 }
 
-static TryExpr *createDecodeCall(ASTContext &C, Type resultType,
-                                 Type codingKeysType,
-                                 EnumElementDecl *codingKey,
-                                 Expr *containerExpr,
-                                 bool useIfPresentVariant) {
+static Expr *createDecodeCall(ASTContext &C, Type resultType,
+                              Type codingKeysType,
+                              EnumElementDecl *codingKey,
+                              Expr *containerExpr,
+                              bool useIfPresentVariant) {
   auto methodName = useIfPresentVariant ? C.Id_decodeIfPresent : C.Id_decode;
 
   // Type.self
@@ -1470,7 +1482,7 @@ deriveBodyDecodable_init(AbstractFunctionDecl *initDecl, void *) {
                                                         varDecl->getName());
       auto *assignExpr = new (C) AssignExpr(varExpr, SourceLoc(), tryExpr,
                                             /*Implicit=*/true);
-      statements.push_back(assignExpr);
+      statements.push_back(wrapInUnsafeIfNeeded(C, assignExpr));
     }
   }
 
@@ -1506,7 +1518,7 @@ deriveBodyDecodable_init(AbstractFunctionDecl *initDecl, void *) {
         // try super.init(from: container.superDecoder())
         auto *tryExpr = new (C) TryExpr(SourceLoc(), callExpr, Type(),
                                         /*Implicit=*/true);
-        statements.push_back(tryExpr);
+        statements.push_back(wrapInUnsafeIfNeeded(C, tryExpr));
       } else {
         // The explicit constructor name is a compound name taking no arguments.
         DeclName initName(C, DeclBaseName::createConstructor(),
@@ -1538,7 +1550,7 @@ deriveBodyDecodable_init(AbstractFunctionDecl *initDecl, void *) {
           callExpr = new (C) TryExpr(SourceLoc(), callExpr, Type(),
                                      /*Implicit=*/true);
 
-        statements.push_back(callExpr);
+        statements.push_back(wrapInUnsafeIfNeeded(C, callExpr));
       }
     }
   }
@@ -1827,7 +1839,7 @@ deriveBodyDecodable_enum_init(AbstractFunctionDecl *initDecl, void *) {
                 new (C) AssignExpr(selfRef, SourceLoc(), selfCaseExpr,
                                    /*Implicit=*/true);
 
-            caseStatements.push_back(assignExpr);
+            caseStatements.push_back(wrapInUnsafeIfNeeded(C, assignExpr));
           } else {
             // Foo.bar(x:)
             SmallVector<Identifier, 3> scratch;
@@ -1845,7 +1857,7 @@ deriveBodyDecodable_enum_init(AbstractFunctionDecl *initDecl, void *) {
                 new (C) AssignExpr(selfRef, SourceLoc(), caseCallExpr,
                                    /*Implicit=*/true);
 
-            caseStatements.push_back(assignExpr);
+            caseStatements.push_back(wrapInUnsafeIfNeeded(C, assignExpr));
           }
 
           auto body =

lib/Sema/TypeCheckEffects.cpp

@@ -4667,18 +4667,25 @@ class CheckEffectsCoverage : public EffectsHandlingWalker<CheckEffectsCoverage>
   }
 
   void diagnoseRedundantUnsafe(UnsafeExpr *E) const {
-    // Silence this warning in the expansion of the _SwiftifyImport macro.
-    // This is a hack because it's tricky to determine when to insert "unsafe".
-    unsigned bufferID =
-        Ctx.SourceMgr.findBufferContainingLoc(E->getUnsafeLoc());
-    if (auto sourceInfo = Ctx.SourceMgr.getGeneratedSourceInfo(bufferID)) {
-      if (sourceInfo->macroName == "_SwiftifyImport")
-        return;
+    // Ignore implicitly-generated "unsafe" expressions; they're allowed to be
+    // overly conservative.
+    if (E->isImplicit())
+      return;
+
+    SourceLoc loc = E->getUnsafeLoc();
+    if (loc.isValid()) {
+      // Silence this warning in the expansion of the _SwiftifyImport macro.
+      // This is a hack because it's tricky to determine when to insert "unsafe".
+      unsigned bufferID = Ctx.SourceMgr.findBufferContainingLoc(loc);
+      if (auto sourceInfo = Ctx.SourceMgr.getGeneratedSourceInfo(bufferID)) {
+        if (sourceInfo->macroName == "_SwiftifyImport")
+          return;
+      }
     }
 
     if (auto *SVE = SingleValueStmtExpr::tryDigOutSingleValueStmtExpr(E)) {
       // For an if/switch expression, produce a tailored warning.
-      Ctx.Diags.diagnose(E->getUnsafeLoc(),
+      Ctx.Diags.diagnose(loc,
                          diag::effect_marker_on_single_value_stmt,
                          "unsafe", SVE->getStmt()->getKind())
         .highlight(E->getUnsafeLoc());

lib/Sema/TypeCheckPattern.cpp

@@ -791,7 +791,7 @@ ExprPatternMatchRequest::evaluate(Evaluator &evaluator,
 
   // If there was an "unsafe", put it outside of the match call.
   if (unsafeExpr) {
-    matchCall = UnsafeExpr::createImplicit(ctx, unsafeExpr->getLoc(), matchCall);
+    matchCall = new (ctx) UnsafeExpr(unsafeExpr->getLoc(), matchCall);
   }
 
   return {matchVar, matchCall};

test/Unsafe/codable_synthesis.swift

@@ -0,0 +1,26 @@
+// RUN: %target-typecheck-verify-swift -strict-memory-safety
+
+@unsafe public struct UnsafeStruct: Codable {
+  public var string: String
+}
+
+
+@unsafe public enum UnsafeEnum: Codable {
+case something(Int)
+}
+
+@safe public struct SafeStruct: Codable {
+  public var us: UnsafeStruct
+}
+
+@safe public enum SafeEnum: Codable {
+case something(UnsafeEnum)
+}
+
+@unsafe public class C1: Codable {
+  public var string: String = ""
+}
+
+@unsafe public class C2: C1 {
+  public var otherString: String = ""
+}

test/Unsafe/hashable_synthesis.swift

@@ -0,0 +1,18 @@
+// RUN: %target-typecheck-verify-swift -strict-memory-safety
+
+@unsafe public struct UnsafeStruct: Hashable {
+  public var string: String
+}
+
+
+@unsafe public enum UnsafeEnum: Hashable {
+case something(Int)
+}
+
+@safe public struct SafeStruct: Hashable {
+  public var us: UnsafeStruct
+}
+
+@safe public enum SafeEnum: Hashable {
+case something(UnsafeEnum)
+}