Merge pull request swiftlang#19849 from atrick/remove-swift3-exclusivity

Remove exclusivity support for Swift 3 mode.

Author: atrick

include/swift/ABI/MetadataValues.h

@@ -1010,8 +1010,6 @@ enum class ExclusivityFlags : uintptr_t {
   // Read or Modify).
   ActionMask       = 0x1,
 
-  // Downgrade exclusivity failures to a warning.
-  WarningOnly      = 0x10,
   // The runtime should track this access to check against subsequent accesses.
   Tracking         = 0x20
 };
@@ -1027,9 +1025,6 @@ static inline ExclusivityFlags getAccessAction(ExclusivityFlags flags) {
   return ExclusivityFlags(uintptr_t(flags)
                         & uintptr_t(ExclusivityFlags::ActionMask));
 }
-static inline bool isWarningOnly(ExclusivityFlags flags) {
-  return uintptr_t(flags) & uintptr_t(ExclusivityFlags::WarningOnly);
-}
 static inline bool isTracking(ExclusivityFlags flags) {
   return uintptr_t(flags) & uintptr_t(ExclusivityFlags::Tracking);
 }

include/swift/AST/DiagnosticsSIL.def

@@ -93,14 +93,6 @@ ERROR(unimplemented_generator_witnesses,none,
       "protocol conformance emission for generator coroutines is unimplemented",
       ())
 
