Skip to content

Commit 7377fff

Browse files
xedinxedin
authored
Merge pull request swiftlang#36947 from xedin/rdar68795727
[Diagnostics] Avoid relying on solution for contextual purpose information
2 parents eca2801 + 71372bc commit 7377fff

File tree

12 files changed

+136
-83
lines changed

12 files changed

+136
-83
lines changed

‎include/swift/Sema/ConstraintLocator.h

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,46 @@ class Pattern;
4141
class SourceManager;
4242
class ProtocolConformance;
4343

44+
/// This specifies the purpose of the contextual type, when specified to
45+
/// typeCheckExpression. This is used for diagnostic generation to produce more
46+
/// specified error messages when the conversion fails.
47+
///
48+
enum ContextualTypePurpose : uint8_t {
49+
CTP_Unused, ///< No contextual type is specified.
50+
CTP_Initialization, ///< Pattern binding initialization.
51+
CTP_ReturnStmt, ///< Value specified to a 'return' statement.
52+
CTP_ReturnSingleExpr, ///< Value implicitly returned from a function.
53+
CTP_YieldByValue, ///< By-value yield operand.
54+
CTP_YieldByReference, ///< By-reference yield operand.
55+
CTP_ThrowStmt, ///< Value specified to a 'throw' statement.
56+
CTP_EnumCaseRawValue, ///< Raw value specified for "case X = 42" in enum.
57+
CTP_DefaultParameter, ///< Default value in parameter 'foo(a : Int = 42)'.
58+
59+
/// Default value in @autoclosure parameter
60+
/// 'foo(a : @autoclosure () -> Int = 42)'.
61+
CTP_AutoclosureDefaultParameter,
62+
63+
CTP_CalleeResult, ///< Constraint is placed on the result of a callee.
64+
CTP_CallArgument, ///< Call to function or operator requires type.
65+
CTP_ClosureResult, ///< Closure result expects a specific type.
66+
CTP_ArrayElement, ///< ArrayExpr wants elements to have a specific type.
67+
CTP_DictionaryKey, ///< DictionaryExpr keys should have a specific type.
68+
CTP_DictionaryValue, ///< DictionaryExpr values should have a specific type.
69+
CTP_CoerceOperand, ///< CoerceExpr operand coerced to specific type.
70+
CTP_AssignSource, ///< AssignExpr source operand coerced to result type.
71+
CTP_SubscriptAssignSource, ///< AssignExpr source operand coerced to subscript
72+
///< result type.
73+
CTP_Condition, ///< Condition expression of various statements e.g.
74+
///< `if`, `for`, `while` etc.
75+
CTP_ForEachStmt, ///< "expression/sequence" associated with 'for-in' loop
76+
///< is expected to conform to 'Sequence' protocol.
77+
CTP_WrappedProperty, ///< Property type expected to match 'wrappedValue' type
78+
CTP_ComposedPropertyWrapper, ///< Composed wrapper type expected to match
79+
///< former 'wrappedValue' type
80+
81+
CTP_CannotFail, ///< Conversion can never fail. abort() if it does.
82+
};
83+
4484
namespace constraints {
4585

4686
class ConstraintSystem;
@@ -851,6 +891,25 @@ class LocatorPathElt::ImplicitConversion final
851891
}
852892
};
853893

