Skip to content

[Property Wrappers] Improve diagnostics for property wrappers initialized out-of-line #32672

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
merged 6 commits into from
Jul 8, 2020
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
6 changes: 6 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -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 "
Expand Down
16 changes: 16 additions & 0 deletions lib/Sema/CSApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -8139,6 +8141,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();
Expand Down Expand Up @@ -8419,6 +8432,9 @@ SolutionApplicationTarget SolutionApplicationTarget::walk(ASTWalker &walker) {

case Kind::patternBinding:
return *this;

case Kind::uninitializedWrappedVar:
return *this;
}

llvm_unreachable("invalid target kind");
Expand Down
32 changes: 26 additions & 6 deletions lib/Sema/CSDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -423,8 +423,9 @@ bool MissingConformanceFailure::diagnoseAsError() {
// with it and if so skip conformance error, otherwise we'd
// produce an unrelated `<type> doesn't conform to Equatable protocol`
// diagnostic.
if (isPatternMatchingOperator(const_cast<Expr *>(anchor))) {
if (auto *binaryOp = dyn_cast_or_null<BinaryExpr>(findParentExpr(anchor))) {
if (isPatternMatchingOperator(anchor)) {
auto *expr = castToExpr(anchor);
if (auto *binaryOp = dyn_cast_or_null<BinaryExpr>(findParentExpr(expr))) {
auto *caseExpr = binaryOp->getArg()->getElement(0);

llvm::SmallPtrSet<Expr *, 4> anchors;
Expand Down Expand Up @@ -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<EnumType>())
return !enumType->getDecl()->hasOnlyCasesWithoutAssociatedValues();
Expand All @@ -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;
Expand Down Expand Up @@ -611,6 +613,10 @@ Optional<Diag<Type, Type>> 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:
Expand Down Expand Up @@ -2204,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:
Expand Down Expand Up @@ -2902,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:
Expand Down Expand Up @@ -4998,8 +5011,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<Expr *>() || getAnchor().is<TypeRepr *>());
if (auto *expr = getAsExpr(getAnchor()))
return diagnoseForAnchor(expr, Parameters);

return diagnoseForAnchor(getAnchor().get<TypeRepr *>(), Parameters);
}

bool diagnosed = false;
for (const auto &scope : scopedParameters)
Expand Down
5 changes: 3 additions & 2 deletions lib/Sema/CSDiagnostics.h
Original file line number Diff line number Diff line change
Expand Up @@ -265,8 +265,9 @@ class RequirementFailure : public FailureDiagnostic {
assert(getGenericContext() &&
"Affected decl not within a generic context?");

if (auto *parentExpr = findParentExpr(getRawAnchor().get<Expr *>()))
Apply = dyn_cast<ApplyExpr>(parentExpr);
if (auto *expr = getAsExpr(getRawAnchor()))
if (auto *parentExpr = findParentExpr(expr))
Apply = dyn_cast<ApplyExpr>(parentExpr);
}

unsigned getRequirementIndex() const {
Expand Down
44 changes: 34 additions & 10 deletions lib/Sema/CSGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3874,35 +3874,38 @@ 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,
cs.addConstraint(ConstraintKind::Equal, wrapperType, wrappedValueType,
locator);
cs.setContextualType(typeRepr, TypeLoc::withoutLoc(wrappedValueType),
CTP_ComposedPropertyWrapper);
}

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;
Expand All @@ -3922,7 +3925,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;

Expand Down Expand Up @@ -4165,6 +4168,27 @@ bool ConstraintSystem::generateConstraints(

return hadError;
}

case SolutionApplicationTarget::Kind::uninitializedWrappedVar: {
auto *wrappedVar = target.getAsUninitializedWrappedVar();
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);
Type propertyType = wrappedVar->getType();
if (!wrappedValueType || propertyType->hasError())
return true;

addConstraint(ConstraintKind::Equal, propertyType, wrappedValueType,
getConstraintLocator(wrappedVar, LocatorPathElt::ContextualType()));
setContextualType(wrappedVar, TypeLoc::withoutLoc(wrappedValueType),
CTP_WrappedProperty);
return false;
}
}
}

Expand Down
2 changes: 2 additions & 0 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10156,6 +10156,8 @@ void ConstraintSystem::addContextualConversionConstraint(
case CTP_CoerceOperand:
case CTP_SubscriptAssignSource:
case CTP_ForEachStmt:
case CTP_WrappedProperty:
case CTP_ComposedPropertyWrapper:
break;
}

Expand Down
29 changes: 27 additions & 2 deletions lib/Sema/ConstraintSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4249,11 +4249,17 @@ Optional<Identifier> 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(">") ||
Expand Down Expand Up @@ -4674,6 +4680,11 @@ SolutionApplicationTarget SolutionApplicationTarget::forForEachStmt(
return target;
}

SolutionApplicationTarget
SolutionApplicationTarget::forUninitializedWrappedVar(VarDecl *wrappedVar) {
return SolutionApplicationTarget(wrappedVar);
}

ContextualPattern
SolutionApplicationTarget::getContextualPattern() const {
assert(kind == Kind::expression);
Expand Down Expand Up @@ -4741,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;
}
Expand Down Expand Up @@ -4781,6 +4794,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(),
Expand Down
Loading