Skip to content

[Exclusivity] Make static access conflict an error in Swift 4 mode #9373

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions include/swift/AST/DiagnosticsSIL.def
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,22 @@ ERROR(inout_argument_alias,none,
NOTE(previous_inout_alias,none,
"previous aliasing argument", ())

// This is temporarily a warning during staging to make it easier to evaluate.
// The intent is to change it to an error before turning it on by default.
WARNING(exclusivity_access_required,none,
WARNING(exclusivity_access_required_swift3,none,
"simultaneous accesses to %0 %1; "
"%select{initialization|read|modification|deinitialization}2 requires "
"exclusive access", (DescriptiveDeclKind, Identifier, unsigned))

WARNING(exclusivity_access_required_unknown_decl,none,
ERROR(exclusivity_access_required,none,
"simultaneous accesses to %0 %1; "
"%select{initialization|read|modification|deinitialization}2 requires "
"exclusive access", (DescriptiveDeclKind, Identifier, unsigned))

ERROR(exclusivity_access_required_unknown_decl,none,
"simultaneous accesses; "
"%select{initialization|read|modification|deinitialization}0 requires "
"exclusive access", (unsigned))

WARNING(exclusivity_access_required_unknown_decl_swift3,none,
"simultaneous accesses; "
"%select{initialization|read|modification|deinitialization}0 requires "
"exclusive access", (unsigned))
Expand Down
14 changes: 11 additions & 3 deletions lib/SILOptimizer/Mandatory/DiagnoseStaticExclusivity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,8 @@ class AccessInfo {

/// Indicates whether a 'begin_access' requires exclusive access
/// or allows shared access. This needs to be kept in sync with
/// diag::exclusivity_access_required and diag::exclusivity_conflicting_access.
/// diag::exclusivity_access_required, exclusivity_access_required_swift3,
/// and diag::exclusivity_conflicting_access.
enum class ExclusiveOrShared_t : unsigned {
ExclusiveAccess = 0,
SharedAccess = 1
Expand Down Expand Up @@ -382,15 +383,22 @@ static void diagnoseExclusivityViolation(const AccessedStorage &Storage,
AccessForMainDiagnostic->getLoc().getSourceRange();

if (const ValueDecl *VD = Storage.getStorageDecl()) {
// We have a declaration, so mention the identifier in the diagnostic.
auto DiagnosticID = (Ctx.LangOpts.isSwiftVersion3() ?
diag::exclusivity_access_required_swift3 :
diag::exclusivity_access_required);
diagnose(Ctx, AccessForMainDiagnostic->getLoc().getSourceLoc(),
diag::exclusivity_access_required,
DiagnosticID,
VD->getDescriptiveKind(),
VD->getName(),
static_cast<unsigned>(AccessForMainDiagnostic->getAccessKind()))
.highlight(rangeForMain);
} else {
auto DiagnosticID = (Ctx.LangOpts.isSwiftVersion3() ?
diag::exclusivity_access_required_unknown_decl_swift3 :
diag::exclusivity_access_required_unknown_decl);
diagnose(Ctx, AccessForMainDiagnostic->getLoc().getSourceLoc(),
diag::exclusivity_access_required_unknown_decl,
DiagnosticID,
static_cast<unsigned>(AccessForMainDiagnostic->getAccessKind()))
.highlight(rangeForMain);
}
Expand Down
26 changes: 13 additions & 13 deletions test/SILOptimizer/exclusivity_static_diagnostics.sil
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ bb0(%0 : $Int):
%3 = project_box %2 : ${ var Int }, 0
store %0 to [trivial] %3 : $*Int
%4 = function_ref @takesTwoInouts : $@convention(thin) (@inout Int, @inout Int) -> ()
%5 = begin_access [modify] [unknown] %3 : $*Int // expected-warning {{simultaneous accesses; modification requires exclusive access}}
%5 = begin_access [modify] [unknown] %3 : $*Int // expected-error {{simultaneous accesses; modification requires exclusive access}}
%6 = begin_access [modify] [unknown] %3 : $*Int // expected-note {{conflicting access is here}}
%7 = apply %4(%5, %6) : $@convention(thin) (@inout Int, @inout Int) -> ()
end_access %6 : $*Int
Expand All @@ -55,7 +55,7 @@ bb0(%0 : $Int):
%3 = project_box %2 : ${ var Int }, 0
store %0 to [trivial] %3 : $*Int
%4 = function_ref @takesTwoInouts : $@convention(thin) (@inout Int, @inout Int) -> ()
%5 = begin_access [modify] [unknown] %3 : $*Int // expected-warning {{simultaneous accesses; modification requires exclusive access}}
%5 = begin_access [modify] [unknown] %3 : $*Int // expected-error {{simultaneous accesses; modification requires exclusive access}}
%6 = begin_access [modify] [unknown] %5 : $*Int
%7 = begin_access [modify] [unknown] %3 : $*Int // expected-note {{conflicting access is here}}
%8 = apply %4(%5, %6) : $@convention(thin) (@inout Int, @inout Int) -> ()
Expand Down Expand Up @@ -147,7 +147,7 @@ bb0(%0 : $Int, %1 : $Builtin.Int1):
br bb1
bb1:
// Make sure we don't diagnose twice.
%4 = begin_access [modify] [unknown] %3 : $*Int // expected-warning {{simultaneous accesses; modification requires exclusive access}}
%4 = begin_access [modify] [unknown] %3 : $*Int // expected-error {{simultaneous accesses; modification requires exclusive access}}
%5 = begin_access [modify] [unknown] %3 : $*Int // expected-note {{conflicting access is here}}
end_access %5: $*Int
end_access %4: $*Int
Expand Down Expand Up @@ -199,7 +199,7 @@ bb0(%0 : $Int):
%2 = project_box %1 : ${ var Int }, 0
store %0 to [trivial] %2 : $*Int
%4 = begin_access [read] [unknown] %2 : $*Int // expected-note {{conflicting access is here}}
%5 = begin_access [modify] [unknown] %2 : $*Int // expected-warning {{simultaneous accesses; modification requires exclusive access}}
%5 = begin_access [modify] [unknown] %2 : $*Int // expected-error {{simultaneous accesses; modification requires exclusive access}}
end_access %5 : $*Int
end_access %4: $*Int
destroy_value %1 : ${ var Int }
Expand All @@ -213,7 +213,7 @@ bb0(%0 : $Int):
%1 = alloc_box ${ var Int }
%2 = project_box %1 : ${ var Int }, 0
store %0 to [trivial] %2 : $*Int
%4 = begin_access [modify] [unknown] %2 : $*Int // expected-warning {{simultaneous accesses; modification requires exclusive access}}
%4 = begin_access [modify] [unknown] %2 : $*Int // expected-error {{simultaneous accesses; modification requires exclusive access}}
%5 = begin_access [read] [unknown] %2 : $*Int // expected-note {{conflicting access is here}}
end_access %5 : $*Int
end_access %4: $*Int
Expand All @@ -232,7 +232,7 @@ sil hidden @classStoredProperty : $@convention(thin) (ClassWithStoredProperty) -
bb0(%0 : $ClassWithStoredProperty):
%1 = ref_element_addr %0 : $ClassWithStoredProperty, #ClassWithStoredProperty.f

// expected-warning@+1{{simultaneous accesses to var 'f'; modification requires exclusive access}}
// expected-error@+1{{simultaneous accesses to var 'f'; modification requires exclusive access}}
%2 = begin_access [modify] [dynamic] %1 : $*Int
%3 = ref_element_addr %0 : $ClassWithStoredProperty, #ClassWithStoredProperty.f

Expand All @@ -252,7 +252,7 @@ bb0(%0 : $ClassWithStoredProperty):
%2 = begin_borrow %0 : $ClassWithStoredProperty
%3 = ref_element_addr %1 : $ClassWithStoredProperty, #ClassWithStoredProperty.f

// expected-warning@+1{{simultaneous accesses to var 'f'; modification requires exclusive access}}
// expected-error@+1{{simultaneous accesses to var 'f'; modification requires exclusive access}}
%4 = begin_access [modify] [dynamic] %3 : $*Int
%5 = ref_element_addr %2 : $ClassWithStoredProperty, #ClassWithStoredProperty.f

Expand All @@ -279,7 +279,7 @@ bb0(%0 : $Int):
store %0 to [trivial] %3 : $*Int
%4 = copy_value %2 : ${ var Int }
%5 = project_box %4 : ${ var Int }, 0
%6 = begin_access [modify] [unknown] %3 : $*Int // expected-warning {{simultaneous accesses; modification requires exclusive access}}
%6 = begin_access [modify] [unknown] %3 : $*Int // expected-error {{simultaneous accesses; modification requires exclusive access}}
%7 = begin_access [modify] [unknown] %5 : $*Int // expected-note {{conflicting access is here}}
end_access %7 : $*Int
end_access %6: $*Int
Expand All @@ -297,7 +297,7 @@ bb0(%0 : $Int):
%3 = project_box %2 : ${ var Int }, 0
store %0 to [trivial] %3 : $*Int
%4 = project_box %2 : ${ var Int }, 0
%5 = begin_access [modify] [unknown] %3 : $*Int // expected-warning {{simultaneous accesses; modification requires exclusive access}}
%5 = begin_access [modify] [unknown] %3 : $*Int // expected-error {{simultaneous accesses; modification requires exclusive access}}
%6 = begin_access [modify] [unknown] %4 : $*Int // expected-note {{conflicting access is here}}
end_access %6 : $*Int
end_access %5: $*Int
Expand All @@ -315,7 +315,7 @@ sil hidden @modifySameGlobal : $@convention(thin) (Int) -> () {
bb0(%0 : $Int):
%1 = global_addr @global1 :$*Int
%2 = global_addr @global1 :$*Int
%3 = begin_access [modify] [unknown] %1 : $*Int // expected-warning {{simultaneous accesses; modification requires exclusive access}}
%3 = begin_access [modify] [unknown] %1 : $*Int // expected-error {{simultaneous accesses; modification requires exclusive access}}
%4 = begin_access [modify] [unknown] %2 : $*Int // expected-note {{conflicting access is here}}
end_access %4 : $*Int
end_access %3: $*Int
Expand Down Expand Up @@ -348,7 +348,7 @@ bb0(%0 : $Int):
store %0 to [trivial] %3 : $*Int
%4 = function_ref @takesTwoInouts : $@convention(thin) (@inout Int, @inout Int) -> ()
%5 = begin_access [read] [unknown] %3 : $*Int // expected-note {{conflicting access is here}}
%6 = begin_access [modify] [unknown] %3 : $*Int // expected-warning {{simultaneous accesses; modification requires exclusive access}}
%6 = begin_access [modify] [unknown] %3 : $*Int // expected-error {{simultaneous accesses; modification requires exclusive access}}
%7 = begin_access [read] [unknown] %3 : $*Int // no-error
%8 = apply %4(%5, %6) : $@convention(thin) (@inout Int, @inout Int) -> ()
end_access %7 : $*Int
Expand All @@ -368,7 +368,7 @@ bb0(%0 : $Int):
%3 = project_box %2 : ${ var Int }, 0
store %0 to [trivial] %3 : $*Int
%4 = function_ref @takesTwoInouts : $@convention(thin) (@inout Int, @inout Int) -> ()
%5 = begin_access [modify] [unknown] %3 : $*Int // expected-warning {{simultaneous accesses; modification requires exclusive access}}
%5 = begin_access [modify] [unknown] %3 : $*Int // expected-error {{simultaneous accesses; modification requires exclusive access}}
%6 = begin_access [modify] [unknown] %3 : $*Int // expected-note {{conflicting access is here}}
%7 = begin_access [modify] [unknown] %3 : $*Int // no-error
%8 = apply %4(%5, %6) : $@convention(thin) (@inout Int, @inout Int) -> ()
Expand All @@ -392,7 +392,7 @@ bb0(%0 : $Int):
%4 = function_ref @takesTwoInouts : $@convention(thin) (@inout Int, @inout Int) -> ()
%5 = begin_access [modify] [unknown] %3 : $*Int // no-note
end_access %5 : $*Int
%6 = begin_access [modify] [unknown] %3 : $*Int // expected-warning {{simultaneous accesses; modification requires exclusive access}}
%6 = begin_access [modify] [unknown] %3 : $*Int // expected-error {{simultaneous accesses; modification requires exclusive access}}
%7 = begin_access [modify] [unknown] %3 : $*Int // expected-note {{conflicting access is here}}
%8 = apply %4(%5, %6) : $@convention(thin) (@inout Int, @inout Int) -> ()
end_access %7 : $*Int
Expand Down
22 changes: 12 additions & 10 deletions test/SILOptimizer/exclusivity_static_diagnostics.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: %target-swift-frontend -enforce-exclusivity=checked -emit-sil -primary-file %s -o /dev/null -verify
// RUN: %target-swift-frontend -enforce-exclusivity=checked -swift-version 4 -emit-sil -primary-file %s -o /dev/null -verify

import Swift

Expand All @@ -7,25 +7,27 @@ func takesTwoInouts<T>(_ p1: inout T, _ p2: inout T) { }
func simpleInoutDiagnostic() {
var i = 7

// FIXME: This diagnostic should be removed if static enforcement is
// turned on by default.
// expected-error@+4{{inout arguments are not allowed to alias each other}}
// expected-note@+3{{previous aliasing argument}}
// expected-warning@+2{{simultaneous accesses to var 'i'; modification requires exclusive access}}
// expected-error@+2{{simultaneous accesses to var 'i'; modification requires exclusive access}}
// expected-note@+1{{conflicting access is here}}
takesTwoInouts(&i, &i)
}

func inoutOnInoutParameter(p: inout Int) {
// expected-error@+4{{inout arguments are not allowed to alias each other}}
// expected-note@+3{{previous aliasing argument}}
// expected-warning@+2{{simultaneous accesses to parameter 'p'; modification requires exclusive access}}
// expected-error@+2{{simultaneous accesses to parameter 'p'; modification requires exclusive access}}
// expected-note@+1{{conflicting access is here}}
takesTwoInouts(&p, &p)
}

func swapNoSuppression(_ i: Int, _ j: Int) {
var a: [Int] = [1, 2, 3]

// expected-warning@+2{{simultaneous accesses to var 'a'; modification requires exclusive access}}
// expected-error@+2{{simultaneous accesses to var 'a'; modification requires exclusive access}}
// expected-note@+1{{conflicting access is here}}
swap(&a[i], &a[j]) // no-warning
}
Expand All @@ -40,21 +42,21 @@ struct StructWithMutatingMethodThatTakesSelfInout {
mutating func callMutatingMethodThatTakesSelfInout() {
// expected-error@+4{{inout arguments are not allowed to alias each other}}
// expected-note@+3{{previous aliasing argument}}
// expected-warning@+2{{simultaneous accesses to parameter 'self'; modification requires exclusive access}}
// expected-error@+2{{simultaneous accesses to parameter 'self'; modification requires exclusive access}}
// expected-note@+1{{conflicting access is here}}
mutate(&self)
}

mutating func callMutatingMethodThatTakesSelfStoredPropInout() {
// expected-warning@+2{{simultaneous accesses to parameter 'self'; modification requires exclusive access}}
// expected-error@+2{{simultaneous accesses to parameter 'self'; modification requires exclusive access}}
// expected-note@+1{{conflicting access is here}}
mutate(&self.f)
}
}

var globalStruct1 = StructWithMutatingMethodThatTakesSelfInout()
func callMutatingMethodThatTakesGlobalStoredPropInout() {
// expected-warning@+2{{simultaneous accesses to var 'globalStruct1'; modification requires exclusive access}}
// expected-error@+2{{simultaneous accesses to var 'globalStruct1'; modification requires exclusive access}}
// expected-note@+1{{conflicting access is here}}
globalStruct1.mutate(&globalStruct1.f)
}
Expand All @@ -66,13 +68,13 @@ class ClassWithFinalStoredProp {
func callMutatingMethodThatTakesClassStoredPropInout() {
s1.mutate(&s2.f) // no-warning

// expected-warning@+2{{simultaneous accesses to var 's1'; modification requires exclusive access}}
// expected-error@+2{{simultaneous accesses to var 's1'; modification requires exclusive access}}
// expected-note@+1{{conflicting access is here}}
s1.mutate(&s1.f)

let local1 = self

// expected-warning@+2{{simultaneous accesses to var 's1'; modification requires exclusive access}}
// expected-error@+2{{simultaneous accesses to var 's1'; modification requires exclusive access}}
// expected-note@+1{{conflicting access is here}}
local1.s1.mutate(&local1.s1.f)
}
Expand All @@ -82,7 +84,7 @@ func violationWithGenericType<T>(_ p: T) {
var local = p
// expected-error@+4{{inout arguments are not allowed to alias each other}}
// expected-note@+3{{previous aliasing argument}}
// expected-warning@+2{{simultaneous accesses to var 'local'; modification requires exclusive access}}
// expected-error@+2{{simultaneous accesses to var 'local'; modification requires exclusive access}}
// expected-note@+1{{conflicting access is here}}
takesTwoInouts(&local, &local)
}
30 changes: 30 additions & 0 deletions test/SILOptimizer/exclusivity_static_diagnostics_swift3.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// RUN: %target-swift-frontend -enforce-exclusivity=checked -swift-version 3 -emit-sil -primary-file %s -o /dev/null -verify

import Swift

// In Swift 3 compatibility mode, diagnostics for exclusive accesses are
// warnings not errors.

func takesTwoInouts<T>(_ p1: inout T, _ p2: inout T) { }

func simpleInoutDiagnostic() {
var i = 7

// expected-error@+4{{inout arguments are not allowed to alias each other}}
// expected-note@+3{{previous aliasing argument}}
// expected-warning@+2{{simultaneous accesses to var 'i'; modification requires exclusive access}}
// expected-note@+1{{conflicting access is here}}
takesTwoInouts(&i, &i)
}

struct X {
var f = 12
}

func diagnoseOnSameField() {
var x = X()

// expected-warning@+2{{simultaneous accesses to var 'x'; modification requires exclusive access}}
// expected-note@+1{{conflicting access is here}}
takesTwoInouts(&x.f, &x.f)
}