Skip to content

Commit f05589f

Browse files
hborlahborla
committed
[Diagnostics] Improve diagnostics for passing an invalid projected
value argument.
1 parent 4e55f43 commit f05589f

File tree

11 files changed

+87
-138
lines changed

11 files changed

+87
-138
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5574,16 +5574,15 @@ ERROR(property_wrapper_param_not_supported,none,
55745574
(DescriptiveDeclKind))
55755575
NOTE(property_wrapper_declared_here,none,
55765576
"property wrapper type %0 declared here", (Identifier))
5577-
ERROR(property_wrapper_param_no_projection,none,
5578-
"cannot use property wrapper projection parameter; "
5579-
"wrapper %0 does not have a 'projectedValue'", (Type))
5580-
ERROR(property_wrapper_param_no_wrapper,none,
5581-
"cannot use property wrapper projection argument; "
5582-
"parameter does not have an attached property wrapper",
5583-
())
5584-
ERROR(property_wrapper_param_projection_invalid,none,
5585-
"cannot use property wrapper projection argument; "
5586-
"pass wrapped value type %0 instead", (Type))
5577+
ERROR(invalid_projection_argument,none,
5578+
"cannot use property wrapper projection argument", ())
5579+
NOTE(property_wrapper_param_no_wrapper,none,
5580+
"parameter %0 does not have an attached property wrapper", (Identifier))
5581+
NOTE(property_wrapper_param_attr_arg,none,
5582+
"property wrapper has arguments in the wrapper attribute", ())
5583+
NOTE(property_wrapper_no_init_projected_value,none,
5584+
"property wrapper type %0 does not support initialization from a "
5585+
"projected value", (Type))
55875586
ERROR(property_wrapper_param_mutating,none,
55885587
"property wrapper applied to parameter must have a nonmutating "
55895588
"'wrappedValue' getter", ())

