SILType: Avoid asking ASTType if it is Noncopyable

It's better to ask SILType if it is MoveOnly than go to the AST type and
 ask if it is noncopyable, because some types in SIL do not have a
 well-defined notion of conformance in the AST.

Author: kavon

include/swift/SIL/SILType.h

@@ -763,9 +763,13 @@ class SILType {
   /// Returns true if this is the AnyObject SILType;
   bool isAnyObject() const { return getASTType()->isAnyObject(); }
 
-  /// Returns true if this type is a first class move only type or a move only
-  /// wrapped type.
-  bool isMoveOnly() const;
+  /// Returns true if this type is a noncopyable type. Otherwise, if the type
+  /// satisfies \c isMoveOnlyWrapped(), then it returns true iff \c orWrapped
+  /// is true. That is,
+  ///
+  /// orWrapped == false -->  isNoncopyable
+  /// orWrapped == true  -->  isNoncopyable || isMoveOnlyWrapped
+  bool isMoveOnly(bool orWrapped=true) const;
 
   /// Return true if this is a value type (struct/enum) that requires
   /// deinitialization beyond destruction of its members.

lib/AST/Type.cpp

@@ -184,10 +184,6 @@ static CanType preprocessTypeForInvertibleQuery(Type orig) {
   // Strip off any StorageType wrapper.
   type = type->getReferenceStorageReferent();
 
-  // Always strip off SILMoveOnlyWrapper.
-  if (auto wrapper = type->getAs<SILMoveOnlyWrappedType>())
-    type = wrapper->getInnerType();
-
   // Pack expansions such as `repeat T` themselves do not have conformances,
   // so check its pattern type for conformance.
   if (auto *pet = type->getAs<PackExpansionType>()) {

lib/SIL/IR/SILType.cpp

@@ -1049,16 +1049,19 @@ bool SILType::isEscapable() const {
   return getASTType()->isEscapable();
 }
 
-bool SILType::isMoveOnly() const {
+bool SILType::isMoveOnly(bool orWrapped) const {
+  // If it's inside the move-only wrapper, return true iff we want to include
+  // such types as "move-only" in this query. Such values are typically
+  // just "no-implicit-copy" and not "move-only".
+  if (isMoveOnlyWrapped())
+    return orWrapped;
+
   // Legacy check.
   if (!getASTContext().LangOpts.hasFeature(Feature::NoncopyableGenerics)) {
-    return getASTType()->isNoncopyable() || isMoveOnlyWrapped();
+    return getASTType()->isNoncopyable();
   }
 
-  // Anything within the move-only wrapper is move-only.
-  if (isMoveOnlyWrapped())
-    return true;
-
+  // NOTE: getASTType strips the MoveOnlyWrapper off!
   auto ty = getASTType();
 
   // All kinds of references are copyable.

lib/SIL/Verifier/SILVerifier.cpp

@@ -6223,7 +6223,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     auto type = ddi->getType();
     require(type == ddi->getOperand()->getType(),
             "Result and operand must have the same type.");
-    require(type.getASTType()->isNoncopyable(),
+    require(type.isMoveOnly(/*orWrapped=*/false),
             "drop_deinit only allowed for move-only types");
     require(type.getNominalOrBoundGenericNominal()
             ->getValueTypeDestructor(), "drop_deinit only allowed for "

lib/SILGen/SILGenApply.cpp

@@ -3267,7 +3267,7 @@ Expr *ArgumentSource::findStorageReferenceExprForMoveOnly(
 
   SILType ty =
       SGF.getLoweredType(type->getWithoutSpecifierType()->getCanonicalType());
-  bool isMoveOnly = ty.getASTType()->isNoncopyable();
+  bool isMoveOnly = ty.isMoveOnly(/*orWrapped=*/false);
   if (auto *pd = dyn_cast<ParamDecl>(storage)) {
     isMoveOnly |= pd->getSpecifier() == ParamSpecifier::Borrowing;
     isMoveOnly |= pd->getSpecifier() == ParamSpecifier::Consuming;

lib/SILGen/SILGenBuilder.cpp

@@ -530,7 +530,7 @@ static ManagedValue createInputFunctionArgument(
          "Function arguments of non-bare functions must have a decl");
   auto *arg = F.begin()->createFunctionArgument(type, decl);
   if (auto *pd = dyn_cast_or_null<ParamDecl>(decl)) {
-    if (!arg->getType().getASTType()->isNoncopyable()) {
+    if (!arg->getType().isMoveOnly()) {
       isNoImplicitCopy |= pd->getSpecifier() == ParamSpecifier::Borrowing;
       isNoImplicitCopy |= pd->getSpecifier() == ParamSpecifier::Consuming;
     }

lib/SILGen/SILGenDecl.cpp

@@ -515,11 +515,14 @@ class LocalVariableInitialization : public SingleBufferInitialization {
     auto instanceType = SGF.SGM.Types.getLoweredRValueType(
         TypeExpansionContext::minimal(), decl->getTypeInContext());
 
-    // If we have a no implicit copy param decl, make our instance type
-    // @moveOnly.
-    if (!instanceType->isNoncopyable()) {
+
+    bool isNoImplicitCopy = instanceType->is<SILMoveOnlyWrappedType>();
+
+    // If our instance type is not already @moveOnly wrapped, and it's a
+    // no-implicit-copy parameter, wrap it.
+    if (!isNoImplicitCopy && !instanceType->isNoncopyable()) {
       if (auto *pd = dyn_cast<ParamDecl>(decl)) {
-        bool isNoImplicitCopy = pd->isNoImplicitCopy();
+        isNoImplicitCopy = pd->isNoImplicitCopy();
         isNoImplicitCopy |= pd->getSpecifier() == ParamSpecifier::Consuming;
         if (pd->isSelfParameter()) {
           auto *dc = pd->getDeclContext();
@@ -533,9 +536,11 @@ class LocalVariableInitialization : public SingleBufferInitialization {
       }
     }
 
+    const bool isCopyable = isNoImplicitCopy || !instanceType->isNoncopyable();
+
     auto boxType = SGF.SGM.Types.getContextBoxTypeForCapture(
         decl, instanceType, SGF.F.getGenericEnvironment(),
-        /*mutable*/ !instanceType->isNoncopyable() || !decl->isLet());
+        /*mutable=*/ isCopyable || !decl->isLet());
 
     // The variable may have its lifetime extended by a closure, heap-allocate
     // it using a box.
@@ -689,7 +694,7 @@ class LetValueInitialization : public Initialization {
       // For noncopyable types, we always need to box them.
       needsTemporaryBuffer =
           (lowering->isAddressOnly() && SGF.silConv.useLoweredAddresses()) ||
-              lowering->getLoweredType().getASTType()->isNoncopyable();
+              lowering->getLoweredType().isMoveOnly(/*orWrapped=*/false);
     }
 
     // Make sure that we have a non-address only type when binding a
@@ -774,7 +779,7 @@ class LetValueInitialization : public Initialization {
     if (value->getOwnershipKind() == OwnershipKind::None) {
       // Then check if we have a pure move only type. In that case, we need to
       // insert a no implicit copy
-      if (value->getType().getASTType()->isNoncopyable()) {
+      if (value->getType().isMoveOnly(/*orWrapped=*/false)) {
         value = SGF.B.createMoveValue(PrologueLoc, value, /*isLexical*/ true);
         return SGF.B.createMarkUnresolvedNonCopyableValueInst(
             PrologueLoc, value,
@@ -824,7 +829,7 @@ class LetValueInitialization : public Initialization {
     // We do this before the begin_borrow "normal" path below since move only
     // types do not have no implicit copy attr on them.
     if (value->getOwnershipKind() == OwnershipKind::Owned &&
-        value->getType().getASTType()->isNoncopyable()) {
+        value->getType().isMoveOnly(/*orWrapped=*/false)) {
       value = SGF.B.createMoveValue(PrologueLoc, value, true /*isLexical*/);
       return SGF.B.createMarkUnresolvedNonCopyableValueInst(
           PrologueLoc, value,

lib/SILGen/SILGenProlog.cpp

@@ -703,7 +703,7 @@ class ArgumentInitHelper {
     // - @_eagerMove
     // - @_noEagerMove
     bool isNoImplicitCopy = pd->isNoImplicitCopy();
-    if (!argrv.getType().getASTType()->isNoncopyable()) {
+    if (!argrv.getType().isMoveOnly(/*orWrapped=*/false)) {
       isNoImplicitCopy |= pd->getSpecifier() == ParamSpecifier::Borrowing;
       isNoImplicitCopy |= pd->getSpecifier() == ParamSpecifier::Consuming;
       if (pd->isSelfParameter()) {
@@ -1167,7 +1167,7 @@ static void emitCaptureArguments(SILGenFunction &SGF,
     // in SIL since it is illegal to capture an inout value in an escaping
     // closure. The later code knows how to handle that we have the
     // mark_unresolved_non_copyable_value here.
-    if (isInOut && ty.getASTType()->isNoncopyable()) {
+    if (isInOut && ty.isMoveOnly(/*orWrapped=*/false)) {
       arg = SGF.B.createMarkUnresolvedNonCopyableValueInst(
           Loc, arg,
           MarkUnresolvedNonCopyableValueInst::CheckKind::

lib/SILOptimizer/SILCombiner/SILCombinerMiscVisitors.cpp

@@ -379,7 +379,7 @@ struct AllocStackAnalyzer : SILInstructionVisitor<AllocStackAnalyzer> {
     // analysis assumes memory is deinitialized on all paths, which is not the
     // case for discarded values. Eventually copyable types may also be
     // discarded; to support that, we will leave a drop_deinit_addr in place.
-    if (ASI->getType().getASTType()->isNoncopyable()) {
+    if (ASI->getType().isMoveOnly(/*orWrapped=*/false)) {
       LegalUsers = false;
       return;
     }

lib/SILOptimizer/Transforms/SILLowerAggregateInstrs.cpp

@@ -52,7 +52,7 @@ static bool shouldExpandShim(SILFunction *fn, SILType type) {
   // shouldExpand returns false for struct-with-deinit types, so bypassing it is
   // incorrect for move-only types
   if (EnableExpandAll) {
-    assert(!type.getASTType()->isNoncopyable()
+    assert(!type.isMoveOnly(/*orWrapped=*/false)
            && "sil-lower-agg-instrs-expand-all is incompatible with move-only "
               "types");
     return true;