Better enforce GenericFunctionType TypeVariableType invariant

Enforce that we don't have any type variables
present in either the result or parameter types.
To ensure the constraint system doesn't violate
this invariant, refactor `getTypeOfMemberReference`
slightly to avoid construction of a
`GenericFunctionType` as a means of opening the
generic parameters of the context for a VarDecl.

Author: hamishknight

lib/AST/ASTContext.cpp

@@ -4178,6 +4178,13 @@ GenericFunctionType *GenericFunctionType::get(GenericSignature sig,
                                               Type result,
                                               Optional<ExtInfo> info) {
   assert(sig && "no generic signature for generic function type?!");
+
+  // We do not allow type variables in GenericFunctionTypes. Note that if this
+  // ever changes, we'll need to setup arena-specific allocation for
+  // GenericFunctionTypes.
+  assert(llvm::none_of(params, [](Param param) {
+    return param.getPlainType()->hasTypeVariable();
+  }));
   assert(!result->hasTypeVariable());
 
   llvm::FoldingSetNodeID id;

lib/Sema/ConstraintSystem.cpp

@@ -2364,6 +2364,13 @@ ConstraintSystem::getTypeOfMemberReference(
     return { openedType, openedType, memberTy, memberTy };
   }
 
+  if (isa<AbstractFunctionDecl>(value) || isa<EnumElementDecl>(value)) {
+    if (value->getInterfaceType()->is<ErrorType>()) {
+      auto genericErrorTy = ErrorType::get(getASTContext());
+      return { genericErrorTy, genericErrorTy, genericErrorTy, genericErrorTy };
+    }
+  }
+
   // Figure out the declaration context to use when opening this type.
   DeclContext *innerDC = value->getInnermostDeclContext();
   DeclContext *outerDC = value->getDeclContext();
@@ -2372,18 +2379,22 @@ ConstraintSystem::getTypeOfMemberReference(
   Type openedType;
   OpenedTypeMap localReplacements;
   auto &replacements = replacementsPtr ? *replacementsPtr : localReplacements;
-  unsigned numRemovedArgumentLabels = getNumRemovedArgumentLabels(
-      value, /*isCurriedInstanceReference*/ !hasAppliedSelf, functionRefKind);
 
-  AnyFunctionType *funcType;
+  // If we have a generic signature, open the parameters. We delay opening
+  // requirements to allow contextual types to affect the situation.
+  auto genericSig = innerDC->getGenericSignatureOfContext();
+  if (genericSig)
+    openGenericParameters(outerDC, genericSig, replacements, locator);
 
   if (isa<AbstractFunctionDecl>(value) || isa<EnumElementDecl>(value)) {
-    if (auto ErrorTy = value->getInterfaceType()->getAs<ErrorType>()) {
-      auto genericErrorTy = ErrorType::get(ErrorTy->getASTContext());
-      return { genericErrorTy, genericErrorTy, genericErrorTy, genericErrorTy };
-    }
     // This is the easy case.
-    funcType = value->getInterfaceType()->castTo<AnyFunctionType>();
+    openedType = value->getInterfaceType()->castTo<AnyFunctionType>();
+
+    if (auto *genericFn = openedType->getAs<GenericFunctionType>()) {
+      openedType = genericFn->substGenericArgs([&](Type type) {
+        return openType(type, replacements);
+      });
+    }
   } else {
     // For a property, build a type (Self) -> PropType.
     // For a subscript, build a type (Self) -> (Indices...) -> ElementType.
@@ -2424,29 +2435,21 @@ ConstraintSystem::getTypeOfMemberReference(
         !selfTy->hasError())
       selfFlags = selfFlags.withInOut(true);
 
-    // If the storage is generic, add a generic signature.
+    // If the storage is generic, open the self and ref types.
+    if (genericSig) {
+      selfTy = openType(selfTy, replacements);
+      refType = openType(refType, replacements);
+    }
     FunctionType::Param selfParam(selfTy, Identifier(), selfFlags);
+
     // FIXME: Verify ExtInfo state is correct, not working by accident.
-    if (auto sig = innerDC->getGenericSignatureOfContext()) {
-      GenericFunctionType::ExtInfo info;
-      funcType = GenericFunctionType::get(sig, {selfParam}, refType, info);
-    } else {
-      FunctionType::ExtInfo info;
-      funcType = FunctionType::get({selfParam}, refType, info);
-    }
+    FunctionType::ExtInfo info;
+    openedType = FunctionType::get({selfParam}, refType, info);
   }
+  assert(!openedType->hasTypeParameter());
 
-  // While opening member function type, let's delay opening requirements
-  // to allow contextual types to affect the situation.
-  if (auto *genericFn = funcType->getAs<GenericFunctionType>()) {
-    openGenericParameters(outerDC, genericFn->getGenericSignature(),
-                          replacements, locator);
-
-    openedType = genericFn->substGenericArgs(
-        [&](Type type) { return openType(type, replacements); });
-  } else {
-    openedType = funcType;
-  }
+  unsigned numRemovedArgumentLabels = getNumRemovedArgumentLabels(
+      value, /*isCurriedInstanceReference*/ !hasAppliedSelf, functionRefKind);
 
   openedType = openedType->removeArgumentLabels(numRemovedArgumentLabels);
 
@@ -2497,9 +2500,9 @@ ConstraintSystem::getTypeOfMemberReference(
   // failing we'll get a generic requirement constraint failure
   // if mismatch is related to generic parameters which is much
   // easier to diagnose.
-  if (auto *genericFn = funcType->getAs<GenericFunctionType>()) {
+  if (genericSig) {
     openGenericRequirements(
-        outerDC, genericFn->getGenericSignature(),
+        outerDC, genericSig,
         /*skipProtocolSelfConstraint=*/true, locator,
         [&](Type type) { return openType(type, replacements); });
   }