Revert "[cfe] Check type variable dependency cycles via extension types"

This reverts commit 9bb61c0.

Reason for revert: NPE detected in google3.

Original change's description:
> [cfe] Check type variable dependency cycles via extension types
>
> Closes #54097
> Closes #54164
> Part of #49731
>
> Change-Id: I73aac5f7e2c7f05fd0872b37e6f39fa7b5ed4862
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/337183
> Commit-Queue: Chloe Stefantsova <[email protected]>
> Reviewed-by: Johnni Winther <[email protected]>

Change-Id: If1e102112546e333f2e7058e432af142ce7da56e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/341922
Bot-Commit: Rubber Stamper <[email protected]>
Reviewed-by: Johnni Winther <[email protected]>
Commit-Queue: Johnni Winther <[email protected]>
Auto-Submit: Chloe Stefantsova <[email protected]>

Author: chloestefantsova

pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart

@@ -2214,7 +2214,7 @@ const Template<Message Function(String name, String string)>
         problemMessageTemplate:
             r"""Type '#name' is a bound of itself via '#string'.""",
         correctionMessageTemplate:
-            r"""Try breaking the cycle by removing at least one of the 'extends' clauses in the cycle.""",
+            r"""Try breaking the cycle by removing at least on of the 'extends' clauses in the cycle.""",
         withArguments: _withArgumentsCycleInTypeVariables);
 
 // DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
@@ -2233,7 +2233,7 @@ Message _withArgumentsCycleInTypeVariables(String name, String string) {
       problemMessage:
           """Type '${name}' is a bound of itself via '${string}'.""",
       correctionMessage:
-          """Try breaking the cycle by removing at least one of the 'extends' clauses in the cycle.""",
+          """Try breaking the cycle by removing at least on of the 'extends' clauses in the cycle.""",
       arguments: {'name': name, 'string': string});
 }
 
@@ -2555,7 +2555,7 @@ const Template<
         Message Function(String name)>("DirectCycleInTypeVariables",
     problemMessageTemplate: r"""Type '#name' can't use itself as a bound.""",
     correctionMessageTemplate:
-        r"""Try breaking the cycle by removing at least one of the 'extends' clauses in the cycle.""",
+        r"""Try breaking the cycle by removing at least on of the 'extends' clauses in the cycle.""",
     withArguments: _withArgumentsDirectCycleInTypeVariables);
 
 // DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
@@ -2570,7 +2570,7 @@ Message _withArgumentsDirectCycleInTypeVariables(String name) {
   return new Message(codeDirectCycleInTypeVariables,
       problemMessage: """Type '${name}' can't use itself as a bound.""",
       correctionMessage:
-          """Try breaking the cycle by removing at least one of the 'extends' clauses in the cycle.""",
+          """Try breaking the cycle by removing at least on of the 'extends' clauses in the cycle.""",
       arguments: {'name': name});
 }
 

pkg/front_end/lib/src/fasta/builder/type_variable_builder.dart

@@ -83,98 +83,6 @@ sealed class TypeVariableBuilderBase extends TypeDeclarationBuilderImpl
 
   void finish(SourceLibraryBuilder library, ClassBuilder object,
       TypeBuilder dynamicType);
