Merge pull request #69924 from nate-chandler/opaque-values/20231116/1/resilient_empty_enum_case

[SIL] Addr-only enums have non-none ownership.

Author: nate-chandler

lib/SIL/IR/ValueOwnership.cpp

@@ -275,7 +275,6 @@ FORWARDING_OWNERSHIP_INST(UnconditionalCheckedCast)
 FORWARDING_OWNERSHIP_INST(Upcast)
 FORWARDING_OWNERSHIP_INST(UncheckedValueCast)
 FORWARDING_OWNERSHIP_INST(UncheckedEnumData)
-FORWARDING_OWNERSHIP_INST(Enum)
 FORWARDING_OWNERSHIP_INST(MarkDependence)
 // NOTE: init_existential_ref from a reference counting perspective is not
 // considered to be "owned" since it doesn't affect reference counts. That being
@@ -296,6 +295,25 @@ FORWARDING_OWNERSHIP_INST(CopyableToMoveOnlyWrapperValue)
 FORWARDING_OWNERSHIP_INST(MoveOnlyWrapperToCopyableBox)
 #undef FORWARDING_OWNERSHIP_INST
 
+ValueOwnershipKind
+ValueOwnershipKindClassifier::visitEnumInst(EnumInst *I) {
+  if (!I->getModule().useLoweredAddresses() && I->getType().isAddressOnly(*I->getFunction())) {
+    // During address lowering, an address-only enum instruction will eventually
+    // be lowered to inject_enum_addr/init_enum_data_addr, initializing a
+    // non-trivial storage location.  So prior to AddressLowering (in opaque
+    // values mode) such an enum instruction produces a non-trivial value,
+    // without regard to whether it is in a trivial case.  Otherwise, non-trivial
+    // storage would fail to be destroy_addr'd.
+    assert(!I->getType().isTrivial(*I->getFunction()));
+    // An enum instruction is representation changing, so its address-only
+    // operand must be owned.
+    return OwnershipKind::Owned;
+  }
+  return I->getType().isTrivial(*I->getFunction())
+             ? ValueOwnershipKind(OwnershipKind::None)
+             : I->getForwardingOwnershipKind();
+}
+
 ValueOwnershipKind
 ValueOwnershipKindClassifier::visitUncheckedOwnershipConversionInst(
     UncheckedOwnershipConversionInst *I) {

lib/SILGen/SILGenBuilder.cpp

@@ -681,7 +681,7 @@ ManagedValue SILGenBuilder::createManagedOptionalNone(SILLocation loc,
   if (!type.isAddressOnly(getFunction()) ||
       !SGF.silConv.useLoweredAddresses()) {
     SILValue noneValue = createOptionalNone(loc, type);
-    return ManagedValue::forObjectRValueWithoutOwnership(noneValue);
+    return SGF.emitManagedRValueWithCleanup(noneValue);
   }
 
   SILValue tempResult = SGF.emitTemporaryAllocation(loc, type);

lib/SILGen/SILGenExpr.cpp

@@ -1028,8 +1028,8 @@ RValue RValueEmitter::visitNilLiteralExpr(NilLiteralExpr *E, SGFContext C) {
 
     ManagedValue noneValue;
     if (enumTy.isLoadable(SGF.F) || !SGF.silConv.useLoweredAddresses()) {
-      noneValue = ManagedValue::forObjectRValueWithoutOwnership(
-          SGF.B.createEnum(E, SILValue(), noneDecl, enumTy));
+      auto *e = SGF.B.createEnum(E, SILValue(), noneDecl, enumTy);
+      noneValue = SGF.emitManagedRValueWithCleanup(e);
     } else {
       noneValue =
           SGF.B.bufferForExpr(E, enumTy, SGF.getTypeLowering(enumTy), C,

test/SILGen/Inputs/opaque_values_silgen_resilient.swift

@@ -0,0 +1,14 @@
+public class ClassEquatable : Equatable {
+  public static func ==(lhs: ClassEquatable, rhs: ClassEquatable) -> Bool { lhs === rhs }
+}
+
+public enum EnumNontrivialWithEmptyCases : Equatable {
+  case empty
+  case other
+  case loaded(ClassEquatable)
+}
+
+public enum EnumTrivial : Equatable {
+  case empty
+  case other
+}

test/SILGen/opaque_values_silgen_imports_resilient.swift

@@ -0,0 +1,32 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-module -disable-availability-checking -o %t -enable-library-evolution -module-name ResilientLibrary %S/Inputs/opaque_values_silgen_resilient.swift
+// RUN: %target-swift-frontend -enable-sil-opaque-values -emit-silgen -disable-availability-checking -I %t %s | %FileCheck %s
+
+import ResilientLibrary
+
+// Verify that an enum instruction producing a trivial is _still_ destroyed if
+// (1) the enum instruction's type is non-trivial
+// (2) the enum is address-only
+// Necessary because such an enum will be lowered to
+// init_enum_data_addr/inject_enum_addr and there isn't generally enough
+// information to determine whether a destroy_addr is required so it is always
+// required.
+// CHECK-LABEL: sil [ossa] @produceSomeEmptyNontrivialAddronlyEnumInstance : {{.*}} {
+// CHECK:         [[EMPTY_CASE:%[^,]+]] = enum $EnumNontrivialWithEmptyCases, #EnumNontrivialWithEmptyCases.empty!enumelt
+// CHECK:         [[SOME_EMPTY_CASE:%[^,]+]] = enum $Optional<EnumNontrivialWithEmptyCases>, #Optional.some!enumelt, [[EMPTY_CASE]]
+// CHECK:         destroy_value [[SOME_EMPTY_CASE]]
+// CHECK-LABEL: } // end sil function 'produceSomeEmptyNontrivialAddronlyEnumInstance'
+@_silgen_name("produceSomeEmptyNontrivialAddronlyEnumInstance")
+public func produceSomeEmptyNontrivialAddronlyEnumInstance(_ one: EnumNontrivialWithEmptyCases?) {
+  if one != .empty {
+  }
+}
+
+// CHECK-LABEL: sil [ossa] @produceNoneEmptyAddronlyEnumInstance : {{.*}} {
+// CHECK:         [[NONE:%[^,]+]] = enum $Optional<EnumNontrivialWithEmptyCases>, #Optional.none!enumelt 
+// CHECK:         return [[NONE]] : $Optional<EnumNontrivialWithEmptyCases> 
+// CHECK-LABEL: } // end sil function 'produceNoneEmptyAddronlyEnumInstance'
+@_silgen_name("produceNoneEmptyAddronlyEnumInstance")
+public func produceNoneEmptyAddronlyEnumInstance() -> EnumNontrivialWithEmptyCases? {
+  return .none
+}