[sil-parser] Fix harmless bug when parsing ossa.

Specifically, we were preferring the always correct ownership kind specified by
the FunctionType and ignoring what we parsed from the argument. This PR changes
ossa to give a nice error when this is detected and fixes the places where this
tests were written incorrectly.

Author: gottesmm

include/swift/AST/DiagnosticsParse.def

@@ -1,4 +1,3 @@
-
 //===--- DiagnosticsParse.def - Diagnostics Text ----------------*- C++ -*-===//
 //
 // This source file is part of the Swift.org open source project
@@ -485,6 +484,10 @@ ERROR(referenced_value_no_accessor,none,
       "referenced declaration has no %select{getter|setter}0", (unsigned))
 ERROR(expected_sil_value_ownership_kind,none,
       "expected value ownership kind in SIL code", ())
+ERROR(silfunc_and_silarg_have_incompatible_sil_value_ownership,none,
+      "SILFunction and SILArgument have mismatching ValueOwnershipKinds. "
+      "Function type specifies: '@%0'. SIL argument specifies: '@%1'.",
+      (StringRef, StringRef))
 ERROR(expected_sil_colon,none,
       "expected ':' before %0", (StringRef))
 ERROR(expected_sil_tuple_index,none,

include/swift/SIL/SILValue.h

@@ -197,6 +197,8 @@ struct ValueOwnershipKind {
           return acc.getValue().merge(x);
         });
   }
+
+  StringRef asString() const;
 };
 
 llvm::raw_ostream &operator<<(llvm::raw_ostream &os, ValueOwnershipKind Kind);

lib/ParseSIL/ParseSIL.cpp

