Merge pull request #13643 from DougGregor/redecl-fewer-validations

[Type checker] Make redeclaration checking validate fewer declarations.

Author: DougGregor

include/swift/AST/Decl.h

@@ -213,10 +213,6 @@ struct OverloadSignature {
   /// The full name of the declaration.
   DeclName Name;
 
-  /// The interface type of the declaration, when relevant to the
-  /// overload signature.
-  CanType InterfaceType;
-
   /// The kind of unary operator.
   UnaryOperatorKind UnaryOperator = UnaryOperatorKind::None;
 
@@ -231,7 +227,8 @@ struct OverloadSignature {
 };
 
 /// Determine whether two overload signatures conflict.
-bool conflicting(const OverloadSignature& sig1, const OverloadSignature& sig2);
+bool conflicting(const OverloadSignature& sig1, const OverloadSignature& sig2,
+                 bool skipProtocolExtensionCheck = false);
 
 
 /// Decl - Base class for all declarations in Swift.
@@ -2251,9 +2248,13 @@ class ValueDecl : public Decl {
   /// Retrieve the declaration that this declaration overrides, if any.
   ValueDecl *getOverriddenDecl() const;
 
-  /// Compute the overload signature for this declaration.
+  /// Compute the untyped overload signature for this declaration.
   OverloadSignature getOverloadSignature() const;
 
+  /// Retrieve the type used to describe this entity for the purposes of
+  /// overload resolution.
+  CanType getOverloadSignatureType() const;
+
   /// Returns true if the decl requires Objective-C interop.
   ///
   /// This can be true even if there is no 'objc' attribute on the declaration.

lib/AST/Decl.cpp

@@ -1654,27 +1654,34 @@ ValueDecl *ValueDecl::getOverriddenDecl() const {
 }
 
 bool swift::conflicting(const OverloadSignature& sig1,
-                        const OverloadSignature& sig2) {
+                        const OverloadSignature& sig2,
+                        bool skipProtocolExtensionCheck) {
   // A member of a protocol extension never conflicts with a member of a
   // protocol.
-  if (sig1.InProtocolExtension != sig2.InProtocolExtension)
+  if (!skipProtocolExtensionCheck &&
+      sig1.InProtocolExtension != sig2.InProtocolExtension)
     return false;
 
   // If the base names are different, they can't conflict.
   if (sig1.Name.getBaseName() != sig2.Name.getBaseName())
     return false;
-  
+
+  // If one is an operator and the other is not, they can't conflict.
+  if (sig1.UnaryOperator != sig2.UnaryOperator)
+    return false;
+
+  // If one is an instance and the other is not, they can't conflict.
+  if (sig1.IsInstanceMember != sig2.IsInstanceMember)
+    return false;
+
   // If one is a compound name and the other is not, they do not conflict
   // if one is a property and the other is a non-nullary function.
   if (sig1.Name.isCompoundName() != sig2.Name.isCompoundName()) {
     return !((sig1.IsProperty && sig2.Name.getArgumentNames().size() > 0) ||
              (sig2.IsProperty && sig1.Name.getArgumentNames().size() > 0));
   }
 
-  return sig1.Name == sig2.Name &&
-         sig1.InterfaceType == sig2.InterfaceType &&
-         sig1.UnaryOperator == sig2.UnaryOperator &&
-         sig1.IsInstanceMember == sig2.IsInstanceMember;
+  return sig1.Name == sig2.Name;
 }
 
 static Type mapSignatureFunctionType(ASTContext &ctx, Type type,
@@ -1791,63 +1798,68 @@ OverloadSignature ValueDecl::getOverloadSignature() const {
   signature.Name = getFullName();
   signature.InProtocolExtension
     = getDeclContext()->getAsProtocolExtensionContext();
+  signature.IsInstanceMember = isInstanceMember();
+  signature.IsProperty = isa<VarDecl>(this);
 
-  // Functions, initializers, and de-initializers include their
-  // interface types in their signatures as well as whether they are
-  // instance members.
-  if (auto afd = dyn_cast<AbstractFunctionDecl>(this)) {
-    signature.InterfaceType =
-        mapSignatureFunctionType(
-            getASTContext(), getInterfaceType(),
-            /*topLevelFunction=*/true,
-            /*isMethod=*/afd->getImplicitSelfDecl() != nullptr,
-            /*isInitializer=*/isa<ConstructorDecl>(afd),
-             afd->getNumParameterLists())->getCanonicalType();
-
-    signature.IsInstanceMember = isInstanceMember();
-    // Unary operators also include prefix/postfix.
-    if (auto func = dyn_cast<FuncDecl>(this)) {
-      if (func->isUnaryOperator()) {
-        signature.UnaryOperator = func->getAttrs().getUnaryOperatorKind();
-      }
+  if (auto func = dyn_cast<FuncDecl>(this)) {
+    if (func->isUnaryOperator()) {
+      signature.UnaryOperator = func->getAttrs().getUnaryOperatorKind();
     }
-  } else if (isa<SubscriptDecl>(this)) {
-    signature.InterfaceType = getInterfaceType()->getCanonicalType();
+  }
 
-    // If the subscript occurs within a generic extension context,
-    // consider the generic signature of the extension.
-    if (auto ext = dyn_cast<ExtensionDecl>(getDeclContext())) {
-      if (auto genericSig = ext->getGenericSignature()) {
-        if (auto funcTy = signature.InterfaceType->getAs<AnyFunctionType>()) {
-          signature.InterfaceType
-            = GenericFunctionType::get(genericSig,
-                                       funcTy->getParams(),
-                                       funcTy->getResult(),
-                                       funcTy->getExtInfo())
-                ->getCanonicalType();
-        }
-      }
-    }
-  } else if (isa<VarDecl>(this)) {
-    signature.IsProperty = true;
-    signature.IsInstanceMember = isInstanceMember();
+  return signature;
+}
 
-    // If the property occurs within a generic extension context,
+CanType ValueDecl::getOverloadSignatureType() const {
+  if (auto afd = dyn_cast<AbstractFunctionDecl>(this)) {
+    return mapSignatureFunctionType(
+                           getASTContext(), getInterfaceType(),
+                           /*topLevelFunction=*/true,
+                           /*isMethod=*/afd->getImplicitSelfDecl() != nullptr,
+                           /*isInitializer=*/isa<ConstructorDecl>(afd),
+                           afd->getNumParameterLists())->getCanonicalType();
+  }
+
+  if (isa<SubscriptDecl>(this)) {
+    CanType interfaceType = getInterfaceType()->getCanonicalType();
+
+    // If the subscript declaration occurs within a generic extension context,
     // consider the generic signature of the extension.
-    if (auto ext = dyn_cast<ExtensionDecl>(getDeclContext())) {
-      if (auto genericSig = ext->getGenericSignature()) {
-        ASTContext &ctx = getASTContext();
-        signature.InterfaceType
-          = GenericFunctionType::get(genericSig,
-                                     TupleType::getEmpty(ctx),
-                                     TupleType::getEmpty(ctx),
-                                     AnyFunctionType::ExtInfo())
+    auto ext = dyn_cast<ExtensionDecl>(getDeclContext());
+    if (!ext) return interfaceType;
+
+    auto genericSig = ext->getGenericSignature();
+    if (!genericSig) return interfaceType;
+
+    if (auto funcTy = interfaceType->getAs<AnyFunctionType>()) {
+      return GenericFunctionType::get(genericSig,
+                                      funcTy->getParams(),
+                                      funcTy->getResult(),
+                                      funcTy->getExtInfo())
               ->getCanonicalType();
-      }
     }
+
+    return interfaceType;
   }
 
-  return signature;
+  if (isa<VarDecl>(this)) {
+    // If the variable declaration occurs within a generic extension context,
+    // consider the generic signature of the extension.
+    auto ext = dyn_cast<ExtensionDecl>(getDeclContext());
+    if (!ext) return CanType();
+
+    auto genericSig = ext->getGenericSignature();
+    if (!genericSig) return CanType();
+
+    ASTContext &ctx = getASTContext();
+    return GenericFunctionType::get(genericSig,
+                                    TupleType::getEmpty(ctx),
+                                    TupleType::getEmpty(ctx),
+                                    AnyFunctionType::ExtInfo())
+             ->getCanonicalType();
+  }
+
+  return CanType();
 }
 
 void ValueDecl::setIsObjC(bool Value) {

lib/AST/LookupVisibleDecls.cpp

@@ -668,35 +668,6 @@ template <> struct DenseMapInfo<FoundDeclTy> {
 
 namespace {
 
-/// Similar to swift::conflicting, but lenient about protocol extensions which
-/// don't affect code completion's concept of overloading.
-static bool relaxedConflicting(const OverloadSignature &sig1,
-                               const OverloadSignature &sig2) {
-
-  // If the base names are different, they can't conflict.
-  if (sig1.Name.getBaseName() != sig2.Name.getBaseName())
-    return false;
-
-  // If one is a compound name and the other is not, they do not conflict
-  // if one is a property and the other is a non-nullary function.
-  if (sig1.Name.isCompoundName() != sig2.Name.isCompoundName()) {
-    return !((sig1.IsProperty && sig2.Name.getArgumentNames().size() > 0) ||
-             (sig2.IsProperty && sig1.Name.getArgumentNames().size() > 0));
-  }
-
-  // Allow null property types to match non-null ones, which only happens when
-  // one property is from a generic extension and the other is not.
-  if (sig1.InterfaceType != sig2.InterfaceType) {
-    if (!sig1.IsProperty || !sig2.IsProperty)
-      return false;
-    if (sig1.InterfaceType && sig2.InterfaceType)
-      return false;
-  }
-
-  return sig1.Name == sig2.Name && sig1.UnaryOperator == sig2.UnaryOperator &&
-         sig1.IsInstanceMember == sig2.IsInstanceMember;
-}
-
 /// Hack to guess at whether substituting into the type of a declaration will
 /// be okay.
 /// FIXME: This is awful. We should either have Type::subst() work for
@@ -803,11 +774,12 @@ class OverrideFilteringConsumer : public VisibleDeclConsumer {
     }
 
     auto FoundSignature = VD->getOverloadSignature();
-    if (FoundSignature.InterfaceType && shouldSubst &&
-        shouldSubstIntoDeclType(FoundSignature.InterfaceType)) {
+    auto FoundSignatureType = VD->getOverloadSignatureType();
+    if (FoundSignatureType && shouldSubst &&
+        shouldSubstIntoDeclType(FoundSignatureType)) {
       auto subs = BaseTy->getMemberSubstitutionMap(M, VD);
-      if (auto CT = FoundSignature.InterfaceType.subst(subs))
-        FoundSignature.InterfaceType = CT->getCanonicalType();
+      if (auto CT = FoundSignatureType.subst(subs))
+        FoundSignatureType = CT->getCanonicalType();
     }
 
     for (auto I = PossiblyConflicting.begin(), E = PossiblyConflicting.end();
@@ -820,14 +792,18 @@ class OverrideFilteringConsumer : public VisibleDeclConsumer {
       }
 
       auto OtherSignature = OtherVD->getOverloadSignature();
-      if (OtherSignature.InterfaceType && shouldSubst &&
-          shouldSubstIntoDeclType(OtherSignature.InterfaceType)) {
+      auto OtherSignatureType = OtherVD->getOverloadSignatureType();
+      if (OtherSignatureType && shouldSubst &&
+          shouldSubstIntoDeclType(OtherSignatureType)) {
         auto subs = BaseTy->getMemberSubstitutionMap(M, OtherVD);
-        if (auto CT = OtherSignature.InterfaceType.subst(subs))
-          OtherSignature.InterfaceType = CT->getCanonicalType();
+        if (auto CT = OtherSignatureType.subst(subs))
+          OtherSignatureType = CT->getCanonicalType();
       }
 
-      if (relaxedConflicting(FoundSignature, OtherSignature)) {
+      if (conflicting(FoundSignature, OtherSignature, true) &&
+          (FoundSignatureType == OtherSignatureType ||
+           FoundSignature.Name.isCompoundName() !=
+             OtherSignature.Name.isCompoundName())) {
         if (VD->getFormalAccess() > OtherVD->getFormalAccess()) {
           PossiblyConflicting.erase(I);
           PossiblyConflicting.insert(VD);

lib/AST/NameLookup.cpp

@@ -193,7 +193,7 @@ bool swift::removeShadowedDecls(SmallVectorImpl<ValueDecl*> &decls,
     // constrained extensions, so use the overload signature's
     // type. This is layering a partial fix upon a total hack.
     if (auto asd = dyn_cast<AbstractStorageDecl>(decl))
-      signature = asd->getOverloadSignature().InterfaceType;
+      signature = asd->getOverloadSignatureType();
 
     // If we've seen a declaration with this signature before, note it.
     auto &knownDecls =

lib/Sema/TypeCheckDecl.cpp

@@ -899,6 +899,7 @@ static void checkRedeclaration(TypeChecker &tc, ValueDecl *current) {
   // Compare this signature against the signature of other
   // declarations with the same name.
   OverloadSignature currentSig = current->getOverloadSignature();
+  CanType currentSigType = current->getOverloadSignatureType();
   ModuleDecl *currentModule = current->getModuleContext();
   for (auto other : otherDefinitions) {
     // Skip invalid declarations and ourselves.
@@ -914,6 +915,12 @@ static void checkRedeclaration(TypeChecker &tc, ValueDecl *current) {
     if (currentDC->isTypeContext() != other->getDeclContext()->isTypeContext())
       continue;
 
+    // Check whether the overload signatures conflict (ignoring the type for
+    // now).
+    auto otherSig = other->getOverloadSignature();
+    if (!conflicting(currentSig, otherSig))
+      continue;
+
     // Validate the declaration.
     tc.validateDecl(other);
     if (other->isInvalid() || !other->hasInterfaceType())
@@ -946,8 +953,12 @@ static void checkRedeclaration(TypeChecker &tc, ValueDecl *current) {
       break;
     }
 
+    // Get the overload signature type.
+    CanType otherSigType = other->getOverloadSignatureType();
+
     // If there is another conflict, complain.
-    if (conflicting(currentSig, other->getOverloadSignature())) {
+    if (currentSigType == otherSigType ||
+        currentSig.Name.isCompoundName() != otherSig.Name.isCompoundName()) {
       // If the two declarations occur in the same source file, make sure
       // we get the diagnostic ordering to be sensible.
       if (auto otherFile = other->getDeclContext()->getParentSourceFile()) {

test/IDE/complete_value_expr.swift

@@ -1650,24 +1650,26 @@ struct dedupS : dedupP {
 
 func testDeDuped(_ x: dedupS) {
   x#^PROTOCOL_EXT_DEDUP_1^#
-// FIXME: Should produce 3 items
-// PROTOCOL_EXT_DEDUP_1: Begin completions, 5 items
+// FIXME: Should produce 3 items (?)
+// PROTOCOL_EXT_DEDUP_1: Begin completions, 6 items
 // PROTOCOL_EXT_DEDUP_1: Decl[InstanceMethod]/CurrNominal:   .foo()[#Int#]; name=foo()
 // PROTOCOL_EXT_DEDUP_1: Decl[InstanceVar]/CurrNominal:      .bar[#Int#]; name=bar
 // PROTOCOL_EXT_DEDUP_1: Decl[Subscript]/CurrNominal:        [{#Int#}][#Int#]; name=[Int]
 // PROTOCOL_EXT_DEDUP_1: End completions
 }
 func testDeDuped2(_ x: dedupP) {
   x#^PROTOCOL_EXT_DEDUP_2^#
-// PROTOCOL_EXT_DEDUP_2: Begin completions, 3 items
+// FIXME: Should produce 3 items (?)
+// PROTOCOL_EXT_DEDUP_2: Begin completions, 4 items
 // PROTOCOL_EXT_DEDUP_2: Decl[InstanceMethod]/CurrNominal:   .foo()[#dedupP.T#]; name=foo()
 // PROTOCOL_EXT_DEDUP_2: Decl[InstanceVar]/CurrNominal:      .bar[#dedupP.T#]; name=bar
 // PROTOCOL_EXT_DEDUP_2: Decl[Subscript]/CurrNominal:        [{#Self.T#}][#Self.T#]; name=[Self.T]
 // PROTOCOL_EXT_DEDUP_2: End completions
 }
 func testDeDuped3<T : dedupP where T.T == Int>(_ x: T) {
+// FIXME: Should produce 3 items (?)
   x#^PROTOCOL_EXT_DEDUP_3^#
-// PROTOCOL_EXT_DEDUP_3: Begin completions, 3 items
+// PROTOCOL_EXT_DEDUP_3: Begin completions, 4 items
 // PROTOCOL_EXT_DEDUP_3: Decl[InstanceMethod]/Super:   .foo()[#Int#]; name=foo()
 // PROTOCOL_EXT_DEDUP_3: Decl[InstanceVar]/Super:      .bar[#Int#]; name=bar
 // PROTOCOL_EXT_DEDUP_3: Decl[Subscript]/Super:        [{#Self.T#}][#Self.T#]; name=[Self.T]

validation-test/compiler_crashers_2_fixed/0126-sr5905.swift

@@ -1,4 +1,4 @@
-// RUN: not %target-swift-frontend %s -typecheck
+// RUN: %target-swift-frontend %s -emit-ir -o /dev/null
 protocol VectorIndex {
     associatedtype Vector8 : Vector where Vector8.Index == Self, Vector8.Element == UInt8
 }

validation-test/compiler_crashers/28790-conformance-getwitness-requirement-nullptr-getdecl-already-have-a-non-optional-w.swift renamed to validation-test/compiler_crashers_fixed/28790-conformance-getwitness-requirement-nullptr-getdecl-already-have-a-non-optional-w.swift

@@ -5,6 +5,6 @@
 // See https://swift.org/LICENSE.txt for license information
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 
-// REQUIRES: asserts
-// RUN: not --crash %target-swift-frontend %s -emit-ir
+
+// RUN: not %target-swift-frontend %s -emit-ir
 class a:P{class a{}{}func a:a{}func a{}}@objc protocol P{{}func a