-WARNING(exclusivity_access_required_warn,none,
-        "overlapping accesses to %0, but "
-        "%select{initialization|read|modification|deinitialization}1 requires "
-        "exclusive access; "
-        "%select{consider copying to a local variable|"
-                "consider calling MutableCollection.swapAt(_:_:)}2",
-        (StringRef, unsigned, bool))
-
 ERROR(exclusivity_access_required,none,
       "overlapping accesses to %0, but "
       "%select{initialization|read|modification|deinitialization}1 requires "
@@ -114,11 +106,6 @@ ERROR(exclusivity_access_required_unknown_decl,none,
         "%select{initialization|read|modification|deinitialization}0 requires "
         "exclusive access; consider copying to a local variable", (unsigned))
 
-WARNING(exclusivity_access_required_unknown_decl_warn,none,
-        "overlapping accesses, but "
-        "%select{initialization|read|modification|deinitialization}0 requires "
-"exclusive access; consider copying to a local variable", (unsigned))
-
 NOTE(exclusivity_conflicting_access,none,
      "conflicting access is here", ())
 

lib/IRGen/IRGenSIL.cpp

@@ -4208,14 +4208,9 @@ static ExclusivityFlags getExclusivityAction(SILAccessKind kind) {
 
 static ExclusivityFlags getExclusivityFlags(SILModule &M,
                                             SILAccessKind kind,
-                                            bool noNestedConflict,
-                                            bool fromBuiltin) {
+                                            bool noNestedConflict) {
   auto flags = getExclusivityAction(kind);
 
-  // In old Swift compatibility modes, downgrade this to a warning.
-  if (!fromBuiltin && M.getASTContext().LangOpts.isSwiftVersion3())
-    flags |= ExclusivityFlags::WarningOnly;
-
   if (!noNestedConflict)
     flags |= ExclusivityFlags::Tracking;
 
@@ -4245,7 +4240,7 @@ static SILAccessEnforcement getEffectiveEnforcement(IRGenFunction &IGF,
 template <class BeginAccessInst>
 static ExclusivityFlags getExclusivityFlags(BeginAccessInst *i) {
   return getExclusivityFlags(i->getModule(), i->getAccessKind(),
-                             i->hasNoNestedConflict(), i->isFromBuiltin());
+                             i->hasNoNestedConflict());
 }
 
 void IRGenSILFunction::visitBeginAccessInst(BeginAccessInst *access) {

lib/SILOptimizer/Mandatory/DiagnoseStaticExclusivity.cpp

@@ -542,15 +542,8 @@ static void diagnoseExclusivityViolation(const ConflictingAccess &Violation,
   unsigned AccessKindForMain =
       static_cast<unsigned>(MainAccess.getAccessKind());
 
-  // For now, all exclusivity violations are warning in Swift 3 mode.
-  // Also treat some violations as warnings to allow them to be staged in.
-  bool DiagnoseAsWarning = Ctx.LangOpts.isSwiftVersion3();
-
   if (const ValueDecl *VD = Storage.getDecl(F)) {
     // We have a declaration, so mention the identifier in the diagnostic.
-    auto DiagnosticID = (DiagnoseAsWarning ?
-                         diag::exclusivity_access_required_warn :
-                         diag::exclusivity_access_required);
     SILType BaseType = FirstAccess.getInstruction()->getType().getAddressType();
     SILModule &M = FirstAccess.getInstruction()->getModule();
     std::string PathDescription = getPathDescription(
@@ -570,18 +563,16 @@ static void diagnoseExclusivityViolation(const ConflictingAccess &Violation,
     }
 
     auto D =
-        diagnose(Ctx, MainAccess.getAccessLoc().getSourceLoc(), DiagnosticID,
+        diagnose(Ctx, MainAccess.getAccessLoc().getSourceLoc(),
+                 diag::exclusivity_access_required,
                  PathDescription, AccessKindForMain, SuggestSwapAt);
     D.highlight(RangeForMain);
     if (SuggestSwapAt)
       addSwapAtFixit(D, CallToReplace, Base, SwapIndex1, SwapIndex2,
                      Ctx.SourceMgr);
   } else {
-    auto DiagnosticID = (DiagnoseAsWarning ?
-                         diag::exclusivity_access_required_unknown_decl_warn :
-                         diag::exclusivity_access_required_unknown_decl);
-    diagnose(Ctx, MainAccess.getAccessLoc().getSourceLoc(), DiagnosticID,
-             AccessKindForMain)
+    diagnose(Ctx, MainAccess.getAccessLoc().getSourceLoc(),
+             diag::exclusivity_access_required_unknown_decl, AccessKindForMain)
         .highlight(RangeForMain);
   }
   diagnose(Ctx, NoteAccess.getAccessLoc().getSourceLoc(),

stdlib/private/CMakeLists.txt

@@ -5,6 +5,7 @@ endif()
 if(SWIFT_BUILD_SDK_OVERLAY)
   # SwiftPrivatePthreadExtras makes use of Darwin/Glibc, which is part of the
   # SDK overlay. It can't be built separately from the SDK overlay.
+  add_subdirectory(RuntimeUnittest)
   add_subdirectory(StdlibUnittest)
   add_subdirectory(StdlibUnicodeUnittest)
   add_subdirectory(StdlibCollectionUnittest)

stdlib/private/RuntimeUnittest/CMakeLists.txt

@@ -0,0 +1,18 @@
+set(swift_stdlib_unittest_compile_flags)
+
+add_swift_library(swiftRuntimeUnittest ${SWIFT_STDLIB_LIBRARY_BUILD_TYPES} IS_STDLIB
+  # This file should be listed the first.  Module name is inferred from the
+  # filename.
+  RuntimeUnittest.swift
+
+  ExclusivityTests.cpp
+
+  SWIFT_MODULE_DEPENDS StdlibUnittest
+  SWIFT_MODULE_DEPENDS_LINUX Glibc
+  SWIFT_MODULE_DEPENDS_FREEBSD Glibc
+  SWIFT_MODULE_DEPENDS_CYGWIN Glibc
+  SWIFT_MODULE_DEPENDS_HAIKU Glibc
+  SWIFT_COMPILE_FLAGS ${swift_stdlib_unittest_compile_flags}
+  TARGET_SDKS ALL_POSIX_PLATFORMS
+  INSTALL_IN_COMPONENT stdlib-experimental)
+

unittests/runtime/Exclusivity.cpp renamed to stdlib/private/RuntimeUnittest/ExclusivityTests.cpp

@@ -1,69 +1,59 @@
-//===--- Exclusivity.cpp --------------------------------------------------===//
-//
-// This source file is part of the Swift.org open source project
-//
-// Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors
-// Licensed under Apache License v2.0 with Runtime Library Exception
-//
-// See https://swift.org/LICENSE.txt for license information
-// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
-//
-//===----------------------------------------------------------------------===//
-
 #include "swift/Runtime/Exclusivity.h"
 #include "swift/Runtime/Metadata.h"
-#include "gtest/gtest.h"
 
 using namespace swift;
 
-TEST(TestExclusivity, testNullPC) {
+SWIFT_CC(swift) SWIFT_RUNTIME_LIBRARY_VISIBILITY extern "C"
+void testExclusivityNullPC() {
   ValueBuffer scratch, scratch2;
   long var;
   swift_beginAccess(&var, &scratch,
-                    ExclusivityFlags::WarningOnly | ExclusivityFlags::Read,
+                    ExclusivityFlags::Read | ExclusivityFlags::Tracking,
                     /*pc=*/0);
-  swift_beginAccess(&var, &scratch2,
-                    ExclusivityFlags::WarningOnly | ExclusivityFlags::Modify,
+  swift_beginAccess(&var, &scratch2, ExclusivityFlags::Modify,
                     /*pc=*/0);
   swift_endAccess(&scratch2);
   swift_endAccess(&scratch);
 }
 
-TEST(TestExclusivity, testPCOne) {
+SWIFT_CC(swift) SWIFT_RUNTIME_LIBRARY_VISIBILITY extern "C"
+void testExclusivityPCOne() {
   ValueBuffer scratch, scratch2;
   long var;
   swift_beginAccess(&var, &scratch,
-                    ExclusivityFlags::WarningOnly | ExclusivityFlags::Read,
+                    ExclusivityFlags::Read | ExclusivityFlags::Tracking,
                     /*pc=*/(void *)1);
-  swift_beginAccess(&var, &scratch2,
-                    ExclusivityFlags::WarningOnly | ExclusivityFlags::Modify,
+  swift_beginAccess(&var, &scratch2, ExclusivityFlags::Modify,
                     /*pc=*/(void *)1);
   swift_endAccess(&scratch2);
   swift_endAccess(&scratch);
 }
 
-TEST(TestExclusivity, testBogusPC) {
+SWIFT_CC(swift) SWIFT_RUNTIME_LIBRARY_VISIBILITY extern "C"
+void testExclusivityBogusPC() {
   ValueBuffer scratch, scratch2;
   long var;
   swift_beginAccess(&var, &scratch,
-                    ExclusivityFlags::WarningOnly | ExclusivityFlags::Read,
+                    ExclusivityFlags::Read | ExclusivityFlags::Tracking,
                     /*pc=*/(void *)0xdeadbeefdeadbeefULL);
-  swift_beginAccess(&var, &scratch2,
-                    ExclusivityFlags::WarningOnly | ExclusivityFlags::Modify,
+  swift_beginAccess(&var, &scratch2, ExclusivityFlags::Modify,
                     /*pc=*/(void *)0xdeadbeefdeadbeefULL);
   swift_endAccess(&scratch2);
   swift_endAccess(&scratch);
 }
 
+
 // rdar://32866493
-TEST(TestExclusivity, testNonNested) {
+SWIFT_CC(swift) SWIFT_RUNTIME_LIBRARY_VISIBILITY extern "C"
+void testExclusivityNonNested() {
   const int N = 5;
   ValueBuffer scratches[N];
   long vars[N];
 
   auto begin = [&](unsigned i) {
     assert(i < N);
-    swift_beginAccess(&vars[i], &scratches[i], ExclusivityFlags::Modify, 0);
+    swift_beginAccess(&vars[i], &scratches[i],
+                      ExclusivityFlags::Modify | ExclusivityFlags::Tracking, 0);
   };
   auto end = [&](unsigned i) {
     assert(i < N);

stdlib/private/RuntimeUnittest/RuntimeUnittest.swift

@@ -0,0 +1,44 @@
+//===--- RuntimeUnittest.swift ---------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2018 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+// Make runtime unit tests available to the stdlib unit test harness.
+//
+//===----------------------------------------------------------------------===//
+
+// namespace
+public enum SwiftRuntimeUnitTest {
+  @_silgen_name("testExclusivityNullPC")
+  private static func _testExclusivityNullPC()
+
+  public static func testExclusivityNullPC() {
+    _testExclusivityNullPC()
+  }
+
+  @_silgen_name("testExclusivityPCOne")
+  private static func _testExclusivityPCOne()
+
+  public static func testExclusivityPCOne() {
+    _testExclusivityPCOne()
+  }
+
+  @_silgen_name("testExclusivityBogusPC")
+  private static func _testExclusivityBogusPC()
+
+  public static func testExclusivityBogusPC() {
+    _testExclusivityBogusPC()
+  }
+
+  @_silgen_name("testExclusivityNonNested")
+  private static func _testExclusivityNonNestedPC()
+
+  public static func testExclusivityNonNestedPC() {
+    _testExclusivityNonNestedPC()
+  }
+}

stdlib/public/runtime/Exclusivity.cpp

@@ -50,15 +50,6 @@ LLVM_ATTRIBUTE_ALWAYS_INLINE
 static void reportExclusivityConflict(ExclusivityFlags oldAction, void *oldPC,
                                       ExclusivityFlags newFlags, void *newPC,
                                       void *pointer) {
-  static std::atomic<long> reportedConflicts{0};
-  constexpr long maxReportedConflicts = 100;
-  // Don't report more that 100 conflicts. Hopefully, this will improve
-  // performance in case there are conflicts inside a tight loop.
-  if (reportedConflicts.fetch_add(1, std::memory_order_relaxed) >=
-      maxReportedConflicts) {
-    return;
-  }
-
   constexpr unsigned maxMessageLength = 100;
   constexpr unsigned maxAccessDescriptionLength = 50;
   char message[maxMessageLength];
@@ -87,8 +78,6 @@ static void reportExclusivityConflict(ExclusivityFlags oldAction, void *oldPC,
   constexpr unsigned framesToSkip = 1;
   printCurrentBacktrace(framesToSkip);
 
-  bool keepGoing = isWarningOnly(newFlags);
-
   RuntimeErrorDetails::Thread secondaryThread = {
     .description = oldAccess,
     .numFrames = 1,
@@ -103,17 +92,7 @@ static void reportExclusivityConflict(ExclusivityFlags oldAction, void *oldPC,
     .numExtraThreads = 1,
     .threads = &secondaryThread
   };
-  uintptr_t flags = RuntimeErrorFlagNone;
-  if (!keepGoing)
-    flags = RuntimeErrorFlagFatal;
-  _swift_reportToDebugger(flags, message, &details);
-
-  if (keepGoing) {
-    return;
-  }
-
-  // 0 means no backtrace will be printed.
-  fatalError(0, "Fatal access conflict detected.\n");
+  _swift_reportToDebugger(RuntimeErrorFlagFatal, message, &details);
 }
 
 namespace {
@@ -200,8 +179,8 @@ class AccessSet {
       reportExclusivityConflict(cur->getAccessAction(), cur->PC,
                                 flags, pc, pointer);
 
-      // If we're only warning, don't report multiple conflicts.
-      break;
+      // 0 means no backtrace will be printed.
+      fatalError(0, "Fatal access conflict detected.\n");
     }
     if (!isTracking(flags))
       return false;