Merge pull request #31938 from theblixguy/fix/SR-12839-5.3

tkremenek · web-flow · commit a242ad8895e7 · 2020-05-21T13:50:05.000-07:00
[5.3] [Typechecker] Emit a specialised diagnostic for redeclaration errors when the declaration is synthesised
diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h
@@ -5217,6 +5217,12 @@ class VarDecl : public AbstractStorageDecl {
   Optional<PropertyWrapperMutability>
       getPropertyWrapperMutability() const;
 
+  /// Returns whether this property is the backing storage property or a storage
+  /// wrapper for wrapper instance's projectedValue. If this property is
+  /// neither, then it returns `None`.
+  Optional<PropertyWrapperSynthesizedPropertyKind>
+  getPropertyWrapperSynthesizedPropertyKind() const;
+
   /// Retrieve the backing storage property for a property that has an
   /// attached property wrapper.
   ///
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -760,12 +760,20 @@ ERROR(invalid_redecl,none,"invalid redeclaration of %0", (DeclName))
 ERROR(invalid_redecl_init,none,
       "invalid redeclaration of synthesized %select{|memberwise }1%0",
       (DeclName, bool))
+ERROR(invalid_redecl_implicit,none,
+      "invalid redeclaration of synthesized "
+      "%select{%0|implementation for protocol requirement}1 %2",
+      (DescriptiveDeclKind, bool, DeclName))
 WARNING(invalid_redecl_swift5_warning,none,
         "redeclaration of %0 is deprecated and will be an error in Swift 5",
         (DeclName))
 
 NOTE(invalid_redecl_prev,none,
      "%0 previously declared here", (DeclName))
+NOTE(invalid_redecl_implicit_wrapper,none,
+     "%0 synthesized for property wrapper "
+     "%select{projected value|backing storage}1",
+     (DeclName, bool))
 
 ERROR(ambiguous_type_base,none,
       "%0 is ambiguous for type lookup in this context", (DeclNameRef))
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
@@ -5950,6 +5950,17 @@ VarDecl::getPropertyWrapperMutability() const {
       None);
 }
 
+Optional<PropertyWrapperSynthesizedPropertyKind>
+VarDecl::getPropertyWrapperSynthesizedPropertyKind() const {
+  if (getOriginalWrappedProperty(
+          PropertyWrapperSynthesizedPropertyKind::Backing))
+    return PropertyWrapperSynthesizedPropertyKind::Backing;
+  if (getOriginalWrappedProperty(
+          PropertyWrapperSynthesizedPropertyKind::StorageWrapper))
+    return PropertyWrapperSynthesizedPropertyKind::StorageWrapper;
+  return None;
+}
+
 VarDecl *VarDecl::getPropertyWrapperBackingProperty() const {
   return getPropertyWrapperBackingPropertyInfo().backingVar;
 }
