[AST] Remove ParenType

Today ParenType is used:

1. As the type of ParenExpr
2. As the payload type of an unlabeled single
   associated value enum case (and the type of
   ParenPattern).
3. As the type for an `(X)` TypeRepr

For 1, this leads to some odd behavior, e.g the
type of `(5.0 * 5).squareRoot()` is `(Double)`. For
2, we should be checking the arity of the enum case
constructor parameters and the presence of
ParenPattern respectively. Eventually we ought to
consider replacing Paren/TuplePattern with a
PatternList node, similar to ArgumentList.

3 is one case where it could be argued that there's
some utility in preserving the sugar of the type
that the user wrote. However it's really not clear
to me that this is particularly desirable since a
bunch of diagnostic logic is already stripping
ParenTypes. In cases where we care about how the
type was written in source, we really ought to be
consulting the TypeRepr.

Author: hamishknight

docs/ABI/Mangling.rst

@@ -934,7 +934,6 @@ productions:
   type ::= base-type "XSq"                       // sugared Optional type
   type ::= base-type "XSa"                       // sugared Array type
   type ::= key-type value-type "XSD"             // sugared Dictionary type
-  type ::= base-type "XSp"                       // sugared Paren type
 
 Generics
 ~~~~~~~~

include/swift/AST/ASTDemangler.h

