[modular_test] Delete support for macro execution

Mostly a revert of https://dart-review.googlesource.com/c/sdk/+/361261.
A straight revert contained merge conflicts that I resolved manually but
I left some of the cleanup or refactoring in place.

Change-Id: I2add9f4951d7c6c2399d8c95f36b1483279eea44
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/409401
Commit-Queue: Nicholas Shahan <[email protected]>
Reviewed-by: Johnni Winther <[email protected]>
Reviewed-by: Jake Macdonald <[email protected]>

Author: nshahan

pkg/compiler/tool/modular_test_suite.dart

@@ -11,7 +11,6 @@ import 'dart:async';
 
 import 'package:modular_test/src/io_pipeline.dart';
 import 'package:modular_test/src/runner.dart';
-import 'package:modular_test/src/steps/macro_precompile_aot.dart';
 import 'modular_test_suite_helper.dart';
 
 Future<void> main(List<String> args) async {
@@ -23,7 +22,6 @@ Future<void> main(List<String> args) async {
       'tests/modular',
       options,
       IOPipeline([
-        PrecompileMacroAotStep(verbose: options.verbose),
         OutlineDillCompilationStep(),
         FullDillCompilationStep(),
         ConcatenateDillsStep(),

pkg/compiler/tool/modular_test_suite_helper.dart

@@ -20,7 +20,6 @@ import 'package:modular_test/src/io_pipeline.dart';
 import 'package:modular_test/src/pipeline.dart';
 import 'package:modular_test/src/runner.dart';
 import 'package:modular_test/src/suite.dart';
-import 'package:modular_test/src/steps/macro_precompile_aot.dart';
 import 'package:modular_test/src/steps/util.dart';
 import 'package:path/path.dart' as p;
 
@@ -157,11 +156,6 @@ abstract class CFEStep extends IOModularStep {
       ...(transitiveDependencies.expand(
         (m) => ['--input-summary', '${toUri(m, inputData)}'],
       )),
-      ...transitiveDependencies
-          .where((m) => m.macroConstructors.isNotEmpty)
-          .expand(
-            (m) => ['--precompiled-macro', '${precompiledMacroArg(m, toUri)};'],
-          ),
       ...(sources.expand((String uri) => ['--source', uri])),
       ...(flags.expand((String flag) => ['--enable-experiment', flag])),
     ];
@@ -196,10 +190,7 @@ class OutlineDillCompilationStep extends CFEStep {
   bool get needsSources => true;
 
   @override
-  List<DataId> get dependencyDataNeeded => const [
-    dillSummaryId,
-    precompiledMacroId,
-  ];
+  List<DataId> get dependencyDataNeeded => const [dillSummaryId];
 
   @override
   List<DataId> get moduleDataNeeded => const [];
@@ -229,10 +220,7 @@ class FullDillCompilationStep extends CFEStep {
   bool get needsSources => true;
 
   @override
-  List<DataId> get dependencyDataNeeded => const [
-    dillSummaryId,
-    precompiledMacroId,
-  ];
+  List<DataId> get dependencyDataNeeded => const [dillSummaryId];
 
   @override
   List<DataId> get moduleDataNeeded => const [];

pkg/dev_compiler/lib/src/command/command.dart

@@ -335,8 +335,7 @@ Future<CompilerResult> _compile(List<String> args,
         environmentDefines: declaredVariables,
         trackNeededDillLibraries: recordUsedInputs,
         nnbdMode:
-            options.soundNullSafety ? fe.NnbdMode.Strong : fe.NnbdMode.Weak,
-        precompiledMacros: options.precompiledMacros);
+            options.soundNullSafety ? fe.NnbdMode.Strong : fe.NnbdMode.Weak);
     var incrementalCompiler = compilerState.incrementalCompiler!;
     var cachedSdkInput = compileSdk
         ? null

pkg/dev_compiler/test/modular_helpers.dart

@@ -9,7 +9,6 @@ import 'package:modular_test/src/create_package_config.dart';
 import 'package:modular_test/src/io_pipeline.dart';
 import 'package:modular_test/src/pipeline.dart';
 import 'package:modular_test/src/runner.dart';
-import 'package:modular_test/src/steps/macro_precompile_aot.dart';
 import 'package:modular_test/src/steps/util.dart';
 import 'package:modular_test/src/suite.dart';
 import 'package:path/path.dart' as p;
@@ -34,7 +33,7 @@ class SourceToSummaryDillStep implements IOModularStep {
   bool get needsSources => true;
 
   @override
-  List<DataId> get dependencyDataNeeded => const [dillId, precompiledMacroId];
+  List<DataId> get dependencyDataNeeded => const [dillId];
 
   @override
   List<DataId> get moduleDataNeeded => const [];
@@ -108,10 +107,6 @@ class SourceToSummaryDillStep implements IOModularStep {
       ...transitiveDependencies
           .where((m) => !m.isSdk)
           .expand((m) => ['--input-summary', '${toUri(m, dillId)}']),
-      ...transitiveDependencies
-          .where((m) => m.macroConstructors.isNotEmpty)
-          .expand((m) =>
-              ['--precompiled-macro', '${precompiledMacroArg(m, toUri)};']),
       ...sources.expand((String uri) => ['--source', uri]),
       ...flags.expand((String flag) => ['--enable-experiment', flag]),
     ];
@@ -125,9 +120,6 @@ class SourceToSummaryDillStep implements IOModularStep {
   void notifyCached(Module module) {
     if (_options.verbose) print('\ncached step: source-to-dill on $module');
   }
-
-  @override
-  bool shouldExecute(Module module) => true;
 }
 
 class DDCStep implements IOModularStep {
@@ -142,7 +134,7 @@ class DDCStep implements IOModularStep {
   bool get needsSources => true;
 
   @override
-  List<DataId> get dependencyDataNeeded => const [dillId, precompiledMacroId];
+  List<DataId> get dependencyDataNeeded => const [dillId];
 
   @override
   List<DataId> get moduleDataNeeded => const [dillId];
@@ -206,10 +198,6 @@ class DDCStep implements IOModularStep {
       ...transitiveDependencies
           .where((m) => !m.isSdk)
           .expand((m) => ['-s', '${toUri(m, dillId)}=${m.name}']),
-      ...transitiveDependencies
-          .where((m) => m.macroConstructors.isNotEmpty)
-          .expand((m) =>
-              ['--precompiled-macro', '${precompiledMacroArg(m, toUri)};']),
       '-o',
       '$output',
     ];
@@ -222,9 +210,6 @@ class DDCStep implements IOModularStep {
   void notifyCached(Module module) {
     if (_options.verbose) print('\ncached step: ddc on $module');
   }
-
-  @override
-  bool shouldExecute(Module module) => true;
 }
 
 class RunD8 implements IOModularStep {
@@ -287,9 +272,6 @@ class RunD8 implements IOModularStep {
   void notifyCached(Module module) {
     if (_options.verbose) print('\ncached step: d8 on $module');
   }
-
-  @override
-  bool shouldExecute(Module module) => true;
 }
 
 String get _d8executable {

pkg/dev_compiler/test/modular_suite.dart

@@ -9,7 +9,6 @@ library;
 
 import 'package:modular_test/src/io_pipeline.dart';
 import 'package:modular_test/src/runner.dart';
-import 'package:modular_test/src/steps/macro_precompile_aot.dart';
 
 import 'modular_helpers.dart';
 
@@ -21,7 +20,6 @@ void main(List<String> args) async {
       'tests/modular',
       options,
       IOPipeline([
-        PrecompileMacroAotStep(verbose: options.verbose),
         SourceToSummaryDillStep(),
         DDCStep(canaryFeatures: false),
         RunD8(),

pkg/dev_compiler/test/modular_suite_canary.dart

@@ -9,7 +9,6 @@ library;
 
 import 'package:modular_test/src/io_pipeline.dart';
 import 'package:modular_test/src/runner.dart';
-import 'package:modular_test/src/steps/macro_precompile_aot.dart';
 
 import 'modular_helpers.dart';
 
@@ -21,7 +20,6 @@ void main(List<String> args) async {
       'tests/modular',
       options,
       IOPipeline([
-        PrecompileMacroAotStep(verbose: options.verbose),
         SourceToSummaryDillStep(),
         DDCStep(canaryFeatures: true),
         RunD8(),

pkg/front_end/lib/src/api_unstable/ddc.dart

@@ -161,8 +161,6 @@ Future<InitializedCompilerState> initializeIncrementalCompiler(
     required Map<ExperimentalFlag, bool> explicitExperimentalFlags,
     required Map<String, String> environmentDefines,
     bool trackNeededDillLibraries = false,
-    bool requirePrebuiltMacros = false,
-    List<String> precompiledMacros = const [],
     required NnbdMode nnbdMode}) {
   return modular.initializeIncrementalCompiler(
       oldState,

pkg/modular_test/lib/src/io_pipeline.dart

@@ -26,10 +26,6 @@ abstract class IOModularStep extends ModularStep {
   /// should be stored under `root.resolveUri(toUri(module, resultKind))`.
   Future<void> execute(Module module, Uri root, ModuleDataToRelativeUri toUri,
       List<String> flags);
-
-  /// Whether this step should apply to [module]. Most steps apply to all
-  /// modules, but not all.
-  bool shouldExecute(Module module) => true;
 }
 
 class IOPipeline extends Pipeline<IOModularStep> {
@@ -104,9 +100,6 @@ class IOPipeline extends Pipeline<IOModularStep> {
   @override
   Future<void> runStep(IOModularStep step, Module module,
       Map<Module, Set<DataId>> visibleData, List<String> flags) async {
-    // Skip it if we aren't expected to run.
-    if (!step.shouldExecute(module)) return;
-
     final resultsFolderUri = _resultsFolderUri!;
     if (cacheSharedModules && module.isShared) {
       // If all expected outputs are already available, skip the step.
@@ -134,13 +127,8 @@ class IOPipeline extends Pipeline<IOModularStep> {
       for (var dataId in visibleData[module]!) {
         var assetUri = resultsFolderUri
             .resolve(_toFileName(module, dataId, configSpecific: true));
-        var originalFile = File.fromUri(assetUri);
-        // Some steps don't actually have an output, if they implement
-        // shouldExecute.
-        if (!(await originalFile.exists())) continue;
-        var newPath =
-            stepFolder.uri.resolve(_toFileName(module, dataId)).toFilePath();
-        await originalFile.copy(newPath);
+        await File.fromUri(assetUri).copy(
+            stepFolder.uri.resolve(_toFileName(module, dataId)).toFilePath());
       }
     }
     if (step.needsSources) {
@@ -163,10 +151,9 @@ class IOPipeline extends Pipeline<IOModularStep> {
             "Step '${step.runtimeType}' on module '${module.name}' didn't "
             "produce an output file");
       }
-      var newPath = resultsFolderUri
+      await outputFile.copy(resultsFolderUri
           .resolve(_toFileName(module, dataId, configSpecific: true))
-          .toFilePath();
-      await outputFile.copy(newPath);
+          .toFilePath());
     }
     await stepFolder.delete(recursive: true);
   }

pkg/modular_test/lib/src/loader.dart

@@ -60,7 +60,7 @@ Future<ModularTest> loadTest(Uri uri) async {
         }
         var relativeUri = Uri.parse(fileName);
         var isMain = moduleName == 'main';
-        var module = Module(moduleName, [], testUri, [relativeUri], {},
+        var module = Module(moduleName, [], testUri, [relativeUri],
             mainSource: isMain ? relativeUri : null,
             isMain: isMain,
             packageBase: Uri.parse('.'));
@@ -87,7 +87,7 @@ Future<ModularTest> loadTest(Uri uri) async {
         return _moduleConflict(moduleName, modules[moduleName]!, testUri);
       }
       var sources = await _listModuleSources(entryUri);
-      modules[moduleName] = Module(moduleName, [], testUri, sources, {},
+      modules[moduleName] = Module(moduleName, [], testUri, sources,
           packageBase: Uri.parse('$moduleName/'));
     }
   }
@@ -109,7 +109,6 @@ Future<ModularTest> loadTest(Uri uri) async {
   await _addModulePerPackage(spec.packages, testUri, modules);
   _attachDependencies(spec.dependencies, modules);
   _attachDependencies(defaultTestSpecification.dependencies, modules);
-  _attachMacros(spec.macros, modules);
   _addSdkDependencies(modules, sdkModule);
   _detectCyclesAndRemoveUnreachable(modules, mainModule);
   var sortedModules = modules.values.toList()
@@ -155,20 +154,6 @@ void _attachDependencies(
   });
 }
 
-void _attachMacros(Map<String, Map<String, Map<String, List<String>>>> macros,
-    Map<String, Module> modules) {
-  macros.forEach((name, macroConstructors) {
-    final module = modules[name];
-    if (module == null) {
-      _invalidTest("declared macros for a nonexistent module named '$name'");
-    }
-    if (module.macroConstructors.isNotEmpty) {
-      _invalidTest("Module macros have already been declared on $name.");
-    }
-    module.macroConstructors.addAll(macroConstructors);
-  });
-}
-
 /// Make every module depend on the sdk module.
 void _addSdkDependencies(Map<String, Module> modules, Module sdkModule) {
   for (var module in modules.values) {
@@ -192,8 +177,7 @@ Future<void> _addModulePerPackage(Map<String, String> packages, Uri configRoot,
       // TODO(sigmund): validate that we don't use a different alias for a
       // module that is part of the test (package name and module name should
       // match).
-      modules[packageName] = Module(
-          packageName, [], packageRootUri, sources, {},
+      modules[packageName] = Module(packageName, [], packageRootUri, sources,
           isPackage: true, packageBase: Uri.parse('lib/'), isShared: true);
     }
   }
@@ -221,7 +205,7 @@ Future<Module> _createSdkModule(Uri root) async {
     }
   }
   sources.sort((a, b) => a.path.compareTo(b.path));
-  return Module('sdk', [], root, sources, {}, isSdk: true, isShared: true);
+  return Module('sdk', [], root, sources, isSdk: true, isShared: true);
 }
 
 /// Trim the set of modules, and detect cycles while we are at it.