‎include/swift/AST/Types.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3329,7 +3329,7 @@ END_CAN_TYPE_WRAPPER(FunctionType, AnyFunctionType)
33293329
struct ParameterListInfo {
33303330
SmallBitVector defaultArguments;
33313331
SmallBitVector acceptsUnlabeledTrailingClosures;
3332-
SmallVector<const ParamDecl *, 4> propertyWrappers;
3332+
SmallBitVector propertyWrappers;
33333333

33343334
public:
33353335
ParameterListInfo() { }
@@ -3346,7 +3346,7 @@ struct ParameterListInfo {
33463346

33473347
/// The ParamDecl at the given index if the parameter has an applied
33483348
/// property wrapper.
3349-
const ParamDecl *getPropertyWrapperParam(unsigned paramIdx) const;
3349+
bool hasExternalPropertyWrapper(unsigned paramIdx) const;
33503350

33513351
/// Retrieve the number of non-defaulted parameters.
33523352
unsigned numNonDefaultedParameters() const {

‎include/swift/Sema/CSFix.h

Lines changed: 11 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -122,12 +122,8 @@ enum class FixKind : uint8_t {
122122
/// the storage or property wrapper.
123123
UseWrappedValue,
124124

125-
/// Add 'var projectedValue' to the property wrapper type to allow passing
126-
/// a projection argument.
127-
AddProjectedValue,
128-
129-
/// Add '@propertyWrapper' to a nominal type declaration.
130-
AddPropertyWrapperAttribute,
125+
/// Remove the '$' prefix from an argument label or parameter name.
126+
RemoveProjectedValueArgument,
131127

132128
/// Instead of spelling out `subscript` directly, use subscript operator.
133129
UseSubscriptOperator,
@@ -984,37 +980,21 @@ class UseWrappedValue final : public ConstraintFix {
984980
ConstraintLocator *locator);
985981
};
986982

987-
class AddProjectedValue final : public ConstraintFix {
983+
class RemoveProjectedValueArgument final : public ConstraintFix {
988984
Type wrapperType;
985+
ParamDecl *param;
989986

990-
AddProjectedValue(ConstraintSystem &cs, Type wrapper,
991-
ConstraintLocator *locator)
992-
: ConstraintFix(cs, FixKind::AddProjectedValue, locator), wrapperType(wrapper) {}
987+
RemoveProjectedValueArgument(ConstraintSystem &cs, Type wrapper,
988+
ParamDecl *param, ConstraintLocator *locator)
989+
: ConstraintFix(cs, FixKind::RemoveProjectedValueArgument, locator),
990+
wrapperType(wrapper), param(param) {}
993991

994992
public:
995-
static AddProjectedValue *create(ConstraintSystem &cs, Type wrapper,
996-
ConstraintLocator *locator);
997-
998-
std::string getName() const override {
999-
return "add 'var projectedValue' to pass a projection argument";
1000-
}
1001-
1002-
bool diagnose(const Solution &solution, bool asNote = false) const override;
1003-
};
1004-
1005-
class AddPropertyWrapperAttribute final : public ConstraintFix {
1006-
Type wrapperType;
1007-
1008-
AddPropertyWrapperAttribute(ConstraintSystem &cs, Type wrapper,
1009-
ConstraintLocator *locator)
1010-
: ConstraintFix(cs, FixKind::AddPropertyWrapperAttribute, locator), wrapperType(wrapper) {}
1011-
1012-
public:
1013-
static AddPropertyWrapperAttribute *create(ConstraintSystem &cs, Type wrapper,
1014-
ConstraintLocator *locator);
993+
static RemoveProjectedValueArgument *create(ConstraintSystem &cs, Type wrapper,
994+
ParamDecl *param, ConstraintLocator *locator);
1015995

1016996
std::string getName() const override {
1017-
return "add '@propertyWrapper'";
997+
return "remove '$' from argument label";
1018998
}
1019999

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

‎lib/AST/Type.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -967,7 +967,7 @@ ParameterListInfo::ParameterListInfo(
967967
}
968968

969969
if (param->hasAttachedPropertyWrapper()) {
970-
propertyWrappers[i] = param;
970+
propertyWrappers.set(i);
971971
}
972972
}
973973
}
@@ -983,9 +983,8 @@ bool ParameterListInfo::acceptsUnlabeledTrailingClosureArgument(
983983
acceptsUnlabeledTrailingClosures[paramIdx];
984984
}
985985

986-
const ParamDecl *
987-
ParameterListInfo::getPropertyWrapperParam(unsigned paramIdx) const {
988-
return paramIdx < propertyWrappers.size() ? propertyWrappers[paramIdx] : nullptr;
986+
bool ParameterListInfo::hasExternalPropertyWrapper(unsigned paramIdx) const {
987+
return paramIdx < propertyWrappers.size() ? propertyWrappers[paramIdx] : false;
989988
}
990989

991990
/// Turn a param list into a symbolic and printable representation that does not

‎lib/Sema/CSApply.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5895,7 +5895,8 @@ Expr *ExprRewriter::coerceCallArguments(
58955895
return true;
58965896
};
58975897

5898-
if (auto *param = paramInfo.getPropertyWrapperParam(paramIdx)) {
5898+
if (paramInfo.hasExternalPropertyWrapper(paramIdx)) {
5899+
auto *param = getParameterAt(callee.getDecl(), paramIdx);
58995900
auto appliedWrapper = appliedPropertyWrappers[appliedWrapperIndex++];
59005901
auto wrapperType = solution.simplifyType(appliedWrapper.wrapperType);
59015902
auto initKind = appliedWrapper.initKind;

‎lib/Sema/CSDiagnostics.cpp

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3306,32 +3306,26 @@ bool MissingPropertyWrapperUnwrapFailure::diagnoseAsError() {
33063306
return true;
33073307
}
33083308

3309-
if (isa<ParamDecl>(getProperty())) {
3310-
auto wrapperType = getToType();
3311-
auto wrappedValueType = computeWrappedValueType(getProperty(), wrapperType);
3312-
emitDiagnostic(diag::property_wrapper_param_projection_invalid, wrappedValueType);
3313-
return true;
3314-
}
3315-
33163309
emitDiagnostic(diag::incorrect_property_wrapper_reference, getPropertyName(),
33173310
getFromType(), getToType(), true)
33183311
.fixItRemoveChars(getLoc(), endLoc);
33193312
return true;
33203313
}
33213314

3322-
bool MissingProjectedValueFailure::diagnoseAsError() {
3323-
emitDiagnostic(diag::property_wrapper_param_no_projection, wrapperType);
3324-
return true;
3325-
}
3326-
3327-
bool MissingPropertyWrapperAttributeFailure::diagnoseAsError() {
3328-
if (auto *param = getAsDecl<ParamDecl>(getAnchor())) {
3315+
bool InvalidProjectedValueArgument::diagnoseAsError() {
3316+
if (param->hasImplicitPropertyWrapper()) {
33293317
emitDiagnostic(diag::invalid_implicit_property_wrapper, wrapperType);
3330-
3331-
// FIXME: emit a note and fix-it to add '@propertyWrapper' if the
3332-
// type is a nominal and in the same module.
33333318
} else {
3334-
emitDiagnostic(diag::property_wrapper_param_no_wrapper);
3319+
emitDiagnostic(diag::invalid_projection_argument);
3320+
3321+
if (!param->hasAttachedPropertyWrapper()) {
3322+
param->diagnose(diag::property_wrapper_param_no_wrapper, param->getName());
3323+
} else if (param->getAttachedPropertyWrappers().front()->getArg()) {
3324+
param->diagnose(diag::property_wrapper_param_attr_arg);
3325+
} else {
3326+
auto backingType = param->getPropertyWrapperBackingPropertyType();
3327+
param->diagnose(diag::property_wrapper_no_init_projected_value, backingType);
3328+
}
33353329
}
33363330

33373331
return true;

‎lib/Sema/CSDiagnostics.h

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1079,24 +1079,15 @@ class MissingPropertyWrapperUnwrapFailure final
10791079
bool diagnoseAsError() override;
10801080
};
10811081

1082-
class MissingProjectedValueFailure final : public FailureDiagnostic {
1082+
class InvalidProjectedValueArgument final : public FailureDiagnostic {
10831083
Type wrapperType;
1084+
ParamDecl *param;
10841085

10851086
public:
1086-
MissingProjectedValueFailure(const Solution &solution, Type wrapper,
1087-
ConstraintLocator *locator)
1088-
: FailureDiagnostic(solution, locator), wrapperType(resolveType(wrapper)) {}
1089-
1090-
bool diagnoseAsError() override;
1091-
};
1092-
1093-
class MissingPropertyWrapperAttributeFailure final : public FailureDiagnostic {
1094-
Type wrapperType;
1095-
1096-
public:
1097-
MissingPropertyWrapperAttributeFailure(const Solution &solution, Type wrapper,
1098-
ConstraintLocator *locator)
1099-
: FailureDiagnostic(solution, locator), wrapperType(resolveType(wrapper)) {}
1087+
InvalidProjectedValueArgument(const Solution &solution, Type wrapper,
1088+
ParamDecl *param, ConstraintLocator *locator)
1089+
: FailureDiagnostic(solution, locator), wrapperType(resolveType(wrapper)),
1090+
param(param) {}
11001091

11011092
bool diagnoseAsError() override;
11021093
};

‎lib/Sema/CSFix.cpp

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -646,25 +646,15 @@ UseWrappedValue *UseWrappedValue::create(ConstraintSystem &cs,
646646
UseWrappedValue(cs, propertyWrapper, base, wrapper, locator);
647647
}
648648

649-
bool AddProjectedValue::diagnose(const Solution &solution, bool asNote) const {
650-
MissingProjectedValueFailure failure(solution, wrapperType, getLocator());
649+
bool RemoveProjectedValueArgument::diagnose(const Solution &solution, bool asNote) const {
650+
InvalidProjectedValueArgument failure(solution, wrapperType, param, getLocator());
651651
return failure.diagnose(asNote);
652652
}
653653

654-
AddProjectedValue *AddProjectedValue::create(ConstraintSystem &cs, Type wrapperType,
655-
ConstraintLocator *locator) {
656-
return new (cs.getAllocator()) AddProjectedValue(cs, wrapperType, locator);
657-
}
658-
659-
bool AddPropertyWrapperAttribute::diagnose(const Solution &solution, bool asNote) const {
660-
MissingPropertyWrapperAttributeFailure failure(solution, wrapperType, getLocator());
661-
return failure.diagnose(asNote);
662-
}
663-
664-
AddPropertyWrapperAttribute *AddPropertyWrapperAttribute::create(ConstraintSystem &cs,
665-
Type wrapperType,
666-
ConstraintLocator *locator) {
667-
return new (cs.getAllocator()) AddPropertyWrapperAttribute(cs, wrapperType, locator);
654+
RemoveProjectedValueArgument *
655+
RemoveProjectedValueArgument::create(ConstraintSystem &cs, Type wrapperType,
656+
ParamDecl *param, ConstraintLocator *locator) {
657+
return new (cs.getAllocator()) RemoveProjectedValueArgument(cs, wrapperType, param, locator);
668658
}
669659

670660
bool UseSubscriptOperator::diagnose(const Solution &solution,

‎lib/Sema/CSGen.cpp

Lines changed: 20 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -4071,12 +4071,29 @@ ConstraintSystem::applyPropertyWrapperToParameter(
40714071
anchor = apply->getFn();
40724072
}
40734073

4074-
if (argLabel.hasDollarPrefix() && (!param || !param->hasAttachedPropertyWrapper())) {
4074+
auto supportsProjectedValueInit = [&](ParamDecl *param) -> bool {
4075+
if (!param->hasAttachedPropertyWrapper())
4076+
return false;
4077+
4078+
if (param->hasImplicitPropertyWrapper())
4079+
return true;
4080+
4081+
if (param->getAttachedPropertyWrappers().front()->getArg())
4082+
return false;
4083+
4084+
auto wrapperInfo = param->getAttachedPropertyWrapperTypeInfo(0);
4085+
return wrapperInfo.projectedValueVar && wrapperInfo.hasProjectedValueInit;
4086+
};
4087+
4088+
if (argLabel.hasDollarPrefix() && (!param || !supportsProjectedValueInit(param))) {
40754089
if (!shouldAttemptFixes())
40764090
return getTypeMatchFailure(locator);
40774091

4078-
addConstraint(matchKind, paramType, wrapperType, locator);
4079-
auto *fix = AddPropertyWrapperAttribute::create(*this, wrapperType, getConstraintLocator(locator));
4092+
if (paramType->hasTypeVariable())
4093+
recordPotentialHole(paramType);
4094+
4095+
auto *loc = getConstraintLocator(locator);
4096+
auto *fix = RemoveProjectedValueArgument::create(*this, wrapperType, param, loc);
40804097
if (recordFix(fix))
40814098
return getTypeMatchFailure(locator);
40824099

@@ -4085,34 +4102,8 @@ ConstraintSystem::applyPropertyWrapperToParameter(
40854102

40864103
PropertyWrapperInitKind initKind;
40874104
if (argLabel.hasDollarPrefix()) {
4088-
auto attemptProjectedValueFix = [&](ConstraintFix *fix) -> ConstraintSystem::TypeMatchResult {
4089-
if (shouldAttemptFixes()) {
4090-
if (paramType->hasTypeVariable())
4091-
recordPotentialHole(paramType);
4092-
4093-
if (!recordFix(fix))
4094-
return getTypeMatchSuccess();
4095-
}
4096-
4097-
return getTypeMatchFailure(locator);
4098-
};
4099-
4100-
auto typeInfo = wrapperType->getAnyNominal()->getPropertyWrapperTypeInfo();
4101-
if (!typeInfo.projectedValueVar) {
4102-
auto *fix = AddProjectedValue::create(*this, wrapperType, getConstraintLocator(locator));
4103-
return attemptProjectedValueFix(fix);
4104-
}
4105-
41064105
Type projectionType = computeProjectedValueType(param, wrapperType);
41074106
addConstraint(matchKind, paramType, projectionType, locator);
4108-
4109-
if (!param->hasImplicitPropertyWrapper() &&
4110-
param->getAttachedPropertyWrappers().front()->getArg()) {
4111-
auto *fix = UseWrappedValue::create(*this, param, /*base=*/Type(), wrapperType,
4112-
getConstraintLocator(locator));
4113-
return attemptProjectedValueFix(fix);
4114-
}
4115-
41164107
initKind = PropertyWrapperInitKind::ProjectedValue;
41174108
} else {
41184109
generateWrappedPropertyTypeConstraints(*this, wrapperType, param, paramType);

‎lib/Sema/CSSimplify.cpp

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1420,11 +1420,11 @@ ConstraintSystem::TypeMatchResult constraints::matchCallArguments(
14201420
cs, cs.getConstraintLocator(loc)));
14211421
}
14221422

1423-
auto *wrappedParam = paramInfo.getPropertyWrapperParam(argIdx);
14241423
auto argLabel = argument.getLabel();
1425-
if (wrappedParam || argLabel.hasDollarPrefix()) {
1426-
if (cs.applyPropertyWrapperToParameter(paramTy, argTy, const_cast<ParamDecl *>(wrappedParam),
1427-
argLabel, subKind, locator).isFailure()) {
1424+
if (paramInfo.hasExternalPropertyWrapper(argIdx) || argLabel.hasDollarPrefix()) {
1425+
auto *param = getParameterAt(callee, argIdx);
1426+
if (cs.applyPropertyWrapperToParameter(paramTy, argTy, const_cast<ParamDecl *>(param),
1427+
argLabel, subKind, loc).isFailure()) {
14281428
return cs.getTypeMatchFailure(loc);
14291429
}
14301430
continue;
@@ -8288,8 +8288,8 @@ bool ConstraintSystem::resolveClosure(TypeVariableType *typeVar,
82888288
var->setInvalid();
82898289
});
82908290

8291-
auto *fix = AddPropertyWrapperAttribute::create(*this, backingType,
8292-
getConstraintLocator(paramDecl));
8291+
auto *fix = RemoveProjectedValueArgument::create(*this, backingType, paramDecl,
8292+
getConstraintLocator(paramDecl));
82938293
recordFix(fix);
82948294
hasError = true;
82958295
}
@@ -10792,8 +10792,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
1079210792
case FixKind::SkipUnhandledConstructInResultBuilder:
1079310793
case FixKind::UsePropertyWrapper:
1079410794
case FixKind::UseWrappedValue:
10795-
case FixKind::AddProjectedValue:
10796-
case FixKind::AddPropertyWrapperAttribute:
10795+
case FixKind::RemoveProjectedValueArgument:
1079710796
case FixKind::ExpandArrayIntoVarargs:
1079810797
case FixKind::UseRawValue:
1079910798
case FixKind::SpecifyBaseTypeForContextualMember:

0 commit comments

Comments
 (0)