mustachio: Separate out the context stack LUB type calculation (#3730)

Author: srawlins

lib/src/generator/templates.aot_renderers_for_html.dart

@@ -3,8 +3,7 @@
 // To change the contents of this library, make changes to the builder source
 // files in the tool/mustachio/ directory.
 
-// There are a few deduplicated render functions which are generated but not
-// used.
+// Some deduplicated render functions are generated but not used.
 // TODO(srawlins): Detect these and do not write them.
 // ignore_for_file: unused_element
 // Sometimes we enter a new section which triggers creating a new variable, but

test/mustachio/foo.aot_renderers_for_html.dart

@@ -3,8 +3,7 @@
 // To change the contents of this library, make changes to the builder source
 // files in the tool/mustachio/ directory.
 
-// There are a few deduplicated render functions which are generated but not
-// used.
+// Some deduplicated render functions are generated but not used.
 // TODO(srawlins): Detect these and do not write them.
 // ignore_for_file: unused_element
 // Sometimes we enter a new section which triggers creating a new variable, but

tool/mustachio/codegen_aot_compiler.dart

@@ -17,6 +17,10 @@ import 'package:dartdoc/src/type_utils.dart';
 import 'package:meta/meta.dart';
 import 'package:path/path.dart' as path;
 
+/// A list of [_VariableLookup]s used by a renderer constitutes a "context
+/// stack".
+typedef ContextStack = List<_VariableLookup>;
+
 /// Compiles all templates specified in [specs] into a Dart library containing
 /// a renderer for each template.
 Future<String> compileTemplatesToRenderers(
@@ -62,8 +66,7 @@ Future<String> compileTemplatesToRenderers(
 // To change the contents of this library, make changes to the builder source
 // files in the tool/mustachio/ directory.
 
-// There are a few deduplicated render functions which are generated but not
-// used.
+// Some deduplicated render functions are generated but not used.
 // TODO(srawlins): Detect these and do not write them.
 // ignore_for_file: unused_element
 // Sometimes we enter a new section which triggers creating a new variable, but
@@ -116,27 +119,23 @@ Future<Map<_AotCompiler, String>> _deduplicateRenderers(
       continue;
     }
     var firstCompiler = compilers.first;
-    var contextStacksLength = firstCompiler._usedContexts.length;
-    if (compilers.any((c) => c._usedContexts.length != contextStacksLength)) {
-      // The stack lengths are different, it is impossible to deduplicate such
-      // partial renderers.
+    var contextStacks = compilers.map((c) => c._usedContextStack).toList();
+    var contextStackTypes = typeSystem.contextStackLub(contextStacks);
+    if (contextStackTypes == null) {
+      // The stack lengths are different; it is impossible to fully deduplicate
+      // such partial renderers.
       continue;
     }
-    // The new list of context types, each of which is the LUB of the associated
-    // context type of each of the compilers.
-    var contextStackTypes = <InterfaceType>[];
-    for (var i = 0; i < contextStacksLength; i++) {
-      var types = compilers.map((c) => c._usedContextStack[i].type);
-      var lubType = types.fold<DartType>(types.first,
-              (value, type) => typeSystem.leastUpperBound(value, type))
-          as InterfaceType;
-      contextStackTypes.add(lubType);
-    }
 
     // Each of the render functions generated by a compiler for this asset can
     // be replaced by a more generic renderer which accepts the LUB types. The
-    // body of each replaced renderer can perform a simple redirect.
+    // body of each replaced renderer can perform a simple redirect to the more
+    // generic renderer.
     var rendererName = filePath.replaceAll('.', '_').replaceAll('/', '_');
+
+    // The names of the renderers which are being replaced all include some
+    // reference to the template/partial which referred to them; we must create
+    // a new renderer name from scratch.
     var lubCompiler = _AotCompiler._(
       contextStackTypes.first,
       '_deduplicated_$rendererName',
@@ -250,12 +249,12 @@ class _AotCompiler {
   final Map<_AotCompiler, String> _compiledPartials = {};
 
   /// The current stack of context objects (as variable lookups).
-  final List<_VariableLookup> _contextStack;
+  final ContextStack _contextStack;
 
   /// The set of context objects which are ultimately used by this compiler.
   final Set<_VariableLookup> _usedContexts = {};
 
-  List<_VariableLookup> get _usedContextStack =>
+  ContextStack get _usedContextStack =>
       [..._contextStack.where(_usedContexts.contains)];
 
   /// A counter for naming partial render functions.
@@ -276,7 +275,7 @@ class _AotCompiler {
     String rendererName,
     String templatePath,
     _BuildData buildData, {
-    List<_VariableLookup> contextStack = const [],
+    ContextStack contextStack = const [],
   }) async {
     var template =
         await File(path.join(buildData._root, templatePath)).readAsString();
@@ -292,7 +291,7 @@ class _AotCompiler {
     this._templatePath,
     this._syntaxTree,
     this._buildData, {
-    required List<_VariableLookup> contextStack,
+    required ContextStack contextStack,
   })  : _contextStack = _rename(contextStack),
         _contextNameCounter = contextStack.length;
 
@@ -301,7 +300,7 @@ class _AotCompiler {
   ///
   /// This ensures that each renderer accepts a simple list of context objects
   /// with predictable names.
-  static List<_VariableLookup> _rename(List<_VariableLookup> original) {
+  static ContextStack _rename(ContextStack original) {
     var result = <_VariableLookup>[];
     var index = original.length - 1;
     for (var variable in original) {
@@ -391,7 +390,7 @@ class _AotCompiler {
 class _BlockCompiler {
   final _AotCompiler _templateCompiler;
 
-  final List<_VariableLookup> _contextStack;
+  final ContextStack _contextStack;
 
   final Set<_VariableLookup> _usedContextTypes = {};
 
@@ -756,3 +755,27 @@ extension on StringBuffer {
     return referencedElements;
   }
 }
+
+extension on TypeSystem {
+  /// Returns a list of types which represents the LUB of each of the types in
+  /// the corresponding positions in [contextStacks].
+  List<InterfaceType>? contextStackLub(List<ContextStack> contextStacks) {
+    // The length of each and every context stack.
+    var contextStacksLength = contextStacks.first.length;
+    if (contextStacks.any((s) => s.length != contextStacksLength)) {
+      return null;
+    }
+
+    // The new list of context types, each of which is the LUB of the associated
+    // context type of each of the compilers.
+    var contextStackTypes = <InterfaceType>[];
+    for (var i = 0; i < contextStacksLength; i++) {
+      var types = contextStacks.map((s) => s[i].type);
+      var lubType =
+          types.fold<DartType>(types.first, leastUpperBound) as InterfaceType;
+      contextStackTypes.add(lubType);
+    }
+
+    return contextStackTypes;
+  }
+}