Skip to content

Commit

Permalink
Fix "without macros" benchmark: notify analyzer of new files. (#138)
Browse files Browse the repository at this point in the history
  • Loading branch information
davidmorgan authored Nov 11, 2024
1 parent 9c1a4f3 commit 28643f3
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 22 deletions.
4 changes: 2 additions & 2 deletions pkgs/_macro_tool/lib/analyzer_macro_runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ class AnalyzerMacroRunner implements MacroRunner {
analysisContext = contextCollection.contexts.single;
}

void notifyChange(SourceFile sourceFile) {
analysisContext.changeFile(sourceFile.path);
void notifyChange(String sourcePath) {
analysisContext.changeFile(sourcePath);
}

@override
Expand Down
6 changes: 3 additions & 3 deletions pkgs/_macro_tool/lib/cfe_macro_runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,8 @@ class CfeMacroRunner implements MacroRunner {
CfeMacroRunner({required this.workspacePath, required this.packageConfigPath})
: sourceFiles = SourceFile.findDartInWorkspace(workspacePath);

void notifyChange(SourceFile sourceFile) {
throw UnimplementedError(
'CfeMacroRunner does not support incremental compilation.');
void notifyChange(String sourcePath) {
// No incremental compile.
}

File get _productPlatformDill {
Expand Down Expand Up @@ -66,6 +65,7 @@ class CfeMacroRunner implements MacroRunner {
final packagesUri = Uri.file(packageConfigPath);

final computeKernelResult = await computeKernel([
'--enable-experiment=macros',
'--no-summary',
'--no-summary-only',
'--target=vm',
Expand Down
2 changes: 1 addition & 1 deletion pkgs/_macro_tool/lib/macro_runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ abstract interface class MacroRunner {
/// Notifies the host that a file changed.
///
/// Call this on changed files then call [run] again for an incremental run.
void notifyChange(SourceFile sourceFile);
void notifyChange(String sourcePath);
}

/// [MacroRunner] result for the whole workspace.
Expand Down
15 changes: 6 additions & 9 deletions pkgs/_macro_tool/lib/macro_tool.dart
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class MacroTool {

for (final result in _applyResult!.fileResults) {
if (result.output != null) {
result.sourceFile.writeOutput(result.output!);
result.sourceFile.writeOutput(macroRunner, result.output!);
}
}
}
Expand All @@ -60,7 +60,7 @@ class MacroTool {

for (final result in _applyResult!.fileResults) {
if (result.output != null) {
result.sourceFile.patchForAnalyzer();
result.sourceFile.patchForAnalyzer(macroRunner);
}
}
}
Expand All @@ -77,7 +77,7 @@ class MacroTool {

for (final result in _applyResult!.fileResults) {
if (result.output != null) {
result.sourceFile.patchForCfe();
result.sourceFile.patchForCfe(macroRunner);
}
}
}
Expand Down Expand Up @@ -110,7 +110,7 @@ class MacroTool {
/// and/or [bustCaches].
void revert() {
for (final sourceFile in macroRunner.sourceFiles) {
sourceFile.revert();
sourceFile.revert(macroRunner);
}
}

Expand Down Expand Up @@ -167,11 +167,8 @@ class MacroTool {
void bustCaches() {
var cacheBusterFound = false;
for (final sourceFile in macroRunner.sourceFiles) {
if (sourceFile.bustCaches()) {
if (sourceFile.bustCaches(macroRunner)) {
cacheBusterFound = true;
// Notify the macro runner of the change so that it will be picked up
// by the next incremental run.
macroRunner.notifyChange(sourceFile);
}
}
if (!cacheBusterFound) {
Expand All @@ -195,7 +192,7 @@ class MacroTool {
_applyResult = await macroRunner.run();
for (final result in _applyResult!.fileResults) {
if (result.output != null) {
result.sourceFile.writeOutput(result.output!);
result.sourceFile.writeOutput(macroRunner, result.output!);
}
}
if (_applyResult!.allErrors.isNotEmpty) {
Expand Down
36 changes: 29 additions & 7 deletions pkgs/_macro_tool/lib/source_file.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import 'dart:math';

import 'package:path/path.dart' as p;

import 'macro_runner.dart';

final _random = Random.secure();

/// Dart source file manipulation for `_macro_tool`.
Expand All @@ -23,26 +25,33 @@ extension type SourceFile(String path) implements String {
.whereType<File>()
.where((f) => f.path.endsWith('.dart'))
.map((f) => SourceFile(f.path))
.toList();
.toList()
..sort((a, b) => a.path.compareTo(b.path));

/// The source file path plus `.macro_tool_output`.
String get toolOutputPath => '$path.macro_tool_output';

/// Writes [output] to [toolOutputPath].
void writeOutput(String output) {
///
/// [macroRunner] is notified of the change.
void writeOutput(MacroRunner macroRunner, String output) {
File(toolOutputPath).writeAsStringSync(output);
macroRunner.notifyChange(toolOutputPath);
}

/// Patches [path] so the analyzer can analyze it without running macros.
///
/// Adds a `part` statement referencing [toolOutputPath].
void patchForAnalyzer() {
///
/// [macroRunner] is notified of the change.
void patchForAnalyzer(MacroRunner macroRunner) {
final partName = p.basename(toolOutputPath);
final line = "part '$partName'; $_addedMarker\n";

final file = File(path);
file.writeAsStringSync(_insertAfterLastImport(
line, _removeToolAddedLinesFromSource(file.readAsStringSync())));
macroRunner.notifyChange(path);
}

String _insertAfterLastImport(String line, String source) {
Expand All @@ -60,28 +69,38 @@ extension type SourceFile(String path) implements String {
///
/// This means changing [toolOutputPath] from using parts to library
/// augmentations and adding `import augment` to [path].
void patchForCfe() {
///
/// [macroRunner] is notified of the change.
void patchForCfe(MacroRunner macroRunner) {
final partName = p.basename(toolOutputPath);
final line = "import augment '$partName'; $_addedMarker\n";

final file = File(path);
file.writeAsStringSync(
line + _removeToolAddedLinesFromSource(file.readAsStringSync()));
macroRunner.notifyChange(path);

final toolOutputFile = File(toolOutputPath);
toolOutputFile.writeAsStringSync(toolOutputFile
.readAsStringSync()
.replaceAll('part of ', 'augment library '));
macroRunner.notifyChange(toolOutputPath);
}

/// Reverts changes to source from any of [patchForAnalyzer], [patchForCfe]
/// and/or [bustCaches].
void revert() {
///
/// [macroRunner] is notified of the change.
void revert(MacroRunner macroRunner) {
final file = File(path);
file.writeAsStringSync(_resetCacheBusters(
_removeToolAddedLinesFromSource(file.readAsStringSync())));
macroRunner.notifyChange(path);
final toolOutputFile = File(toolOutputPath);
if (toolOutputFile.existsSync()) toolOutputFile.deleteSync();
if (toolOutputFile.existsSync()) {
toolOutputFile.deleteSync();
macroRunner.notifyChange(toolOutputPath);
}
}

/// Returns [source] with lines added by [_addImportAugment] removed.
Expand All @@ -96,7 +115,9 @@ extension type SourceFile(String path) implements String {
/// If there is an augmentation output file, updates that too.
///
/// Returns whether any change was made to a file.
bool bustCaches() {
///
/// [macroRunner] is notified of the change.
bool bustCaches(MacroRunner macroRunner) {
final token = _random.nextInt(1 << 32).toRadixString(16) +
_random.nextInt(1 << 32).toRadixString(16);
var cacheBusterFound = false;
Expand All @@ -111,6 +132,7 @@ extension type SourceFile(String path) implements String {
cacheBusterFound = true;
file.writeAsStringSync(
source.replaceAll(_cacheBusterRegexp, '$_cacheBusterString$token'));
macroRunner.notifyChange(path);
}
}
return cacheBusterFound;
Expand Down

0 comments on commit 28643f3

Please sign in to comment.