Merge pull request #36732 from Strieker/composed_property_wrapper_improved_error_handling

Improved error handling for composed property wrapper mismatches

Author: xedin

include/swift/AST/DiagnosticsSema.def

@@ -5672,8 +5672,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 "

include/swift/Sema/CSFix.h

@@ -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 {
@@ -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,

include/swift/Sema/ConstraintLocator.h

@@ -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;
@@ -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);
@@ -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)

include/swift/Sema/ConstraintLocatorPathElts.def

@@ -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)
 

lib/Sema/CSDiagnostics.cpp

@@ -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();
@@ -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:
@@ -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:

lib/Sema/CSDiagnostics.h

@@ -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> {}

lib/Sema/CSFix.cpp

@@ -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).
 ///

lib/Sema/CSGen.cpp

@@ -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
@@ -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);
   }

lib/Sema/CSSimplify.cpp

@@ -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)));
@@ -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;
   }

lib/Sema/ConstraintLocator.cpp

@@ -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:
@@ -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>();
 }
@@ -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";
@@ -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;

test/decl/var/property_wrappers.swift

@@ -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=)}}

validation-test/compiler_crashers_2_fixed/sr11599.swift

@@ -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'}}
 }