Merge pull request #82115 from slavapestov/get-superclass-for-decl

Fix RequirementMachine crash with invalid AST via getSuperclassForDecl()

Author: slavapestov

lib/AST/ConformanceLookupTable.cpp

@@ -862,8 +862,6 @@ DeclContext *ConformanceLookupTable::getConformingContext(
       Type classTy = nominal->getDeclaredInterfaceType();
       do {
         Type superclassTy = classTy->getSuperclassForDecl(superclassDecl);
-        if (superclassTy->is<ErrorType>())
-          return nullptr;
         auto inheritedConformance = swift::lookupConformance(
             superclassTy, protocol, /*allowMissing=*/false);
         if (inheritedConformance)
@@ -936,8 +934,6 @@ ConformanceLookupTable::getConformance(NominalTypeDecl *nominal,
     // declared.
     auto *conformingClass = cast<ClassDecl>(conformingNominal);
     Type superclassTy = type->getSuperclassForDecl(conformingClass);
-    if (superclassTy->is<ErrorType>())
-      return nullptr;
 
     // Look up the inherited conformance.
     auto inheritedConformance = swift::lookupConformance(

lib/AST/RequirementMachine/Symbol.cpp

@@ -366,6 +366,8 @@ Symbol Symbol::forLayout(LayoutConstraint layout,
 /// Creates a superclass symbol, representing a superclass constraint.
 Symbol Symbol::forSuperclass(CanType type, ArrayRef<Term> substitutions,
                              RewriteContext &ctx) {
+  ASSERT(type.getClassOrBoundGenericClass() != nullptr);
+
   llvm::FoldingSetNodeID id;
   id.AddInteger(unsigned(Kind::Superclass));
   id.AddPointer(type.getPointer());

lib/AST/TypeSubstitution.cpp

@@ -523,14 +523,20 @@ Type TypeBase::getSuperclassForDecl(const ClassDecl *baseClass,
     t = t->getSuperclass(useArchetypes);
   }
 
-#ifndef NDEBUG
-  auto *currentClass = getConcreteTypeForSuperclassTraversing(this)
-      ->getClassOrBoundGenericClass();
-  assert(baseClass->isSuperclassOf(currentClass) &&
-         "no inheritance relationship between given classes");
-#endif
-
-  return ErrorType::get(this);
+  if (CONDITIONAL_ASSERT_enabled()) {
+    auto *currentClass = getConcreteTypeForSuperclassTraversing(this)
+        ->getClassOrBoundGenericClass();
+    ASSERT(baseClass->isSuperclassOf(currentClass) &&
+           "no inheritance relationship between given classes");
+  }
+
+  // We can end up here if the AST is invalid, because then
+  // getSuperclassDecl() might resolve to a decl, and yet
+  // getSuperclass() is just an ErrorType. Make sure we still
+  // return a nominal type as the result though, and not an
+  // ErrorType, because that's what callers expect.
+  return baseClass->getDeclaredInterfaceType()
+      .subst(SubstitutionMap())->getCanonicalType();
 }
 
 SubstitutionMap TypeBase::getContextSubstitutionMap() {
@@ -546,7 +552,7 @@ SubstitutionMap TypeBase::getContextSubstitutionMap() {
 
   Type baseTy(this);
 
-  assert(!baseTy->hasLValueType() &&
+  assert(!baseTy->is<LValueType>() &&
          !baseTy->is<AnyMetatypeType>() &&
          !baseTy->is<ErrorType>());
 
@@ -628,7 +634,7 @@ TypeBase::getContextSubstitutions(const DeclContext *dc,
   assert(dc->isTypeContext());
   Type baseTy(this);
 
-  assert(!baseTy->hasLValueType() &&
+  assert(!baseTy->is<LValueType>() &&
          !baseTy->is<AnyMetatypeType>() &&
          !baseTy->is<ErrorType>());
 

lib/Sema/TypeCheckStorage.cpp

@@ -1146,12 +1146,7 @@ static Expr *buildStorageReference(AccessorDecl *accessor,
       auto *baseClass = override->getDeclContext()->getSelfClassDecl();
       selfTypeForAccess = selfTypeForAccess->getSuperclassForDecl(baseClass);
 
-      // Error recovery path. We get an ErrorType here if getSuperclassForDecl()
-      // fails (because, for instance, a generic parameter of a generic nominal
-      // type cannot be resolved).
-      if (!selfTypeForAccess->is<ErrorType>()) {
-        subs = selfTypeForAccess->getContextSubstitutionMap(baseClass);
-      }
+      subs = selfTypeForAccess->getContextSubstitutionMap(baseClass);
 
       storage = override;
 

test/Generics/unify_superclass_types_invalid.swift

@@ -0,0 +1,10 @@
+// RUN: %target-typecheck-verify-swift
+// RUN: not %target-swift-frontend -typecheck -debug-generic-signatures %s 2>&1 | %FileCheck %s
+
+class Base<T> {}
+class Derived : Base<Foo> {}
+// expected-error@-1 {{cannot find type 'Foo' in scope}}
+
+// CHECK-LABEL: unify_superclass_types_invalid.(file).f@
+// CHECK: Generic signature: <T where T : Derived>
+func f<T>(_: T) where T: Base<Int>, T: Derived {}

validation-test/compiler_crashers_2/2819a60354d8389.swift renamed to validation-test/compiler_crashers_2_fixed/2819a60354d8389.swift

@@ -1,3 +1,3 @@
 // {"signature":"swift::rewriting::PropertyMap::addSuperclassProperty(swift::rewriting::Term, swift::rewriting::Symbol, unsigned int)"}
-// RUN: not --crash %target-swift-frontend -typecheck %s
+// RUN: not %target-swift-frontend -typecheck %s
 class a < b class c : a func d < b where b : a<Int>, b : c

validation-test/compiler_crashers_2/9b91d843ce95d4f.swift renamed to validation-test/compiler_crashers_2_fixed/9b91d843ce95d4f.swift

@@ -1,5 +1,5 @@
 // {"signature":"swift::ide::printTypeUSR(swift::Type, llvm::raw_ostream&)"}
-// RUN: not --crash %target-swift-frontend -typecheck %s
+// RUN: not %target-swift-frontend -typecheck %s
 class a {
   class b < c class e : a<> {
     d = b

validation-test/compiler_crashers_2/faa012128bc0b7c7.swift renamed to validation-test/compiler_crashers_2_fixed/faa012128bc0b7c7.swift

@@ -1,3 +1,3 @@
 // {"signature":"swift::TypeBase::getContextSubstitutions(swift::DeclContext const*, swift::GenericEnvironment*)"}
-// RUN: not --crash %target-swift-frontend -typecheck %s
+// RUN: not %target-swift-frontend -typecheck %s
 {                      $0?={