Skip to content

Improved error handling for composed property wrapper mismatches #36732

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -5678,8 +5678,8 @@ 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))
"composed wrapper type %0 does not match type of '%1.wrappedValue', which is %2",
(Type, StringRef, Type))

ERROR(property_wrapper_type_access,none,
"%select{%select{variable|constant}0|property}1 "
Expand Down
20 changes: 20 additions & 0 deletions include/swift/Sema/CSFix.h
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,12 @@ enum class FixKind : uint8_t {
/// even though result type of the reference doesn't conform
/// to an expected protocol.
AllowInvalidStaticMemberRefOnProtocolMetatype,

/// Allow the wrappedValue type of any property wrapper that is a
/// part of a composed property wrapper to mismatch the type of
/// another property wrapper that is a part of the same composed
/// property wrapper.
AllowWrappedValueMismatch,
};

class ConstraintFix {
Expand Down Expand Up @@ -615,6 +621,20 @@ class TreatArrayLiteralAsDictionary final : public ContextualMismatch {
ConstraintLocator *loc);
};

class AllowWrappedValueMismatch : public ContextualMismatch {
AllowWrappedValueMismatch(ConstraintSystem &cs, Type lhs, Type rhs,
ConstraintLocator *locator)
: ContextualMismatch(cs, FixKind::AllowWrappedValueMismatch, lhs, rhs, locator) {}

public:
std::string getName() const override { return "fix wrapped value type mismatch"; }

bool diagnose(const Solution &solution, bool asNote = false) const override;

static AllowWrappedValueMismatch *create(ConstraintSystem &cs, Type lhs, Type rhs,
ConstraintLocator *locator);
};

/// Mark function type as explicitly '@escaping'.
class MarkExplicitlyEscaping final : public ContextualMismatch {
MarkExplicitlyEscaping(ConstraintSystem &cs, Type lhs, Type rhs,
Expand Down
25 changes: 25 additions & 0 deletions include/swift/Sema/ConstraintLocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,12 @@ class ConstraintLocator : public llvm::FoldingSetNode {
/// Determine whether this locator points to the generic parameter.
bool isForGenericParameter() const;

/// Determine whether this locator points to any of the
/// property wrappers' types for a given composed property
/// wrapper, with the exception of the innermost property wrapper's
/// type.
bool isForWrappedValue() const;

/// Determine whether this locator points to the element type of a
/// sequence in a for ... in ... loop.
bool isForSequenceElementType() const;
Expand Down Expand Up @@ -356,6 +362,11 @@ class ConstraintLocator : public llvm::FoldingSetNode {
/// If this locator points to generic parameter return its type.
GenericTypeParamType *getGenericParameter() const;

/// If this locator points to any of the property wrappers' types
/// for a given composed property wrapper, with the exception
/// of the innermost property wrapper's type.
Type getWrappedValue() const;

/// Produce a profile of this locator, for use in a folding set.
static void Profile(llvm::FoldingSetNodeID &id, ASTNode anchor,
ArrayRef<PathElement> path);
Expand Down Expand Up @@ -692,6 +703,20 @@ class LocatorPathElt::GenericParameter final : public StoredPointerElement<Gener
}
};

class LocatorPathElt::WrappedValue final : public StoredPointerElement<TypeBase> {
public:
WrappedValue(Type type)
: StoredPointerElement(PathElementKind::WrappedValue, type.getPointer()) {
assert(type.getPointer() != nullptr);
}

Type getType() const { return getStoredPointer(); }

static bool classof(const LocatorPathElt *elt) {
return elt->getKind() == PathElementKind::WrappedValue;
}
};

class LocatorPathElt::OpenedGeneric final : public StoredPointerElement<GenericSignatureImpl> {
public:
OpenedGeneric(GenericSignature sig)
Expand Down
5 changes: 5 additions & 0 deletions include/swift/Sema/ConstraintLocatorPathElts.def
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,11 @@ SIMPLE_LOCATOR_PATH_ELT(InstanceType)
/// Also contains the generic parameter type itself.
CUSTOM_LOCATOR_PATH_ELT(GenericParameter)

/// The wrappedValue type for a property wrapper.
///
/// Must equal the property declaration's property wrapper type.
CUSTOM_LOCATOR_PATH_ELT(WrappedValue)

/// A component of a key path.
CUSTOM_LOCATOR_PATH_ELT(KeyPathComponent)

Expand Down
16 changes: 12 additions & 4 deletions lib/Sema/CSDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,16 @@ bool RequirementFailure::isStaticOrInstanceMember(const ValueDecl *decl) {
return decl->isStatic();
}

bool WrappedValueMismatch::diagnoseAsError() {
auto *locator = getLocator();
auto elt = locator->castLastElementTo<LocatorPathElt::WrappedValue>();

emitDiagnostic(diag::composed_property_wrapper_mismatch, getFromType(),
resolveType(elt.getType())->getString(), getToType());

return true;
}

bool RequirementFailure::diagnoseAsError() {
const auto *reqDC = getRequirementDC();
auto *genericCtx = getGenericContext();
Expand Down Expand Up @@ -639,11 +649,10 @@ Optional<Diag<Type, Type>> GenericArgumentsMismatchFailure::getDiagnosticFor(
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_ComposedPropertyWrapper:
case CTP_Unused:
case CTP_CannotFail:
case CTP_YieldByReference:
Expand Down Expand Up @@ -3132,11 +3141,10 @@ ContextualFailure::getDiagnosticFor(ContextualTypePurpose context,

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_ComposedPropertyWrapper:
case CTP_Unused:
case CTP_CannotFail:
case CTP_YieldByReference:
Expand Down
10 changes: 10 additions & 0 deletions lib/Sema/CSDiagnostics.h
Original file line number Diff line number Diff line change
Expand Up @@ -769,6 +769,16 @@ class MissingOptionalUnwrapFailure final : public ContextualFailure {
void offerForceUnwrapFixIt(const Expr *expr) const;
};

class WrappedValueMismatch final : public ContextualFailure {
public:
WrappedValueMismatch(const Solution &solution, Type fromType,
Type toType, ConstraintLocator *locator)
: ContextualFailure(solution, fromType, toType, locator) {
}

bool diagnoseAsError() override;
};

/// Diagnostics for mismatched generic arguments e.g
/// ```swift
/// struct F<G> {}
Expand Down
12 changes: 12 additions & 0 deletions lib/Sema/CSFix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,18 @@ ContextualMismatch *ContextualMismatch::create(ConstraintSystem &cs, Type lhs,
return new (cs.getAllocator()) ContextualMismatch(cs, lhs, rhs, locator);
}

bool AllowWrappedValueMismatch::diagnose(const Solution &solution, bool asError) const {
WrappedValueMismatch failure(solution, getFromType(), getToType(), getLocator());
return failure.diagnoseAsError();
}

AllowWrappedValueMismatch *AllowWrappedValueMismatch::create(ConstraintSystem &cs,
Type lhs,
Type rhs,
ConstraintLocator *locator) {
return new (cs.getAllocator()) AllowWrappedValueMismatch(cs, lhs, rhs, locator);
}

/// Computes the contextual type information for a type mismatch of a
/// component in a structural type (tuple or function type).
///
Expand Down
18 changes: 11 additions & 7 deletions lib/Sema/CSGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3590,6 +3590,7 @@ static bool generateWrappedPropertyTypeConstraints(
auto dc = wrappedVar->getInnermostDeclContext();

Type wrappedValueType;
Type wrapperType;
auto wrapperAttributes = wrappedVar->getAttachedPropertyWrappers();
for (unsigned i : indices(wrapperAttributes)) {
// FIXME: We should somehow pass an OpenUnboundGenericTypeFn to
Expand All @@ -3601,22 +3602,25 @@ static bool generateWrappedPropertyTypeConstraints(
return true;

auto *typeExpr = wrapperAttributes[i]->getTypeExpr();
auto *locator = cs.getConstraintLocator(typeExpr);
auto wrapperType = cs.replaceInferableTypesWithTypeVars(rawWrapperType, locator);
cs.setType(typeExpr, wrapperType);

if (!wrappedValueType) {
// Equate the outermost wrapper type to the initializer type.
auto *locator = cs.getConstraintLocator(typeExpr);
wrapperType =
cs.replaceInferableTypesWithTypeVars(rawWrapperType, locator);
if (initializerType)
cs.addConstraint(ConstraintKind::Equal, wrapperType, initializerType, locator);
} else {
// The former wrappedValue type must be equal to the current wrapper type
cs.addConstraint(ConstraintKind::Equal, wrapperType, wrappedValueType,
cs.getConstraintLocator(locator, LocatorPathElt::ContextualType()));
cs.setContextualType(typeExpr, TypeLoc::withoutLoc(wrappedValueType),
CTP_ComposedPropertyWrapper);
auto *locator = cs.getConstraintLocator(
typeExpr, LocatorPathElt::WrappedValue(wrapperType));
wrapperType =
cs.replaceInferableTypesWithTypeVars(rawWrapperType, locator);
cs.addConstraint(ConstraintKind::Equal, wrapperType, wrappedValueType, locator);
}

cs.setType(typeExpr, wrapperType);

wrappedValueType = wrapperType->getTypeOfMember(
dc->getParentModule(), wrapperInfo.valueVar);
}
Expand Down
7 changes: 7 additions & 0 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4312,6 +4312,12 @@ bool ConstraintSystem::repairFailures(
break;
}

case ConstraintLocator::WrappedValue: {
conversionsOrFixes.push_back(AllowWrappedValueMismatch::create(
*this, lhs, rhs, getConstraintLocator(locator)));
break;
}

case ConstraintLocator::FunctionArgument: {
auto *argLoc = getConstraintLocator(
locator.withPathElement(LocatorPathElt::SynthesizedArgument(0)));
Expand Down Expand Up @@ -11254,6 +11260,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
case FixKind::AllowUnsupportedRuntimeCheckedCast:
case FixKind::AllowAlwaysSucceedCheckedCast:
case FixKind::AllowInvalidStaticMemberRefOnProtocolMetatype:
case FixKind::AllowWrappedValueMismatch:
case FixKind::RemoveExtraneousArguments: {
return recordFix(fix) ? SolutionKind::Error : SolutionKind::Solved;
}
Expand Down
17 changes: 17 additions & 0 deletions lib/Sema/ConstraintLocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ unsigned LocatorPathElt::getNewSummaryFlags() const {
case ConstraintLocator::DynamicType:
case ConstraintLocator::SubscriptMember:
case ConstraintLocator::OpenedGeneric:
case ConstraintLocator::WrappedValue:
case ConstraintLocator::GenericParameter:
case ConstraintLocator::GenericArgument:
case ConstraintLocator::NamedTupleElement:
Expand Down Expand Up @@ -188,6 +189,10 @@ bool ConstraintLocator::isForGenericParameter() const {
return isLastElement<LocatorPathElt::GenericParameter>();
}

bool ConstraintLocator::isForWrappedValue() const {
return isLastElement<LocatorPathElt::WrappedValue>();
}

bool ConstraintLocator::isForSequenceElementType() const {
return isLastElement<LocatorPathElt::SequenceElementType>();
}
Expand Down Expand Up @@ -218,6 +223,12 @@ GenericTypeParamType *ConstraintLocator::getGenericParameter() const {
castLastElementTo<LocatorPathElt::GenericParameter>().getType() : nullptr;
}

Type ConstraintLocator::getWrappedValue() const {
return isForWrappedValue()
? castLastElementTo<LocatorPathElt::WrappedValue>().getType()
: Type();
}

void ConstraintLocator::dump(SourceManager *sm) const {
dump(sm, llvm::errs());
llvm::errs() << "\n";
Expand Down Expand Up @@ -270,6 +281,12 @@ void ConstraintLocator::dump(SourceManager *sm, raw_ostream &out) const {
out << "generic parameter '" << gpElt.getType()->getString(PO) << "'";
break;
}
case WrappedValue: {
auto wrappedValueElt = elt.castTo<LocatorPathElt::WrappedValue>();
out << "composed property wrapper type '"
<< wrappedValueElt.getType()->getString(PO) << "'";
break;
}
case ApplyArgument:
out << "apply argument";
break;
Expand Down
6 changes: 4 additions & 2 deletions test/decl/var/property_wrappers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1096,8 +1096,10 @@ struct TestComposition {
@WrapperD<WrapperE, Int, String> @WrapperE var p3: Int?
@WrapperD<WrapperC, Int, String> @WrapperC var p4: Int?
@WrapperD<WrapperC, Int, String> @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<Int>' does not match former 'wrappedValue' type 'WrapperC<Value>'}}
// expected-note@-1 {{explicitly specify the generic arguments to fix this issue}}
// expected-error@-2 {{composed wrapper type 'WrapperE<Int>' does not match type of 'WrapperD<WrapperC<Value>, Int, String>.wrappedValue', which is 'WrapperC<Value>'}}

@Wrapper<String> @Wrapper var value: Int // expected-error{{composed wrapper type 'Wrapper<Int>' does not match type of 'Wrapper<String>.wrappedValue', which is 'String'}}

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=)}}
Expand Down
2 changes: 1 addition & 1 deletion validation-test/compiler_crashers_2_fixed/sr11599.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@ struct B {
}

struct C {
@A @B var foo: Int // expected-error{{composed wrapper type 'B' does not match former 'wrappedValue' type 'Int'}}
@A @B var foo: Int // expected-error{{composed wrapper type 'B' does not match type of 'A.wrappedValue', which is 'Int'}}
}