Skip to content

Commit 607732f

Browse files
authored
Merge pull request #8281 from DougGregor/redundant-explicit-via-inferred
[GSB] Diagnose explicit constraints made redundant by inferred ones.
2 parents e7390d1 + 2e28311 commit 607732f

27 files changed

+110
-89
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1574,22 +1574,25 @@ ERROR(protocol_typealias_conflict, none,
15741574
WARNING(redundant_conformance_constraint,none,
15751575
"redundant conformance constraint %0: %1", (Type, ProtocolDecl *))
15761576
NOTE(redundant_conformance_here,none,
1577-
"conformance constraint %1: %2 %select{written here|implied here}0",
1578-
(bool, Type, ProtocolDecl *))
1577+
"conformance constraint %1: %2 %select{written here|implied here|"
1578+
"inferred from type here}0",
1579+
(unsigned, Type, ProtocolDecl *))
15791580

15801581
WARNING(redundant_same_type_to_concrete,none,
15811582
"redundant same-type constraint %0 == %1", (Type, Type))
15821583
NOTE(same_type_redundancy_here,none,
1583-
"same-type constraint %1 == %2 %select{written here|implied here}0",
1584-
(bool, Type, Type))
1584+
"same-type constraint %1 == %2 %select{written here|implied here|"
1585+
"inferred from type here}0",
1586+
(unsigned, Type, Type))
15851587
ERROR(requires_superclass_conflict,none,
15861588
"%select{generic parameter |protocol |}0%1 cannot be a subclass of both "
15871589
"%2 and %3", (unsigned, Type, Type, Type))
15881590
WARNING(redundant_superclass_constraint,none,
15891591
"redundant superclass constraint %0 : %1", (Type, Type))
15901592
NOTE(superclass_redundancy_here,none,
1591-
"superclass constraint %1 : %2 %select{written here|implied here}0",
1592-
(bool, Type, Type))
1593+
"superclass constraint %1 : %2 %select{written here|implied here|"
1594+
"inferred from type here}0",
1595+
(unsigned, Type, Type))
15931596

15941597
ERROR(conflicting_layout_constraints,none,
15951598
"%select{generic parameter |protocol |}0%1 has conflicting layout "
@@ -1598,15 +1601,16 @@ ERROR(conflicting_layout_constraints,none,
15981601
WARNING(redundant_layout_constraint,none,
15991602
"redundant layout constraint %0 : %1", (Type, LayoutConstraint))
16001603
NOTE(previous_layout_constraint, none,
1601-
"layout constraint constraint %1 : %2 %select{written here|implied here}0",
1602-
(bool, Type, LayoutConstraint))
1604+
"layout constraint constraint %1 : %2 %select{written here|implied here|"
1605+
"inferred from type here}0",
1606+
(unsigned, Type, LayoutConstraint))
16031607

16041608
WARNING(redundant_same_type_constraint,none,
16051609
"redundant same-type constraint %0 == %1", (Type, Type))
16061610
NOTE(previous_same_type_constraint, none,
16071611
"previous same-type constraint %1 == %2 "
1608-
"%select{written here|implied here}0",
1609-
(bool, Type, Type))
1612+
"%select{written here|implied here|inferred from type here}0",
1613+
(unsigned, Type, Type))
16101614

16111615
ERROR(generic_param_access,none,
16121616
"%0 %select{must be declared %select{"

include/swift/AST/GenericSignatureBuilder.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,7 @@ class GenericSignatureBuilder {
476476
Optional<Diag<unsigned, Type, T, T>>
477477
conflictingDiag,
478478
Diag<Type, T> redundancyDiag,
479-
Diag<bool, Type, T> otherNoteDiag);
479+
Diag<unsigned, Type, T> otherNoteDiag);
480480

481481
/// Check a list of constraints, removing self-derived constraints
482482
/// and diagnosing redundant constraints.
@@ -500,7 +500,7 @@ class GenericSignatureBuilder {
500500
Optional<Diag<unsigned, Type, DiagT, DiagT>>
501501
conflictingDiag,
502502
Diag<Type, DiagT> redundancyDiag,
503-
Diag<bool, Type, DiagT> otherNoteDiag,
503+
Diag<unsigned, Type, DiagT> otherNoteDiag,
504504
llvm::function_ref<DiagT(const T&)> diagValue,
505505
bool removeSelfDerived);
506506

