Merge pull request #22821 from bob-wilson/cherry-picks-for-5.1

Cherry pick fixes from 5.0 to swift-5.1-branch

Author: bob-wilson

lib/IDE/ExprContextAnalysis.cpp

@@ -16,6 +16,7 @@
 #include "swift/AST/Decl.h"
 #include "swift/AST/DeclContext.h"
 #include "swift/AST/Expr.h"
+#include "swift/AST/Initializer.h"
 #include "swift/AST/LazyResolver.h"
 #include "swift/AST/Module.h"
 #include "swift/AST/Pattern.h"
@@ -691,7 +692,11 @@ class ExprContextAnalyzer {
       } else
         return false;
     });
-    DC->walkContext(Finder);
+
+    // For 'Initializer' context, we need to look into its parent because it
+    // might constrain the initializer's type.
+    auto analyzeDC = isa<Initializer>(DC) ? DC->getParent() : DC;
+    analyzeDC->walkContext(Finder);
 
     if (Finder.Ancestors.empty())
       return;

lib/SILOptimizer/Transforms/AccessEnforcementOpts.cpp

@@ -988,6 +988,12 @@ static void mergeEndAccesses(BeginAccessInst *parentIns,
 }
 
 static bool canMergeEnd(BeginAccessInst *parentIns, BeginAccessInst *childIns) {
+  // A [read] access cannot be converted to a [modify] without potentially
+  // introducing new conflicts that were previously ignored. Merging read/modify
+  // will require additional data flow information.
+  if (childIns->getAccessKind() != parentIns->getAccessKind())
+    return false;
+
   auto *endP = getSingleEndAccess(parentIns);
   if (!endP)
     return false;
@@ -1087,13 +1093,6 @@ static bool mergeAccesses(
     LLVM_DEBUG(llvm::dbgs()
                << "Merging: " << *childIns << " into " << *parentIns << "\n");
 
-    // Change the type of access of parent:
-    // should be the worse between it and child
-    auto childAccess = childIns->getAccessKind();
-    if (parentIns->getAccessKind() < childAccess) {
-      parentIns->setAccessKind(childAccess);
-    }
-
     // Change the no nested conflict of parent:
     // should be the worst case scenario: we might merge to non-conflicting
     // scopes to a conflicting one. f the new result does not conflict,

test/IDE/complete_in_accessors.swift

@@ -150,7 +150,7 @@ func returnsInt() -> Int {}
 
 // WITH_MEMBER_DECLS_INIT: Begin completions
 // WITH_MEMBER_DECLS_INIT-DAG: Decl[Struct]/CurrModule:          FooStruct[#FooStruct#]{{; name=.+$}}
-// WITH_MEMBER_DECLS_INIT-DAG: Decl[FreeFunction]/CurrModule:    returnsInt()[#Int#]{{; name=.+$}}
+// WITH_MEMBER_DECLS_INIT-DAG: Decl[FreeFunction]/CurrModule/TypeRelation[Identical]: returnsInt()[#Int#]{{; name=.+$}}
 // WITH_MEMBER_DECLS_INIT-DAG: Decl[InstanceMethod]/CurrNominal: instanceFunc({#self: MemberAccessors#})[#(Int) -> Float#]{{; name=.+$}}
 // WITH_MEMBER_DECLS_INIT: End completions
 

test/IDE/complete_unresolved_members.swift

@@ -100,6 +100,8 @@
 // RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=GENERICPARAM_20 | %FileCheck %s -check-prefix=GENERICPARAM_1
 // RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=GENERICPARAM_21 | %FileCheck %s -check-prefix=GENERICPARAM_1
 
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=DECL_MEMBER_INIT_1 | %FileCheck %s -check-prefix=UNRESOLVED_3
+
 enum SomeEnum1 {
   case South
   case North
@@ -645,3 +647,7 @@ func testingGenericParam2<X>(obj: C<X>) {
   obj.t = .#^GENERICPARAM_21^#
   // Same as GENERICPARAM_1.
 }
+
+struct TestingStruct {
+  var value: SomeEnum1 = .#^DECL_MEMBER_INIT_1^#
+}

test/SILOptimizer/access_enforcement_opts.sil

@@ -506,14 +506,19 @@ bb0(%0 : ${ var Int64 }):
 // }
 // Preserve the scope of the outer inout access. Runtime trap expected.
 //
+// FIXME: The optimization should be able to merge these accesses, but
+// it must first prove that no other conflicting read accesses occur
+// within the existing read access scopes.
+//
 // CHECK-LABEL: sil @$s17enforce_with_opts24testInoutWriteEscapeReadyyF : $@convention(thin) () -> () {
 // CHECK: [[BOX:%.*]] = alloc_box ${ var Int64 }, var, name "x"
 // CHECK: [[BOXADR:%.*]] = project_box [[BOX]] : ${ var Int64 }, 0
 // CHECK: [[BEGIN:%.*]] = begin_access [modify] [dynamic] [[BOXADR]] : $*Int64
 // CHECK: apply
-// CHECK-NOT: begin_access
 // CHECK: end_access [[BEGIN]]
-// CHECK-NOT: begin_access
+// CHECK: [[BEGIN2:%.*]] = begin_access [read] [dynamic] [no_nested_conflict] [[BOXADR]] : $*Int64
+// CHECK: load [[BEGIN2]]
+// CHECK: end_access [[BEGIN2]]
 // CHECK-LABEL: } // end sil function '$s17enforce_with_opts24testInoutWriteEscapeReadyyF'
 sil @$s17enforce_with_opts24testInoutWriteEscapeReadyyF : $@convention(thin) () -> () {
 bb0:
@@ -579,14 +584,19 @@ bb0(%0 : ${ var Int64 }):
 // }
 // Preserve the scope of the outer inout access. Runtime trap expected.
 //
+// FIXME: The optimization should be able to merge these accesses, but
+// it must first prove that no other conflicting read accesses occur
+// within the existing read access scopes.
+//
 // CHECK-LABEL: sil @$s17enforce_with_opts020testInoutWriteEscapeF0yyF : $@convention(thin) () -> () {
 // CHECK: [[BOX:%.*]] = alloc_box ${ var Int64 }, var, name "x"
 // CHECK: [[BOXADR:%.*]] = project_box [[BOX]] : ${ var Int64 }, 0
 // CHECK: [[BEGIN:%.*]] = begin_access [modify] [dynamic] [[BOXADR]] : $*Int64
 // CHECK: apply
-// CHECK-NOT: begin_access
 // CHECK: end_access [[BEGIN]]
-// CHECK-NOT: begin_access
+// CHECK: [[BEGIN2:%.*]] = begin_access [read] [dynamic] [no_nested_conflict] [[BOXADR]] : $*Int64
+// CHECK: load [[BEGIN2]]
+// CHECK: end_access [[BEGIN2]]
 // CHECK-LABEL: } // end sil function '$s17enforce_with_opts020testInoutWriteEscapeF0yyF'
 sil @$s17enforce_with_opts020testInoutWriteEscapeF0yyF : $@convention(thin) () -> () {
 bb0:
@@ -999,14 +1009,21 @@ bb0:
 // public func testOldToNewMapWrite) {
 // Checks merging of 3 scopes resulting in a larger modify scope
 //
+// FIXME: The optimization should be able to merge these accesses, but
+// it must first prove that no other conflicting read accesses occur
+// within the existing read access scopes.
+//
 // CHECK-LABEL: sil @testOldToNewMapWrite : $@convention(thin) () -> () {
 // CHECK: [[GLOBAL:%.*]] = global_addr @globalX : $*X
-// CHECK-NEXT: [[BEGIN:%.*]] = begin_access [modify] [dynamic] [[GLOBAL]] : $*X
-// CHECK-NEXT: load [[BEGIN]] : $*X
-// CHECK: store {{.*}} to [[BEGIN]] : $*X
+// CHECK-NEXT: [[BEGIN:%.*]] = begin_access [read] [dynamic] [no_nested_conflict] [[GLOBAL]] : $*X
 // CHECK-NEXT: load [[BEGIN]] : $*X
 // CHECK-NEXT: end_access [[BEGIN]] : $*X
-// CHECK-NOT: begin_access
+// CHECK: [[BEGIN2:%.*]] = begin_access [modify] [dynamic] [no_nested_conflict] [[GLOBAL]] : $*X
+// CHECK-NEXT: store {{.*}} to [[BEGIN2]] : $*X
+// CHECK-NEXT: end_access [[BEGIN2]] : $*X
+// CHECK: [[BEGIN3:%.*]] = begin_access [read] [dynamic] [no_nested_conflict] [[GLOBAL]] : $*X
+// CHECK-NEXT: load [[BEGIN3]] : $*X
+// CHECK-NEXT: end_access [[BEGIN3]] : $*X
 // CHECK-LABEL: } // end sil function 'testOldToNewMapWrite'
 sil @testOldToNewMapWrite : $@convention(thin) () -> () {
 bb0:
@@ -1031,15 +1048,20 @@ bb0:
 // public func testDataFlowAcrossBBs() {
 // Checks merging of scopes across basic blocks - propagating that information
 //
+// FIXME: The optimization should be able to merge these accesses, but
+// it must first prove that no other conflicting read accesses occur
+// within the existing read access scopes.
+//
 // CHECK-LABEL: sil @testDataFlowAcrossBBs : $@convention(thin) () -> () {
 // CHECK: [[GLOBAL:%.*]] = global_addr @globalX : $*X
-// CHECK-NEXT: [[BEGIN:%.*]] = begin_access [modify] [dynamic] [[GLOBAL]] : $*X
+// CHECK-NEXT: [[BEGIN:%.*]] = begin_access [modify] [dynamic] [no_nested_conflict] [[GLOBAL]] : $*X
 // CHECK-NEXT: load [[BEGIN]] : $*X
+// CHECK-NEXT: end_access [[BEGIN]] : $*X
 // CHECK-NEXT: br bb1
 // CHECK: br bb2
-// CHECK: load [[BEGIN]] : $*X
-// CHECK-NEXT: end_access [[BEGIN]] : $*X
-// CHECK-NOT: begin_access
+// CHECK: [[BEGIN2:%.*]] = begin_access [read] [dynamic] [no_nested_conflict] [[GLOBAL]] : $*X
+// CHECK-NEXT: load [[BEGIN2]] : $*X
+// CHECK-NEXT: end_access [[BEGIN2]] : $*X
 // CHECK-LABEL: } // end sil function 'testDataFlowAcrossBBs'
 sil @testDataFlowAcrossBBs : $@convention(thin) () -> () {
 bb0:
@@ -1063,15 +1085,20 @@ bb2:
 // public func testDataFlowAcrossInnerLoop() {
 // Checks merging of scopes across an inner loop
 //
+// FIXME: The optimization should be able to merge these accesses, but
+// it must first prove that no other conflicting read accesses occur
+// within the existing read access scopes.
+//
 // CHECK-LABEL: sil @testDataFlowAcrossInnerLoop : $@convention(thin) () -> () {
 // CHECK: [[GLOBAL:%.*]] = global_addr @globalX : $*X
-// CHECK-NEXT: [[BEGIN:%.*]] = begin_access [modify] [dynamic] [[GLOBAL]] : $*X
+// CHECK-NEXT: [[BEGIN:%.*]] = begin_access [modify] [dynamic] [no_nested_conflict] [[GLOBAL]] : $*X
 // CHECK-NEXT: load [[BEGIN]] : $*X
+// CHECK-NEXT: end_access [[BEGIN]] : $*X
 // CHECK-NEXT: br bb1
 // CHECK: cond_br {{.*}}, bb1, bb2
-// CHECK: load [[BEGIN]] : $*X
-// CHECK-NEXT: end_access [[BEGIN]] : $*X
-// CHECK-NOT: begin_access
+// CHECK: [[BEGIN2:%.*]] = begin_access [read] [dynamic] [no_nested_conflict] [[GLOBAL]] : $*X
+// CHECK-NEXT: load [[BEGIN2]] : $*X
+// CHECK-NEXT: end_access [[BEGIN2]] : $*X
 // CHECK-LABEL: } // end sil function 'testDataFlowAcrossInnerLoop'
 sil @testDataFlowAcrossInnerLoop : $@convention(thin) () -> () {
 bb0:
@@ -1248,13 +1275,19 @@ bb4:
 // Checks detection of irreducible control flow / bail + parts that we *can* merge
 // See disableCrossBlock in the algorithm: this detects this corner case
 //
+// FIXME: The optimization should be able to merge these accesses, but
+// it must first prove that no other conflicting read accesses occur
+// within the existing read access scopes.
+//
 // CHECK-LABEL: sil @testIrreducibleGraph2 : $@convention(thin) () -> () {
 // CHECK: [[GLOBAL:%.*]] = global_addr @globalX : $*X
-// CHECK-NEXT: [[BEGIN:%.*]] = begin_access [modify] [dynamic] [[GLOBAL]] : $*X
+// CHECK-NEXT: [[BEGIN:%.*]] = begin_access [read] [dynamic] [no_nested_conflict] [[GLOBAL]] : $*X
 // CHECK-NEXT: load [[BEGIN]] : $*X
-// CHECK-NEXT: br bb1
-// CHECK: load [[BEGIN]] : $*X
 // CHECK-NEXT: end_access [[BEGIN]] : $*X
+// CHECK-NEXT: br bb1
+// CHECK: [[BEGIN1:%.*]] = begin_access [modify] [dynamic] [no_nested_conflict] [[GLOBAL]] : $*X
+// CHECK-NEXT: load [[BEGIN1]] : $*X
+// CHECK-NEXT: end_access [[BEGIN1]] : $*X
 // CHECK: cond_br {{.*}}, bb2, bb3
 // CHECK: [[BEGIN2:%.*]] = begin_access [read] [dynamic] [no_nested_conflict] [[GLOBAL]] : $*X
 // CHECK-NEXT: load [[BEGIN2]] : $*X
@@ -1390,13 +1423,19 @@ bb0(%0 : $RefElemNoConflictClass):
 // During the merge optimization,
 // Check that we don't merge cross strongly component boundaries for now
 //
+// FIXME: The optimization should be able to merge these accesses, but
+// it must first prove that no other conflicting read accesses occur
+// within the existing read access scopes.
+//
 // CHECK-LABEL: sil @testStronglyConnectedComponent : $@convention(thin) () -> () {
 // CHECK: [[GLOBAL:%.*]] = global_addr @globalX : $*X
-// CHECK-NEXT: [[BEGIN:%.*]] = begin_access [modify] [dynamic] [[GLOBAL]] : $*X
+// CHECK-NEXT: [[BEGIN:%.*]] = begin_access [read] [dynamic] [no_nested_conflict] [[GLOBAL]] : $*X
 // CHECK-NEXT: load [[BEGIN]] : $*X
-// CHECK-NEXT: br bb1
-// CHECK: load [[BEGIN]] : $*X
 // CHECK-NEXT: end_access [[BEGIN]] : $*X
+// CHECK-NEXT: br bb1
+// CHECK: [[BEGIN1:%.*]] = begin_access [modify] [dynamic] [no_nested_conflict] [[GLOBAL]] : $*X
+// CHECK-NEXT: load [[BEGIN1]] : $*X
+// CHECK-NEXT: end_access [[BEGIN1]] : $*X
 // CHECK: cond_br {{.*}}, bb2, bb3
 // CHECK: [[BEGIN2:%.*]] = begin_access [read] [dynamic] [no_nested_conflict] [[GLOBAL]] : $*X
 // CHECK-NEXT: load [[BEGIN2]] : $*X
@@ -1555,3 +1594,59 @@ bb3(%1 : $*X):
   return %10 : $()
 }
 
+// --- rdar://48239213: Fatal access conflict detected.
+//
+// The read/modify pair of accesses in testReadModifyConflictPair
+// cannot be merged without introducing a false conflict.
+
+public class TestClass {
+  @_hasStorage @_hasInitialValue var flags: Int64 { get set }
+}
+
+// CHECK-LABEL: sil hidden [noinline] @readFlags : $@convention(method) (Int64, @guaranteed TestClass) -> Bool {
+// CHECK: bb0(%0 : $Int64, %1 : $TestClass):
+// CHECK:   [[ADR:%.*]] = ref_element_addr %1 : $TestClass, #TestClass.flags
+// CHECK:   begin_access [read] [dynamic] [no_nested_conflict] [[ADR]] : $*Int64
+// CHECK:   load %4 : $*Builtin.Int64
+// CHECK:   end_access
+// CHECK-LABEL: } // end sil function 'readFlags'
+sil hidden [noinline] @readFlags : $@convention(method) (Int64, @guaranteed TestClass) -> Bool {
+bb0(%0 : $Int64, %1 : $TestClass):
+  %2 = ref_element_addr %1 : $TestClass, #TestClass.flags
+  %3 = begin_access [read] [dynamic] [no_nested_conflict] %2 : $*Int64
+  %4 = struct_element_addr %3 : $*Int64, #Int64._value
+  %5 = load %4 : $*Builtin.Int64
+  end_access %3 : $*Int64
+  %7 = struct_extract %0 : $Int64, #Int64._value
+  %8 = builtin "cmp_eq_Int64"(%5 : $Builtin.Int64, %7 : $Builtin.Int64) : $Builtin.Int1
+  %9 = struct $Bool (%8 : $Builtin.Int1)
+  return %9 : $Bool
+}
+
+// CHECK-LABEL: sil @testReadModifyConflictPair : $@convention(method) (@guaranteed TestClass) -> () {
+// CHECK: bb0(%0 : $TestClass):
+// CHECK:   [[ADR:%.*]] = ref_element_addr %0 : $TestClass, #TestClass.flags
+// CHECK:   begin_access [read] [dynamic] [no_nested_conflict] [[ADR]] : $*Int64
+// CHECK:   load
+// CHECK:   end_access
+// CHECK:   apply {{.*}} : $@convention(method) (Int64, @guaranteed TestClass) -> Bool
+// CHECK:   begin_access [modify] [dynamic] [no_nested_conflict] [[ADR]] : $*Int64
+// CHECK:   store
+// CHECK:   end_access
+// CHECK-LABEL: } // end sil function 'testReadModifyConflictPair'
+sil @testReadModifyConflictPair : $@convention(method) (@guaranteed TestClass) -> () {
+bb0(%0 : $TestClass):
+  %1 = ref_element_addr %0 : $TestClass, #TestClass.flags
+  %2 = begin_access [read] [dynamic] %1 : $*Int64
+  %3 = load %2 : $*Int64
+  end_access %2 : $*Int64
+  %5 = function_ref @readFlags : $@convention(method) (Int64, @guaranteed TestClass) -> Bool
+  %6 = apply %5(%3, %0) : $@convention(method) (Int64, @guaranteed TestClass) -> Bool
+  %7 = integer_literal $Builtin.Int64, 3
+  %8 = struct $Int64 (%7 : $Builtin.Int64)
+  %9 = begin_access [modify] [dynamic] %1 : $*Int64
+  store %8 to %9 : $*Int64
+  end_access %9 : $*Int64
+  %12 = tuple ()
+  return %12 : $()
+}