-
-  TypeBuilder? _unaliasAndErase(TypeBuilder? typeBuilder) {
-    if (typeBuilder is! NamedTypeBuilder) {
-      return typeBuilder;
-    } else {
-      TypeDeclarationBuilder? declaration = typeBuilder.declaration;
-      if (declaration is TypeAliasBuilder) {
-        // We pass empty lists as [unboundTypes] and [unboundTypeVariables]
-        // because new builders can be generated during unaliasing. We ignore
-        // the returned builders, however, because they will not be used in the
-        // output and are needed only for the checks.
-        //
-        // We also don't instantiate-to-bound raw types because it won't affect
-        // the dependency cycle analysis.
-        return _unaliasAndErase(declaration.unalias(typeBuilder.typeArguments,
-            unboundTypes: [], unboundTypeVariables: []));
-      } else if (declaration is ExtensionTypeDeclarationBuilder) {
-        TypeBuilder? representationType =
-            declaration.declaredRepresentationTypeBuilder;
-        if (representationType == null) {
-          return null;
-        } else {
-          List<NominalVariableBuilder>? typeParameters =
-              declaration.typeParameters;
-          List<TypeBuilder>? typeArguments = typeBuilder.typeArguments;
-          if (typeParameters != null && typeArguments != null) {
-            representationType = representationType.subst(
-                new Map<NominalVariableBuilder, TypeBuilder>.fromIterables(
-                    typeParameters, typeArguments));
-          }
-          return _unaliasAndErase(representationType);
-        }
-      } else {
-        return typeBuilder;
-      }
-    }
-  }
-
-  TypeVariableCyclicDependency? findCyclicDependency(
-      {Map<TypeVariableBuilderBase, TypeVariableTraversalState>?
-          typeVariablesTraversalState,
-      Map<TypeVariableBuilderBase, TypeVariableBuilderBase>? cycleElements}) {
-    typeVariablesTraversalState ??= {};
-    cycleElements ??= {};
-
-    switch (typeVariablesTraversalState[this] ??=
-        TypeVariableTraversalState.unvisited) {
-      case TypeVariableTraversalState.visited:
-        return null;
-      case TypeVariableTraversalState.active:
-        typeVariablesTraversalState[this] = TypeVariableTraversalState.visited;
-        List<TypeVariableBuilderBase>? viaTypeVariables;
-        TypeVariableBuilderBase? nextViaTypeVariable = cycleElements[this];
-        while (nextViaTypeVariable != null && nextViaTypeVariable != this) {
-          (viaTypeVariables ??= []).add(nextViaTypeVariable);
-          nextViaTypeVariable = cycleElements[nextViaTypeVariable];
-        }
-        return new TypeVariableCyclicDependency(this,
-            viaTypeVariables: viaTypeVariables);
-      case TypeVariableTraversalState.unvisited:
-        typeVariablesTraversalState[this] = TypeVariableTraversalState.active;
-        TypeBuilder? bound = this.bound;
-        if (bound is NamedTypeBuilder) {
-          TypeBuilder? unaliasedAndErasedBound = _unaliasAndErase(bound);
-          TypeDeclarationBuilder? unaliasedAndErasedBoundDeclaration =
-              unaliasedAndErasedBound?.declaration;
-          TypeVariableBuilderBase? nextVariable;
-          if (unaliasedAndErasedBoundDeclaration is TypeVariableBuilderBase) {
-            nextVariable = unaliasedAndErasedBoundDeclaration;
-          }
-
-          if (nextVariable != null) {
-            cycleElements[this] = nextVariable;
-            TypeVariableCyclicDependency? result =
-                nextVariable.findCyclicDependency(
-                    typeVariablesTraversalState: typeVariablesTraversalState,
-                    cycleElements: cycleElements);
-            typeVariablesTraversalState[this] =
-                TypeVariableTraversalState.visited;
-            return result;
-          } else {
-            typeVariablesTraversalState[this] =
-                TypeVariableTraversalState.visited;
-            return null;
-          }
-        } else {
-          typeVariablesTraversalState[this] =
-              TypeVariableTraversalState.visited;
-          return null;
-        }
-    }
-  }
 }
 
 class NominalVariableBuilder extends TypeVariableBuilderBase {
@@ -402,17 +310,21 @@ class NominalVariableBuilder extends TypeVariableBuilderBase {
   }
 }
 
