Skip to content

Commit

Permalink
Merge pull request apple#32672 from hborla/property-wrapper-diagnostics
Browse files Browse the repository at this point in the history
[Property Wrappers] Improve diagnostics for property wrappers initialized out-of-line
  • Loading branch information
hborla committed Jul 8, 2020
2 parents d6982cf + 9b4f1f0 commit 063d420
Show file tree
Hide file tree
Showing 14 changed files with 214 additions and 130 deletions.
6 changes: 6 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Expand Up @@ -5036,6 +5036,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
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
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 @@ -613,6 +615,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 @@ -2207,6 +2213,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 @@ -2905,6 +2913,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 @@ -5001,8 +5014,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
Expand Up @@ -260,8 +260,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
Expand Up @@ -3879,35 +3879,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 @@ -3929,7 +3932,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 @@ -4172,6 +4175,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
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
Expand Up @@ -4237,11 +4237,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 @@ -4662,6 +4668,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 @@ -4729,6 +4740,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 @@ -4769,6 +4782,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

0 comments on commit 063d420

Please sign in to comment.