diff --git a/lib/Sema/TypeCheckDeclPrimary.cpp b/lib/Sema/TypeCheckDeclPrimary.cpp
@@ -711,7 +711,75 @@ CheckRedeclarationRequest::evaluate(Evaluator &eval, ValueDecl *current) const {
             current->diagnose(diag::invalid_redecl_init,
                               current->getName(),
                               otherInit->isMemberwiseInitializer());
-        } else if (!current->isImplicit() && !other->isImplicit()) {
+        } else if (current->isImplicit() || other->isImplicit()) {
+          // If both declarations are implicit, we do not diagnose anything
+          // as it would lead to misleading diagnostics and it's likely that
+          // there's nothing actionable about it due to its implicit nature.
+          // One special case for this is property wrappers.
+          //
+          // Otherwise, if 'current' is implicit, then we diagnose 'other'
+          // since 'other' is a redeclaration of 'current'. Similarly, if
+          // 'other' is implicit, we diagnose 'current'.
+          const Decl *declToDiagnose = nullptr;
+          if (current->isImplicit() && other->isImplicit()) {
+            // If 'current' is a property wrapper backing storage property
+            // or projected value property, then diagnose the wrapped
+            // property.
+            if (auto VD = dyn_cast<VarDecl>(current)) {
+              if (auto originalWrappedProperty =
+                      VD->getOriginalWrappedProperty()) {
+                declToDiagnose = originalWrappedProperty;
+              }
+            }
+          } else {
+            declToDiagnose = current->isImplicit() ? other : current;
+          }
+
+          if (declToDiagnose) {
+            // Figure out if the the declaration we've redeclared is a
+            // synthesized witness for a protocol requirement.
+            bool isProtocolRequirement = false;
+            if (auto VD = dyn_cast<ValueDecl>(current->isImplicit() ? current
+                                                                    : other)) {
+              isProtocolRequirement = llvm::any_of(
+                  VD->getSatisfiedProtocolRequirements(), [&](ValueDecl *req) {
+                    return req->getName() == VD->getName();
+                  });
+            }
+            declToDiagnose->diagnose(diag::invalid_redecl_implicit,
+                                     current->getDescriptiveKind(),
+                                     isProtocolRequirement, other->getName());
+          }
+
+          // Emit a specialized note if the one of the declarations is
+          // the backing storage property ('_foo') or projected value
+          // property ('$foo') for a wrapped property. The backing or
+          // projected var has the same source location as the wrapped
+          // property we diagnosed above, so we don't need to extract
+          // the original property.
+          const VarDecl *varToDiagnose = nullptr;
+          auto kind = PropertyWrapperSynthesizedPropertyKind::Backing;
+          if (auto currentVD = dyn_cast<VarDecl>(current)) {
+            if (auto currentKind =
+                    currentVD->getPropertyWrapperSynthesizedPropertyKind()) {
+              varToDiagnose = currentVD;
+              kind = *currentKind;
+            }
+          }
+          if (auto otherVD = dyn_cast<VarDecl>(other)) {
+            if (auto otherKind =
+                    otherVD->getPropertyWrapperSynthesizedPropertyKind()) {
+              varToDiagnose = otherVD;
+              kind = *otherKind;
+            }
+          }
+
+          if (varToDiagnose) {
+            varToDiagnose->diagnose(
+                diag::invalid_redecl_implicit_wrapper, varToDiagnose->getName(),
+                kind == PropertyWrapperSynthesizedPropertyKind::Backing);
+          }
+        } else {
           ctx.Diags.diagnoseWithNotes(
             current->diagnose(diag::invalid_redecl,
                               current->getName()), [&]() {
diff --git a/test/Sema/enum_conformance_synthesis.swift b/test/Sema/enum_conformance_synthesis.swift
@@ -61,7 +61,7 @@ func customHashable() {
 enum InvalidCustomHashable {
   case A, B
 
-  var hashValue: String { return "" }
+  var hashValue: String { return "" } // expected-error {{invalid redeclaration of synthesized implementation for protocol requirement 'hashValue'}}
 }
 func ==(x: InvalidCustomHashable, y: InvalidCustomHashable) -> String {
   return ""
@@ -369,7 +369,6 @@ func canEatHotChip(_ birthyear:Birthyear) -> Bool {
   return birthyear > .nineties(3)
 }
 // FIXME: Remove -verify-ignore-unknown.
-// <unknown>:0: error: unexpected error produced: invalid redeclaration of 'hashValue'
 // <unknown>:0: error: unexpected note produced: candidate has non-matching type '(Foo, Foo) -> Bool'
 // <unknown>:0: error: unexpected note produced: candidate has non-matching type '<T> (Generic<T>, Generic<T>) -> Bool'
 // <unknown>:0: error: unexpected note produced: candidate has non-matching type '(InvalidCustomHashable, InvalidCustomHashable) -> Bool'
diff --git a/test/decl/var/property_wrappers.swift b/test/decl/var/property_wrappers.swift
@@ -849,11 +849,28 @@ struct WrapperWithProjectedValue<T> {
   var projectedValue: T { return wrappedValue }
 }
 
-class TestInvalidRedeclaration {
+class TestInvalidRedeclaration1 {
+
   @WrapperWithProjectedValue var i = 17
   // expected-note@-1 {{'i' previously declared here}}
+  // expected-note@-2 {{'$i' synthesized for property wrapper projected value}}
+  // expected-note@-3 {{'_i' synthesized for property wrapper backing storage}}
+
   @WrapperWithProjectedValue var i = 39
   // expected-error@-1 {{invalid redeclaration of 'i'}}
+  // expected-error@-2 {{invalid redeclaration of synthesized property '$i'}}
+  // expected-error@-3 {{invalid redeclaration of synthesized property '_i'}}
+}
+
+// SR-12839
+struct TestInvalidRedeclaration2 {
+  var _foo1 = 123 // expected-error {{invalid redeclaration of synthesized property '_foo1'}}
+  @WrapperWithInitialValue var foo1 = 123 // expected-note {{'_foo1' synthesized for property wrapper backing storage}}
+}
+
+struct TestInvalidRedeclaration3 {
+  @WrapperWithInitialValue var foo1 = 123 // expected-note {{'_foo1' synthesized for property wrapper backing storage}}
+  var _foo1 = 123 // expected-error {{invalid redeclaration of synthesized property '_foo1'}}
 }
 
 // ---------------------------------------------------------------------------