-List<TypeVariableBuilderBase> sortAllTypeVariablesTopologically(
-    Iterable<TypeVariableBuilderBase> typeVariables) {
+List< /* TypeVariableBuilder | FunctionTypeTypeVariableBuilder */ Object>
+    sortAllTypeVariablesTopologically(
+        Iterable< /* TypeVariableBuilder | FunctionTypeTypeVariableBuilder */
+                Object>
+            typeVariables) {
   assert(typeVariables.every((typeVariable) =>
       typeVariable is NominalVariableBuilder ||
       typeVariable is StructuralVariableBuilder));
 
-  Set<TypeVariableBuilderBase> unhandled =
-      new Set<TypeVariableBuilderBase>.identity()..addAll(typeVariables);
-  List<TypeVariableBuilderBase> result = <TypeVariableBuilderBase>[];
+  Set< /* TypeVariableBuilder | FunctionTypeTypeVariableBuilder */ Object>
+      unhandled = new Set<Object>.identity()..addAll(typeVariables);
+  List< /* TypeVariableBuilder | FunctionTypeTypeVariableBuilder */ Object>
+      result = <Object>[];
   while (unhandled.isNotEmpty) {
-    TypeVariableBuilderBase rootVariable = unhandled.first;
+    Object rootVariable = unhandled.first;
     unhandled.remove(rootVariable);
 
     TypeBuilder? rootVariableBound;
@@ -772,47 +684,3 @@ class FreshStructuralVariableBuildersFromNominalVariableBuilders {
   FreshStructuralVariableBuildersFromNominalVariableBuilders(
       this.freshStructuralVariableBuilders, this.substitutionMap);
 }
-
-/// This enum is used internally for dependency analysis of type variables.
-enum TypeVariableTraversalState {
-  /// An [unvisited] type variable isn't yet visited by the traversal algorithm.
-  unvisited,
-
-  /// An [active] type variable is traversed, but not fully processed.
-  active,
-
-  /// A [visited] type variable is fully processed.
-  visited;
-}
-
-/// Represents a cyclic dependency of a type variable on itself.
-///
-/// An examples of such dependencies are X  in the following cases.
-///
-///   typedef F<Y> = Y;
-///   extension type E<Y>(Y it) {}
-///
-///   class A<X extends X> {} // Error.
-///   class B<X extends Y, Y extends X> {} // Error.
-///   class C<X extends F<Y>, Y extends X> {} // Error.
-///   class D<X extends E<Y>, Y extends X> {} // Error.
-class TypeVariableCyclicDependency {
-  /// Type variable that's the bound of itself.
-  final TypeVariableBuilderBase typeVariableBoundOfItself;
-
-  /// The elements in a non-trivial self-dependency cycle.
-  ///
-  /// The loop is considered non-trivial if it includes more than one type
-  /// variable.
-  final List<TypeVariableBuilderBase>? viaTypeVariables;
-
-  TypeVariableCyclicDependency(this.typeVariableBoundOfItself,
-      {this.viaTypeVariables});
-
-  @override
-  String toString() {
-    return "TypeVariableCyclicDependency("
-        "typeVariableBoundOfItself=${typeVariableBoundOfItself}, "
-        "viaTypeVariable=${viaTypeVariables})";
-  }
-}

pkg/front_end/lib/src/fasta/kernel/body_builder.dart

@@ -8786,7 +8786,6 @@ class BodyBuilder extends StackListenerImpl
     // Peek to leave type parameters on top of stack.
     List<TypeVariableBuilderBase> typeVariables =
         peek() as List<TypeVariableBuilderBase>;
-    libraryBuilder.checkTypeVariableDependencies(typeVariables);
 
     List<TypeBuilder> unboundTypes = [];
     List<StructuralVariableBuilder> unboundTypeVariables = [];

pkg/front_end/lib/src/fasta/source/outline_builder.dart

@@ -3643,6 +3643,70 @@ class OutlineBuilder extends StackListenerImpl {
           libraryFeatures.enhancedEnums, beginToken.charOffset, noLength);
     }
 
+    // Peek to leave type parameters on top of stack.
+    List<TypeVariableBuilderBase>? typeParameters =
+        peek() as List<TypeVariableBuilderBase>?;
+
+    Map<String, TypeVariableBuilderBase>? typeVariablesByName;
+    if (typeParameters != null) {
+      for (TypeVariableBuilderBase builder in typeParameters) {
+        if (builder.bound != null) {
+          typeVariablesByName ??= <String, TypeVariableBuilderBase>{
+            for (TypeVariableBuilderBase builder in typeParameters)
+              builder.name: builder
+          };
+
+          // Find cycle: If there's no cycle we can at most step through all
+          // `typeParameters` (at which point the last builders bound will be
+          // null).
+          // If there is a cycle with `builder` 'inside' the steps to get back
+          // to it will also be bound by `typeParameters.length`.
+          // If there is a cycle without `builder` 'inside' we will just ignore
+          // it for now. It will be reported when processing one of the
+          // `builder`s that is in fact `inside` the cycle. This matches the
+          // cyclic class hierarchy error.
+          TypeVariableBuilderBase? bound = builder;
+          for (int steps = 0;
+              bound!.bound != null && steps < typeParameters.length;
+              ++steps) {
+            TypeName? typeName = bound.bound!.typeName;
+            bound = typeVariablesByName[typeName?.name];
+            if (bound == null || bound == builder) break;
+          }
+          if (bound == builder && bound!.bound != null) {
+            // Write out cycle.
+            List<String> via = <String>[];
+            TypeName? typeName = bound.bound!.typeName;
+            bound = typeVariablesByName[typeName?.name];
+            while (bound != builder) {
+              via.add(bound!.name);
+              TypeName? typeName = bound.bound!.typeName;
+              bound = typeVariablesByName[typeName?.name];
+            }
+            Message message = via.isEmpty
+                ? templateDirectCycleInTypeVariables.withArguments(builder.name)
+                : templateCycleInTypeVariables.withArguments(
+                    builder.name, via.join("', '"));
+            addProblem(message, builder.charOffset, builder.name.length);
+            builder.bound = new NamedTypeBuilderImpl(
+                new SyntheticTypeName(builder.name, builder.charOffset),
+                const NullabilityBuilder.omitted(),
+                fileUri: uri,
+                charOffset: builder.charOffset,
+                instanceTypeVariableAccess:
+                    //InstanceTypeVariableAccessState.Unexpected
+                    declarationContext.instanceTypeVariableAccessState)
+              ..bind(
+                  libraryBuilder,
+                  new InvalidTypeDeclarationBuilder(
+                      builder.name,
+                      message.withLocation(
+                          uri, builder.charOffset, builder.name.length)));
+          }
+        }
+      }
+    }
+
     if (inConstructorName) {
       addProblem(messageConstructorWithTypeParameters,
           offsetForToken(beginToken), lengthOfSpan(beginToken, endToken));