Merge pull request #67202 from gottesmm/pr-bf5ceafdd45802deafe22132048f87ea30198725

[reference-bindings] Add Sema checks for inout reference bindings to make sure we can only attach to lvalues

Author: gottesmm

include/swift/AST/DiagnosticsSema.def

@@ -7477,5 +7477,14 @@ NOTE(out_of_order_access_in_init_accessor,none,
      "is called before %1 is initialized",
      (Identifier, Identifier))
 
+//------------------------------------------------------------------------------
+// MARK: Reference Bindings Errors
+//------------------------------------------------------------------------------
+
+ERROR(referencebindings_binding_must_have_initial_value,none,
+      "%0 bindings must have an initial value", (StringRef))
+ERROR(referencebindings_binding_must_be_to_lvalue,none,
+      "%0 bindings must be bound to an lvalue", (StringRef))
+
 #define UNDEFINE_DIAGNOSTIC_MACROS
 #include "DefineDiagnosticMacros.h"

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -2283,6 +2283,22 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
     }
   }
 
+  bool checkBoundInOutVarDecl(PatternBindingDecl *pbd, unsigned patternIndex,
+                              const Pattern *p, VarDecl *vd) {
+    // If our var decl doesn't have an initial value, error. We always want an
+    // inout var decl to have an lvalue initial value that it is binding to.
+    auto *expr = pbd->getInit(patternIndex);
+    assert(expr && "Code assumes that we checked for validity");
+
+    // Next make sure that our initial value was an lvalue.
+    if (!isa<LoadExpr>(expr)) {
+      vd->diagnose(diag::referencebindings_binding_must_be_to_lvalue, "inout");
+      return false;
+    }
+
+    return true;
+  }
+
   void visitPatternBindingDecl(PatternBindingDecl *PBD) {
     DeclContext *DC = PBD->getDeclContext();
 
@@ -2308,6 +2324,11 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
       Pat->forEachVariable([&](VarDecl *var) {
         this->visitBoundVariable(var);
 
+        auto markVarAndPBDInvalid = [PBD, var] {
+          PBD->setInvalid();
+          var->setInvalid();
+        };
+
         if (PBD->isInitialized(i)) {
           // Add the attribute that preserves the "has an initializer" value
           // across module generation, as required for TBDGen.
@@ -2316,6 +2337,15 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
             var->getAttrs().add(new (Ctx)
                                     HasInitialValueAttr(/*IsImplicit=*/true));
           }
+
+          // If we fail to check the bound inout introducer, mark the variable
+          // and pbd invalid().
+          if (var->getIntroducer() == VarDecl::Introducer::InOut) {
+            if (!checkBoundInOutVarDecl(PBD, i, Pat, var)) {
+              markVarAndPBDInvalid();
+            }
+          }
+
           return;
         }
 
@@ -2333,11 +2363,6 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
         if (var->isInvalid() || PBD->isInvalid())
           return;
 
-        auto markVarAndPBDInvalid = [PBD, var] {
-          PBD->setInvalid();
-          var->setInvalid();
-        };
-        
         // Properties with an opaque return type need an initializer to
         // determine their underlying type.
         if (var->getOpaqueResultTypeDecl()) {
@@ -2393,6 +2418,14 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
           markVarAndPBDInvalid();
           return;
         }
+
+        // Inout VarDecls need to have an initializer.
+        if (var->getIntroducer() == VarDecl::Introducer::InOut) {
+          var->diagnose(diag::referencebindings_binding_must_have_initial_value,
+                        "inout");
+          markVarAndPBDInvalid();
+          return;
+        }
       });
     }
 

test/Sema/reference_bindings.swift

@@ -0,0 +1,54 @@
+// RUN: %target-typecheck-verify-swift -enable-experimental-feature ReferenceBindings
+
+var globalValue = String()
+class Klass {
+    var sStored: String = ""
+    var sGetter: String { fatalError() }
+    var sModify: String {
+        _read {
+            fatalError()
+        }
+        _modify {
+            fatalError()
+        }
+    }
+}
+
+struct S : ~Copyable {
+    var sStored: String = ""
+    var sGetter: String { fatalError() }
+    var sModify: String {
+        _read {
+            fatalError()
+        }
+        _modify {
+            fatalError()
+        }
+    }    
+}
+
+func foo(_ arg: String, _ consumingArg: consuming String, _ borrowingArg: borrowing String, _ inoutArg: inout String) {
+    let letValue = 5
+    var varValue = 5
+
+    inout noInitialValue: String // expected-error {{inout bindings must have an initial value}}
+    inout literalX = 5 // expected-error {{inout bindings must be bound to an lvalue}}
+    inout consumingArgX = consumingArg
+    inout borrowingArgX = borrowingArg // expected-error {{inout bindings must be bound to an lvalue}}
+    inout inoutArgX = inoutArg
+    inout letValueX = letValue // expected-error {{inout bindings must be bound to an lvalue}}
+    inout varValueX = varValue
+    inout globalValueX = globalValue
+    let k = Klass()
+    inout storedKlassFieldX = k.sStored
+    inout getterKlassFieldX = k.sGetter // expected-error {{inout bindings must be bound to an lvalue}}
+    inout modifyKlassFieldX = k.sModify
+    let letStruct = S()
+    inout storedStructLetFieldX = letStruct.sStored // expected-error {{inout bindings must be bound to an lvalue}}
+    inout getterStructLetFieldX = letStruct.sGetter // expected-error {{inout bindings must be bound to an lvalue}}
+    inout modifyStructLetFieldX = letStruct.sModify // expected-error {{inout bindings must be bound to an lvalue}}
+    var varStruct = S()
+    inout storedStructVarFieldX = varStruct.sStored
+    inout getterStructVarFieldX = varStruct.sGetter // expected-error {{inout bindings must be bound to an lvalue}}
+    inout modifyStructVarFieldX = varStruct.sModify
+}