@@ -244,8 +244,6 @@ class ASTBuilder {
 
   Type createDictionaryType(Type key, Type value);
 
-  Type createParenType(Type base);
-
   Type createIntegerType(intptr_t value);
 
   Type createNegativeIntegerType(intptr_t value);

include/swift/AST/TypeNodes.def

@@ -209,7 +209,6 @@ UNCHECKED_TYPE(TypeVariable, Type)
 UNCHECKED_TYPE(ErrorUnion, Type)
 ALWAYS_CANONICAL_TYPE(Integer, Type)
 ABSTRACT_SUGARED_TYPE(Sugar, Type)
-  SUGARED_TYPE(Paren, SugarType)
   SUGARED_TYPE(TypeAlias, SugarType)
   ABSTRACT_SUGARED_TYPE(SyntaxSugar, SugarType)
     ABSTRACT_SUGARED_TYPE(UnarySyntaxSugar, SyntaxSugarType)
@@ -219,7 +218,7 @@ ABSTRACT_SUGARED_TYPE(Sugar, Type)
       TYPE_RANGE(UnarySyntaxSugar, ArraySlice, VariadicSequence)
     SUGARED_TYPE(Dictionary, SyntaxSugarType)
     TYPE_RANGE(SyntaxSugar, ArraySlice, Dictionary)
-  TYPE_RANGE(Sugar, Paren, Dictionary)
+  TYPE_RANGE(Sugar, TypeAlias, Dictionary)
 LAST_TYPE(Dictionary) // Sugared types are last to make isa<SugarType>() fast.
 
 #endif

include/swift/AST/TypeTransform.h

@@ -94,13 +94,10 @@ class TypeTransform {
 
 public:
   Type doIt(Type t, TypePosition pos) {
-    if (!isa<ParenType>(t.getPointer())) {
-      // Transform this type node.
-      if (std::optional<Type> transformed = asDerived().transform(t.getPointer(), pos))
-        return *transformed;
-
-      // Recur.
-    }
+    // Transform this type node.
+    if (std::optional<Type> transformed =
+            asDerived().transform(t.getPointer(), pos))
+      return *transformed;
 
     // Recur into children of this type.
     TypeBase *base = t.getPointer();
@@ -536,18 +533,6 @@ case TypeKind::Id:
                                 newUnderlyingTy);
     }
 
-    case TypeKind::Paren: {
-      auto paren = cast<ParenType>(base);
-      Type underlying = doIt(paren->getUnderlyingType(), pos);
-      if (!underlying)
-        return Type();
-
-      if (underlying.getPointer() == paren->getUnderlyingType().getPointer())
-        return t;
-
-      return ParenType::get(ctx, underlying);
-    }
-
     case TypeKind::ErrorUnion: {
       auto errorUnion = cast<ErrorUnionType>(base);
       bool anyChanged = false;

include/swift/AST/Types.h

@@ -426,8 +426,6 @@ class alignas(1 << TypeAlignInBits) TypeBase
     HasCachedType : 1
   );
 
-  SWIFT_INLINE_BITFIELD_EMPTY(ParenType, SugarType);
-
   SWIFT_INLINE_BITFIELD_FULL(AnyFunctionType, TypeBase, NumAFTExtInfoBits+1+1+1+1+16,
     /// Extra information which affects how the function is called, like
     /// regparm and the calling convention.
@@ -688,9 +686,6 @@ class alignas(1 << TypeAlignInBits) TypeBase
   /// Returns true if this contextual type is (Escapable && !isNoEscape).
   bool mayEscape() { return !isNoEscape() && isEscapable(); }
 
-  /// Does the type have outer parenthesis?
-  bool hasParenSugar() const { return getKind() == TypeKind::Paren; }
-
   /// Are values of this type essentially just class references,
   /// possibly with some sort of additional information?
   ///
@@ -1288,9 +1283,6 @@ class alignas(1 << TypeAlignInBits) TypeBase
   /// argument labels removed.
   Type removeArgumentLabels(unsigned numArgumentLabels);
 
-  /// Retrieve the type without any parentheses around it.
-  Type getWithoutParens();
-
   /// Replace the base type of the result type of the given function
   /// type with a new result type, as per a DynamicSelf or other
   /// covariant return transformation.  The optionality of the
@@ -2589,21 +2581,6 @@ class YieldTypeFlags {
   uint8_t toRaw() const { return value.toRaw(); }
 };
 
-/// ParenType - A paren type is a type that's been written in parentheses.
-class ParenType : public SugarType {
-  ParenType(Type UnderlyingType, RecursiveTypeProperties properties);
-
-public:
-  static ParenType *get(const ASTContext &C, Type underlying);
-
-  Type getUnderlyingType() const { return getSinglyDesugaredType(); }
-
-  // Implement isa/cast/dyncast/etc.
-  static bool classof(const TypeBase *T) {
-    return T->getKind() == TypeKind::Paren;
-  }
-};
-
 /// TupleTypeElt - This represents a single element of a tuple.
 class TupleTypeElt {
   /// An optional name for the field.

include/swift/Demangling/DemangleNodes.def

@@ -328,7 +328,7 @@ NODE(AssociatedTypeGenericParamRef)
 NODE(SugaredOptional)
 NODE(SugaredArray)
 NODE(SugaredDictionary)
-NODE(SugaredParen)
+NODE(SugaredParen) // Removed in Swift 6.TBD
 
 // Added in Swift 5.1
 NODE(AccessorFunctionReference)

include/swift/Demangling/TypeDecoder.h

@@ -1479,7 +1479,9 @@ class TypeDecoder {
       if (base.isError())
         return base;
 
-      return Builder.createParenType(base.getType());
+      // ParenType has been removed, return the base type for backwards
+      // compatibility.
+      return base.getType();
     }
     case NodeKind::OpaqueType: {
       if (Node->getNumChildren() < 3)

include/swift/Sema/CSBindings.h

@@ -94,7 +94,7 @@ struct PotentialBinding {
 public:
   PotentialBinding(Type type, AllowedBindingKind kind, Constraint *source)
       : PotentialBinding(
-            type->getWithoutParens(), kind,
+            type, kind,
             PointerUnion<Constraint *, ConstraintLocator *>(source)) {}
 
   bool isDefaultableBinding() const {

lib/APIDigester/ModuleAnalyzerNodes.cpp

@@ -1065,13 +1065,12 @@ static StringRef getPrintedName(SDKContext &Ctx, Type Ty,
   if (IsImplicitlyUnwrappedOptional)
     PO.PrintOptionalAsImplicitlyUnwrapped = true;
 
-  Ty->getWithoutParens().print(OS, PO);
+  Ty.print(OS, PO);
   return Ctx.buffer(OS.str());
 }
 
 static StringRef getTypeName(SDKContext &Ctx, Type Ty,
                              bool IsImplicitlyUnwrappedOptional) {
-  Ty = Ty->getWithoutParens();
   if (Ty->isVoid()) {
     return Ctx.buffer("Void");
   }

lib/AST/ASTContext.cpp

@@ -542,7 +542,6 @@ struct ASTContext::Implementation {
     llvm::DenseMap<Type, VariadicSequenceType*> VariadicSequenceTypes;
     llvm::DenseMap<std::pair<Type, Type>, DictionaryType *> DictionaryTypes;
     llvm::DenseMap<Type, OptionalType*> OptionalTypes;
-    llvm::DenseMap<Type, ParenType*> ParenTypes;
     llvm::DenseMap<uintptr_t, ReferenceStorageType*> ReferenceStorageTypes;
     llvm::DenseMap<Type, LValueType*> LValueTypes;
     llvm::DenseMap<Type, InOutType*> InOutTypes;
@@ -3240,7 +3239,6 @@ size_t ASTContext::Implementation::Arena::getTotalMemory() const {
     llvm::capacity_in_bytes(DictionaryTypes) +
     llvm::capacity_in_bytes(OptionalTypes) +
     llvm::capacity_in_bytes(VariadicSequenceTypes) +
-    llvm::capacity_in_bytes(ParenTypes) +
     llvm::capacity_in_bytes(ReferenceStorageTypes) +
     llvm::capacity_in_bytes(LValueTypes) +
     llvm::capacity_in_bytes(InOutTypes) +
@@ -3279,7 +3277,6 @@ void ASTContext::Implementation::Arena::dump(llvm::raw_ostream &os) const {
     SIZE_AND_BYTES(VariadicSequenceTypes);
     SIZE_AND_BYTES(DictionaryTypes);
     SIZE_AND_BYTES(OptionalTypes);
-    SIZE_AND_BYTES(ParenTypes);
     SIZE_AND_BYTES(ReferenceStorageTypes);
     SIZE_AND_BYTES(LValueTypes);
     SIZE_AND_BYTES(InOutTypes);
@@ -3629,18 +3626,6 @@ BuiltinVectorType *BuiltinVectorType::get(const ASTContext &context,
   return vecTy;
 }
 
-ParenType *ParenType::get(const ASTContext &C, Type underlying) {
-  auto properties = underlying->getRecursiveProperties();
-  auto arena = getArena(properties);
-  ParenType *&Result = C.getImpl().getArena(arena).ParenTypes[underlying];
-  if (Result == nullptr) {
-    Result = new (C, arena) ParenType(underlying, properties);
-    assert((C.hadError() || !underlying->is<InOutType>()) &&
-           "Cannot wrap InOutType");
-  }
-  return Result;
-}
-
 CanTupleType TupleType::getEmpty(const ASTContext &C) {
   return cast<TupleType>(CanType(C.TheEmptyTupleType));
 }
@@ -4567,7 +4552,7 @@ Type AnyFunctionType::composeTuple(ASTContext &ctx, ArrayRef<Param> params,
     elements.emplace_back(param.getParameterType(), param.getLabel());
   }
   if (elements.size() == 1 && !elements[0].hasName())
-    return ParenType::get(ctx, elements[0].getType());
+    return elements[0].getType();
   return TupleType::get(elements, ctx);
 }
 

lib/AST/ASTDemangler.cpp

@@ -1053,10 +1053,6 @@ Type ASTBuilder::createDictionaryType(Type key, Type value) {
   return DictionaryType::get(key, value);
 }
 
-Type ASTBuilder::createParenType(Type base) {
-  return ParenType::get(Ctx, base);
-}
-
 Type ASTBuilder::createIntegerType(intptr_t value) {
   return IntegerType::get(std::to_string(value), /*isNegative*/ false, Ctx);
 }

lib/AST/ASTDumper.cpp

@@ -4140,14 +4140,6 @@ namespace {
       printFoot();
     }
 
-    void visitParenType(ParenType *T, StringRef label) {
-      printCommon("paren_type", label);
-
-      printRec(T->getUnderlyingType());
-
-      printFoot();
-    }
-
     void visitTupleType(TupleType *T, StringRef label) {
       printCommon("tuple_type", label);
 

lib/AST/ASTMangler.cpp

@@ -1396,12 +1396,6 @@ void ASTMangler::appendType(Type type, GenericSignature sig,
       return;
     }
 
-    case TypeKind::Paren:
-      assert(DWARFMangling && "sugared types are only legal for the debugger");
-      appendType(cast<ParenType>(tybase)->getUnderlyingType(), sig, forDecl);
-      appendOperator("XSp");
-      return;
-
     case TypeKind::ArraySlice:
       assert(DWARFMangling && "sugared types are only legal for the debugger");
       appendType(cast<ArraySliceType>(tybase)->getBaseType(), sig, forDecl);

lib/AST/ASTPrinter.cpp

@@ -6102,12 +6102,6 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
     }
   }
 
-  void visitParenType(ParenType *T) {
-    Printer << "(";
-    visit(T->getUnderlyingType()->getInOutObjectType());
-    Printer << ")";
-  }
-
   void visitPackType(PackType *T) {
     if (Options.PrintExplicitPackTypes || Options.PrintTypesForDebugging)
       Printer << "Pack{";
@@ -6185,7 +6179,7 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
         Printer << ": ";
       } else if (e == 1 && !EltType->is<PackExpansionType>()) {
         // Unlabeled one-element tuples always print the empty label to
-        // distinguish them from the older syntax for ParenType.
+        // distinguish them from the older syntax for parens.
         Printer << "_: ";
       }
       visit(EltType);

lib/AST/ASTVerifier.cpp

@@ -2135,16 +2135,6 @@ class Verifier : public ASTWalker {
       verifyCheckedBase(E);
     }
 
-    void verifyChecked(ParenExpr *E) {
-      PrettyStackTraceExpr debugStack(Ctx, "verifying ParenExpr", E);
-      auto ty = dyn_cast<ParenType>(E->getType().getPointer());
-      if (!ty) {
-        Out << "ParenExpr not of ParenType\n";
-        abort();
-      }
-      verifyCheckedBase(E);
-    }
-
     void verifyChecked(AnyTryExpr *E) {
       PrettyStackTraceExpr debugStack(Ctx, "verifying AnyTryExpr", E);
 

lib/AST/ArgumentList.cpp

@@ -239,7 +239,7 @@ Expr *ArgumentList::packIntoImplicitTupleOrParen(
   if (auto *unary = getUnlabeledUnaryExpr()) {
     auto *paren = new (ctx) ParenExpr(getLParenLoc(), unary, getRParenLoc());
     if (auto ty = getType(unary))
-      paren->setType(ParenType::get(ctx, ty));
+      paren->setType(ty);
     paren->setImplicit();
     return paren;
   }

lib/AST/DiagnosticEngine.cpp

@@ -707,8 +707,8 @@ static bool typeSpellingIsAmbiguous(Type type,
   for (auto arg : Args) {
     if (arg.getKind() == DiagnosticArgumentKind::Type) {
       auto argType = arg.getAsType();
-      if (argType && argType->getWithoutParens().getPointer() != type.getPointer() &&
-          argType->getWithoutParens().getString(PO) == type.getString(PO)) {
+      if (argType && argType.getPointer() != type.getPointer() &&
+          argType.getString(PO) == type.getString(PO)) {
         // Currently, existential types are spelled the same way
         // as protocols and compositions. We can remove this once
         // existenials are printed with 'any'.
@@ -900,41 +900,20 @@ static void formatDiagnosticArgument(StringRef Modifier,
     // Compute the appropriate print options for this argument.
     auto printOptions = PrintOptions::forDiagnosticArguments();
     if (Arg.getKind() == DiagnosticArgumentKind::Type) {
-      type = Arg.getAsType()->getWithoutParens();
-      if (type.isNull()) {
-        // FIXME: We should never receive a nullptr here, but this is causing
-        // crashes (rdar://75740683). Remove once ParenType never contains
-        // nullptr as the underlying type.
-        Out << "<null>";
-        break;
-      }
+      type = Arg.getAsType();
       if (type->getASTContext().TypeCheckerOpts.PrintFullConvention)
         printOptions.PrintFunctionRepresentationAttrs =
             PrintOptions::FunctionRepresentationMode::Full;
       needsQualification = typeSpellingIsAmbiguous(type, Args, printOptions);
     } else if (Arg.getKind() == DiagnosticArgumentKind::FullyQualifiedType) {
-      type = Arg.getAsFullyQualifiedType().getType()->getWithoutParens();
-      if (type.isNull()) {
-        // FIXME: We should never receive a nullptr here, but this is causing
-        // crashes (rdar://75740683). Remove once ParenType never contains
-        // nullptr as the underlying type.
-        Out << "<null>";
-        break;
-      }
+      type = Arg.getAsFullyQualifiedType().getType();
       if (type->getASTContext().TypeCheckerOpts.PrintFullConvention)
         printOptions.PrintFunctionRepresentationAttrs =
             PrintOptions::FunctionRepresentationMode::Full;
       needsQualification = true;
     } else {
       assert(Arg.getKind() == DiagnosticArgumentKind::WitnessType);
-      type = Arg.getAsWitnessType().getType()->getWithoutParens();
-      if (type.isNull()) {
-        // FIXME: We should never receive a nullptr here, but this is causing
-        // crashes (rdar://75740683). Remove once ParenType never contains
-        // nullptr as the underlying type.
-        Out << "<null>";
-        break;
-      }
+      type = Arg.getAsWitnessType().getType();
       printOptions.PrintGenericRequirements = false;
       printOptions.PrintInverseRequirements = false;
       needsQualification = typeSpellingIsAmbiguous(type, Args, printOptions);