Skip to content

Commit 7258861

Browse files
authored
Merge pull request #6515 from xedin/crasher-28588
[QoI] While merging equivalence classes don't forget to merge fixed types (if any).
2 parents a8da5b8 + fac59ce commit 7258861

File tree

6 files changed

+82
-49
lines changed

6 files changed

+82
-49
lines changed

lib/Sema/CSDiag.cpp

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4296,6 +4296,59 @@ void ConstraintSystem::diagnoseAssignmentFailure(Expr *dest, Type destTy,
42964296
diag::assignment_lhs_not_lvalue);
42974297
}
42984298

4299+
//===----------------------------------------------------------------------===//
4300+
// Diagnose assigning variable to itself.
4301+
//===----------------------------------------------------------------------===//
4302+
4303+
static Decl *findSimpleReferencedDecl(const Expr *E) {
4304+
if (auto *LE = dyn_cast<LoadExpr>(E))
4305+
E = LE->getSubExpr();
4306+
4307+
if (auto *DRE = dyn_cast<DeclRefExpr>(E))
4308+
return DRE->getDecl();
4309+
4310+
return nullptr;
4311+
}
4312+
4313+
static std::pair<Decl *, Decl *> findReferencedDecl(const Expr *E) {
4314+
E = E->getValueProvidingExpr();
4315+
4316+
if (auto *LE = dyn_cast<LoadExpr>(E))
4317+
return findReferencedDecl(LE->getSubExpr());
4318+
4319+
if (auto *AE = dyn_cast<AssignExpr>(E))
4320+
return findReferencedDecl(AE->getDest());
4321+
4322+
if (auto *D = findSimpleReferencedDecl(E))
4323+
return std::make_pair(nullptr, D);
4324+
4325+
if (auto *MRE = dyn_cast<MemberRefExpr>(E)) {
4326+
if (auto *BaseDecl = findSimpleReferencedDecl(MRE->getBase()))
4327+
return std::make_pair(BaseDecl, MRE->getMember().getDecl());
4328+
}
4329+
4330+
return std::make_pair(nullptr, nullptr);
4331+
}
4332+
4333+
bool TypeChecker::diagnoseSelfAssignment(const Expr *E) {
4334+
auto AE = dyn_cast<AssignExpr>(E);
4335+
if (!AE)
4336+
return false;
4337+
4338+
auto LHSDecl = findReferencedDecl(AE->getDest());
4339+
auto RHSDecl = findReferencedDecl(AE->getSrc());
4340+
4341+
if (LHSDecl.second && LHSDecl == RHSDecl) {
4342+
diagnose(AE->getLoc(), LHSDecl.first ? diag::self_assignment_prop
4343+
: diag::self_assignment_var)
4344+
.highlight(AE->getDest()->getSourceRange())
4345+
.highlight(AE->getSrc()->getSourceRange());
4346+
return true;
4347+
}
4348+
4349+
return false;
4350+
}
4351+
42994352
static bool isSymmetricBinaryOperator(const CalleeCandidateInfo &CCI) {
43004353
// If we don't have at least one known candidate, don't trigger.
43014354
if (CCI.candidates.empty()) return false;
@@ -6047,6 +6100,9 @@ bool FailureDiagnosis::visitAssignExpr(AssignExpr *assignExpr) {
60476100
return true;
60486101
}
60496102

6103+
if (CS->TC.diagnoseSelfAssignment(assignExpr))
6104+
return true;
6105+
60506106
// Type check the destination first, so we can coerce the source to it.
60516107
auto destExpr = typeCheckChildIndependently(assignExpr->getDest(),
60526108
TCC_AllowLValue);

lib/Sema/CSGen.cpp

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,19 @@ static bool mergeRepresentativeEquivalenceClasses(ConstraintSystem &CS,
7171
auto rep2 = CS.getRepresentative(tyvar2);
7272

7373
if (rep1 != rep2) {
74+
auto fixedType2 = CS.getFixedType(rep2);
75+
76+
// If the there exists fixed type associated with the second
77+
// type variable, and we simply merge two types together it would
78+
// mean that portion of the constraint graph previously associated
79+
// with that (second) variable is going to be disconnected from its
80+
// new equivalence class, which is going to lead to incorrect solutions,
81+
// so we need to make sure to re-bind fixed to the new representative.
82+
if (fixedType2) {
83+
CS.addConstraint(ConstraintKind::Bind, fixedType2, rep1,
84+
rep1->getImpl().getLocator());
85+
}
86+
7487
CS.mergeEquivalenceClasses(rep1, rep2, /*updateWorkList*/ false);
7588
return true;
7689
}
@@ -137,7 +150,12 @@ namespace {
137150

138151
// We'd like to look at the elements of arrays and dictionaries.
139152
isa<ArrayExpr>(expr) ||
140-
isa<DictionaryExpr>(expr)) {
153+
isa<DictionaryExpr>(expr) ||
154+
155+
// assignment expression can involve anonymous closure parameters
156+
// as source and destination, so it's beneficial for diagnostics if
157+
// we look at the assignment.
158+
isa<AssignExpr>(expr)) {
141159
LinkedExprs.push_back(expr);
142160
return {false, expr};
143161
}

lib/Sema/MiscDiagnostics.cpp

Lines changed: 1 addition & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -38,52 +38,6 @@ static Expr *isImplicitPromotionToOptional(Expr *E) {
3838
return nullptr;
3939
}
4040

41-
//===----------------------------------------------------------------------===//
42-
// Diagnose assigning variable to itself.
43-
//===----------------------------------------------------------------------===//
44-
45-
static Decl *findSimpleReferencedDecl(const Expr *E) {
46-
if (auto *LE = dyn_cast<LoadExpr>(E))
47-
E = LE->getSubExpr();
48-
49-
if (auto *DRE = dyn_cast<DeclRefExpr>(E))
50-
return DRE->getDecl();
51-
52-
return nullptr;
53-
}
54-
55-
static std::pair<Decl *, Decl *> findReferencedDecl(const Expr *E) {
56-
if (auto *LE = dyn_cast<LoadExpr>(E))
57-
E = LE->getSubExpr();
58-
59-
if (auto *D = findSimpleReferencedDecl(E))
60-
return std::make_pair(nullptr, D);
61-
62-
if (auto *MRE = dyn_cast<MemberRefExpr>(E)) {
63-
if (auto *BaseDecl = findSimpleReferencedDecl(MRE->getBase()))
64-
return std::make_pair(BaseDecl, MRE->getMember().getDecl());
65-
}
66-
67-
return std::make_pair(nullptr, nullptr);
68-
}
69-
70-
/// Diagnose assigning variable to itself.
71-
static void diagSelfAssignment(TypeChecker &TC, const Expr *E) {
72-
auto *AE = dyn_cast<AssignExpr>(E);
73-
if (!AE)
74-
return;
75-
76-
auto LHSDecl = findReferencedDecl(AE->getDest());
77-
auto RHSDecl = findReferencedDecl(AE->getSrc());
78-
if (LHSDecl.second && LHSDecl == RHSDecl) {
79-
TC.diagnose(AE->getLoc(), LHSDecl.first ? diag::self_assignment_prop
80-
: diag::self_assignment_var)
81-
.highlight(AE->getDest()->getSourceRange())
82-
.highlight(AE->getSrc()->getSourceRange());
83-
}
84-
}
85-
86-
8741
/// Diagnose syntactic restrictions of expressions.
8842
///
8943
/// - Module values may only occur as part of qualification.
@@ -3832,7 +3786,7 @@ static void diagnoseUnintendedOptionalBehavior(TypeChecker &TC, const Expr *E,
38323786
void swift::performSyntacticExprDiagnostics(TypeChecker &TC, const Expr *E,
38333787
const DeclContext *DC,
38343788
bool isExprStmt) {
3835-
diagSelfAssignment(TC, E);
3789+
TC.diagnoseSelfAssignment(E);
38363790
diagSyntacticUseRestrictions(TC, E, DC, isExprStmt);
38373791
diagRecursivePropertyAccess(TC, E, DC);
38383792
diagnoseImplicitSelfUseInClosure(TC, E, DC);

lib/Sema/TypeChecker.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2039,6 +2039,9 @@ class TypeChecker final : public LazyResolver {
20392039
VarDecl *getSelfForInitDelegationInConstructor(DeclContext *DC,
20402040
UnresolvedDotExpr *ctorRef);
20412041

2042+
/// Diagnose assigning variable to itself.
2043+
bool diagnoseSelfAssignment(const Expr *E);
2044+
20422045
/// When referencing a class initializer, check that the base expression is
20432046
/// either a static metatype or that the initializer is 'required'.
20442047
bool

test/Constraints/closures.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,3 +384,5 @@ func g_2994(arg: Int) -> Double {
384384
C_2994<S_2994>(arg: { (r: S_2994) in f_2994(arg: g_2994(arg: r.dataOffset)) }) // expected-error {{cannot convert value of type 'Double' to expected argument type 'String'}}
385385

386386
let _ = { $0[$1] }(1, 1) // expected-error {{cannot subscript a value of incorrect or ambiguous type}}
387+
let _ = { $0 = ($0 = {}) } // expected-error {{assigning a variable to itself}}
388+
let _ = { $0 = $0 = 42 } // expected-error {{assigning a variable to itself}}

validation-test/compiler_crashers/28588-unreachable-executed-at-swift-lib-sema-csapply-cpp-5770.swift renamed to validation-test/compiler_crashers_fixed/28588-unreachable-executed-at-swift-lib-sema-csapply-cpp-5770.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,5 @@
55
// See https://swift.org/LICENSE.txt for license information
66
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
77

8-
// RUN: not --crash %target-swift-frontend %s -emit-ir
8+
// RUN: not %target-swift-frontend %s -emit-ir
99
{$0=($0={

0 commit comments

Comments
 (0)