Merge pull request #71560 from kavon/ncgenerics-test-fixes-kavon-v10

Ncgenerics test fixes kavon v10

Author: kavon

include/swift/AST/InverseMarking.h

@@ -57,6 +57,11 @@ struct InverseMarking {
     }
     operator bool() const { return isPresent(); }
 
+    // Is there any kind of explicit marking?
+    bool isAnyExplicit() const {
+      return is(Kind::Explicit) || is(Kind::LegacyExplicit);
+    }
+
     SourceLoc getLoc() const { return loc; }
 
     void set(Kind k, SourceLoc l = SourceLoc()) {

include/swift/AST/ProtocolConformance.h

@@ -1157,6 +1157,19 @@ class BuiltinProtocolConformance final : public RootProtocolConformance {
     return getBuiltinConformanceKind() == BuiltinConformanceKind::Missing;
   }
 
+  bool isInvalid() const {
+    switch (getBuiltinConformanceKind()) {
+    case BuiltinConformanceKind::Synthesized:
+      return false;
+    case BuiltinConformanceKind::Missing:
+      return true;
+    }
+  }
+
+  SourceLoc getLoc() const {
+    return SourceLoc();
+  }
+
   /// Get any requirements that must be satisfied for this conformance to apply.
   llvm::Optional<ArrayRef<Requirement>>
   getConditionalRequirementsIfAvailable() const {
@@ -1191,6 +1204,10 @@ class BuiltinProtocolConformance final : public RootProtocolConformance {
     llvm_unreachable("builtin-conformances never have associated types");
   }
 
+  bool hasWitness(ValueDecl *requirement) const {
+    llvm_unreachable("builtin-conformances never have requirement witnesses");
+  }
+
   /// Retrieve the type witness and type decl (if one exists)
   /// for the given associated type.
   TypeWitnessAndDecl
@@ -1199,6 +1216,10 @@ class BuiltinProtocolConformance final : public RootProtocolConformance {
     llvm_unreachable("builtin-conformances never have associated types");
   }
 
+  Witness getWitness(ValueDecl *requirement) const {
+    llvm_unreachable("builtin-conformances never have requirement witnesses");
+  }
+
   /// Given that the requirement signature of the protocol directly states
   /// that the given dependent type must conform to the given protocol,
   /// return its associated conformance.

lib/AST/ConformanceLookup.cpp

@@ -29,6 +29,7 @@
 #include "swift/AST/DiagnosticsSema.h"
 #include "swift/AST/ExistentialLayout.h"
 #include "swift/AST/GenericEnvironment.h"
+#include "swift/AST/InverseMarking.h"
 #include "swift/AST/NameLookup.h"
 #include "swift/AST/NameLookupRequests.h"
 #include "swift/AST/PackConformance.h"
@@ -402,6 +403,41 @@ static ProtocolConformanceRef getBuiltinMetaTypeTypeConformance(
   return ProtocolConformanceRef::forMissingOrInvalid(type, protocol);
 }
 
+static ProtocolConformanceRef
+getBuiltinInvertibleProtocolConformance(NominalTypeDecl *nominal,
+                                        Type type,
+                                        ProtocolDecl *protocol) {
+  assert(isa<ClassDecl>(nominal));
+  ASTContext &ctx = protocol->getASTContext();
+
+  auto ip = protocol->getInvertibleProtocolKind();
+  switch (*ip) {
+  case InvertibleProtocolKind::Copyable:
+    // If move-only classes is enabled, we'll check the markings.
+    if (ctx.LangOpts.hasFeature(Feature::MoveOnlyClasses)) {
+      auto marking = nominal->getMarking(*ip);
+      switch (marking.getInverse().getKind()) {
+      case InverseMarking::Kind::LegacyExplicit:
+      case InverseMarking::Kind::Explicit:
+        // An inverse ~Copyable prevents conformance.
+        return ProtocolConformanceRef::forInvalid();
+
+      case InverseMarking::Kind::Inferred: // ignore "inferred" inverse marking
+      case InverseMarking::Kind::None:
+        break;
+      }
+    }
+    break;
+  case InvertibleProtocolKind::Escapable:
+    // Always conforms.
+    break;
+  }
+
+  return ProtocolConformanceRef(
+      ctx.getBuiltinConformance(type, protocol,
+                                BuiltinConformanceKind::Synthesized));
+}
+
 /// Synthesize a builtin type conformance to the given protocol, if
 /// appropriate.
 static ProtocolConformanceRef
@@ -472,6 +508,9 @@ LookupConformanceInModuleRequest::evaluate(
   auto *protocol = desc.PD;
   ASTContext &ctx = mod->getASTContext();
 
+  // Remove SIL reference ownership wrapper, if present.
+  type = type->getReferenceStorageReferent();
+
   // A dynamic Self type conforms to whatever its underlying type
   // conforms to.
   if (auto selfType = type->getAs<DynamicSelfType>())
@@ -588,6 +627,13 @@ LookupConformanceInModuleRequest::evaluate(
   if (!nominal || isa<ProtocolDecl>(nominal))
     return ProtocolConformanceRef::forMissingOrInvalid(type, protocol);
 
+  // We specially avoid recording conformances to invertible protocols in a
+  // class's conformance table. This prevents an evaluator cycle.
+  if (ctx.LangOpts.hasFeature(Feature::NoncopyableGenerics)
+      && isa<ClassDecl>(nominal)
+      && protocol->getInvertibleProtocolKind())
+        return getBuiltinInvertibleProtocolConformance(nominal, type, protocol);
+
   // Expand conformances added by extension macros.
   //
   // FIXME: This expansion should only be done if the

lib/AST/ProtocolConformance.cpp

@@ -102,9 +102,10 @@ switch (getKind()) {                                                         \
     return cast<NormalProtocolConformance>(this)->Method Args;               \
   case ProtocolConformanceKind::Self:                                        \
     return cast<SelfProtocolConformance>(this)->Method Args;                 \
+  case ProtocolConformanceKind::Builtin:                                     \
+    return cast<BuiltinProtocolConformance>(this)->Method Args;              \
   case ProtocolConformanceKind::Specialized:                                 \
   case ProtocolConformanceKind::Inherited:                                   \
-  case ProtocolConformanceKind::Builtin:                                     \
     llvm_unreachable("not a root conformance");                              \
 }                                                                            \
 llvm_unreachable("bad ProtocolConformanceKind");
@@ -1090,19 +1091,21 @@ void NominalTypeDecl::prepareConformanceTable() const {
   // Synthesize the unconditional conformances to invertible protocols.
   // For conditional ones, see findSynthesizedConformances .
   if (ctx.LangOpts.hasFeature(Feature::NoncopyableGenerics)) {
-    bool missingOne = false;
-    for (auto ip : InvertibleProtocolSet::full()) {
-      auto invertible = getMarking(ip);
-      if (!invertible.getInverse() || bool(invertible.getPositive()))
-        addSynthesized(ctx.getProtocol(getKnownProtocolKind(ip)));
-      else
-        missingOne = true;
-    }
-
-    // FIXME: rdar://122289155 (NCGenerics: convert Equatable, Hashable, and RawRepresentable to ~Copyable.)
-    if (missingOne)
-      return;
+    // Classes get their conformances during ModuleDecl::lookupConformance.
+    if (!isa<ClassDecl>(this)) {
+      bool missingOne = false;
+      for (auto ip : InvertibleProtocolSet::full()) {
+        auto invertible = getMarking(ip);
+        if (!invertible.getInverse() || bool(invertible.getPositive()))
+          addSynthesized(ctx.getProtocol(getKnownProtocolKind(ip)));
+        else
+          missingOne = true;
+      }
 
+      // FIXME: rdar://122289155 (NCGenerics: convert Equatable, Hashable, and RawRepresentable to ~Copyable.)
+      if (missingOne)
+        return;
+    }
   } else if (!canBeCopyable()) {
     return; // No synthesized conformances for move-only nominals.
   }

lib/Frontend/Frontend.cpp

@@ -1501,8 +1501,8 @@ bool CompilerInstance::loadStdlibIfNeeded() {
 
   // If we failed to load, we should have already diagnosed.
   if (M->failedToLoad()) {
-//    assert(Diagnostics.hadAnyError() &&
-//           "Module failed to load but nothing was diagnosed?");
+    assert(Diagnostics.hadAnyError() &&
+           "stdlib module failed to load but nothing was diagnosed?");
     return true;
   }
   return false;

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -3183,6 +3183,29 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
     }
   }
 
+  static void diagnoseInverseOnClass(ClassDecl *decl) {
+    auto &ctx = decl->getASTContext();
+
+    for (auto ip : InvertibleProtocolSet::full()) {
+      auto marking = decl->getMarking(ip);
+
+      // Inferred inverses are already ignored for classes.
+      // FIXME: we can also diagnose @_moveOnly here if we use `isAnyExplicit`
+      if (!marking.getInverse().is(InverseMarking::Kind::Explicit))
+        continue;
+
+      // Allow ~Copyable when MoveOnlyClasses is enabled
+      if (ip == InvertibleProtocolKind::Copyable
+          && ctx.LangOpts.hasFeature(Feature::MoveOnlyClasses))
+        continue;
+
+
+      ctx.Diags.diagnose(marking.getInverse().getLoc(),
+                         diag::inverse_on_class,
+                         getProtocolName(getKnownProtocolKind(ip)));
+    }
+  }
+
   /// check to see if a move-only type can ever conform to the given type.
   /// \returns true iff a diagnostic was emitted because it was not compatible
   static bool diagnoseIncompatibleWithMoveOnlyType(SourceLoc loc,
@@ -3415,6 +3438,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
 
     diagnoseIncompatibleProtocolsForMoveOnlyType(CD);
 
+    diagnoseInverseOnClass(CD);
   }
 
   void visitProtocolDecl(ProtocolDecl *PD) {

lib/Sema/TypeCheckInvertible.cpp

@@ -171,20 +171,14 @@ static bool checkInvertibleConformanceCommon(ProtocolConformance *conformance,
   auto &ctx = nom->getASTContext();
   bool conforms = true;
 
-  // An explicit `~IP` prevents conformance if any of these are true:
+  // An explicit `~IP` prevents conformance if it appears on the same
+  // declaration that also declares the conformance.
   //
-  // 1. It appears on a class.
-  // 2. Appears on the same declaration that also declares the conformance.
-  //    So, if the nominal has `~Copyable` but this conformance is
-  //    written in an extension, then we do not raise an error.
+  // So, if the nominal has `~Copyable` but this conformance is
+  // written in an extension, then we do not raise an error.
   auto marking = nom->getMarking(ip);
-  if (marking.getInverse().getKind() == InverseMarking::Kind::Explicit) {
-    if (isa<ClassDecl>(nom)) {
-      ctx.Diags.diagnose(marking.getInverse().getLoc(),
-                         diag::inverse_on_class,
-                         getProtocolName(kp));
-      conforms &= false;
-    } else if (conformance->getDeclContext() == nom) {
+  if (marking.getInverse().isAnyExplicit()) {
+    if (conformance->getDeclContext() == nom) {
       ctx.Diags.diagnose(marking.getInverse().getLoc(),
                          diag::inverse_but_also_conforms,
                          nom, getProtocolName(kp));
@@ -198,7 +192,7 @@ static bool checkInvertibleConformanceCommon(ProtocolConformance *conformance,
 
   // Protocols do not directly define any storage.
   if (isa<ProtocolDecl, BuiltinTupleDecl>(nom))
-    llvm_unreachable("unexpected nominal to check Copyable conformance");
+    llvm_unreachable("unexpected nominal to check invertible's conformance");
 
   // A deinit prevents a struct or enum from conforming to Copyable.
   if (ip == InvertibleProtocolKind::Copyable) {
@@ -343,6 +337,7 @@ ProtocolConformance *deriveConformanceForInvertible(Evaluator &evaluator,
   if (!ip)
     llvm_unreachable("not an invertible protocol");
 
+  assert(!isa<ClassDecl>(nominal) && "classes aren't handled here");
   auto file = cast<FileUnit>(nominal->getModuleScopeContext());
 
   // Generates a conformance for the nominal to the protocol.
@@ -403,20 +398,6 @@ ProtocolConformance *deriveConformanceForInvertible(Evaluator &evaluator,
     return generateConformance(ext);
   };
 
-  switch (*ip) {
-  case InvertibleProtocolKind::Copyable:
-    // If move-only classes is enabled, we'll check the markings.
-    if (ctx.LangOpts.hasFeature(Feature::MoveOnlyClasses))
-      break;
-
-  LLVM_FALLTHROUGH;
-  case InvertibleProtocolKind::Escapable:
-    // Always derive unconditional IP conformance for classes
-    if (isa<ClassDecl>(nominal))
-      return generateConformance(nominal);
-    break;
-  }
-
   auto marking = nominal->getMarking(*ip);
 
   // Unexpected to have any positive marking for IP if we're deriving it.
@@ -430,10 +411,8 @@ ProtocolConformance *deriveConformanceForInvertible(Evaluator &evaluator,
     return nullptr; // No positive IP conformance will be inferred.
 
   case InverseMarking::Kind::Inferred:
-    if (!isa<ClassDecl>(nominal))
-      return generateConditionalConformance();
+    return generateConditionalConformance();
 
-  LLVM_FALLTHROUGH;
   case InverseMarking::Kind::None:
     // All types already start with conformances to the invertible protocols in
     // this case, within `NominalTypeDecl::prepareConformanceTable`.

lib/Sema/TypeCheckType.cpp

@@ -1083,14 +1083,6 @@ Type TypeResolution::applyUnboundGenericArguments(
   if (didDiagnoseMoveOnlyGenericArgs(ctx, loc, resultType, genericArgs, dc))
     return ErrorType::get(ctx);
 
-  if (options.contains(TypeResolutionFlags::SILType)) {
-    if (auto nominal = dyn_cast<NominalTypeDecl>(decl)) {
-      if (nominal->isOptionalDecl()) {
-        skipRequirementsCheck = true;
-      }
-    }
-  }
-
   // Get the substitutions for outer generic parameters from the parent
   // type.
   if (parentTy) {

test/Generics/inverse_generics.swift

@@ -169,7 +169,12 @@ protocol NeedsCopyable {}
 
 struct Silly: ~Copyable, Copyable {} // expected-error {{struct 'Silly' required to be 'Copyable' but is marked with '~Copyable'}}
 enum Sally: Copyable, ~Copyable, NeedsCopyable {} // expected-error {{enum 'Sally' required to be 'Copyable' but is marked with '~Copyable'}}
+
 class NiceTry: ~Copyable, Copyable {} // expected-error {{classes cannot be '~Copyable'}}
+                                      // expected-error@-1 {{class 'NiceTry' required to be 'Copyable' but is marked with '~Copyable}}
+
+@_moveOnly class NiceTry2: Copyable {} // expected-error {{'@_moveOnly' attribute is only valid on structs or enums}}
+                                       // expected-error@-1 {{class 'NiceTry2' required to be 'Copyable' but is marked with '~Copyable'}}
 
 struct OopsConformance1: ~Copyable, NeedsCopyable {}
 // expected-error@-1 {{type 'OopsConformance1' does not conform to protocol 'NeedsCopyable'}}