@@ -395,7 +395,8 @@ namespace {
     bool parseSILOwnership(ValueOwnershipKind &OwnershipKind) {
       // We parse here @ <identifier>.
       if (!P.consumeIf(tok::at_sign)) {
-        // If we fail, we must have @any ownership.
+        // If we fail, we must have @any ownership. We check elsewhere in the
+        // parser that this matches what the function signature wants.
         OwnershipKind = ValueOwnershipKind::Any;
         return false;
       }
@@ -5339,6 +5340,18 @@ bool SILParser::parseSILBasicBlock(SILBuilder &B) {
         SILArgument *Arg;
         if (IsEntry) {
           Arg = BB->createFunctionArgument(Ty);
+          // Today, we construct the ownership kind straight from the function
+          // type. Make sure they are in sync, otherwise bail. We want this to
+          // be an exact check rather than a compatibility check since we do not
+          // want incompatibilities in between @any and other types of ownership
+          // to be ignored.
+          if (F->hasOwnership() && Arg->getOwnershipKind() != OwnershipKind) {
+            auto diagID =
+                diag::silfunc_and_silarg_have_incompatible_sil_value_ownership;
+            P.diagnose(NameLoc, diagID, Arg->getOwnershipKind().asString(),
+                       OwnershipKind.asString());
+            return true;
+          }
         } else {
           Arg = BB->createPhiArgument(Ty, OwnershipKind);
         }

lib/SIL/SILValue.cpp

@@ -188,22 +188,25 @@ ValueOwnershipKind::ValueOwnershipKind(const SILFunction &F, SILType Type,
   }
 }
 
-llvm::raw_ostream &swift::operator<<(llvm::raw_ostream &os,
-                                     ValueOwnershipKind Kind) {
-  switch (Kind) {
+StringRef ValueOwnershipKind::asString() const {
+  switch (Value) {
   case ValueOwnershipKind::Unowned:
-    return os << "unowned";
+    return "unowned";
   case ValueOwnershipKind::Owned:
-    return os << "owned";
+    return "owned";
   case ValueOwnershipKind::Guaranteed:
-    return os << "guaranteed";
+    return "guaranteed";
   case ValueOwnershipKind::Any:
-    return os << "any";
+    return "any";
   }
-
   llvm_unreachable("Unhandled ValueOwnershipKind in switch.");
 }
 
+llvm::raw_ostream &swift::operator<<(llvm::raw_ostream &os,
+                                     ValueOwnershipKind kind) {
+  return os << kind.asString();
+}
+
 Optional<ValueOwnershipKind>
 ValueOwnershipKind::merge(ValueOwnershipKind RHS) const {
   auto LHSVal = Value;

test/SIL/Parser/apply_with_conformance.sil

@@ -22,7 +22,7 @@ sil @_TFV4test1S3foofS0_US_1P__FQ_T_ : $@convention(method) <T where T : P> (@in
 // CHECK: call
 // test.bar (test.S, test.X) -> ()
 sil [ossa] @_TF4test3barFTVS_1SVS_1X_T_ : $@convention(thin) (S, X) -> () {
-bb0(%0 : @unowned $S, %1 : @unowned $X):
+bb0(%0 : $S, %1 : $X):
   debug_value %0 : $S  // let s                   // id: %2
   debug_value %1 : $X  // let x                   // id: %3
   // function_ref test.S.foo (test.S)<A : test.P>(A) -> ()

test/SIL/Parser/ossa_needs_ownership_on_args.sil

@@ -0,0 +1,19 @@
+// RUN: not %sil-opt -verify %s
+
+// Make sure that we error if ossa functions do not parse unless the argument
+// has the proper ownership specifier on it.
+
+import Builtin
+
+sil [ossa] @test1 : $@convention(thin) (@owned Builtin.NativeObject) -> () {
+bb0(%0 : $Builtin.NativeObject): // {{expected-error: SILFunction and SILArgument have mismatching ValueOwnershipKinds. Function type specifies: '@owned'. SIL argument specifies: '@any'.}}
+  destroy_value %0 : $Builtin.NativeObject
+  %9999 = tuple()
+  return %9999 : $()
+}
+
+sil [ossa] @test2 : $@convention(thin) (Builtin.Int32) -> () {
+bb0(%0 : @owned $Builtin.Int32): // {{expected-error: SILFunction and SILArgument have mismatching ValueOwnershipKinds. Function type specifies: '@any'. SIL argument specifies: '@owned'.}}
+  %9999 = tuple()
+  return %9999 : $()
+}

test/SIL/Parser/overloaded_member.sil

@@ -52,7 +52,7 @@ class B {
 }
 
 sil [ossa] @test_overloaded_subscript : $@convention(thin) (@guaranteed B, B.Index) -> () {
-bb0(%0 : $B, %1 : $B.Index):
+bb0(%0 : @guaranteed $B, %1 : $B.Index):
   %reader = class_method %0 : $B, #B.subscript!read.1 : (B) -> (B.Index) -> (), $@convention(method) @yield_once (B.Index, @guaranteed B) -> @yields @guaranteed B.Element
   (%element, %token) = begin_apply %reader(%1, %0) : $@convention(method) @yield_once (B.Index, @guaranteed B) -> @yields @guaranteed B.Element
   end_apply %token

test/SIL/Parser/unmanaged.sil

@@ -16,7 +16,7 @@ bb0(%0 : $*Optional<U>):
 }
 
 sil [ossa] @retain_release : $@convention(thin) (@sil_unmanaged Optional<C>) -> () {
-bb0(%0 : @unowned $@sil_unmanaged Optional<C>):
+bb0(%0 : $@sil_unmanaged Optional<C>):
   %1 = unmanaged_to_ref %0 : $@sil_unmanaged Optional<C> to $Optional<C>
   unmanaged_retain_value %1 : $Optional<C>
   unmanaged_autorelease_value %1 : $Optional<C>

test/SIL/Serialization/Inputs/function_param_convention_input.sil

@@ -1,10 +1,18 @@
 
-struct X {}
+sil_stage raw
+
+import Builtin
+
+struct X {
+  let x: Builtin.NativeObject
+}
 
 // Make sure that we can deserialize an apply with various parameter calling
 // conventions.
 sil [serialized] [ossa] @foo : $@convention(thin) (@in X, @inout X, @in_guaranteed X, @owned X, X, @guaranteed X) -> @out X {
 bb0(%0 : $*X, %1 : $*X, %2 : $*X, %3 : $*X, %4 : @owned $X, %5 : @unowned $X, %6 : @guaranteed $X):
+  copy_addr [take] %1 to [initialization] %0 : $*X
+  destroy_value %4 : $X
   %9999 = tuple()
   return %9999 : $()
 }

test/SIL/Serialization/Recovery/function.sil

@@ -13,7 +13,7 @@ import Types
 // CHECK-LABEL: sil @missingParam : $@convention(thin) (SoonToBeMissing) -> () {
 // CHECK-RECOVERY-NEGATIVE-NOT: sil [ossa] @missingParam
 sil [ossa] @missingParam : $@convention(thin) (SoonToBeMissing) -> () {
-entry(%arg : @unowned $SoonToBeMissing):
+entry(%arg : $SoonToBeMissing):
   %9999 = tuple()
   return %9999 : $()
 }