894+
class LocatorPathElt::ContextualType final : public StoredIntegerElement<1> {
895+
public:
896+
ContextualType(ContextualTypePurpose purpose)
897+
: StoredIntegerElement(ConstraintLocator::ContextualType,
898+
static_cast<unsigned>(purpose)) {}
899+
900+
ContextualTypePurpose getPurpose() const {
901+
return static_cast<ContextualTypePurpose>(getValue());
902+
}
903+
904+
bool isFor(ContextualTypePurpose purpose) const {
905+
return getPurpose() == purpose;
906+
}
907+
908+
static bool classof(const LocatorPathElt *elt) {
909+
return elt->getKind() == ConstraintLocator::ContextualType;
910+
}
911+
};
912+
854913
namespace details {
855914
template <typename CustomPathElement>
856915
class PathElement {

‎include/swift/Sema/ConstraintLocatorPathElts.def

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ CUSTOM_LOCATOR_PATH_ELT(ClosureBody)
6161
SIMPLE_LOCATOR_PATH_ELT(ConstructorMember)
6262

6363
/// The desired contextual type passed in to the constraint system.
64-
SIMPLE_LOCATOR_PATH_ELT(ContextualType)
64+
CUSTOM_LOCATOR_PATH_ELT(ContextualType)
6565

6666
/// A result of an expression involving dynamic lookup.
6767
SIMPLE_LOCATOR_PATH_ELT(DynamicLookupResult)

‎include/swift/Sema/ConstraintSystem.h

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -90,46 +90,6 @@ void *operator new(size_t bytes, swift::constraints::ConstraintSystem& cs,
9090

9191
namespace swift {
9292

93-
/// This specifies the purpose of the contextual type, when specified to
94-
/// typeCheckExpression. This is used for diagnostic generation to produce more
95-
/// specified error messages when the conversion fails.
96-
///
97-
enum ContextualTypePurpose {
98-
CTP_Unused, ///< No contextual type is specified.
99-
CTP_Initialization, ///< Pattern binding initialization.
100-
CTP_ReturnStmt, ///< Value specified to a 'return' statement.
101-
CTP_ReturnSingleExpr, ///< Value implicitly returned from a function.
102-
CTP_YieldByValue, ///< By-value yield operand.
103-
CTP_YieldByReference, ///< By-reference yield operand.
104-
CTP_ThrowStmt, ///< Value specified to a 'throw' statement.
105-
CTP_EnumCaseRawValue, ///< Raw value specified for "case X = 42" in enum.
106-
CTP_DefaultParameter, ///< Default value in parameter 'foo(a : Int = 42)'.
107-
108-
/// Default value in @autoclosure parameter
109-
/// 'foo(a : @autoclosure () -> Int = 42)'.
110-
CTP_AutoclosureDefaultParameter,
111-
112-
CTP_CalleeResult, ///< Constraint is placed on the result of a callee.
113-
CTP_CallArgument, ///< Call to function or operator requires type.
114-
CTP_ClosureResult, ///< Closure result expects a specific type.
115-
CTP_ArrayElement, ///< ArrayExpr wants elements to have a specific type.
116-
CTP_DictionaryKey, ///< DictionaryExpr keys should have a specific type.
117-
CTP_DictionaryValue, ///< DictionaryExpr values should have a specific type.
118-
CTP_CoerceOperand, ///< CoerceExpr operand coerced to specific type.
119-
CTP_AssignSource, ///< AssignExpr source operand coerced to result type.
120-
CTP_SubscriptAssignSource, ///< AssignExpr source operand coerced to subscript
121-
///< result type.
122-
CTP_Condition, ///< Condition expression of various statements e.g.
123-
///< `if`, `for`, `while` etc.
124-
CTP_ForEachStmt, ///< "expression/sequence" associated with 'for-in' loop
125-
///< is expected to conform to 'Sequence' protocol.
126-
CTP_WrappedProperty, ///< Property type expected to match 'wrappedValue' type
127-
CTP_ComposedPropertyWrapper, ///< Composed wrapper type expected to match
128-
///< former 'wrappedValue' type
129-
130-
CTP_CannotFail, ///< Conversion can never fail. abort() if it does.
131-
};
132-
13393
/// Specify how we handle the binding of underconstrained (free) type variables
13494
/// within a solution to a constraint system.
13595
enum class FreeTypeVariableBinding {

‎lib/Sema/BuilderTransform.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -767,7 +767,7 @@ class BuilderClosureVisitor
767767
// If needed, generate constraints for everything in the case statement.
768768
if (cs) {
769769
auto locator = cs->getConstraintLocator(
770-
subjectExpr, LocatorPathElt::ContextualType());
770+
subjectExpr, LocatorPathElt::ContextualType(CTP_Initialization));
771771
Type subjectType = cs->getType(subjectExpr);
772772

773773
if (cs->generateConstraints(caseStmt, dc, subjectType, locator)) {
@@ -851,7 +851,7 @@ class BuilderClosureVisitor
851851
cs->addConstraint(
852852
ConstraintKind::Equal, cs->getType(arrayInitExpr), arrayType,
853853
cs->getConstraintLocator(
854-
arrayInitExpr, LocatorPathElt::ContextualType()));
854+
arrayInitExpr, LocatorPathElt::ContextualType(CTP_Initialization)));
855855

856856
// Form a call to Array.append(_:) to add the result of executing each
857857
// iteration of the loop body to the array formed above.

‎lib/Sema/CSApply.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8336,8 +8336,8 @@ static Optional<SolutionApplicationTarget> applySolutionToInitialization(
83368336

83378337
// Convert the initializer to the type of the pattern.
83388338
auto &cs = solution.getConstraintSystem();
8339-
auto locator =
8340-
cs.getConstraintLocator(initializer, LocatorPathElt::ContextualType());
8339+
auto locator = cs.getConstraintLocator(
8340+
initializer, LocatorPathElt::ContextualType(CTP_Initialization));
83418341
initializer = solution.coerceToType(initializer, initType, locator);
83428342
if (!initializer)
83438343
return None;

‎lib/Sema/CSDiagnostics.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2447,8 +2447,9 @@ bool ContextualFailure::diagnoseConversionToNil() const {
24472447

24482448
Optional<ContextualTypePurpose> CTP;
24492449
// Easy case were failure has been identified as contextual already.
2450-
if (locator->isLastElement<LocatorPathElt::ContextualType>()) {
2451-
CTP = getContextualTypePurpose();
2450+
if (auto contextualTy =
2451+
locator->getLastElementAs<LocatorPathElt::ContextualType>()) {
2452+
CTP = contextualTy->getPurpose();
24522453
} else {
24532454
// Here we need to figure out where where `nil` is located.
24542455
// It could be e.g. an argument to a subscript/call, assignment
@@ -6692,8 +6693,9 @@ bool UnableToInferClosureParameterType::diagnoseAsError() {
66926693

66936694
// If there is a contextual mismatch associated with this
66946695
// closure, let's not diagnose any parameter type issues.
6695-
if (hasFixFor(solution, getConstraintLocator(
6696-
closure, LocatorPathElt::ContextualType())))
6696+
if (hasFixFor(solution,
6697+
getConstraintLocator(closure, LocatorPathElt::ContextualType(
6698+
CTP_Initialization))))
66976699
return false;
66986700

66996701
if (auto *parentExpr = findParentExpr(closure)) {

‎lib/Sema/CSDiagnostics.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -594,8 +594,11 @@ class ContextualFailure : public FailureDiagnostic {
594594
ConstraintLocator *locator)
595595
: ContextualFailure(
596596
solution,
597-
solution.getConstraintSystem().getContextualTypePurpose(
598-
locator->getAnchor()),
597+
locator->isForContextualType()
598+
? locator->castLastElementTo<LocatorPathElt::ContextualType>()
599+
.getPurpose()
600+
: solution.getConstraintSystem().getContextualTypePurpose(
601+
locator->getAnchor()),
599602
lhs, rhs, locator) {}
600603

601604
ContextualFailure(const Solution &solution, ContextualTypePurpose purpose,

‎lib/Sema/CSFix.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -391,15 +391,16 @@ AllowWrappedValueMismatch *AllowWrappedValueMismatch::create(ConstraintSystem &c
391391
/// and the contextual type.
392392
static Optional<std::tuple<ContextualTypePurpose, Type, Type>>
393393
getStructuralTypeContext(const Solution &solution, ConstraintLocator *locator) {
394-
if (locator->findLast<LocatorPathElt::ContextualType>()) {
394+
if (auto contextualTypeElt =
395+
locator->findLast<LocatorPathElt::ContextualType>()) {
395396
assert(locator->isLastElement<LocatorPathElt::ContextualType>() ||
396397
locator->isLastElement<LocatorPathElt::FunctionArgument>());
397398

398399
auto &cs = solution.getConstraintSystem();
399400
auto anchor = locator->getAnchor();
400401
auto contextualType = cs.getContextualType(anchor);
401402
auto exprType = cs.getType(anchor);
402-
return std::make_tuple(cs.getContextualTypePurpose(anchor), exprType,
403+
return std::make_tuple(contextualTypeElt->getPurpose(), exprType,
403404
contextualType);
404405
} else if (auto argApplyInfo = solution.getFunctionArgApplyInfo(locator)) {
405406
return std::make_tuple(CTP_CallArgument,
@@ -1340,7 +1341,8 @@ bool IgnoreAssignmentDestinationType::diagnose(const Solution &solution,
13401341

13411342
AssignmentTypeMismatchFailure failure(
13421343
solution, CTP, getFromType(), getToType(),
1343-
cs.getConstraintLocator(AE->getSrc(), LocatorPathElt::ContextualType()));
1344+
cs.getConstraintLocator(AE->getSrc(),
1345+
LocatorPathElt::ContextualType(CTP)));
13441346
return failure.diagnose(asNote);
13451347
}
13461348

‎lib/Sema/CSGen.cpp

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3626,8 +3626,10 @@ static bool generateWrappedPropertyTypeConstraints(
36263626
}
36273627

36283628
// The property type must be equal to the wrapped value type
3629-
cs.addConstraint(ConstraintKind::Equal, propertyType, wrappedValueType,
3630-
cs.getConstraintLocator(wrappedVar, LocatorPathElt::ContextualType()));
3629+
cs.addConstraint(
3630+
ConstraintKind::Equal, propertyType, wrappedValueType,
3631+
cs.getConstraintLocator(
3632+
wrappedVar, LocatorPathElt::ContextualType(CTP_WrappedProperty)));
36313633
cs.setContextualType(wrappedVar, TypeLoc::withoutLoc(wrappedValueType),
36323634
CTP_WrappedProperty);
36333635
return false;
@@ -3637,8 +3639,8 @@ static bool generateWrappedPropertyTypeConstraints(
36373639
static bool generateInitPatternConstraints(
36383640
ConstraintSystem &cs, SolutionApplicationTarget target, Expr *initializer) {
36393641
auto pattern = target.getInitializationPattern();
3640-
auto locator =
3641-
cs.getConstraintLocator(initializer, LocatorPathElt::ContextualType());
3642+
auto locator = cs.getConstraintLocator(
3643+
initializer, LocatorPathElt::ContextualType(CTP_Initialization));
36423644
Type patternType = cs.generateConstraints(
36433645
pattern, locator, target.shouldBindPatternVarsOneWay(),
36443646
target.getInitializationPatternBindingDecl(),
@@ -3669,8 +3671,8 @@ generateForEachStmtConstraints(
36693671
bool isAsync = stmt->getAwaitLoc().isValid();
36703672

36713673
auto locator = cs.getConstraintLocator(sequence);
3672-
auto contextualLocator =
3673-
cs.getConstraintLocator(sequence, LocatorPathElt::ContextualType());
3674+
auto contextualLocator = cs.getConstraintLocator(
3675+
sequence, LocatorPathElt::ContextualType(CTP_ForEachStmt));
36743676

36753677
// The expression type must conform to the Sequence protocol.
36763678
auto sequenceProto = TypeChecker::getProtocol(
@@ -3787,8 +3789,9 @@ bool ConstraintSystem::generateConstraints(
37873789
if (target.isOptionalSomePatternInit()) {
37883790
assert(!target.getExprContextualType() &&
37893791
"some pattern cannot have contextual type pre-configured");
3790-
auto *convertTypeLocator =
3791-
getConstraintLocator(expr, LocatorPathElt::ContextualType());
3792+
auto *convertTypeLocator = getConstraintLocator(
3793+
expr, LocatorPathElt::ContextualType(
3794+
target.getExprContextualTypePurpose()));
37923795
Type var = createTypeVariable(convertTypeLocator, TVO_CanBindToNoEscape);
37933796
target.setExprConversionType(TypeChecker::getOptionalType(expr->getLoc(), var));
37943797
}
@@ -3810,7 +3813,7 @@ bool ConstraintSystem::generateConstraints(
38103813
ContextualTypePurpose ctp = target.getExprContextualTypePurpose();
38113814
bool isOpaqueReturnType = target.infersOpaqueReturnType();
38123815
auto *convertTypeLocator =
3813-
getConstraintLocator(expr, LocatorPathElt::ContextualType());
3816+
getConstraintLocator(expr, LocatorPathElt::ContextualType(ctp));
38143817

38153818
auto getLocator = [&](Type ty) -> ConstraintLocator * {
38163819
// If we have a placeholder originating from a PlaceholderTypeRepr,
@@ -3973,11 +3976,10 @@ bool ConstraintSystem::generateConstraints(StmtCondition condition,
39733976
return true;
39743977
}
39753978

3976-
addConstraint(ConstraintKind::Conversion,
3977-
getType(condExpr),
3978-
boolTy,
3979-
getConstraintLocator(condExpr,
3980-
LocatorPathElt::ContextualType()));
3979+
addConstraint(
3980+
ConstraintKind::Conversion, getType(condExpr), boolTy,
3981+
getConstraintLocator(condExpr,
3982+
LocatorPathElt::ContextualType(CTP_Condition)));
39813983
continue;
39823984
}
39833985

‎lib/Sema/CSSimplify.cpp

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4784,14 +4784,14 @@ bool ConstraintSystem::repairFailures(
47844784
// truth and produce a contextual mismatch instead of per-branch failure,
47854785
// because it's a better pointer than potential then-to-else type mismatch.
47864786
if (auto contextualType = getContextualType(anchor)) {
4787+
auto purpose = getContextualTypePurpose(anchor);
47874788
if (contextualType->isEqual(rhs)) {
4788-
auto *loc =
4789-
getConstraintLocator(anchor, LocatorPathElt::ContextualType());
4789+
auto *loc = getConstraintLocator(
4790+
anchor, LocatorPathElt::ContextualType(purpose));
47904791
if (hasFixFor(loc, FixKind::ContextualMismatch))
47914792
return true;
47924793

4793-
if (contextualType->isVoid() &&
4794-
getContextualTypePurpose(anchor) == CTP_ReturnStmt) {
4794+
if (contextualType->isVoid() && purpose == CTP_ReturnStmt) {
47954795
conversionsOrFixes.push_back(RemoveReturn::create(*this, lhs, loc));
47964796
break;
47974797
}
@@ -5754,12 +5754,8 @@ ConstraintSystem::matchTypes(Type type1, Type type2, ConstraintKind kind,
57545754
}
57555755

57565756
// Single expression function with implicit `return`.
5757-
if (elt->is<LocatorPathElt::ContextualType>()) {
5758-
auto anchor = locator.getAnchor();
5759-
auto contextualInfo = getContextualTypeInfo(anchor);
5760-
assert(contextualInfo &&
5761-
"Found contextual type locator without additional information");
5762-
if (contextualInfo->purpose == CTP_ReturnSingleExpr) {
5757+
if (auto contextualType = elt->getAs<LocatorPathElt::ContextualType>()) {
5758+
if (contextualType->isFor(CTP_ReturnSingleExpr)) {
57635759
increaseScore(SK_FunctionConversion);
57645760
return getTypeMatchSuccess();
57655761
}
@@ -6135,7 +6131,8 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyConformsToConstraint(
61356131
if (auto *Nil = getAsExpr<NilLiteralExpr>(anchor)) {
61366132
auto *fixLocator = getConstraintLocator(
61376133
getContextualType(Nil)
6138-
? locator.withPathElement(LocatorPathElt::ContextualType())
6134+
? locator.withPathElement(LocatorPathElt::ContextualType(
6135+
getContextualTypePurpose(Nil)))
61396136
: locator);
61406137

61416138
// Here the roles are reversed - `nil` is something we are trying to
@@ -11788,7 +11785,7 @@ void ConstraintSystem::addContextualConversionConstraint(
1178811785

1178911786
// Add the constraint.
1179011787
auto *convertTypeLocator =
11791-
getConstraintLocator(expr, LocatorPathElt::ContextualType());
11788+
getConstraintLocator(expr, LocatorPathElt::ContextualType(purpose));
1179211789
addConstraint(constraintKind, getType(expr), conversionType,
1179311790
convertTypeLocator, /*isFavored*/ true);
1179411791
}

‎unittests/Sema/BindingInferenceTests.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -187,9 +187,9 @@ TEST_F(SemaTest, TestTransitiveProtocolInference) {
187187
auto *typeVar = cs.createTypeVariable(cs.getConstraintLocator({}),
188188
/*options=*/0);
189189

190-
cs.addConstraint(
191-
ConstraintKind::Conversion, typeVar, GPT1,
192-
cs.getConstraintLocator({}, LocatorPathElt::ContextualType()));
190+
cs.addConstraint(ConstraintKind::Conversion, typeVar, GPT1,
191+
cs.getConstraintLocator({}, LocatorPathElt::ContextualType(
192+
CTP_Initialization)));
193193

194194
auto bindings = inferBindings(cs, typeVar);
195195
ASSERT_TRUE(bindings.getConformanceRequirements().empty());

‎validation-test/Sema/SwiftUI/rdar68795727.swift

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// RUN: %target-typecheck-verify-swift -target %target-cpu-apple-macosx10.15 -swift-version 5
2+
// REQUIRES: objc_interop
3+
// REQUIRES: OS=macosx
4+
5+
import SwiftUI
6+
7+
struct UnitVolumeView: View {
8+
var measurement: Measurement<Unit>
9+
10+
var body: some View {
11+
Text("")
12+
}
13+
}
14+
struct UnitView_Previews: PreviewProvider {
15+
static var previews: some View {
16+
let volume: Measurement<Unit> = Measurement<UnitVolume>(value: 200, unit: UnitVolume.milliliters)
17+
// expected-error@-1 {{cannot assign value of type 'Measurement<UnitVolume>' to type 'Measurement<Unit>'}}
18+
// expected-note@-2 {{arguments to generic parameter 'UnitType' ('UnitVolume' and 'Unit') are expected to be equal}}
19+
20+
Group {
21+
ForEach(["en", "de", "fr"], id: \.self) { id in
22+
UnitVolumeView(measurement: volume)
23+
.previewLayout(PreviewLayout.sizeThatFits)
24+
.environment(\.locale, .init(identifier: id))
25+
}
26+
}
27+
}
28+
}

0 commit comments

Comments
 (0)