From 2ac8d7ea1786f8edf1b4666c6e5d901bb3800674 Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Wed, 1 Jul 2020 13:17:47 -0700 Subject: [PATCH 1/6] [ConstraintSystem] Add support for type checking an out-of-line initialized wrapped property via SolutionApplicationTarget. --- lib/Sema/CSApply.cpp | 14 +++++++++ lib/Sema/CSGen.cpp | 39 ++++++++++++++++++------ lib/Sema/ConstraintSystem.cpp | 17 +++++++++++ lib/Sema/ConstraintSystem.h | 57 +++++++++++++++++++++++++++++++++++ 4 files changed, 118 insertions(+), 9 deletions(-) diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index 43bac6311dfda..06ba202376052 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -8139,6 +8139,17 @@ ExprWalker::rewriteTarget(SolutionApplicationTarget target) { patternBinding->setInit(index, resultTarget->getAsExpr()); } + return target; + } else if (auto *wrappedVar = target.getAsUninitializedWrappedVar()) { + // Get the outermost wrapper type from the solution + auto outermostWrapper = wrappedVar->getAttachedPropertyWrappers().front(); + auto backingType = solution.simplifyType( + solution.getType(outermostWrapper->getTypeRepr())); + + auto &ctx = solution.getConstraintSystem().getASTContext(); + ctx.setSideCachedPropertyWrapperBackingPropertyType( + wrappedVar, backingType->mapTypeOutOfContext()); + return target; } else { auto fn = *target.getAsFunction(); @@ -8419,6 +8430,9 @@ SolutionApplicationTarget SolutionApplicationTarget::walk(ASTWalker &walker) { case Kind::patternBinding: return *this; + + case Kind::uninitializedWrappedVar: + return *this; } llvm_unreachable("invalid target kind"); diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index ae23c2a859b86..2752a0030957c 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -3874,35 +3874,36 @@ static Expr *generateConstraintsFor(ConstraintSystem &cs, Expr *expr, /// \param wrappedVar The property that has a property wrapper. /// \returns the type of the property. static Type generateWrappedPropertyTypeConstraints( - ConstraintSystem &cs, Type initializerType, - VarDecl *wrappedVar, ConstraintLocator *locator) { + ConstraintSystem &cs, Type initializerType, VarDecl *wrappedVar) { auto dc = wrappedVar->getInnermostDeclContext(); Type wrapperType = LValueType::get(initializerType); Type wrappedValueType; - for (unsigned i : indices(wrappedVar->getAttachedPropertyWrappers())) { + auto wrapperAttributes = wrappedVar->getAttachedPropertyWrappers(); + for (unsigned i : indices(wrapperAttributes)) { Type rawWrapperType = wrappedVar->getAttachedPropertyWrapperType(i); - if (!rawWrapperType || rawWrapperType->hasError()) + auto wrapperInfo = wrappedVar->getAttachedPropertyWrapperTypeInfo(i); + if (rawWrapperType->hasError() || !wrapperInfo) return Type(); // The former wrappedValue type must be equal to the current wrapper type if (wrappedValueType) { + auto *typeRepr = wrapperAttributes[i]->getTypeRepr(); + auto *locator = + cs.getConstraintLocator(typeRepr, LocatorPathElt::ContextualType()); wrapperType = cs.openUnboundGenericTypes(rawWrapperType, locator); cs.addConstraint(ConstraintKind::Equal, wrappedValueType, wrapperType, locator); } - auto wrapperInfo = wrappedVar->getAttachedPropertyWrapperTypeInfo(i); - if (!wrapperInfo) - return Type(); - wrappedValueType = wrapperType->getTypeOfMember( dc->getParentModule(), wrapperInfo.valueVar); } // Set up an equality constraint to drop the lvalue-ness of the value // type we produced. + auto locator = cs.getConstraintLocator(wrappedVar); Type propertyType = cs.createTypeVariable(locator, 0); cs.addConstraint(ConstraintKind::Equal, propertyType, wrappedValueType, locator); return propertyType; @@ -3922,7 +3923,7 @@ static bool generateInitPatternConstraints( if (auto wrappedVar = target.getInitializationWrappedVar()) { Type propertyType = generateWrappedPropertyTypeConstraints( - cs, cs.getType(target.getAsExpr()), wrappedVar, locator); + cs, cs.getType(target.getAsExpr()), wrappedVar); if (!propertyType) return true; @@ -4165,6 +4166,26 @@ bool ConstraintSystem::generateConstraints( return hadError; } + + case SolutionApplicationTarget::Kind::uninitializedWrappedVar: { + auto *wrappedVar = target.getAsUninitializedWrappedVar(); + Type propertyType = wrappedVar->getType(); + if (propertyType->hasError()) + return true; + + auto *outermostWrapper = wrappedVar->getAttachedPropertyWrappers().front(); + auto *typeRepr = outermostWrapper->getTypeRepr(); + auto backingType = openUnboundGenericTypes(outermostWrapper->getType(), + getConstraintLocator(typeRepr)); + setType(typeRepr, backingType); + + auto wrappedValueType = + generateWrappedPropertyTypeConstraints(*this, backingType, wrappedVar); + + addConstraint(ConstraintKind::Equal, propertyType, wrappedValueType, + getConstraintLocator(wrappedVar, LocatorPathElt::ContextualType())); + return false; + } } } diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 940a4deb6923f..39b78350bf572 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -4674,6 +4674,11 @@ SolutionApplicationTarget SolutionApplicationTarget::forForEachStmt( return target; } +SolutionApplicationTarget +SolutionApplicationTarget::forUninitializedWrappedVar(VarDecl *wrappedVar) { + return SolutionApplicationTarget(wrappedVar); +} + ContextualPattern SolutionApplicationTarget::getContextualPattern() const { assert(kind == Kind::expression); @@ -4781,6 +4786,18 @@ void ConstraintSystem::diagnoseFailureFor(SolutionApplicationTarget target) { // diagnostic various cases that come up. DE.diagnose(expr->getLoc(), diag::type_of_expression_is_ambiguous) .highlight(expr->getSourceRange()); + } else if (auto *wrappedVar = target.getAsUninitializedWrappedVar()) { + auto *wrapper = wrappedVar->getAttachedPropertyWrappers().back(); + Type propertyType = wrappedVar->getInterfaceType(); + Type wrapperType = wrapper->getType(); + + // Emit the property wrapper fallback diagnostic + wrappedVar->diagnose(diag::property_wrapper_incompatible_property, + propertyType, wrapperType); + if (auto nominal = wrapperType->getAnyNominal()) { + nominal->diagnose(diag::property_wrapper_declared_here, + nominal->getName()); + } } else { // Emit a poor fallback message. DE.diagnose(target.getAsFunction()->getLoc(), diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h index 348fe54a66baa..a8e96bd1c2cf2 100644 --- a/lib/Sema/ConstraintSystem.h +++ b/lib/Sema/ConstraintSystem.h @@ -856,6 +856,7 @@ class SolutionApplicationTargetsKey { stmtCondElement, stmt, patternBindingEntry, + varDecl, }; private: @@ -870,6 +871,8 @@ class SolutionApplicationTargetsKey { const PatternBindingDecl *patternBinding; unsigned index; } patternBindingEntry; + + const VarDecl *varDecl; } storage; public: @@ -895,6 +898,11 @@ class SolutionApplicationTargetsKey { storage.patternBindingEntry.index = index; } + SolutionApplicationTargetsKey(const VarDecl *varDecl) { + kind = Kind::varDecl; + storage.varDecl = varDecl; + } + friend bool operator==( SolutionApplicationTargetsKey lhs, SolutionApplicationTargetsKey rhs) { if (lhs.kind != rhs.kind) @@ -916,6 +924,9 @@ class SolutionApplicationTargetsKey { == rhs.storage.patternBindingEntry.patternBinding) && (lhs.storage.patternBindingEntry.index == rhs.storage.patternBindingEntry.index); + + case Kind::varDecl: + return lhs.storage.varDecl == rhs.storage.varDecl; } llvm_unreachable("invalid SolutionApplicationTargetsKey kind"); } @@ -951,6 +962,11 @@ class SolutionApplicationTargetsKey { storage.patternBindingEntry.patternBinding), DenseMapInfo::getHashValue( storage.patternBindingEntry.index)); + + case Kind::varDecl: + return hash_combine( + DenseMapInfo::getHashValue(static_cast(kind)), + DenseMapInfo::getHashValue(storage.varDecl)); } llvm_unreachable("invalid statement kind"); } @@ -1343,6 +1359,7 @@ class SolutionApplicationTarget { stmtCondition, caseLabelItem, patternBinding, + uninitializedWrappedVar, } kind; private: @@ -1409,6 +1426,8 @@ class SolutionApplicationTarget { } caseLabelItem; PatternBindingDecl *patternBinding; + + VarDecl *uninitializedWrappedVar; }; // If the pattern contains a single variable that has an attached @@ -1454,6 +1473,11 @@ class SolutionApplicationTarget { this->patternBinding = patternBinding; } + SolutionApplicationTarget(VarDecl *wrappedVar) { + kind = Kind::uninitializedWrappedVar; + this->uninitializedWrappedVar= wrappedVar; + } + /// Form a target for the initialization of a pattern from an expression. static SolutionApplicationTarget forInitialization( Expr *initializer, DeclContext *dc, Type patternType, Pattern *pattern, @@ -1471,6 +1495,11 @@ class SolutionApplicationTarget { ForEachStmt *stmt, ProtocolDecl *sequenceProto, DeclContext *dc, bool bindPatternVarsOneWay); + /// Form a target for a property with an attached property wrapper that is + /// initialized out-of-line. + static SolutionApplicationTarget forUninitializedWrappedVar( + VarDecl *wrappedVar); + Expr *getAsExpr() const { switch (kind) { case Kind::expression: @@ -1480,6 +1509,7 @@ class SolutionApplicationTarget { case Kind::stmtCondition: case Kind::caseLabelItem: case Kind::patternBinding: + case Kind::uninitializedWrappedVar: return nullptr; } llvm_unreachable("invalid expression type"); @@ -1501,6 +1531,9 @@ class SolutionApplicationTarget { case Kind::patternBinding: return patternBinding->getDeclContext(); + + case Kind::uninitializedWrappedVar: + return uninitializedWrappedVar->getDeclContext(); } llvm_unreachable("invalid decl context type"); } @@ -1653,6 +1686,7 @@ class SolutionApplicationTarget { case Kind::stmtCondition: case Kind::caseLabelItem: case Kind::patternBinding: + case Kind::uninitializedWrappedVar: return None; case Kind::function: @@ -1667,6 +1701,7 @@ class SolutionApplicationTarget { case Kind::function: case Kind::caseLabelItem: case Kind::patternBinding: + case Kind::uninitializedWrappedVar: return None; case Kind::stmtCondition: @@ -1681,6 +1716,7 @@ class SolutionApplicationTarget { case Kind::function: case Kind::stmtCondition: case Kind::patternBinding: + case Kind::uninitializedWrappedVar: return None; case Kind::caseLabelItem: @@ -1695,6 +1731,7 @@ class SolutionApplicationTarget { case Kind::function: case Kind::stmtCondition: case Kind::caseLabelItem: + case Kind::uninitializedWrappedVar: return nullptr; case Kind::patternBinding: @@ -1702,6 +1739,20 @@ class SolutionApplicationTarget { } } + VarDecl *getAsUninitializedWrappedVar() const { + switch (kind) { + case Kind::expression: + case Kind::function: + case Kind::stmtCondition: + case Kind::caseLabelItem: + case Kind::patternBinding: + return nullptr; + + case Kind::uninitializedWrappedVar: + return uninitializedWrappedVar; + } + } + BraceStmt *getFunctionBody() const { assert(kind == Kind::function); return function.body; @@ -1730,6 +1781,9 @@ class SolutionApplicationTarget { case Kind::patternBinding: return patternBinding->getSourceRange(); + + case Kind::uninitializedWrappedVar: + return uninitializedWrappedVar->getSourceRange(); } llvm_unreachable("invalid target type"); } @@ -1751,6 +1805,9 @@ class SolutionApplicationTarget { case Kind::patternBinding: return patternBinding->getLoc(); + + case Kind::uninitializedWrappedVar: + return uninitializedWrappedVar->getLoc(); } llvm_unreachable("invalid target type"); } From 142e9f628969ede72ccab9be0fd431d24685c8a2 Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Wed, 1 Jul 2020 16:52:00 -0700 Subject: [PATCH 2/6] [Diagnostics] Tweak some diagnostics code to allow diagnosing a fix anchored at a TypeRepr rather than an Expr. --- lib/Sema/CSDiagnostics.cpp | 21 +++++++++++++++------ lib/Sema/CSDiagnostics.h | 5 +++-- lib/Sema/ConstraintSystem.cpp | 10 ++++++++-- lib/Sema/ConstraintSystem.h | 8 ++++---- 4 files changed, 30 insertions(+), 14 deletions(-) diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp index 26a3d77a76f11..d1c928b356fb8 100644 --- a/lib/Sema/CSDiagnostics.cpp +++ b/lib/Sema/CSDiagnostics.cpp @@ -414,7 +414,7 @@ void RequirementFailure::emitRequirementNote(const Decl *anchor, Type lhs, } bool MissingConformanceFailure::diagnoseAsError() { - auto *anchor = castToExpr(getAnchor()); + auto anchor = getAnchor(); auto nonConformingType = getLHS(); auto protocolType = getRHS(); @@ -423,8 +423,9 @@ bool MissingConformanceFailure::diagnoseAsError() { // with it and if so skip conformance error, otherwise we'd // produce an unrelated ` doesn't conform to Equatable protocol` // diagnostic. - if (isPatternMatchingOperator(const_cast(anchor))) { - if (auto *binaryOp = dyn_cast_or_null(findParentExpr(anchor))) { + if (isPatternMatchingOperator(anchor)) { + auto *expr = castToExpr(anchor); + if (auto *binaryOp = dyn_cast_or_null(findParentExpr(expr))) { auto *caseExpr = binaryOp->getArg()->getElement(0); llvm::SmallPtrSet anchors; @@ -452,6 +453,7 @@ bool MissingConformanceFailure::diagnoseAsError() { // says that conformances for enums with associated values can't be // synthesized. if (isStandardComparisonOperator(anchor)) { + auto *expr = castToExpr(anchor); auto isEnumWithAssociatedValues = [](Type type) -> bool { if (auto *enumType = type->getAs()) return !enumType->getDecl()->hasOnlyCasesWithoutAssociatedValues(); @@ -464,7 +466,7 @@ bool MissingConformanceFailure::diagnoseAsError() { (protocol->isSpecificProtocol(KnownProtocolKind::Equatable) || protocol->isSpecificProtocol(KnownProtocolKind::Comparable))) { if (RequirementFailure::diagnoseAsError()) { - auto opName = getOperatorName(anchor); + auto opName = getOperatorName(expr); emitDiagnostic(diag::no_binary_op_overload_for_enum_with_payload, opName->str()); return true; @@ -4998,8 +5000,15 @@ bool MissingGenericArgumentsFailure::diagnoseAsError() { scopedParameters[base].push_back(GP); }); - if (!isScoped) - return diagnoseForAnchor(castToExpr(getAnchor()), Parameters); + // FIXME: this code should be generalized now that we can anchor the + // fixes on the TypeRepr with the missing generic arg. + if (!isScoped) { + assert(getAnchor().is() || getAnchor().is()); + if (auto *expr = getAsExpr(getAnchor())) + return diagnoseForAnchor(expr, Parameters); + + return diagnoseForAnchor(getAnchor().get(), Parameters); + } bool diagnosed = false; for (const auto &scope : scopedParameters) diff --git a/lib/Sema/CSDiagnostics.h b/lib/Sema/CSDiagnostics.h index 0c21f1de32997..bc354d7a4af9f 100644 --- a/lib/Sema/CSDiagnostics.h +++ b/lib/Sema/CSDiagnostics.h @@ -265,8 +265,9 @@ class RequirementFailure : public FailureDiagnostic { assert(getGenericContext() && "Affected decl not within a generic context?"); - if (auto *parentExpr = findParentExpr(getRawAnchor().get())) - Apply = dyn_cast(parentExpr); + if (auto *expr = getAsExpr(getRawAnchor())) + if (auto *parentExpr = findParentExpr(expr)) + Apply = dyn_cast(parentExpr); } unsigned getRequirementIndex() const { diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 39b78350bf572..27ed7adf6587a 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -4249,11 +4249,17 @@ Optional constraints::getOperatorName(Expr *expr) { return None; } -bool constraints::isPatternMatchingOperator(Expr *expr) { +bool constraints::isPatternMatchingOperator(ASTNode node) { + auto *expr = getAsExpr(node); + if (!expr) return false; + return isOperator(expr, "~="); } -bool constraints::isStandardComparisonOperator(Expr *expr) { +bool constraints::isStandardComparisonOperator(ASTNode node) { + auto *expr = getAsExpr(node); + if (!expr) return false; + if (auto opName = getOperatorName(expr)) { return opName->is("==") || opName->is("!=") || opName->is("===") || opName->is("!==") || opName->is("<") || opName->is(">") || diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h index a8e96bd1c2cf2..516724a61421f 100644 --- a/lib/Sema/ConstraintSystem.h +++ b/lib/Sema/ConstraintSystem.h @@ -5224,13 +5224,13 @@ bool isArgumentOfPatternMatchingOperator(ConstraintLocator *locator); /// associated with `===` and `!==` operators. bool isArgumentOfReferenceEqualityOperator(ConstraintLocator *locator); -/// Determine whether given expression is a reference to a +/// Determine whether the given AST node is a reference to a /// pattern-matching operator `~=` -bool isPatternMatchingOperator(Expr *expr); +bool isPatternMatchingOperator(ASTNode node); -/// Determine whether given expression is a reference to a +/// Determine whether the given AST node is a reference to a /// "standard" comparison operator such as "==", "!=", ">" etc. -bool isStandardComparisonOperator(Expr *expr); +bool isStandardComparisonOperator(ASTNode node); /// If given expression references operator overlaod(s) /// extract and produce name of the operator. From 238637408cbc4e41ac52d6c1fcf0cd1e1b8ba247 Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Wed, 1 Jul 2020 16:55:15 -0700 Subject: [PATCH 3/6] [Property Wrappers] Type check out-of-line initialized property wrappers via SolutionApplicationTarget. This allows fixes to be applied and diagnosed for better error messages in the case of failures, and removes code duplication for generating property wrapper constraints. --- lib/Sema/TypeCheckPropertyWrapper.cpp | 75 +++++---------------------- test/decl/var/property_wrappers.swift | 31 +++++------ 2 files changed, 29 insertions(+), 77 deletions(-) diff --git a/lib/Sema/TypeCheckPropertyWrapper.cpp b/lib/Sema/TypeCheckPropertyWrapper.cpp index 6866e96a2100c..c46d9de1a0f38 100644 --- a/lib/Sema/TypeCheckPropertyWrapper.cpp +++ b/lib/Sema/TypeCheckPropertyWrapper.cpp @@ -592,72 +592,23 @@ PropertyWrapperBackingPropertyTypeRequest::evaluate( (void)var->getInterfaceType(); if (!binding->isInitializerChecked(index)) TypeChecker::typeCheckPatternBinding(binding, index); - - ASTContext &ctx = var->getASTContext(); - Type type = ctx.getSideCachedPropertyWrapperBackingPropertyType(var); - assert(type || ctx.Diags.hadAnyError()); - return type; - } - - // Compute the type of the property to plug in to the wrapper type. - Type propertyType = var->getType(); - if (propertyType->hasError()) - return Type(); - - using namespace constraints; - auto dc = var->getInnermostDeclContext(); - ConstraintSystem cs(dc, None); - auto emptyLocator = cs.getConstraintLocator({}); - - auto wrapperAttrs = var->getAttachedPropertyWrappers(); - Type valueMemberType; - Type outermostOpenedWrapperType; - for (unsigned i : indices(wrapperAttrs)) { - Type rawWrapperType = var->getAttachedPropertyWrapperType(i); - if (!rawWrapperType) + } else { + using namespace constraints; + auto dc = var->getInnermostDeclContext(); + ConstraintSystem cs(dc, ConstraintSystemFlags::AllowFixes); + auto target = SolutionApplicationTarget::forUninitializedWrappedVar(var); + auto solutions = cs.solve(target, FreeTypeVariableBinding::Disallow); + + if (!solutions || !cs.applySolution(solutions->front(), target)) { + var->setInvalid(); return Type(); - - // Open the type. - Type openedWrapperType = - cs.openUnboundGenericTypes(rawWrapperType, emptyLocator); - if (!outermostOpenedWrapperType) - outermostOpenedWrapperType = openedWrapperType; - - // If we already have a value member type, it must be equivalent to - // this opened wrapper type. - if (valueMemberType) { - cs.addConstraint(ConstraintKind::Equal, valueMemberType, - openedWrapperType, emptyLocator); } - - // Retrieve the type of the wrapped value. - auto wrapperInfo = var->getAttachedPropertyWrapperTypeInfo(i); - if (!wrapperInfo) - return Type(); - - valueMemberType = openedWrapperType->getTypeOfMember( - dc->getParentModule(), wrapperInfo.valueVar); - } - - // The resulting value member type must be equivalent to the property - // type. - cs.addConstraint(ConstraintKind::Equal, valueMemberType, - propertyType, emptyLocator); - - SmallVector solutions; - if (cs.solve(solutions) || solutions.size() != 1) { - var->diagnose(diag::property_wrapper_incompatible_property, - propertyType, rawType); - var->setInvalid(); - if (auto nominalWrapper = rawType->getAnyNominal()) { - nominalWrapper->diagnose(diag::property_wrapper_declared_here, - nominalWrapper->getName()); - } - return Type(); } - Type wrapperType = solutions.front().simplifyType(outermostOpenedWrapperType); - return wrapperType->mapTypeOutOfContext(); + ASTContext &ctx = var->getASTContext(); + Type type = ctx.getSideCachedPropertyWrapperBackingPropertyType(var); + assert(type || ctx.Diags.hadAnyError()); + return type; } Type swift::computeWrappedValueType(VarDecl *var, Type backingStorageType, diff --git a/test/decl/var/property_wrappers.swift b/test/decl/var/property_wrappers.swift index 89980ca80e472..090c4dcefef42 100644 --- a/test/decl/var/property_wrappers.swift +++ b/test/decl/var/property_wrappers.swift @@ -304,7 +304,7 @@ struct IntWrapper { } @propertyWrapper -struct WrapperForHashable { // expected-note{{property wrapper type 'WrapperForHashable' declared here}} +struct WrapperForHashable { // expected-note{{where 'T' = 'NotHashable'}} var wrappedValue: T } @@ -319,9 +319,8 @@ struct UseWrappersWithDifferentForm { @IntWrapper var x: Int - // FIXME: Diagnostic should be better here - @WrapperForHashable - var y: NotHashable // expected-error{{property type 'NotHashable' does not match that of the 'wrappedValue' property of its wrapper type 'WrapperForHashable'}} + @WrapperForHashable // expected-error {{generic struct 'WrapperForHashable' requires that 'NotHashable' conform to 'Hashable'}} + var y: NotHashable @WrapperForHashable var yOkay: Int @@ -329,9 +328,8 @@ struct UseWrappersWithDifferentForm { @WrapperWithTwoParams var zOkay: (Int, Float) - // FIXME: Need a better diagnostic here - @HasNestedWrapper.NestedWrapper - var w: Int // expected-error{{property type 'Int' does not match that of the 'wrappedValue' property of its wrapper type 'HasNestedWrapper.NestedWrapper'}} + @HasNestedWrapper.NestedWrapper // expected-error {{generic parameter 'T' could not be inferred}} expected-note {{explicitly specify}} + var w: Int @HasNestedWrapper.NestedWrapper var wOkay: Int @@ -341,14 +339,16 @@ struct UseWrappersWithDifferentForm { } @propertyWrapper -struct Function { // expected-note{{property wrapper type 'Function' declared here}} +struct Function { // expected-note{{'U' declared as parameter to type 'Function'}} var wrappedValue: (T) -> U? } struct TestFunction { @Function var f: (Int) -> Float? - - @Function var f2: (Int) -> Float // expected-error{{property type '(Int) -> Float' does not match that of the 'wrappedValue' property of its wrapper type 'Function'}} + + // FIXME: This diagnostic should be more specific + @Function var f2: (Int) -> Float // expected-error{{generic parameter 'U' could not be inferred}} + // expected-note@-1 {{explicitly specify}} func test() { let _: Int = _f // expected-error{{cannot convert value of type 'Function' to specified type 'Int'}} @@ -358,9 +358,9 @@ struct TestFunction { // --------------------------------------------------------------------------- // Nested wrappers // --------------------------------------------------------------------------- -struct HasNestedWrapper { +struct HasNestedWrapper { // expected-note {{'T' declared as parameter to type 'HasNestedWrapper'}} @propertyWrapper - struct NestedWrapper { // expected-note{{property wrapper type 'NestedWrapper' declared here}} + struct NestedWrapper { var wrappedValue: U init(wrappedValue initialValue: U) { self.wrappedValue = initialValue @@ -1026,7 +1026,7 @@ struct WrapperB { } @propertyWrapper -struct WrapperC { +struct WrapperC { // expected-note {{'Value' declared as parameter to type 'WrapperC'}} var wrappedValue: Value? init(wrappedValue initialValue: Value?) { @@ -1035,7 +1035,7 @@ struct WrapperC { } @propertyWrapper -struct WrapperD { // expected-note{{property wrapper type 'WrapperD' declared here}} +struct WrapperD { var wrappedValue: Value } @@ -1049,7 +1049,8 @@ struct TestComposition { @WrapperA @WrapperB @WrapperC var p2 = "Hello" @WrapperD @WrapperE var p3: Int? @WrapperD @WrapperC var p4: Int? - @WrapperD @WrapperE var p5: Int // expected-error{{property type 'Int' does not match that of the 'wrappedValue' property of its wrapper type 'WrapperD'}} + @WrapperD @WrapperE var p5: Int // expected-error{{generic parameter 'Value' could not be inferred}} + // expected-note@-1 {{explicitly specify the generic arguments to fix this issue}} func triggerErrors(d: Double) { // expected-note 6 {{mark method 'mutating' to make 'self' mutable}} {{2-2=mutating }} p1 = d // expected-error{{cannot assign value of type 'Double' to type 'Int'}} {{8-8=Int(}} {{9-9=)}} From f555bfefc9f87fcac0edea7baf87bf02befbdda5 Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Wed, 1 Jul 2020 19:42:01 -0700 Subject: [PATCH 4/6] [Property Wrappers] Add new contextual type purposes for property wrappers to produce better diagnostics when there's a 'wrappedValue' type mismatch. --- include/swift/AST/DiagnosticsSema.def | 6 ++++++ lib/Sema/CSApply.cpp | 2 ++ lib/Sema/CSDiagnostics.cpp | 11 +++++++++++ lib/Sema/CSGen.cpp | 6 +++++- lib/Sema/CSSimplify.cpp | 2 ++ lib/Sema/ConstraintSystem.cpp | 2 ++ lib/Sema/TypeChecker.h | 3 +++ test/decl/var/property_wrappers.swift | 6 ++++-- 8 files changed, 35 insertions(+), 3 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index bbc6763b8db62..ff70f4549de71 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -5010,6 +5010,12 @@ NOTE(property_wrapper_direct_init,none, ERROR(property_wrapper_incompatible_property, none, "property type %0 does not match that of the 'wrappedValue' property of " "its wrapper type %1", (Type, Type)) +ERROR(wrapped_value_mismatch, none, + "property type %0 does not match 'wrappedValue' type %1", + (Type, Type)) +ERROR(composed_property_wrapper_mismatch, none, + "composed wrapper type %0 does not match former 'wrappedValue' type %1", + (Type, Type)) ERROR(property_wrapper_type_access,none, "%select{%select{variable|constant}0|property}1 " diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index 06ba202376052..36ee5f7a6c501 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -8039,6 +8039,8 @@ ExprWalker::rewriteTarget(SolutionApplicationTarget target) { case swift::CTP_AssignSource: case swift::CTP_SubscriptAssignSource: case swift::CTP_Condition: + case swift::CTP_WrappedProperty: + case swift::CTP_ComposedPropertyWrapper: case swift::CTP_CannotFail: result.setExpr(rewrittenExpr); break; diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp index d1c928b356fb8..155b863e82c49 100644 --- a/lib/Sema/CSDiagnostics.cpp +++ b/lib/Sema/CSDiagnostics.cpp @@ -613,6 +613,10 @@ Optional> GenericArgumentsMismatchFailure::getDiagnosticFor( return diag::cannot_convert_subscript_assign; case CTP_Condition: return diag::cannot_convert_condition_value; + case CTP_WrappedProperty: + return diag::wrapped_value_mismatch; + case CTP_ComposedPropertyWrapper: + return diag::composed_property_wrapper_mismatch; case CTP_ThrowStmt: case CTP_ForEachStmt: @@ -2206,6 +2210,8 @@ getContextualNilDiagnostic(ContextualTypePurpose CTP) { case CTP_ThrowStmt: case CTP_ForEachStmt: case CTP_YieldByReference: + case CTP_WrappedProperty: + case CTP_ComposedPropertyWrapper: return None; case CTP_EnumCaseRawValue: @@ -2904,6 +2910,11 @@ ContextualFailure::getDiagnosticFor(ContextualTypePurpose context, case CTP_Condition: return diag::cannot_convert_condition_value; + case CTP_WrappedProperty: + return diag::wrapped_value_mismatch; + case CTP_ComposedPropertyWrapper: + return diag::composed_property_wrapper_mismatch; + case CTP_ThrowStmt: case CTP_ForEachStmt: case CTP_Unused: diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index 2752a0030957c..f449a5d4213a9 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -3893,8 +3893,10 @@ static Type generateWrappedPropertyTypeConstraints( auto *locator = cs.getConstraintLocator(typeRepr, LocatorPathElt::ContextualType()); wrapperType = cs.openUnboundGenericTypes(rawWrapperType, locator); - cs.addConstraint(ConstraintKind::Equal, wrappedValueType, wrapperType, + cs.addConstraint(ConstraintKind::Equal, wrapperType, wrappedValueType, locator); + cs.setContextualType(typeRepr, TypeLoc::withoutLoc(wrappedValueType), + CTP_ComposedPropertyWrapper); } wrappedValueType = wrapperType->getTypeOfMember( @@ -4184,6 +4186,8 @@ bool ConstraintSystem::generateConstraints( addConstraint(ConstraintKind::Equal, propertyType, wrappedValueType, getConstraintLocator(wrappedVar, LocatorPathElt::ContextualType())); + setContextualType(wrappedVar, TypeLoc::withoutLoc(wrappedValueType), + CTP_WrappedProperty); return false; } } diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 6341a41ea7237..2bf03eae13e7a 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -10156,6 +10156,8 @@ void ConstraintSystem::addContextualConversionConstraint( case CTP_CoerceOperand: case CTP_SubscriptAssignSource: case CTP_ForEachStmt: + case CTP_WrappedProperty: + case CTP_ComposedPropertyWrapper: break; } diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 27ed7adf6587a..9d27ee0a6cec5 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -4752,6 +4752,8 @@ bool SolutionApplicationTarget::contextualTypeIsOnlyAHint() const { case CTP_AssignSource: case CTP_SubscriptAssignSource: case CTP_Condition: + case CTP_WrappedProperty: + case CTP_ComposedPropertyWrapper: case CTP_CannotFail: return false; } diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index f4c1c6ca61cf4..bb5afff9d1201 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -144,6 +144,9 @@ enum ContextualTypePurpose { ///< `if`, `for`, `while` etc. CTP_ForEachStmt, ///< "expression/sequence" associated with 'for-in' loop ///< is expected to conform to 'Sequence' protocol. + CTP_WrappedProperty, ///< Property type expected to match 'wrappedValue' type + CTP_ComposedPropertyWrapper, ///< Composed wrapper type expected to match + ///< former 'wrappedValue' type CTP_CannotFail, ///< Conversion can never fail. abort() if it does. }; diff --git a/test/decl/var/property_wrappers.swift b/test/decl/var/property_wrappers.swift index 090c4dcefef42..18bf7666bd28a 100644 --- a/test/decl/var/property_wrappers.swift +++ b/test/decl/var/property_wrappers.swift @@ -347,8 +347,9 @@ struct TestFunction { @Function var f: (Int) -> Float? // FIXME: This diagnostic should be more specific - @Function var f2: (Int) -> Float // expected-error{{generic parameter 'U' could not be inferred}} - // expected-note@-1 {{explicitly specify}} + @Function var f2: (Int) -> Float // expected-error {{property type '(Int) -> Float' does not match 'wrappedValue' type '(Int) -> U?'}} + // expected-error@-1 {{generic parameter 'U' could not be inferred}} + // expected-note@-2 {{explicitly specify}} func test() { let _: Int = _f // expected-error{{cannot convert value of type 'Function' to specified type 'Int'}} @@ -1051,6 +1052,7 @@ struct TestComposition { @WrapperD @WrapperC var p4: Int? @WrapperD @WrapperE var p5: Int // expected-error{{generic parameter 'Value' could not be inferred}} // expected-note@-1 {{explicitly specify the generic arguments to fix this issue}} + // expected-error@-2 {{composed wrapper type 'WrapperE' does not match former 'wrappedValue' type 'WrapperC'}} func triggerErrors(d: Double) { // expected-note 6 {{mark method 'mutating' to make 'self' mutable}} {{2-2=mutating }} p1 = d // expected-error{{cannot assign value of type 'Double' to type 'Int'}} {{8-8=Int(}} {{9-9=)}} From 7dffff79dafc78dda6e50b997031118d3e2ed098 Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Wed, 1 Jul 2020 19:57:47 -0700 Subject: [PATCH 5/6] [Property Wrappers] Let `PropertyWrapperBackingPropertyTypeRequest` handle all errors with a property wrapper backing type. --- lib/Sema/CSGen.cpp | 7 +++---- lib/Sema/TypeCheckPropertyWrapper.cpp | 3 --- lib/Sema/TypeCheckStorage.cpp | 21 --------------------- test/decl/var/property_wrappers.swift | 4 ++-- 4 files changed, 5 insertions(+), 30 deletions(-) diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index f449a5d4213a9..dc73314efa2ca 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -4171,10 +4171,6 @@ bool ConstraintSystem::generateConstraints( case SolutionApplicationTarget::Kind::uninitializedWrappedVar: { auto *wrappedVar = target.getAsUninitializedWrappedVar(); - Type propertyType = wrappedVar->getType(); - if (propertyType->hasError()) - return true; - auto *outermostWrapper = wrappedVar->getAttachedPropertyWrappers().front(); auto *typeRepr = outermostWrapper->getTypeRepr(); auto backingType = openUnboundGenericTypes(outermostWrapper->getType(), @@ -4183,6 +4179,9 @@ bool ConstraintSystem::generateConstraints( auto wrappedValueType = generateWrappedPropertyTypeConstraints(*this, backingType, wrappedVar); + Type propertyType = wrappedVar->getType(); + if (!wrappedValueType || propertyType->hasError()) + return true; addConstraint(ConstraintKind::Equal, propertyType, wrappedValueType, getConstraintLocator(wrappedVar, LocatorPathElt::ContextualType())); diff --git a/lib/Sema/TypeCheckPropertyWrapper.cpp b/lib/Sema/TypeCheckPropertyWrapper.cpp index c46d9de1a0f38..a47909b6004fa 100644 --- a/lib/Sema/TypeCheckPropertyWrapper.cpp +++ b/lib/Sema/TypeCheckPropertyWrapper.cpp @@ -577,9 +577,6 @@ PropertyWrapperBackingPropertyTypeRequest::evaluate( if (!rawType || rawType->hasError()) return Type(); - if (!rawType->hasUnboundGenericType()) - return rawType->mapTypeOutOfContext(); - auto binding = var->getParentPatternBinding(); if (!binding) return Type(); diff --git a/lib/Sema/TypeCheckStorage.cpp b/lib/Sema/TypeCheckStorage.cpp index 609c828010faa..64448c4505e1a 100644 --- a/lib/Sema/TypeCheckStorage.cpp +++ b/lib/Sema/TypeCheckStorage.cpp @@ -2605,27 +2605,6 @@ PropertyWrapperBackingPropertyInfoRequest::evaluate(Evaluator &evaluator, Type storageInterfaceType = wrapperType; Type storageType = dc->mapTypeIntoContext(storageInterfaceType); - // Make sure that the property type matches the value of the - // wrapper type. - if (!storageInterfaceType->hasError()) { - Type expectedPropertyType = - computeWrappedValueType(var, storageInterfaceType); - Type propertyType = var->getValueInterfaceType(); - assert(propertyType); - if (!expectedPropertyType->hasError() && - !propertyType->hasError() && - !propertyType->isEqual(expectedPropertyType)) { - var->diagnose(diag::property_wrapper_incompatible_property, - propertyType, wrapperType); - var->setInvalid(); - if (auto nominalWrapper = wrapperType->getAnyNominal()) { - nominalWrapper->diagnose(diag::property_wrapper_declared_here, - nominalWrapper->getName()); - } - return PropertyWrapperBackingPropertyInfo(); - } - } - // Create the backing storage property and note it in the cache. VarDecl *backingVar = new (ctx) VarDecl(/*IsStatic=*/var->isStatic(), VarDecl::Introducer::Var, diff --git a/test/decl/var/property_wrappers.swift b/test/decl/var/property_wrappers.swift index 18bf7666bd28a..f20d237a61c41 100644 --- a/test/decl/var/property_wrappers.swift +++ b/test/decl/var/property_wrappers.swift @@ -958,12 +958,12 @@ struct UsesWrapperRequiringP { // SR-10899 / rdar://problem/51588022 @propertyWrapper -struct SR_10899_Wrapper { // expected-note{{property wrapper type 'SR_10899_Wrapper' declared here}} +struct SR_10899_Wrapper { var wrappedValue: String { "hi" } } struct SR_10899_Usage { - @SR_10899_Wrapper var thing: Bool // expected-error{{property type 'Bool' does not match that of the 'wrappedValue' property of its wrapper type 'SR_10899_Wrapper'}} + @SR_10899_Wrapper var thing: Bool // expected-error{{property type 'Bool' does not match 'wrappedValue' type 'String'}} } // SR-11061 / rdar://problem/52593304 assertion with DeclContext mismatches From 9b4f1f0935c90effc340634a130f1128c90a93ff Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Thu, 2 Jul 2020 09:02:22 -0700 Subject: [PATCH 6/6] [NFC] Update validation tests with new property wrapper diagnostics --- validation-test/compiler_crashers_2_fixed/sr11599.swift | 2 +- validation-test/compiler_crashers_2_fixed/sr11684.swift | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/validation-test/compiler_crashers_2_fixed/sr11599.swift b/validation-test/compiler_crashers_2_fixed/sr11599.swift index 0b51e69354bc5..38e471214cbb6 100644 --- a/validation-test/compiler_crashers_2_fixed/sr11599.swift +++ b/validation-test/compiler_crashers_2_fixed/sr11599.swift @@ -11,5 +11,5 @@ struct B { } struct C { - @A @B var foo: Int // expected-error{{extraneous argument label 'wrappedValue:' in call}} + @A @B var foo: Int // expected-error{{composed wrapper type 'B' does not match former 'wrappedValue' type 'Int'}} } diff --git a/validation-test/compiler_crashers_2_fixed/sr11684.swift b/validation-test/compiler_crashers_2_fixed/sr11684.swift index 0590643e120e9..006db437564b3 100644 --- a/validation-test/compiler_crashers_2_fixed/sr11684.swift +++ b/validation-test/compiler_crashers_2_fixed/sr11684.swift @@ -1,13 +1,13 @@ // RUN: %target-swift-frontend %s -typecheck -verify @propertyWrapper -struct Wrapper1 { // expected-note {{property wrapper type 'Wrapper1' declared here}} +struct Wrapper1 { var wrappedValue: Int? } class Test1 { @Wrapper1 var user: Int - // expected-error@-1 {{property type 'Int' does not match that of the 'wrappedValue' property of its wrapper type 'Wrapper1'}} + // expected-error@-1 {{property type 'Int' does not match 'wrappedValue' type 'Int?'}} } @propertyWrapper