Skip to content

Commit 5b537ea

Browse files
DougGregorDougGregor
authored
Merge pull request #59958 from DougGregor/constraint-solver-recoverable-error
[Constraint solver] Improve fix behaviors for better concurrency-related recovery and overloading
2 parents 35ff605 + 6cab661 commit 5b537ea

File tree

10 files changed

+242
-113
lines changed

10 files changed

+242
-113
lines changed

‎include/swift/Sema/CSFix.h

Lines changed: 46 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "swift/AST/Types.h"
2626
#include "swift/Basic/Debug.h"
2727
#include "swift/Sema/ConstraintLocator.h"
28+
#include "swift/Sema/FixBehavior.h"
2829
#include "llvm/ADT/ArrayRef.h"
2930
#include "llvm/ADT/SmallVector.h"
3031
#include "llvm/Support/TrailingObjects.h"
@@ -45,6 +46,7 @@ class ConstraintSystem;
4546
class ConstraintLocator;
4647
class ConstraintLocatorBuilder;
4748
enum class ConversionRestrictionKind;
49+
enum ScoreKind: unsigned int;
4850
class Solution;
4951
struct MemberLookupResult;
5052

@@ -410,13 +412,12 @@ class ConstraintFix {
410412
ConstraintLocator *Locator;
411413

412414
/// The behavior limit to apply to the diagnostics emitted.
413-
DiagnosticBehavior behaviorLimit;
415+
FixBehavior fixBehavior;
414416

415417
public:
416418
ConstraintFix(ConstraintSystem &cs, FixKind kind, ConstraintLocator *locator,
417-
DiagnosticBehavior behaviorLimit =
418-
DiagnosticBehavior::Unspecified)
419-
: CS(cs), Kind(kind), Locator(locator), behaviorLimit(behaviorLimit) {}
419+
FixBehavior fixBehavior = FixBehavior::Error)
420+
: CS(cs), Kind(kind), Locator(locator), fixBehavior(fixBehavior) {}
420421

421422
virtual ~ConstraintFix();
422423