@@ -870,6 +870,9 @@ class GenericSignatureBuilder::RequirementSource final
870870
return getRoot()->kind == Inferred;
871871
}
872872

873+
/// Classify the kind of this source for diagnostic purposes.
874+
unsigned classifyDiagKind() const;
875+
873876
/// Whether the requirement can be derived from something in its path.
874877
///
875878
/// Derived requirements will not be recorded in a minimized generic

lib/AST/GenericSignatureBuilder.cpp

Lines changed: 35 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,12 @@ const void *RequirementSource::getOpaqueStorage3() const {
173173
return nullptr;
174174
}
175175

176+
unsigned RequirementSource::classifyDiagKind() const {
177+
if (isInferredRequirement()) return 2;
178+
if (isDerivedRequirement()) return 1;
179+
return 0;
180+
}
181+
176182
bool RequirementSource::isDerivedRequirement() const {
177183
switch (kind) {
178184
case Explicit:
@@ -2870,12 +2876,13 @@ namespace {
28702876
continue;
28712877
}
28722878

2873-
// We prefer constraints rooted at explicit requirements to ones rooted
2874-
// on inferred requirements.
2879+
// We prefer constraints rooted at inferred requirements to ones rooted
2880+
// on explicit requirements, because the former won't be diagnosed
2881+
// directly.
28752882
bool thisIsInferred = constraint.source->isInferredRequirement();
28762883
bool representativeIsInferred = representativeConstraint->source->isInferredRequirement();
28772884
if (thisIsInferred != representativeIsInferred) {
2878-
if (representativeIsInferred)
2885+
if (thisIsInferred)
28792886
representativeConstraint = constraint;
28802887
continue;
28812888
}
@@ -3178,7 +3185,7 @@ Constraint<T> GenericSignatureBuilder::checkConstraintList(
31783185
Optional<Diag<unsigned, Type, T, T>>
31793186
conflictingDiag,
31803187
Diag<Type, T> redundancyDiag,
3181-
Diag<bool, Type, T> otherNoteDiag) {
3188+
Diag<unsigned, Type, T> otherNoteDiag) {
31823189
return checkConstraintList<T, T>(genericParams, constraints,
31833190
isSuitableRepresentative, checkConstraint,
31843191
conflictingDiag, redundancyDiag,
@@ -3198,7 +3205,7 @@ Constraint<T> GenericSignatureBuilder::checkConstraintList(
31983205
Optional<Diag<unsigned, Type, DiagT, DiagT>>
31993206
conflictingDiag,
32003207
Diag<Type, DiagT> redundancyDiag,
3201-
Diag<bool, Type, DiagT> otherNoteDiag,
3208+
Diag<unsigned, Type, DiagT> otherNoteDiag,
32023209
llvm::function_ref<DiagT(const T&)> diagValue,
32033210
bool removeSelfDerived) {
32043211
assert(!constraints.empty() && "No constraints?");
@@ -3227,7 +3234,7 @@ Constraint<T> GenericSignatureBuilder::checkConstraintList(
32273234

32283235
Diags.diagnose(representativeConstraint->source->getLoc(),
32293236
otherNoteDiag,
3230-
representativeConstraint->source->isDerivedRequirement(),
3237+
representativeConstraint->source->classifyDiagKind(),
32313238
representativeConstraint->archetype->
32323239
getDependentType(genericParams, /*allowUnresolved=*/true),
32333240
diagValue(representativeConstraint->value));
@@ -3302,7 +3309,6 @@ Constraint<T> GenericSignatureBuilder::checkConstraintList(
33023309
// location) complain that it is redundant.
33033310
if (!constraint.source->isDerivedRequirement() &&
33043311
!constraint.source->isInferredRequirement() &&
3305-
!representativeConstraint->source->isInferredRequirement() &&
33063312
constraint.source->getLoc().isValid()) {
33073313
Diags.diagnose(constraint.source->getLoc(),
33083314
redundancyDiag,
@@ -3472,8 +3478,18 @@ namespace {
34723478

34733479
friend bool operator<(const IntercomponentEdge &lhs,
34743480
const IntercomponentEdge &rhs) {
3475-
return std::make_tuple(lhs.source, lhs.target, lhs.constraint)
3476-
< std::make_tuple(rhs.source, rhs.target, rhs.constraint);
3481+
if (lhs.source != rhs.source)
3482+
return lhs.source < rhs.source;
3483+
if (lhs.target != rhs.target)
3484+
return lhs.target < rhs.target;
3485+
3486+
// Prefer non-inferred requirement sources.
3487+
bool lhsIsInferred = lhs.constraint.source->isInferredRequirement();
3488+
bool rhsIsInferred = rhs.constraint.source->isInferredRequirement();
3489+
if (lhsIsInferred != rhsIsInferred)
3490+
return rhsIsInferred;;
3491+
3492+
return lhs.constraint < rhs.constraint;
34773493
}
34783494
};
34793495
}
@@ -3570,10 +3586,8 @@ void GenericSignatureBuilder::checkSameTypeConstraints(
35703586
"Must not be derived");
35713587

35723588
// Ignore inferred requirements; we don't want to diagnose them.
3573-
if (!constraint.source->isInferredRequirement()) {
3574-
intercomponentEdges.push_back(
3575-
IntercomponentEdge(firstComponent, secondComponent, constraint));
3576-
}
3589+
intercomponentEdges.push_back(
3590+
IntercomponentEdge(firstComponent, secondComponent, constraint));
35773591
}
35783592
}
35793593

@@ -3620,6 +3634,10 @@ void GenericSignatureBuilder::checkSameTypeConstraints(
36203634
rhs.constraint.source->getLoc().isInvalid())
36213635
return true;
36223636

3637+
// If the constraint source is inferred, don't diagnose it.
3638+
if (lhs.constraint.source->isInferredRequirement())
3639+
return true;
3640+
36233641
Diags.diagnose(lhs.constraint.source->getLoc(),
36243642
diag::redundant_same_type_constraint,
36253643
lhs.constraint.archetype->getDependentType(
@@ -3628,7 +3646,7 @@ void GenericSignatureBuilder::checkSameTypeConstraints(
36283646
genericParams, true));
36293647
Diags.diagnose(rhs.constraint.source->getLoc(),
36303648
diag::previous_same_type_constraint,
3631-
false,
3649+
rhs.constraint.source->classifyDiagKind(),
36323650
rhs.constraint.archetype->getDependentType(
36333651
genericParams, true),
36343652
rhs.constraint.value->getDependentType(
@@ -3649,6 +3667,7 @@ void GenericSignatureBuilder::checkSameTypeConstraints(
36493667
// not part of the spanning tree.
36503668
if (connected[edge.source] && connected[edge.target]) {
36513669
if (edge.constraint.source->getLoc().isValid() &&
3670+
!edge.constraint.source->isInferredRequirement() &&
36523671
firstEdge.constraint.source->getLoc().isValid()) {
36533672
Diags.diagnose(edge.constraint.source->getLoc(),
36543673
diag::redundant_same_type_constraint,
@@ -3659,7 +3678,7 @@ void GenericSignatureBuilder::checkSameTypeConstraints(
36593678

36603679
Diags.diagnose(firstEdge.constraint.source->getLoc(),
36613680
diag::previous_same_type_constraint,
3662-
false,
3681+
firstEdge.constraint.source->classifyDiagKind(),
36633682
firstEdge.constraint.archetype->getDependentType(
36643683
genericParams, true),
36653684
firstEdge.constraint.value->getDependentType(
@@ -3774,7 +3793,7 @@ void GenericSignatureBuilder::checkSuperclassConstraints(
37743793
representativeConstraint.archetype)) {
37753794
Diags.diagnose(existing->source->getLoc(),
37763795
diag::same_type_redundancy_here,
3777-
existing->source->isDerivedRequirement(),
3796+
existing->source->classifyDiagKind(),
37783797
existing->archetype->getDependentType(
37793798
genericParams,
37803799
/*allowUnresolved=*/true),

stdlib/private/StdlibCollectionUnittest/CheckMutableCollectionType.swift.gyb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,15 +68,15 @@ internal func _mapInPlace<C : MutableCollection>(
6868
}
6969

7070
internal func makeBufferAccessLoggingMutableCollection<
71-
C : MutableCollection & BidirectionalCollection
71+
C
7272
>(wrapping c: C) -> BufferAccessLoggingMutableBidirectionalCollection<C> {
7373
return BufferAccessLoggingMutableBidirectionalCollection(wrapping: c)
7474
}
7575

7676
internal func makeBufferAccessLoggingMutableCollection<
77-
C : MutableCollection & RandomAccessCollection
78-
>(wrapping c: C) -> BufferAccessLoggingMutableBidirectionalCollection<C> {
79-
return BufferAccessLoggingMutableBidirectionalCollection(wrapping: c)
77+
C
78+
>(wrapping c: C) -> BufferAccessLoggingMutableRandomAccessCollection<C> {
79+
return BufferAccessLoggingMutableRandomAccessCollection(wrapping: c)
8080
}
8181

8282
extension TestSuite {

stdlib/private/StdlibCollectionUnittest/LoggingWrappers.swift.gyb

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ extension LoggingType {
4242
//===----------------------------------------------------------------------===//
4343

4444
public class IteratorLog {
45-
public static func dispatchTester<I : IteratorProtocol>(
45+
public static func dispatchTester<I>(
4646
_ iterator: I
4747
) -> LoggingIterator<LoggingIterator<I>> {
4848
return LoggingIterator(wrapping: LoggingIterator(wrapping: iterator))
@@ -72,7 +72,7 @@ public struct LoggingIterator<Base : IteratorProtocol>
7272
//===----------------------------------------------------------------------===//
7373

7474
public class SequenceLog {
75-
public static func dispatchTester<S : Sequence>(
75+
public static func dispatchTester<S>(
7676
_ s: S
7777
) -> LoggingSequence<LoggingSequence<S>> {
7878
return LoggingSequence(wrapping: LoggingSequence(wrapping: s))
@@ -96,7 +96,7 @@ public class SequenceLog {
9696
}
9797

9898
public class CollectionLog : SequenceLog {
99-
public class func dispatchTester<C : Collection>(
99+
public class func dispatchTester<C>(
100100
_ c: C
101101
) -> LoggingCollection<LoggingCollection<C>> {
102102
return LoggingCollection(wrapping: LoggingCollection(wrapping: c))
@@ -123,7 +123,7 @@ public class CollectionLog : SequenceLog {
123123
}
124124

125125
public class BidirectionalCollectionLog : SequenceLog {
126-
public class func dispatchTester<C : BidirectionalCollection>(
126+
public class func dispatchTester<C>(
127127
_ c: C
128128
) -> LoggingBidirectionalCollection<LoggingBidirectionalCollection<C>> {
129129
return LoggingBidirectionalCollection(
@@ -135,7 +135,7 @@ public class BidirectionalCollectionLog : SequenceLog {
135135
}
136136

137137
public class MutableCollectionLog : CollectionLog {
138-
public class func dispatchTester<C : MutableCollection>(
138+
public class func dispatchTester<C>(
139139
_ c: C
140140
) -> LoggingMutableCollection<LoggingMutableCollection<C>> {
141141
return LoggingMutableCollection(
@@ -172,7 +172,7 @@ public class RangeReplaceableCollectionLog : CollectionLog {
172172
public static var replaceSubrange = TypeIndexed(0)
173173
public static var reserveCapacity = TypeIndexed(0)
174174

175-
public class func dispatchTester<C : RangeReplaceableCollection>(
175+
public class func dispatchTester<C>(
176176
_ rrc: C
177177
) -> LoggingRangeReplaceableCollection<LoggingRangeReplaceableCollection<C>> {
178178
return LoggingRangeReplaceableCollection(

stdlib/private/StdlibUnittest/RaceTest.swift

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -413,19 +413,15 @@ class _RaceTestSharedState<RT : RaceTestWithPerTrialData> {
413413
}
414414
}
415415

416-
func _masterThreadStopWorkers<RT : RaceTestWithPerTrialData>(
417-
_ sharedState: _RaceTestSharedState<RT>
418-
) {
416+
func _masterThreadStopWorkers<RT>( _ sharedState: _RaceTestSharedState<RT>) {
419417
// Workers are proceeding to the first barrier in _workerThreadOneTrial.
420418
sharedState.stopNow.store(1)
421419
// Allow workers to proceed past that first barrier. They will then see
422420
// stopNow==true and stop.
423421
sharedState.trialBarrier.wait()
424422
}
425423

426-
func _masterThreadOneTrial<RT : RaceTestWithPerTrialData>(
427-
_ sharedState: _RaceTestSharedState<RT>
428-
) {
424+
func _masterThreadOneTrial<RT>(_ sharedState: _RaceTestSharedState<RT>) {
429425
let racingThreadCount = sharedState.racingThreadCount
430426
let raceDataCount = racingThreadCount * racingThreadCount
431427
let rt = RT()
@@ -485,7 +481,7 @@ func _masterThreadOneTrial<RT : RaceTestWithPerTrialData>(
485481
}
486482
}
487483

488-
func _workerThreadOneTrial<RT : RaceTestWithPerTrialData>(
484+
func _workerThreadOneTrial<RT>(
489485
_ tid: Int, _ sharedState: _RaceTestSharedState<RT>
490486
) -> Bool {
491487
sharedState.trialBarrier.wait()

stdlib/private/StdlibUnittest/StdlibUnittest.swift.gyb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ public func expectGE<T : Comparable>(_ lhs: T, _ rhs: T, ${TRACE}) {
290290
extension Range
291291
% if 'Countable' in OtherRange:
292292
where
293-
Bound : Comparable & _Strideable, Bound.Stride : SignedInteger
293+
Bound : _Strideable, Bound.Stride : SignedInteger
294294
% end
295295
{
296296
internal func _contains(_ other: ${OtherRange}<Bound>) -> Bool {

stdlib/public/SDK/CoreData/NSManagedObjectContext.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@
1414
import Foundation
1515

1616
extension NSManagedObjectContext {
17-
public func fetch<T: NSFetchRequestResult>(_ request: NSFetchRequest<T>) throws -> [T] {
17+
public func fetch<T>(_ request: NSFetchRequest<T>) throws -> [T] {
1818
return try fetch(unsafeDowncast(request, to: NSFetchRequest<NSFetchRequestResult>.self)) as! [T]
1919
}
2020

21-
public func count<T: NSFetchRequestResult>(for request: NSFetchRequest<T>) throws -> Int {
21+
public func count<T>(for request: NSFetchRequest<T>) throws -> Int {
2222
return try count(for: unsafeDowncast(request, to: NSFetchRequest<NSFetchRequestResult>.self))
2323
}
2424
}

stdlib/public/SDK/Foundation/Measurement.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ extension Measurement {
165165
///
166166
/// If `lhs.unit == rhs.unit`, returns `lhs.value == rhs.value`. Otherwise, converts `rhs` to the same unit as `lhs` and then compares the resulting values.
167167
/// - returns: `true` if the measurements are equal.
168-
public static func ==<LeftHandSideType : Unit, RightHandSideType : Unit>(_ lhs: Measurement<LeftHandSideType>, _ rhs: Measurement<RightHandSideType>) -> Bool {
168+
public static func ==<LeftHandSideType, RightHandSideType>(_ lhs: Measurement<LeftHandSideType>, _ rhs: Measurement<RightHandSideType>) -> Bool {
169169
if lhs.unit == rhs.unit {
170170
return lhs.value == rhs.value
171171
} else {
@@ -183,7 +183,7 @@ extension Measurement {
183183

184184
/// Compare two measurements of the same `Unit`.
185185
/// - returns: `true` if the measurements can be compared and the `lhs` is less than the `rhs` converted value.
186-
public static func <<LeftHandSideType : Unit, RightHandSideType : Unit>(lhs: Measurement<LeftHandSideType>, rhs: Measurement<RightHandSideType>) -> Bool {
186+
public static func <<LeftHandSideType, RightHandSideType>(lhs: Measurement<LeftHandSideType>, rhs: Measurement<RightHandSideType>) -> Bool {
187187
if lhs.unit == rhs.unit {
188188
return lhs.value < rhs.value
189189
} else {
@@ -240,7 +240,7 @@ extension NSMeasurement : _HasCustomAnyHashableRepresentation {
240240
// This workaround is required for the time being, because Swift doesn't support covariance for Measurement (26607639)
241241
@available(OSX 10.12, iOS 10.0, watchOS 3.0, tvOS 10.0, *)
242242
extension MeasurementFormatter {
243-
public func string<UnitType: Unit>(from measurement: Measurement<UnitType>) -> String {
243+
public func string<UnitType>(from measurement: Measurement<UnitType>) -> String {
244244
if let result = string(for: measurement) {
245245
return result
246246
} else {

0 commit comments

Comments
 (0)