Merge pull request #21726 from xedin/fix-repl-method-override

[TypeChecker] Don't verify override access control if it has been dis…

Author: xedin

lib/Sema/TypeCheckDeclOverride.cpp

@@ -759,55 +759,15 @@ SmallVector<OverrideMatch, 2> OverrideMatcher::match(
   return matches;
 }
 
-bool OverrideMatcher::checkOverride(ValueDecl *baseDecl,
-                                    OverrideCheckingAttempt attempt) {
-  auto &diags = ctx.Diags;
-  auto baseTy = getMemberTypeForComparison(ctx, baseDecl, decl);
-  bool emittedMatchError = false;
-
-  // If the name of our match differs from the name we were looking for,
-  // complain.
-  if (decl->getFullName() != baseDecl->getFullName()) {
-    auto diag = diags.diagnose(decl, diag::override_argument_name_mismatch,
-                               isa<ConstructorDecl>(decl),
-                               decl->getFullName(),
-                               baseDecl->getFullName());
-    fixDeclarationName(diag, decl, baseDecl->getFullName());
-    emittedMatchError = true;
-  }
+static void checkOverrideAccessControl(ValueDecl *baseDecl, ValueDecl *decl,
+                                       ASTContext &ctx) {
+  if (!ctx.LangOpts.EnableAccessControl)
+    return;
 
-  // If we have an explicit ownership modifier and our parent doesn't,
-  // complain.
-  auto parentAttr =
-      baseDecl->getAttrs().getAttribute<ReferenceOwnershipAttr>();
-  if (auto ownershipAttr =
-          decl->getAttrs().getAttribute<ReferenceOwnershipAttr>()) {
-    ReferenceOwnership parentOwnership;
-    if (parentAttr)
-      parentOwnership = parentAttr->get();
-    else
-      parentOwnership = ReferenceOwnership::Strong;
-    if (parentOwnership != ownershipAttr->get()) {
-      diags.diagnose(decl, diag::override_ownership_mismatch,
-                     parentOwnership, ownershipAttr->get());
-      diags.diagnose(baseDecl, diag::overridden_here);
-    }
-  }
+  auto &diags = ctx.Diags;
 
-  // If a super method returns Self, and the subclass overrides it to
-  // instead return the subclass type, complain.
-  // This case gets this far because the type matching above specifically
-  // strips out dynamic self via replaceCovariantResultType(), and that
-  // is helpful in several cases - just not this one.
   auto dc = decl->getDeclContext();
   auto classDecl = dc->getSelfClassDecl();
-  if (decl->getASTContext().isSwiftVersionAtLeast(5) &&
-      baseDecl->getInterfaceType()->hasDynamicSelfType() &&
-      !decl->getInterfaceType()->hasDynamicSelfType() &&
-      !classDecl->isFinal()) {
-    diags.diagnose(decl, diag::override_dynamic_self_mismatch);
-    diags.diagnose(baseDecl, diag::overridden_here);
-  }
 
   bool isAccessor = isa<AccessorDecl>(decl);
 
@@ -891,6 +851,59 @@ bool OverrideMatcher::checkOverride(ValueDecl *baseDecl,
       diags.diagnose(baseDecl, diag::overridden_here);
     }
   }
+}
+
+bool OverrideMatcher::checkOverride(ValueDecl *baseDecl,
+                                    OverrideCheckingAttempt attempt) {
+  auto &diags = ctx.Diags;
+  auto baseTy = getMemberTypeForComparison(ctx, baseDecl, decl);
+  bool emittedMatchError = false;
+
+  // If the name of our match differs from the name we were looking for,
+  // complain.
+  if (decl->getFullName() != baseDecl->getFullName()) {
+    auto diag = diags.diagnose(decl, diag::override_argument_name_mismatch,
+                               isa<ConstructorDecl>(decl),
+                               decl->getFullName(),
+                               baseDecl->getFullName());
+    fixDeclarationName(diag, decl, baseDecl->getFullName());
+    emittedMatchError = true;
+  }
+
+  // If we have an explicit ownership modifier and our parent doesn't,
+  // complain.
+  auto parentAttr =
+      baseDecl->getAttrs().getAttribute<ReferenceOwnershipAttr>();
+  if (auto ownershipAttr =
+          decl->getAttrs().getAttribute<ReferenceOwnershipAttr>()) {
+    ReferenceOwnership parentOwnership;
+    if (parentAttr)
+      parentOwnership = parentAttr->get();
+    else
+      parentOwnership = ReferenceOwnership::Strong;
+    if (parentOwnership != ownershipAttr->get()) {
+      diags.diagnose(decl, diag::override_ownership_mismatch,
+                     parentOwnership, ownershipAttr->get());
+      diags.diagnose(baseDecl, diag::overridden_here);
+    }
+  }
+
+  // If a super method returns Self, and the subclass overrides it to
+  // instead return the subclass type, complain.
+  // This case gets this far because the type matching above specifically
+  // strips out dynamic self via replaceCovariantResultType(), and that
+  // is helpful in several cases - just not this one.
+  auto dc = decl->getDeclContext();
+  auto classDecl = dc->getSelfClassDecl();
+  if (decl->getASTContext().isSwiftVersionAtLeast(5) &&
+      baseDecl->getInterfaceType()->hasDynamicSelfType() &&
+      !decl->getInterfaceType()->hasDynamicSelfType() &&
+      !classDecl->isFinal()) {
+    diags.diagnose(decl, diag::override_dynamic_self_mismatch);
+    diags.diagnose(baseDecl, diag::overridden_here);
+  }
+
+  checkOverrideAccessControl(baseDecl, decl, ctx);
 
   bool mayHaveMismatchedOptionals =
       (attempt == OverrideCheckingAttempt::MismatchedOptional ||

test/Interpreter/repl.swift

@@ -223,11 +223,32 @@ if true && true { if true && true { print(true && true) } }
 // CHECK: = "ok"
 
 // Make sure that class inheritance works
-class A {}
+class A {
+  var foo: String { return "" }
+  func bar() -> String { return "" }
+  subscript(_ x: Int) -> String { return "" }
+}
+
 class B : A {
+  override var foo: String {
+    return "property ok"
+  }
+
   override init() {}
-  func foo() -> String { return "ok" }
+
+  override func bar() -> String {
+    return "instance ok"
+  }
+
+  override subscript(_ x: Int) -> String {
+    return "subscript ok"
+  }
 }
 
-let _ = B().foo()
-// CHECK: = "ok"
+let b = B()
+let _ = b.foo
+// CHECK: = "property ok"
+let _ = b.bar()
+// CHECK: = "instance ok"
+let _ = b[42]
+// CHECK: = "subscript ok"

test/attr/Inputs/disabled_access_control_base.swift

@@ -0,0 +1,6 @@
+public class A {
+  public init() {}
+  public var foo: String { return "" }
+  public func bar() {}
+  public subscript(_ x: Int) -> String { return "" }
+}

test/attr/override_with_disabled_access_control.swift

@@ -0,0 +1,20 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-module -o %t/disabled_access_control_base.swiftmodule %S/Inputs/disabled_access_control_base.swift
+// RUN: %target-swift-frontend -disable-access-control -I %t -typecheck %s
+
+import disabled_access_control_base
+
+class B : A {
+  public override var foo: String {
+    return "ok"
+  }
+
+  override init() { }
+
+  override func bar() {
+  }
+
+  override subscript(_ x: Int) -> String {
+    return "ok"
+  }
+}