Merge pull request #4578 from timbodeit/fix-mutable-protocol-let-consts

[SILOptimizer] Fix let constant of non-class protocol type being mutable

Author: jckarter

include/swift/AST/DiagnosticsSIL.def

@@ -173,6 +173,9 @@ ERROR(immutable_value_passed_inout,none,
 ERROR(assignment_to_immutable_value,none,
       "immutable value '%0' may not be assigned to",
       (StringRef))
+ERROR(mutating_protocol_witness_method_on_let_constant,none,
+      "cannot perform mutating operation: '%0' is a 'let' constant",
+      (StringRef))
 
 
 // Control flow diagnostics.

lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp

@@ -518,6 +518,7 @@ namespace {
 
 
     void handleStoreUse(unsigned UseID);
+    void handleLoadUse(unsigned UseID);
     void handleInOutUse(const DIMemoryUse &Use);
     void handleEscapeUse(const DIMemoryUse &Use);
 
@@ -807,15 +808,16 @@ void LifetimeChecker::doIt() {
       handleStoreUse(i);
       break;
 
-    case DIUseKind::IndirectIn:
-    case DIUseKind::Load: {
+    case DIUseKind::IndirectIn: {
       bool IsSuperInitComplete, FailedSelfUse;
       // If the value is not definitively initialized, emit an error.
       if (!isInitializedAtUse(Use, &IsSuperInitComplete, &FailedSelfUse))
         handleLoadUseFailure(Use, IsSuperInitComplete, FailedSelfUse);
       break;
     }
-
+    case DIUseKind::Load:
+      handleLoadUse(i);
+      break;
     case DIUseKind::InOutUse:
       handleInOutUse(Use);
       break;
@@ -855,6 +857,55 @@ void LifetimeChecker::doIt() {
     handleConditionalDestroys(ControlVariable);
 }
 
+void LifetimeChecker::handleLoadUse(unsigned UseID) {
+  DIMemoryUse &Use = Uses[UseID];
+  SILInstruction *LoadInst = Use.Inst;
+
+  bool IsSuperInitComplete, FailedSelfUse;
+  // If the value is not definitively initialized, emit an error.
+  if (!isInitializedAtUse(Use, &IsSuperInitComplete, &FailedSelfUse))
+    return handleLoadUseFailure(Use, IsSuperInitComplete, FailedSelfUse);
+
+  // If this is an OpenExistentialAddrInst in preparation for applying
+  // a witness method, analyze its use to make sure, that no mutation of
+  // lvalue let constants occurs.
+  auto* OEAI = dyn_cast<OpenExistentialAddrInst>(LoadInst);
+  if (OEAI != nullptr && TheMemory.isElementLetProperty(Use.FirstElement)) {
+    for (auto OEAUse : OEAI->getUses()) {
+      auto* AI = dyn_cast<ApplyInst>(OEAUse->getUser());
+
+      if (AI == nullptr)
+        // User is not an ApplyInst
+        continue;
+
+      unsigned OperandNumber = OEAUse->getOperandNumber();
+      if (OperandNumber < 1 || OperandNumber > AI->getNumCallArguments())
+        // Not used as a call argument
+        continue;
+
+      unsigned ArgumentNumber = OperandNumber - 1;
+
+      CanSILFunctionType calleeType = AI->getSubstCalleeType();
+      SILParameterInfo parameterInfo = calleeType->getParameters()[ArgumentNumber];
+
+      if (!parameterInfo.isIndirectMutating() ||
+          parameterInfo.getType().isAnyClassReferenceType())
+        continue;
+
+      if (!shouldEmitError(LoadInst))
+        continue;
+
+      std::string PropertyName;
+      auto *VD = TheMemory.getPathStringToElement(Use.FirstElement, PropertyName);
+      diagnose(Module, LoadInst->getLoc(),
+               diag::mutating_protocol_witness_method_on_let_constant, PropertyName);
+
+      if (auto *Var = dyn_cast<VarDecl>(VD)) {
+        Var->emitLetToVarNoteIfSimple(nullptr);
+      }
+    }
+  }
+}
 
 void LifetimeChecker::handleStoreUse(unsigned UseID) {
   DIMemoryUse &InstInfo = Uses[UseID];

test/SILOptimizer/definite_init_lvalue_let_witness_methods.swift

@@ -0,0 +1,61 @@
+// RUN: %target-swift-frontend -emit-sil -disable-objc-attr-requires-foundation-module -verify %s
+
+// High-level tests that DI rejects passing let constants to
+// mutating witness methods
+
+
+// Mark: General Definitions
+
+protocol TestProtocol {
+    var foo: Int { get set }
+}
+
+struct TestStruct: TestProtocol {
+    var foo: Int
+}
+
+// Mark: - Case1: Illegaly mutating let property of class in initializer
+
+class TestClass {
+    let testObject: TestProtocol // expected-note {{change 'let' to 'var' to make it mutable}}
+    init() {
+        testObject = TestStruct(foo: 42)
+        testObject.foo = 666 // expected-error {{cannot perform mutating operation: 'self.testObject' is a 'let' constant}}
+    }
+}
+
+// Mark: - Case2: Illegaly mutating global let constant
+
+let testObject: TestProtocol  // expected-note {{change 'let' to 'var' to make it mutable}}
+testObject = TestStruct(foo: 42)
+
+testObject.foo = 666 // expected-error {{cannot perform mutating operation: 'testObject' is a 'let' constant}}
+
+extension TestProtocol {
+    mutating func messThingsUp() {
+        foo = 666
+    }
+}
+
+// Mark: - Case3: Illegaly muatating let constant in a function scope
+
+let testObject2: TestProtocol  // expected-note {{change 'let' to 'var' to make it mutable}}
+testObject2 = TestStruct(foo: 42)
+testObject2.messThingsUp() // expected-error {{cannot perform mutating operation: 'testObject2' is a 'let' constant}}
+
+func testFunc() {
+    let testObject: TestProtocol // expected-note {{change 'let' to 'var' to make it mutable}}
+
+    testObject = TestStruct(foo: 42)
+    testObject.foo = 666 // expected-error {{cannot perform mutating operation: 'testObject' is a 'let' constant}}
+}
+
+// Mark: - Case4: Illegaly passing a let constants property as an inout parameter
+
+let testObject3: TestProtocol  // expected-note {{change 'let' to 'var' to make it mutable}}
+testObject3 = TestStruct(foo: 42)
+
+func mutateThis(mutatee: inout Int) {
+    mutatee = 666
+}
+mutateThis(mutatee: &testObject3.foo) // expected-error {{cannot perform mutating operation: 'testObject3' is a 'let' constant}}