Skip to content

Commit 4343227

Browse files
committed
[Diagnostics] Determine affected elements early in collection element mismatches
Diagnostics cannot assume that solution would always be applied to the constraint system, so all of the elements affected by the mismatch have to be determined by the fix. Resolves: rdar://85021348
1 parent ce77849 commit 4343227

File tree

4 files changed

+85
-40
lines changed

4 files changed

+85
-40
lines changed

include/swift/Sema/CSFix.h

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1858,18 +1858,35 @@ class RemoveReturn final : public ContextualMismatch {
18581858
}
18591859
};
18601860

1861-
class CollectionElementContextualMismatch final : public ContextualMismatch {
1862-
CollectionElementContextualMismatch(ConstraintSystem &cs, Type srcType,
1863-
Type dstType, ConstraintLocator *locator)
1861+
class CollectionElementContextualMismatch final
1862+
: public ContextualMismatch,
1863+
private llvm::TrailingObjects<CollectionElementContextualMismatch,
1864+
Expr *> {
1865+
friend TrailingObjects;
1866+
1867+
unsigned NumElements;
1868+
1869+
CollectionElementContextualMismatch(ConstraintSystem &cs,
1870+
ArrayRef<Expr *> affectedElements,
1871+
Type srcType, Type dstType,
1872+
ConstraintLocator *locator)
18641873
: ContextualMismatch(cs,
18651874
FixKind::IgnoreCollectionElementContextualMismatch,
1866-
srcType, dstType, locator) {}
1875+
srcType, dstType, locator),
1876+
NumElements(affectedElements.size()) {
1877+
std::uninitialized_copy(affectedElements.begin(), affectedElements.end(),
1878+
getElementBuffer().begin());
1879+
}
18671880

18681881
public:
18691882
std::string getName() const override {
18701883
return "fix collection element contextual mismatch";
18711884
}
18721885

1886+
ArrayRef<Expr *> getElements() const {
1887+
return {getTrailingObjects<Expr *>(), NumElements};
1888+
}
1889+
18731890
bool diagnose(const Solution &solution, bool asNote = false) const override;
18741891

18751892
static CollectionElementContextualMismatch *
@@ -1879,6 +1896,11 @@ class CollectionElementContextualMismatch final : public ContextualMismatch {
18791896
static bool classof(ConstraintFix *fix) {
18801897
return fix->getKind() == FixKind::IgnoreCollectionElementContextualMismatch;
18811898
}
1899+
1900+
private:
1901+
MutableArrayRef<Expr *> getElementBuffer() {
1902+
return {getTrailingObjects<Expr *>(), NumElements};
1903+
}
18821904
};
18831905

18841906
class DefaultGenericArgument final : public ConstraintFix {

lib/Sema/CSDiagnostics.cpp

Lines changed: 24 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -5441,6 +5441,13 @@ bool CollectionElementContextualFailure::diagnoseAsError() {
54415441
auto eltType = getFromType();
54425442
auto contextualType = getToType();
54435443

5444+
auto diagnoseAllOccurances = [&](Diag<Type, Type> diagnostic) {
5445+
assert(AffectedElements.size() > 1);
5446+
for (auto *element : AffectedElements) {
5447+
emitDiagnosticAt(element->getLoc(), diagnostic, eltType, contextualType);
5448+
}
5449+
};
5450+
54445451
auto isFixedToDictionary = [&](ArrayExpr *anchor) {
54455452
return llvm::any_of(getSolution().Fixes, [&](ConstraintFix *fix) {
54465453
auto *fixAnchor = getAsExpr<ArrayExpr>(fix->getAnchor());
@@ -5453,8 +5460,10 @@ bool CollectionElementContextualFailure::diagnoseAsError() {
54535460
Optional<InFlightDiagnostic> diagnostic;
54545461
if (auto *AE = getAsExpr<ArrayExpr>(anchor)) {
54555462
if (!(treatAsDictionary = isFixedToDictionary(AE))) {
5456-
if (diagnoseMergedLiteralElements())
5463+
if (AffectedElements.size() > 1) {
5464+
diagnoseAllOccurances(diag::cannot_convert_array_element);
54575465
return true;
5466+
}
54585467

54595468
diagnostic.emplace(emitDiagnostic(diag::cannot_convert_array_element,
54605469
eltType, contextualType));
@@ -5464,15 +5473,27 @@ bool CollectionElementContextualFailure::diagnoseAsError() {
54645473
if (treatAsDictionary || isExpr<DictionaryExpr>(anchor)) {
54655474
auto eltLoc = locator->castLastElementTo<LocatorPathElt::TupleElement>();
54665475
switch (eltLoc.getIndex()) {
5467-
case 0: // key
5476+
case 0: { // key
5477+
if (AffectedElements.size() > 1) {
5478+
diagnoseAllOccurances(diag::cannot_convert_dict_key);
5479+
return true;
5480+
}
5481+
54685482
diagnostic.emplace(emitDiagnostic(diag::cannot_convert_dict_key, eltType,
54695483
contextualType));
54705484
break;
5485+
}
5486+
5487+
case 1: { // value
5488+
if (AffectedElements.size() > 1) {
5489+
diagnoseAllOccurances(diag::cannot_convert_dict_value);
5490+
return true;
5491+
}
54715492

5472-
case 1: // value
54735493
diagnostic.emplace(emitDiagnostic(diag::cannot_convert_dict_value,
54745494
eltType, contextualType));
54755495
break;
5496+
}
54765497

54775498
default:
54785499
break;
@@ -5508,30 +5529,6 @@ bool CollectionElementContextualFailure::diagnoseAsError() {
55085529
return true;
55095530
}
55105531

5511-
bool CollectionElementContextualFailure::diagnoseMergedLiteralElements() {
5512-
auto elementAnchor = simplifyLocatorToAnchor(getLocator());
5513-
if (!elementAnchor)
5514-
return false;
5515-
5516-
auto *typeVar = getRawType(elementAnchor)->getAs<TypeVariableType>();
5517-
if (!typeVar || !typeVar->getImpl().getAtomicLiteralKind())
5518-
return false;
5519-
5520-
// This element is a literal whose type variable could have been merged with others,
5521-
// but the conversion constraint to the array element type was only placed on one
5522-
// of them. So, we want to emit the error for each element whose type variable is in
5523-
// this equivalence class.
5524-
auto &cs = getConstraintSystem();
5525-
auto node = cs.getRepresentative(typeVar)->getImpl().getGraphNode();
5526-
for (const auto *typeVar : node->getEquivalenceClass()) {
5527-
auto anchor = typeVar->getImpl().getLocator()->getAnchor();
5528-
emitDiagnosticAt(constraints::getLoc(anchor), diag::cannot_convert_array_element,
5529-
getFromType(), getToType());
5530-
}
5531-
5532-
return true;
5533-
}
5534-
55355532
bool MissingContextualConformanceFailure::diagnoseAsError() {
55365533
auto anchor = getAnchor();
55375534
auto path = getLocator()->getPath();

lib/Sema/CSDiagnostics.h

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1761,15 +1761,18 @@ class ExtraneousReturnFailure final : public FailureDiagnostic {
17611761
/// let _: [Int] = ["hello"]
17621762
/// ```
17631763
class CollectionElementContextualFailure final : public ContextualFailure {
1764+
SmallVector<Expr *, 4> AffectedElements;
1765+
17641766
public:
1765-
CollectionElementContextualFailure(const Solution &solution, Type eltType,
1766-
Type contextualType,
1767+
CollectionElementContextualFailure(const Solution &solution,
1768+
ArrayRef<Expr *> affectedElements,
1769+
Type eltType, Type contextualType,
17671770
ConstraintLocator *locator)
1768-
: ContextualFailure(solution, eltType, contextualType, locator) {}
1771+
: ContextualFailure(solution, eltType, contextualType, locator) {
1772+
AffectedElements.append(affectedElements.begin(), affectedElements.end());
1773+
}
17691774

17701775
bool diagnoseAsError() override;
1771-
1772-
bool diagnoseMergedLiteralElements();
17731776
};
17741777

17751778
class MissingContextualConformanceFailure final : public ContextualFailure {

lib/Sema/CSFix.cpp

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1128,17 +1128,40 @@ RemoveReturn *RemoveReturn::create(ConstraintSystem &cs, Type resultTy,
11281128

11291129
bool CollectionElementContextualMismatch::diagnose(const Solution &solution,
11301130
bool asNote) const {
1131-
CollectionElementContextualFailure failure(solution, getFromType(),
1132-
getToType(), getLocator());
1131+
CollectionElementContextualFailure failure(
1132+
solution, getElements(), getFromType(), getToType(), getLocator());
11331133
return failure.diagnose(asNote);
11341134
}
11351135

11361136
CollectionElementContextualMismatch *
11371137
CollectionElementContextualMismatch::create(ConstraintSystem &cs, Type srcType,
11381138
Type dstType,
11391139
ConstraintLocator *locator) {
1140-
return new (cs.getAllocator())
1141-
CollectionElementContextualMismatch(cs, srcType, dstType, locator);
1140+
// It's common for a single literal element to represent types of other
1141+
// literal elements of the same kind, let's check whether that is the case
1142+
// here and record all of the affected positions.
1143+
1144+
SmallVector<Expr *, 4> affected;
1145+
{
1146+
if (auto *elementLoc = getAsExpr(simplifyLocatorToAnchor(locator))) {
1147+
auto *typeVar = cs.getType(elementLoc)->getAs<TypeVariableType>();
1148+
if (typeVar && typeVar->getImpl().getAtomicLiteralKind()) {
1149+
const auto *node =
1150+
cs.getRepresentative(typeVar)->getImpl().getGraphNode();
1151+
for (auto *typeVar : node->getEquivalenceClass()) {
1152+
auto *locator = typeVar->getImpl().getLocator();
1153+
if (auto *eltLoc = getAsExpr(simplifyLocatorToAnchor(locator)))
1154+
affected.push_back(eltLoc);
1155+
}
1156+
}
1157+
}
1158+
}
1159+
1160+
unsigned size = totalSizeToAlloc<Expr *>(affected.size());
1161+
void *mem = cs.getAllocator().Allocate(
1162+
size, alignof(CollectionElementContextualMismatch));
1163+
return new (mem) CollectionElementContextualMismatch(cs, affected, srcType,
1164+
dstType, locator);
11421165
}
11431166

11441167
bool DefaultGenericArgument::coalesceAndDiagnose(

0 commit comments

Comments
 (0)