Make sure ErrorTypes containing type variables are marked as such. (#7963)

In some cases, the type checker will produce error types with the
"original type" intact for recovery purposes. Like other types, when
the original type contains a type variable, the ErrorType instance
will be allocated in the "temporary" memory arena associated with the
active constraint solver, because there's no way that particular error
will come up again once the constraint system containing that type
variable has been destroyed.

However, we weren't propagating that "contains a type variable"
information to the newly-created ErrorType, which meant that any type
/containing/ that ErrorType would be allocated in the "permanent"
arena. In practice, this would always be a DependentMemberType; not
too many types are created without looking at their base types at all.
The arena containing the ErrorType would then be deallocated, and its
memory reused later on for a /different/ type. If we ever tried to
make a DependentMemberType whose base was this new type, we'd find the
old DependentMemberType instance in our cache and return that. The
result was that we'd have a DependentMemberType whose "HasError" bit
was set even though the base type was not an error type, and which was
considered canonical whether or not the base type was. This would then
either hit an assertion later on or result in nonsensical errors like
"'C.Iterator' is not the same type as 'C.Iterator'".

Because the reused address always referred to a valid type, none of
the usual dynamic analysis tools could catch the problem. It really
comes down to using a pointer address as a key in a map---but even
without that, we were allocating types in the permanent arena that
really should be temporary, which is a waste of memory.

Likely fixes rdar://problem/30382791, a nondeterministic failure we've
been seeing for weeks on the bots and on developer machines.

Author: jrose-apple

include/swift/AST/Types.h

@@ -990,8 +990,10 @@ class alignas(1 << TypeAlignInBits) TypeBase {
 class ErrorType : public TypeBase {
   friend class ASTContext;
   // The Error type is always canonical.
-  ErrorType(ASTContext &C, Type originalType)
-      : TypeBase(TypeKind::Error, &C, RecursiveTypeProperties::HasError) {
+  ErrorType(ASTContext &C, Type originalType,
+            RecursiveTypeProperties properties)
+      : TypeBase(TypeKind::Error, &C, properties) {
+    assert(properties.hasError());
     if (originalType) {
       ErrorTypeBits.HasOriginalType = true;
       *reinterpret_cast<Type *>(this + 1) = originalType;

lib/AST/ASTContext.cpp

@@ -404,7 +404,8 @@ ASTContext::ASTContext(LangOptions &langOpts, SearchPathOptions &SearchPathOpts,
     SwiftShimsModuleName(getIdentifier(SWIFT_SHIMS_NAME)),
     TypeCheckerDebug(new StderrTypeCheckerDebugConsumer()),
     TheErrorType(
-      new (*this, AllocationArena::Permanent) ErrorType(*this, Type())),
+      new (*this, AllocationArena::Permanent)
+        ErrorType(*this, Type(), RecursiveTypeProperties::HasError)),
     TheUnresolvedType(new (*this, AllocationArena::Permanent)
                       UnresolvedType(*this)),
     TheEmptyTupleType(TupleType::get(ArrayRef<TupleTypeElt>(), *this)),
@@ -2424,16 +2425,19 @@ Type ErrorType::get(const ASTContext &C) { return C.TheErrorType; }
 Type ErrorType::get(Type originalType) {
   assert(originalType);
 
-  auto properties = originalType->getRecursiveProperties();
-  auto arena = getArena(properties);
+  auto originalProperties = originalType->getRecursiveProperties();
+  auto arena = getArena(originalProperties);
 
   auto &ctx = originalType->getASTContext();
   auto &entry = ctx.Impl.getArena(arena).ErrorTypesWithOriginal[originalType];
   if (entry) return entry;
 
   void *mem = ctx.Allocate(sizeof(ErrorType) + sizeof(Type),
                            alignof(ErrorType), arena);
-  return entry = new (mem) ErrorType(ctx, originalType);
+  RecursiveTypeProperties properties = RecursiveTypeProperties::HasError;
+  if (originalProperties.hasTypeVariable())
+    properties |= RecursiveTypeProperties::HasTypeVariable;
+  return entry = new (mem) ErrorType(ctx, originalType, properties);
 }
 
 BuiltinIntegerType *BuiltinIntegerType::get(BuiltinIntegerWidth BitWidth,

lib/AST/Type.cpp

@@ -384,7 +384,8 @@ TypeBase::getTypeVariables(SmallVectorImpl<TypeVariableType *> &typeVariables) {
 
       return false;
     });
-    assert(!typeVariables.empty() && "Did not find type variables!");
+    assert((!typeVariables.empty() || hasError()) &&
+           "Did not find type variables!");
   }
 }