Merge pull request #72659 from AnthonyLatsis/many-any-bugs

AnthonyLatsis · web-flow · commit f02d300de284 · 2024-06-05T18:19:52.000+03:00
TypeCheckType: Fix some bugs in the `any` syntax checker
diff --git a/include/swift/AST/TypeRepr.h b/include/swift/AST/TypeRepr.h
@@ -184,6 +184,10 @@ class alignas(1 << TypeReprAlignInBits) TypeRepr
   /// opaque return type reprs.
   bool hasOpaque();
 
+  /// Returns a Boolean value indicating whether this written type is
+  /// parenthesized, that is, matches the following grammar: `'(' type ')'`.
+  bool isParenType() const;
+
   /// Retrieve the type repr without any parentheses around it.
   ///
   /// The use of this function must be restricted to contexts where
diff --git a/lib/AST/TypeRepr.cpp b/lib/AST/TypeRepr.cpp
@@ -109,14 +109,18 @@ bool TypeRepr::hasOpaque() {
     findIf([](TypeRepr *ty) { return isa<OpaqueReturnTypeRepr>(ty); });
 }
 
+bool TypeRepr::isParenType() const {
+  auto *tuple = dyn_cast<TupleTypeRepr>(this);
+  return tuple && tuple->isParenType();
+}
+
 TypeRepr *TypeRepr::getWithoutParens() const {
-  auto *repr = const_cast<TypeRepr *>(this);
-  while (auto *tupleRepr = dyn_cast<TupleTypeRepr>(repr)) {
-    if (!tupleRepr->isParenType())
-      break;
-    repr = tupleRepr->getElementType(0);
+  auto *result = this;
+  while (result->isParenType()) {
+    result = cast<TupleTypeRepr>(result)->getElementType(0);
   }
-  return repr;
+
+  return const_cast<TypeRepr *>(result);
 }
 
 bool TypeRepr::isSimpleUnqualifiedIdentifier(Identifier identifier) const {
diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp
@@ -5980,31 +5980,6 @@ class ExistentialTypeSyntaxChecker : public ASTWalker {
     if (T->isInvalid())
       return Action::SkipNode();
 
-    // Arbitrary protocol constraints are OK on opaque types.
-    if (isa<OpaqueReturnTypeRepr>(T))
-      return Action::SkipNode();
-
-    // Arbitrary protocol constraints are okay for 'any' types.
-    if (isa<ExistentialTypeRepr>(T))
-      return Action::SkipNode();
-
-    // Suppressed conformance needs to be within any/some.
-    if (auto inverse = dyn_cast<InverseTypeRepr>(T)) {
-      // Find an enclosing protocol composition, if there is one, so we
-      // can insert 'any' before that.
-      SourceLoc anyLoc = inverse->getTildeLoc();
-      if (!reprStack.empty()) {
-        if (isa<CompositionTypeRepr>(reprStack.back())) {
-          anyLoc = reprStack.back()->getStartLoc();
-        }
-      }
-
-      Ctx.Diags.diagnose(inverse->getTildeLoc(), diag::inverse_requires_any)
-        .highlight(inverse->getConstraint()->getSourceRange())
-        .fixItInsert(anyLoc, "any ");
-      return Action::SkipNode();
-    }
-
     reprStack.push_back(T);
 
     auto *declRefTR = dyn_cast<DeclRefTypeRepr>(T);
@@ -6057,9 +6032,12 @@ class ExistentialTypeSyntaxChecker : public ASTWalker {
   }
 
 private:
-  bool existentialNeedsParens(TypeRepr *parent) const {
+  /// Returns a Boolean value indicating whether the insertion of `any` before
+  /// a type representation with the given parent requires paretheses.
+  static bool anySyntaxNeedsParens(TypeRepr *parent) {
     switch (parent->getKind()) {
     case TypeReprKind::Optional:
+    case TypeReprKind::ImplicitlyUnwrappedOptional:
     case TypeReprKind::Protocol:
       return true;
     case TypeReprKind::Metatype:
@@ -6074,7 +6052,6 @@ class ExistentialTypeSyntaxChecker : public ASTWalker {
     case TypeReprKind::UnqualifiedIdent:
     case TypeReprKind::QualifiedIdent:
     case TypeReprKind::Dictionary:
-    case TypeReprKind::ImplicitlyUnwrappedOptional:
     case TypeReprKind::Inverse:
     case TypeReprKind::Tuple:
     case TypeReprKind::Fixed:
@@ -6097,40 +6074,94 @@ class ExistentialTypeSyntaxChecker : public ASTWalker {
   }
 
   void emitInsertAnyFixit(InFlightDiagnostic &diag, DeclRefTypeRepr *T) const {
-    TypeRepr *replaceRepr = T;
+    TypeRepr *replacementT = T;
 
     // Insert parens in expression context for '(any P).self'
     bool needsParens = (exprCount != 0);
+
+    // Compute the replacement node (the node to which to apply `any`).
     if (reprStack.size() > 1) {
-      auto parentIt = reprStack.end() - 2;
-      needsParens = existentialNeedsParens(*parentIt);
-
-      // Expand to include parenthesis before checking if the parent needs
-      // to be replaced.
-      while (parentIt != reprStack.begin() &&
-             (*parentIt)->getWithoutParens() != *parentIt)
-        parentIt -= 1;
-
-      // For existential metatypes, 'any' is applied to the metatype.
-      if ((*parentIt)->getKind() == TypeReprKind::Metatype) {
-        replaceRepr = *parentIt;
-        if (parentIt != reprStack.begin())
-          needsParens = existentialNeedsParens(*(parentIt - 1));
+      auto it = reprStack.end() - 1;
+      auto replacementIt = it;
+
+      // Backtrack the stack and expand the replacement range to any parent
+      // inverses, compositions or `.Type` metatypes, skipping only parentheses.
+      //
+      // E.g. `(X & ~P).Type` → `any (X & ~P).Type`.
+      //             ↑
+      //             We're here
+      do {
+        --it;
+        if ((*it)->isParenType()) {
+          continue;
+        }
+
+        if (isa<InverseTypeRepr>(*it) || isa<CompositionTypeRepr>(*it) ||
+            isa<MetatypeTypeRepr>(*it)) {
+          replacementIt = it;
+          continue;
+        }
+
+        break;
+      } while (it != reprStack.begin());
+
+      // Whether parentheses are necessary is determined by the immediate parent
+      // of the replacement node.
+      if (replacementIt != reprStack.begin()) {
+        needsParens = anySyntaxNeedsParens(*(replacementIt - 1));
       }
+
+      replacementT = *replacementIt;
     }
 
     llvm::SmallString<64> fix;
     {
       llvm::raw_svector_ostream OS(fix);
       if (needsParens)
         OS << "(";
-      ExistentialTypeRepr existential(SourceLoc(), replaceRepr);
+      ExistentialTypeRepr existential(SourceLoc(), replacementT);
       existential.print(OS);
       if (needsParens)
         OS << ")";
     }
 
-    diag.fixItReplace(replaceRepr->getSourceRange(), fix);
+    diag.fixItReplace(replacementT->getSourceRange(), fix);
+  }
+
+  /// Returns a Boolean value indicating whether the type representation being
+  /// visited, assuming it is a constraint type demanding `any` or `some`, is
+  /// missing either keyword.
+  bool isAnyOrSomeMissing() const {
+    assert(isa<DeclRefTypeRepr>(reprStack.back()));
+
+    if (reprStack.size() < 2) {
+      return true;
+    }
+
+    auto it = reprStack.end() - 1;
+    while (true) {
+      --it;
+      if (it == reprStack.begin()) {
+        break;
+      }
+
+      // Look through parens, inverses, metatypes, and compositions.
+      if ((*it)->isParenType() || isa<InverseTypeRepr>(*it) ||
+          isa<CompositionTypeRepr>(*it) || isa<MetatypeTypeRepr>(*it)) {
+        continue;
+      }
+
+      // Look through '?' and '!' too; `any P?` et al. is diagnosed in the
+      // type resolver.
+      if (isa<OptionalTypeRepr>(*it) ||
+          isa<ImplicitlyUnwrappedOptionalTypeRepr>(*it)) {
+        continue;
+      }
+
+      break;
+    }
+
+    return !(isa<OpaqueReturnTypeRepr>(*it) || isa<ExistentialTypeRepr>(*it));
   }
 
   void checkDeclRefTypeRepr(DeclRefTypeRepr *T) const {
@@ -6140,13 +6171,32 @@ class ExistentialTypeSyntaxChecker : public ASTWalker {
       return;
     }
 
+    // Backtrack the stack, looking just through parentheses and metatypes. If
+    // we find an inverse (which always requires `any`), diagnose it specially.
+    if (reprStack.size() > 1) {
+      auto it = reprStack.end() - 2;
+      while (it != reprStack.begin() &&
+             ((*it)->isParenType() || isa<MetatypeTypeRepr>(*it))) {
+        --it;
+        continue;
+      }
+
+      if (auto *inverse = dyn_cast<InverseTypeRepr>(*it);
+          inverse && isAnyOrSomeMissing()) {
+        auto diag = Ctx.Diags.diagnose(inverse->getTildeLoc(),
+                                       diag::inverse_requires_any);
+        emitInsertAnyFixit(diag, T);
+        return;
+      }
+    }
+
     auto *decl = T->getBoundDecl();
     if (!decl) {
       return;
     }
 
     if (auto *proto = dyn_cast<ProtocolDecl>(decl)) {
-      if (proto->existentialRequiresAny()) {
+      if (proto->existentialRequiresAny() && isAnyOrSomeMissing()) {
         auto diag =
             Ctx.Diags.diagnose(T->getNameLoc(), diag::existential_requires_any,
                                proto->getDeclaredInterfaceType(),
@@ -6176,7 +6226,7 @@ class ExistentialTypeSyntaxChecker : public ASTWalker {
         if (auto *PCT = type->getAs<ProtocolCompositionType>())
           diagnose |= !PCT->getInverses().empty();
 
-        if (diagnose) {
+        if (diagnose && isAnyOrSomeMissing()) {
           auto diag = Ctx.Diags.diagnose(
               T->getNameLoc(), diag::existential_requires_any,
               alias->getDeclaredInterfaceType(),
diff --git a/test/Parse/inverses.swift b/test/Parse/inverses.swift
@@ -92,8 +92,6 @@ func what(one: ~Copyable..., // expected-error {{noncopyable type '~Copyable' ca
 
 struct A { struct B { struct C {} } }
 
-typealias Z0 = (~Copyable).Type // expected-error{{constraint that suppresses conformance requires 'any'}}{{17-17=any }}
-typealias Z1 = ~Copyable.Type // expected-error{{constraint that suppresses conformance requires 'any'}}{{16-16=any }}
 typealias Z2 = ~A.B.C // expected-error {{type 'A.B.C' cannot be suppressed}}
 typealias Z3 = ~A? // expected-error {{type 'A?' cannot be suppressed}}
 typealias Z4 = ~Rope<Int> // expected-error {{type 'Rope<Int>' cannot be suppressed}}
@@ -120,13 +118,8 @@ func typeInExpression() {
   _ = X<(borrowing any ~Copyable) -> Void>()
 
   _ = ~Copyable.self // expected-error{{unary operator '~' cannot be applied to an operand of type '(any Copyable).Type'}}
-  _ = (~Copyable).self // expected-error{{constraint that suppresses conformance requires 'any'}}{{8-8=any }}
   _ = (any ~Copyable).self
 }
 
-func param1(_ t: borrowing ~Copyable) {} // expected-error{{constraint that suppresses conformance requires 'any'}}{{28-28=any }}
-func param2(_ t: ~Copyable.Type) {} // expected-error{{constraint that suppresses conformance requires 'any'}}{{18-18=any }}
 func param3(_ t: borrowing any ~Copyable) {}
 func param4(_ t: any ~Copyable.Type) {}
-
-func param3(_ t: borrowing ExtraNoncopyProto & ~Copyable) {} // expected-error{{constraint that suppresses conformance requires 'any'}}{{28-28=any }}
diff --git a/test/decl/protocol/protocols.swift b/test/decl/protocol/protocols.swift
@@ -103,9 +103,9 @@ struct DoesNotConform : Up {
 // Circular protocols
 
 protocol CircleMiddle : CircleStart { func circle_middle() }
-// expected-note@-1 2 {{protocol 'CircleMiddle' declared here}}
-protocol CircleStart : CircleEnd { func circle_start() } // expected-error 2 {{protocol 'CircleStart' refines itself}}
-protocol CircleEnd : CircleMiddle { func circle_end()} // expected-note 2 {{protocol 'CircleEnd' declared here}}
+// expected-note@-1 3 {{protocol 'CircleMiddle' declared here}}
+protocol CircleStart : CircleEnd { func circle_start() } // expected-error 3 {{protocol 'CircleStart' refines itself}}
+protocol CircleEnd : CircleMiddle { func circle_end()} // expected-note 3 {{protocol 'CircleEnd' declared here}}
 
 protocol CircleEntry : CircleTrivial { }
 protocol CircleTrivial : CircleTrivial { } // expected-error {{protocol 'CircleTrivial' refines itself}}
diff --git a/test/type/explicit_existential.swift b/test/type/explicit_existential.swift
diff --git a/test/type/explicit_existential_swift6.swift b/test/type/explicit_existential_swift6.swift