@@ -427,14 +428,27 @@ class ConstraintFix {
427428

428429
FixKind getKind() const { return Kind; }
429430

430-
bool isWarning() const {
431-
return behaviorLimit == DiagnosticBehavior::Warning ||
432-
behaviorLimit == DiagnosticBehavior::Ignore;
431+
/// Whether it is still possible to "apply" a solution containing this kind
432+
/// of fix to get a usable AST.
433+
bool canApplySolution() const {
434+
switch (fixBehavior) {
435+
case FixBehavior::AlwaysWarning:
436+
case FixBehavior::DowngradeToWarning:
437+
case FixBehavior::Suppress:
438+
return true;
439+
440+
case FixBehavior::Error:
441+
return false;
442+
}
433443
}
434444

445+
/// Whether this kind of fix affects the solution score, and which score
446+
/// it affects.
447+
Optional<ScoreKind> affectsSolutionScore() const;
448+
435449
/// The diagnostic behavior limit that will be applied to any emitted
436450
/// diagnostics.
437-
DiagnosticBehavior diagBehaviorLimit() const { return behaviorLimit; }
451+
FixBehavior diagfixBehavior() const { return fixBehavior; }
438452

439453
virtual std::string getName() const = 0;
440454

@@ -680,16 +694,15 @@ class ContextualMismatch : public ConstraintFix {
680694

681695
ContextualMismatch(ConstraintSystem &cs, Type lhs, Type rhs,
682696
ConstraintLocator *locator,
683-
DiagnosticBehavior behaviorLimit)
684-
: ConstraintFix(cs, FixKind::ContextualMismatch, locator, behaviorLimit),
697+
FixBehavior fixBehavior)
698+
: ConstraintFix(cs, FixKind::ContextualMismatch, locator, fixBehavior),
685699
LHS(lhs), RHS(rhs) {}
686700

687701
protected:
688702
ContextualMismatch(ConstraintSystem &cs, FixKind kind, Type lhs, Type rhs,
689703
ConstraintLocator *locator,
690-
DiagnosticBehavior behaviorLimit =
691-
DiagnosticBehavior::Unspecified)
692-
: ConstraintFix(cs, kind, locator, behaviorLimit), LHS(lhs), RHS(rhs) {}
704+
FixBehavior fixBehavior = FixBehavior::Error)
705+
: ConstraintFix(cs, kind, locator, fixBehavior), LHS(lhs), RHS(rhs) {}
693706

694707
public:
695708
std::string getName() const override { return "fix contextual mismatch"; }
@@ -777,9 +790,9 @@ class MarkExplicitlyEscaping final : public ContextualMismatch {
777790
class MarkGlobalActorFunction final : public ContextualMismatch {
778791
MarkGlobalActorFunction(ConstraintSystem &cs, Type lhs, Type rhs,
779792
ConstraintLocator *locator,
780-
DiagnosticBehavior behaviorLimit)
793+
FixBehavior fixBehavior)
781794
: ContextualMismatch(cs, FixKind::MarkGlobalActorFunction, lhs, rhs,
782-
locator, behaviorLimit) {
795+
locator, fixBehavior) {
783796
}
784797

785798
public:
@@ -789,7 +802,7 @@ class MarkGlobalActorFunction final : public ContextualMismatch {
789802

790803
static MarkGlobalActorFunction *create(ConstraintSystem &cs, Type lhs,
791804
Type rhs, ConstraintLocator *locator,
792-
DiagnosticBehavior behaviorLimit);
805+
FixBehavior fixBehavior);
793806

794807
static bool classof(ConstraintFix *fix) {
795808
return fix->getKind() == FixKind::MarkGlobalActorFunction;
@@ -825,9 +838,9 @@ class ForceOptional final : public ContextualMismatch {
825838
class AddSendableAttribute final : public ContextualMismatch {
826839
AddSendableAttribute(ConstraintSystem &cs, FunctionType *fromType,
827840
FunctionType *toType, ConstraintLocator *locator,
828-
DiagnosticBehavior behaviorLimit)
841+
FixBehavior fixBehavior)
829842
: ContextualMismatch(cs, FixKind::AddSendableAttribute, fromType, toType,
830-
locator, behaviorLimit) {
843+
locator, fixBehavior) {
831844
assert(fromType->isSendable() != toType->isSendable());
832845
}
833846

@@ -840,7 +853,7 @@ class AddSendableAttribute final : public ContextualMismatch {
840853
FunctionType *fromType,
841854
FunctionType *toType,
842855
ConstraintLocator *locator,
843-
DiagnosticBehavior behaviorLimit);
856+
FixBehavior fixBehavior);
844857

845858
static bool classof(ConstraintFix *fix) {
846859
return fix->getKind() == FixKind::AddSendableAttribute;
@@ -1403,11 +1416,14 @@ class AllowTypeOrInstanceMember final : public AllowInvalidMemberRef {
14031416
};
14041417

14051418
class AllowInvalidPartialApplication final : public ConstraintFix {
1419+
bool isWarning;
1420+
14061421
AllowInvalidPartialApplication(bool isWarning, ConstraintSystem &cs,
14071422
ConstraintLocator *locator)
14081423
: ConstraintFix(cs, FixKind::AllowInvalidPartialApplication, locator,
1409-
isWarning ? DiagnosticBehavior::Warning
1410-
: DiagnosticBehavior::Unspecified) {}
1424+
isWarning ? FixBehavior::AlwaysWarning
1425+
: FixBehavior::Error),
1426+
isWarning(isWarning) {}
14111427

14121428
public:
14131429
std::string getName() const override {
@@ -2141,10 +2157,9 @@ class AllowArgumentMismatch : public ContextualMismatch {
21412157

21422158
AllowArgumentMismatch(ConstraintSystem &cs, FixKind kind, Type argType,
21432159
Type paramType, ConstraintLocator *locator,
2144-
DiagnosticBehavior behaviorLimit =
2145-
DiagnosticBehavior::Unspecified)
2160+
FixBehavior fixBehavior = FixBehavior::Error)
21462161
: ContextualMismatch(
2147-
cs, kind, argType, paramType, locator, behaviorLimit) {}
2162+
cs, kind, argType, paramType, locator, fixBehavior) {}
21482163

21492164
public:
21502165
std::string getName() const override {
@@ -2292,9 +2307,9 @@ class TreatEphemeralAsNonEphemeral final : public AllowArgumentMismatch {
22922307
TreatEphemeralAsNonEphemeral(ConstraintSystem &cs, ConstraintLocator *locator,
22932308
Type srcType, Type dstType,
22942309
ConversionRestrictionKind conversionKind,
2295-
DiagnosticBehavior behaviorLimit)
2310+
FixBehavior fixBehavior)
22962311
: AllowArgumentMismatch(cs, FixKind::TreatEphemeralAsNonEphemeral,
2297-
srcType, dstType, locator, behaviorLimit),
2312+
srcType, dstType, locator, fixBehavior),
22982313
ConversionKind(conversionKind) {}
22992314

23002315
public:
@@ -2458,7 +2473,7 @@ class AllowCoercionToForceCast final : public ContextualMismatch {
24582473
AllowCoercionToForceCast(ConstraintSystem &cs, Type fromType, Type toType,
24592474
ConstraintLocator *locator)
24602475
: ContextualMismatch(cs, FixKind::AllowCoercionToForceCast, fromType,
2461-
toType, locator, DiagnosticBehavior::Warning) {}
2476+
toType, locator, FixBehavior::AlwaysWarning) {}
24622477

24632478
public:
24642479
std::string getName() const override {
@@ -2572,7 +2587,7 @@ class SpecifyLabelToAssociateTrailingClosure final : public ConstraintFix {
25722587
SpecifyLabelToAssociateTrailingClosure(ConstraintSystem &cs,
25732588
ConstraintLocator *locator)
25742589
: ConstraintFix(cs, FixKind::SpecifyLabelToAssociateTrailingClosure,
2575-
locator, DiagnosticBehavior::Warning) {}
2590+
locator, FixBehavior::AlwaysWarning) {}
25762591

25772592
public:
25782593
std::string getName() const override {
@@ -2742,7 +2757,7 @@ class SpecifyBaseTypeForOptionalUnresolvedMember final : public ConstraintFix {
27422757
DeclNameRef memberName,
27432758
ConstraintLocator *locator)
27442759
: ConstraintFix(cs, FixKind::SpecifyBaseTypeForOptionalUnresolvedMember,
2745-
locator, DiagnosticBehavior::Warning),
2760+
locator, FixBehavior::AlwaysWarning),
27462761
MemberName(memberName) {}
27472762
DeclNameRef MemberName;
27482763

@@ -2773,7 +2788,7 @@ class CheckedCastContextualMismatchWarning : public ContextualMismatch {
27732788
CheckedCastKind kind,
27742789
ConstraintLocator *locator)
27752790
: ContextualMismatch(cs, fixKind, fromType, toType, locator,
2776-
DiagnosticBehavior::Warning),
2791+
FixBehavior::AlwaysWarning),
27772792
CastKind(kind) {}
27782793
CheckedCastKind CastKind;
27792794
};
@@ -2932,7 +2947,7 @@ class AllowTupleLabelMismatch final : public ContextualMismatch {
29322947
AllowTupleLabelMismatch(ConstraintSystem &cs, Type fromType, Type toType,
29332948
ConstraintLocator *locator)
29342949
: ContextualMismatch(cs, FixKind::AllowTupleLabelMismatch, fromType,
2935-
toType, locator, DiagnosticBehavior::Warning) {}
2950+
toType, locator, FixBehavior::AlwaysWarning) {}
29362951

29372952
public:
29382953
std::string getName() const override { return "allow tuple label mismatch"; }

‎include/swift/Sema/ConstraintSystem.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -819,7 +819,7 @@ class FunctionArgApplyInfo {
819819

820820
/// Describes an aspect of a solution that affects its overall score, i.e., a
821821
/// user-defined conversions.
822-
enum ScoreKind {
822+
enum ScoreKind: unsigned int {
823823
// These values are used as indices into a Score value.
824824

825825
/// A fix needs to be applied to the source.

‎include/swift/Sema/FixBehavior.h

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
//===--- FixBehavior.h - Constraint Fix Behavior --------------------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2018 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
//
13+
// This file provides information about how a constraint fix should behavior.
14+
//
15+
//===----------------------------------------------------------------------===//
16+
17+
#ifndef SWIFT_SEMA_FIXBEHAVIOR_H
18+
#define SWIFT_SEMA_FIXBEHAVIOR_H
19+
20+
namespace swift {
21+
namespace constraints {
22+
23+
/// Describes the behavior of the diagnostic corresponding to a given fix.
24+
enum class FixBehavior {
25+
/// The diagnostic is an error, and should prevent constraint application.
26+
Error,
27+
/// The diagnostic is always a warning, which should not prevent constraint
28+
/// application.
29+
AlwaysWarning,
30+
/// The diagnostic should be downgraded to a warning, and not prevent
31+
/// constraint application.
32+
DowngradeToWarning,
33+
/// The diagnostic should be suppressed, and not prevent constraint
34+
/// application.
35+
Suppress
36+
};
37+
38+
}
39+
}
40+
41+
#endif // SWIFT_SEMA_FIXBEHAVIOR_H

‎lib/Sema/CSApply.cpp

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8496,7 +8496,7 @@ bool ConstraintSystem::applySolutionFixes(const Solution &solution) {
84968496

84978497
auto diagnosed =
84988498
primaryFix->coalesceAndDiagnose(solution, secondaryFixes);
8499-
if (primaryFix->isWarning()) {
8499+
if (primaryFix->canApplySolution()) {
85008500
assert(diagnosed && "warnings should always be diagnosed");
85018501
(void)diagnosed;
85028502
} else {
@@ -9148,16 +9148,23 @@ Optional<SolutionApplicationTarget> ConstraintSystem::applySolution(
91489148
Solution &solution, SolutionApplicationTarget target) {
91499149
// If any fixes needed to be applied to arrive at this solution, resolve
91509150
// them to specific expressions.
9151+
unsigned numResolvableFixes = 0;
91519152
if (!solution.Fixes.empty()) {
91529153
if (shouldSuppressDiagnostics())
91539154
return None;
91549155

91559156
bool diagnosedErrorsViaFixes = applySolutionFixes(solution);
9157+
bool canApplySolution = true;
9158+
for (const auto fix : solution.Fixes) {
9159+
if (!fix->canApplySolution())
9160+
canApplySolution = false;
9161+
if (fix->affectsSolutionScore() == SK_Fix && fix->canApplySolution())
9162+
++numResolvableFixes;
9163+
}
9164+
91569165
// If all of the available fixes would result in a warning,
91579166
// we can go ahead and apply this solution to AST.
9158-
if (!llvm::all_of(solution.Fixes, [](const ConstraintFix *fix) {
9159-
return fix->isWarning();
9160-
})) {
9167+
if (!canApplySolution) {
91619168
// If we already diagnosed any errors via fixes, that's it.
91629169
if (diagnosedErrorsViaFixes)
91639170
return None;
@@ -9174,7 +9181,7 @@ Optional<SolutionApplicationTarget> ConstraintSystem::applySolution(
91749181
// produce a fallback diagnostic to highlight the problem.
91759182
{
91769183
const auto &score = solution.getFixedScore();
9177-
if (score.Data[SK_Fix] > 0 || score.Data[SK_Hole] > 0) {
9184+
if (score.Data[SK_Fix] > numResolvableFixes || score.Data[SK_Hole] > 0) {
91789185
maybeProduceFallbackDiagnostic(target);
91799186
return None;
91809187
}

‎lib/Sema/CSDiagnostics.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,22 @@ template <typename... ArgTypes>
103103
InFlightDiagnostic
104104
FailureDiagnostic::emitDiagnosticAt(ArgTypes &&... Args) const {
105105
auto &DE = getASTContext().Diags;
106+
DiagnosticBehavior behaviorLimit;
107+
switch (fixBehavior) {
108+
case FixBehavior::Error:
109+
case FixBehavior::AlwaysWarning:
110+
behaviorLimit = DiagnosticBehavior::Unspecified;
111+
break;
112+
113+
case FixBehavior::DowngradeToWarning:
114+
behaviorLimit = DiagnosticBehavior::Warning;
115+
break;
116+
117+
case FixBehavior::Suppress:
118+
behaviorLimit = DiagnosticBehavior::Ignore;
119+
break;
120+
}
121+
106122
return std::move(DE.diagnose(std::forward<ArgTypes>(Args)...)
107123
.limitBehavior(behaviorLimit));
108124
}

0 commit comments

Comments
 (0)