Merge remote-tracking branch 'origin/master' into master-next

Author: swift-ci

include/swift/AST/Decl.h

@@ -7190,8 +7190,7 @@ inline bool Decl::isPotentiallyOverridable() const {
       isa<SubscriptDecl>(this) ||
       isa<FuncDecl>(this) ||
       isa<DestructorDecl>(this)) {
-    return getDeclContext()->getSelfClassDecl() ||
-           isa<ProtocolDecl>(getDeclContext());
+    return getDeclContext()->getSelfClassDecl();
   } else {
     return false;
   }

lib/AST/TypeCheckRequests.cpp

@@ -257,7 +257,7 @@ void IsFinalRequest::cacheResult(bool value) const {
   decl->LazySemanticInfo.isFinalComputed = true;
   decl->LazySemanticInfo.isFinal = value;
 
-  // Register Final in attributes, to preserve print order
+  // Add an attribute for printing
   if (value && !decl->getAttrs().hasAttribute<FinalAttr>())
     decl->getAttrs().add(new (decl->getASTContext()) FinalAttr(/*Implicit=*/true));
 }
@@ -289,6 +289,10 @@ Optional<bool> IsDynamicRequest::getCachedResult() const {
 void IsDynamicRequest::cacheResult(bool value) const {
   auto decl = std::get<0>(getStorage());
   decl->setIsDynamic(value);
+
+  // Add an attribute for printing
+  if (value && !decl->getAttrs().hasAttribute<DynamicAttr>())
+    decl->getAttrs().add(new (decl->getASTContext()) DynamicAttr(/*Implicit=*/true));
 }
 
 //----------------------------------------------------------------------------//

lib/Sema/TypeCheckDecl.cpp

@@ -1099,31 +1099,10 @@ static bool inferFinalAndDiagnoseIfNeeded(ValueDecl *D, ClassDecl *cls,
   return true;
 }
 
-/// Make the given declaration 'dynamic', if it isn't already marked as such.
-static void makeDynamic(ValueDecl *decl) {
-  // If there isn't already a 'dynamic' attribute, add an inferred one.
-  if (decl->getAttrs().hasAttribute<DynamicAttr>())
-    return;
-  
-  auto attr = new (decl->getASTContext()) DynamicAttr(/*implicit=*/true);
-  decl->getAttrs().add(attr);
-}
-
-static llvm::Expected<bool> isStorageDynamic(Evaluator &evaluator,
-                                             AccessorDecl *accessor) {
-  auto isDynamicResult = evaluator(IsDynamicRequest{accessor->getStorage()});
-
-  if (!isDynamicResult)
-    return isDynamicResult;
-
-  return *isDynamicResult;
-}
-
 /// Runtime-replacable accessors are dynamic when their storage declaration
 /// is dynamic and they were explicitly defined or they are implicitly defined
 /// getter/setter because no accessor was defined.
-static llvm::Expected<bool>
-doesAccessorNeedDynamicAttribute(AccessorDecl *accessor, Evaluator &evaluator) {
+static bool doesAccessorNeedDynamicAttribute(AccessorDecl *accessor) {
   auto kind = accessor->getAccessorKind();
   auto storage = accessor->getStorage();
   bool isObjC = storage->isObjC();
@@ -1134,40 +1113,40 @@ doesAccessorNeedDynamicAttribute(AccessorDecl *accessor, Evaluator &evaluator) {
     if (!isObjC &&
         (readImpl == ReadImplKind::Read || readImpl == ReadImplKind::Address))
       return false;
-    return isStorageDynamic(evaluator, accessor);
+    return storage->isDynamic();
   }
   case AccessorKind::Set: {
     auto writeImpl = storage->getWriteImpl();
     if (!isObjC && (writeImpl == WriteImplKind::Modify ||
                     writeImpl == WriteImplKind::MutableAddress ||
                     writeImpl == WriteImplKind::StoredWithObservers))
       return false;
-    return isStorageDynamic(evaluator, accessor);
+    return storage->isDynamic();
   }
   case AccessorKind::Read:
     if (!isObjC && storage->getReadImpl() == ReadImplKind::Read)
-      return isStorageDynamic(evaluator, accessor);
+      return storage->isDynamic();
     return false;
   case AccessorKind::Modify: {
     if (!isObjC && storage->getWriteImpl() == WriteImplKind::Modify)
-      return isStorageDynamic(evaluator, accessor);
+      return storage->isDynamic();
     return false;
   }
   case AccessorKind::MutableAddress: {
     if (!isObjC && storage->getWriteImpl() == WriteImplKind::MutableAddress)
-      return isStorageDynamic(evaluator, accessor);
+      return storage->isDynamic();
     return false;
   }
   case AccessorKind::Address: {
     if (!isObjC && storage->getReadImpl() == ReadImplKind::Address)
-      return isStorageDynamic(evaluator, accessor);
+      return storage->isDynamic();
     return false;
   }
   case AccessorKind::DidSet:
   case AccessorKind::WillSet:
     if (!isObjC &&
         storage->getWriteImpl() == WriteImplKind::StoredWithObservers)
-      return isStorageDynamic(evaluator, accessor);
+      return storage->isDynamic();
     return false;
   }
   llvm_unreachable("covered switch");
@@ -1284,40 +1263,17 @@ IsDynamicRequest::evaluate(Evaluator &evaluator, ValueDecl *decl) const {
   }
 
   if (auto accessor = dyn_cast<AccessorDecl>(decl)) {
-    // Swift 5: Runtime-replacable accessors are dynamic when their storage declaration
+    // Runtime-replacable accessors are dynamic when their storage declaration
     // is dynamic and they were explicitly defined or they are implicitly defined
     // getter/setter because no accessor was defined.
-    if (decl->getASTContext().LangOpts.isSwiftVersionAtLeast(5))
-      return doesAccessorNeedDynamicAttribute(accessor, evaluator);
-
-    // Pre Swift 5: Runtime-replacable accessors are dynamic when their storage declaration
-    // is dynamic. Other accessors are never dynamic.
-    switch (accessor->getAccessorKind()) {
-    case AccessorKind::Get:
-    case AccessorKind::Set: {
-      auto isDynamicResult = evaluator(
-          IsDynamicRequest{accessor->getStorage()});
-      if (isDynamicResult && *isDynamicResult)
-        makeDynamic(decl);
-      
-      return isDynamicResult;  
-    }
-
-#define OBJC_ACCESSOR(ID, KEYWORD)
-#define ACCESSOR(ID) \
-    case AccessorKind::ID:
-#include "swift/AST/AccessorKinds.def"
-      return false;
-    }
+    return doesAccessorNeedDynamicAttribute(accessor);
   }
 
   // The 'NSManaged' attribute implies 'dynamic'.
   // FIXME: Use a semantic check for NSManaged rather than looking for the
   // attribute (which could be ill-formed).
-  if (decl->getAttrs().hasAttribute<NSManagedAttr>()) {
-    makeDynamic(decl);
+  if (decl->getAttrs().hasAttribute<NSManagedAttr>())
     return true;
-  }
 
   // The presence of 'final' blocks the inference of 'dynamic'.
   if (decl->isFinal())
@@ -1334,10 +1290,8 @@ IsDynamicRequest::evaluate(Evaluator &evaluator, ValueDecl *decl) const {
   // @objc declarations in class extensions are implicitly dynamic.
   // This is intended to enable overriding the declarations.
   auto dc = decl->getDeclContext();
-  if (isa<ExtensionDecl>(dc) && dc->getSelfClassDecl()) {
-    makeDynamic(decl);
+  if (isa<ExtensionDecl>(dc) && dc->getSelfClassDecl())
     return true;
-  }
 
   // If any of the declarations overridden by this declaration are dynamic
   // or were imported from Objective-C, this declaration is dynamic.
@@ -1347,10 +1301,8 @@ IsDynamicRequest::evaluate(Evaluator &evaluator, ValueDecl *decl) const {
   auto overriddenDecls = evaluateOrDefault(evaluator,
     OverriddenDeclsRequest{decl}, {});
   for (auto overridden : overriddenDecls) {
-    if (overridden->isDynamic() || overridden->hasClangNode()) {
-      makeDynamic(decl);
+    if (overridden->isDynamic() || overridden->hasClangNode())
       return true;
-    }
   }
 
   return false;

lib/Sema/TypeCheckDeclOverride.cpp

@@ -831,10 +831,14 @@ static void checkOverrideAccessControl(ValueDecl *baseDecl, ValueDecl *decl,
   if (ctx.isAccessControlDisabled())
     return;
 
+  if (isa<ProtocolDecl>(decl->getDeclContext()))
+    return;
+
   auto &diags = ctx.Diags;
 
   auto dc = decl->getDeclContext();
   auto classDecl = dc->getSelfClassDecl();
+  assert(classDecl != nullptr && "Should have ruled out protocols above");
 
   bool isAccessor = isa<AccessorDecl>(decl);
 
@@ -851,8 +855,7 @@ static void checkOverrideAccessControl(ValueDecl *baseDecl, ValueDecl *decl,
   if (!isAccessor &&
       !baseHasOpenAccess &&
       baseDecl->getModuleContext() != decl->getModuleContext() &&
-      !isa<ConstructorDecl>(decl) &&
-      !isa<ProtocolDecl>(decl->getDeclContext())) {
+      !isa<ConstructorDecl>(decl)) {
     // NSObject.hashValue was made non-overridable in Swift 5; one should
     // override NSObject.hash instead.
     if (isNSObjectHashValue(baseDecl)) {
@@ -875,8 +878,7 @@ static void checkOverrideAccessControl(ValueDecl *baseDecl, ValueDecl *decl,
     }
     diags.diagnose(baseDecl, diag::overridden_here);
 
-  } else if (!isa<ConstructorDecl>(decl) &&
-             !isa<ProtocolDecl>(decl->getDeclContext())) {
+  } else if (!isa<ConstructorDecl>(decl)) {
     auto matchAccessScope =
       baseDecl->getFormalAccessScope(dc);
     auto classAccessScope =

test/multifile/Inputs/protocol-override-other.swift

@@ -0,0 +1,3 @@
+protocol Base {
+  var foo: String { get }
+}

test/multifile/protocol-override.swift

@@ -0,0 +1,9 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-module-path %t/other.swiftmodule %S/Inputs/protocol-override-other.swift -module-name other -enable-testing
+// RUN: %target-swift-frontend -typecheck %s -I %t
+
+@testable import other
+
+protocol Sub : Base {
+  var foo: String { get set }
+}