Skip to content

Commit 9fb8440

Browse files
stereotype441Commit Queue
authored andcommitted
[front_end] Fix bootstrapping of message generation.
The dart formatter depends on `package:analyzer`, which depends on the code generated diagnostic message file `pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart`. This means that if the code generator for this file tries to format it using the formatter's programmatic API (the `DartFormatter` class), there will be a bootstrapping problem, because the code generator will include the generated file in its transitive imports. To avoid this, `generate_messages.dart` is changed so that it first writes out an unformatted file, and then uses the `DartFormat` class (from `package:analyzer_utilities`) to do the formatting. The `DartFormat` class runs `dart format` as a subprocess, so it doesn't depend have to import `package:analyzer`. In addition to fixing the bootstrapping problem, this has two side benefits: - By avoiding the import of `package:analyzer`, it makes the code generator run much faster. I see approximately a 4x speedup. - Previously, if formatting failed due to a syntax error in the generated code, it no output would be generated, making the bug difficult to diagnose. Now, if the generated code contains a syntax error, the unformatted file is left on disk. A previous fix for the bootstrapping problem `generate_messages_failsafe.dart`, didn't have these advantages. It is removed since it's no longer needed. The test `generated_files_up_to_date_git_test.dart` is updated so that it does the formatting using the `DartFormatter` class. This isn't a problem for bootstrapping, because this test is only required to work properly when the code is in a fully generated state. Note that the code generator can also be invoked indirectly through the `pkg/front_end/tool/cfe.dart` tool. This, unfortunately, still has the bootstrapping problem, so a hint message is added to `cfe.dart` to encourage the user to run `generate_messages.dart` directly. Change-Id: I6a6a6964fec4259a4462c6ec5204fea2dcd28314 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/447920 Commit-Queue: Paul Berry <[email protected]> Reviewed-by: Johnni Winther <[email protected]>
1 parent 9bb2d49 commit 9fb8440

File tree

10 files changed

+38
-74
lines changed

10 files changed

+38
-74
lines changed

pkg/analyzer/tool/messages/error_code_info.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,7 @@ class CfeToAnalyzerErrorCodeTables {
476476
throw '''
477477
$frontEndCode specifies index $index but indices must be 1 or greater.
478478
For more information run:
479-
pkg/front_end/tool/fasta generate-messages
479+
dart pkg/front_end/tool/generate_messages.dart
480480
''';
481481
}
482482
if (indexToInfo.length <= index) {

pkg/analyzer/tool/messages/generate.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
/// containing the error and the name of the error separated by a `.`
1313
/// (e.g. ParserErrorCode.EQUALITY_CANNOT_BE_EQUALITY_OPERAND).
1414
///
15-
/// It is expected that 'pkg/front_end/tool/fasta generate-messages'
15+
/// It is expected that 'dart pkg/front_end/tool/generate_messages.dart'
1616
/// has already been successfully run.
1717
library;
1818

pkg/front_end/messages.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,14 @@
44

55
# Run
66
#
7-
# dart pkg/front_end/tool/cfe.dart generate-messages
7+
# dart pkg/front_end/tool/generate_messages.dart
88
#
99
# to regenerate messages after having edited this file.
1010
#
1111
# If entries with 'analyzerCode', 'sharedName', or 'index' have been changed,
1212
# run both
1313
#
14-
# dart pkg/front_end/tool/cfe.dart generate-messages
14+
# dart pkg/front_end/tool/generate_messages.dart
1515
# dart pkg/analyzer/tool/messages/generate.dart
1616
#
1717
# to regenerate analyzer messages.

pkg/front_end/pubspec.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ dependencies:
2323
# Use 'any' constraints here; we get our versions from the DEPS file.
2424
dev_dependencies:
2525
analyzer: any
26+
analyzer_utilities: any
2627
args: any
2728
build_integration: any
2829
compiler: any

pkg/front_end/test/generated_files_up_to_date_git_test.dart

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
import "dart:io" show File, exitCode;
66

7+
import 'package:dart_style/dart_style.dart' show DartFormatter;
8+
79
import "../tool/generate_experimental_flags.dart" as generateExperimentalFlags;
810
import "../tool/generate_messages.dart" as generateMessages;
911
import "../tool/parser_ast_helper_creator.dart" as generateParserAstHelper;
@@ -12,7 +14,7 @@ import '../tool/generate_ast_coverage.dart' as generateAstCoverage;
1214
import '../tool/generate_ast_equivalence.dart' as generateAstEquivalence;
1315
import "parser_test_listener_creator.dart" as generateParserTestListener;
1416
import "parser_test_parser_creator.dart" as generateParserTestParser;
15-
import 'utils/io_utils.dart' show computeRepoDirUri;
17+
import 'utils/io_utils.dart' show computeRepoDirUri, getPackageVersionFor;
1618

1719
final Uri repoDir = computeRepoDirUri();
1820

@@ -131,16 +133,20 @@ void messages() {
131133

132134
Uri generatedFile = generateMessages.computeSharedGeneratedFile(repoDir);
133135
check(
134-
messages.sharedMessages,
136+
new DartFormatter(
137+
languageVersion: getPackageVersionFor('_fe_analyzer_shared'),
138+
).format(messages.sharedMessages),
135139
generatedFile,
136-
"dart pkg/front_end/tool/cfe.dart generate-messages",
140+
"dart pkg/front_end/tool/generate_messages.dart",
137141
);
138142

139143
Uri cfeGeneratedFile = generateMessages.computeCfeGeneratedFile(repoDir);
140144
check(
141-
messages.cfeMessages,
145+
new DartFormatter(
146+
languageVersion: getPackageVersionFor('front_end'),
147+
).format(messages.cfeMessages),
142148
cfeGeneratedFile,
143-
"dart pkg/front_end/tool/cfe.dart generate-messages",
149+
"dart pkg/front_end/tool/generate_messages.dart",
144150
);
145151
}
146152

pkg/front_end/test/spell_checking_list_tests.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ bold
107107
boo
108108
boollist
109109
bools
110+
bootstrapping
110111
bots
111112
boundaries
112113
boundarykey

pkg/front_end/tool/cfe.dart

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,10 @@ Future<void> main(List<String> args) async {
9696
scriptArguments.add('--config=${repoDir}/pkg/front_end/testing.json');
9797
break;
9898
case 'generate-messages':
99+
print(
100+
'Hint: running `generate_messages.dart` directly is faster and avoids '
101+
'bootstrapping problems.',
102+
);
99103
mainFunction = generate_messages.main;
100104
script = '${toolDir}/generate_messages.dart';
101105
break;

pkg/front_end/tool/generate_messages.dart

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
import 'dart:io' show File;
66

7-
import 'package:dart_style/dart_style.dart' show DartFormatter;
7+
import 'package:analyzer_utilities/tools.dart';
88

99
import '../test/utils/io_utils.dart' show computeRepoDirUri;
1010
import 'generate_messages_lib.dart';
@@ -21,18 +21,22 @@ void main(List<String> arguments) {
2121
"Refusing to overwrite with empty file!",
2222
);
2323
} else {
24-
new File.fromUri(
25-
computeSharedGeneratedFile(repoDir),
26-
).writeAsStringSync(message.sharedMessages, flush: true);
27-
new File.fromUri(
28-
computeCfeGeneratedFile(repoDir),
29-
).writeAsStringSync(message.cfeMessages, flush: true);
24+
_writeAndFormat(
25+
new File.fromUri(computeSharedGeneratedFile(repoDir)),
26+
message.sharedMessages,
27+
);
28+
_writeAndFormat(
29+
new File.fromUri(computeCfeGeneratedFile(repoDir)),
30+
message.cfeMessages,
31+
);
3032
}
3133
}
3234

35+
void _writeAndFormat(File file, String contents) {
36+
file.writeAsStringSync(contents, flush: true);
37+
DartFormat.formatFile(file);
38+
}
39+
3340
Messages generateMessagesFiles(Uri repoDir) {
34-
return generateMessagesFilesRaw(
35-
repoDir,
36-
(s, version) => new DartFormatter(languageVersion: version).format(s),
37-
);
41+
return generateMessagesFilesRaw(repoDir);
3842
}

pkg/front_end/tool/generate_messages_failsafe.dart

Lines changed: 0 additions & 43 deletions
This file was deleted.

pkg/front_end/tool/generate_messages_lib.dart

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,8 @@ import 'dart:io' show File, exitCode;
66

77
import "package:_fe_analyzer_shared/src/messages/severity.dart"
88
show severityEnumNames;
9-
import 'package:pub_semver/pub_semver.dart' show Version;
109
import 'package:yaml/yaml.dart' show loadYaml;
1110

12-
import '../test/utils/io_utils.dart';
13-
1411
Uri computeSharedGeneratedFile(Uri repoDir) {
1512
return repoDir.resolve(
1613
"pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart",
@@ -30,10 +27,7 @@ class Messages {
3027
Messages(this.sharedMessages, this.cfeMessages);
3128
}
3229

33-
Messages generateMessagesFilesRaw(
34-
Uri repoDir,
35-
String Function(String, Version) formatter,
36-
) {
30+
Messages generateMessagesFilesRaw(Uri repoDir) {
3731
Uri messagesFile = repoDir.resolve("pkg/front_end/messages.yaml");
3832
Map<dynamic, dynamic> yaml = loadYaml(
3933
new File.fromUri(messagesFile).readAsStringSync(),
@@ -151,10 +145,7 @@ part of 'cfe_codes.dart';
151145
return new Messages('', '');
152146
}
153147

154-
return new Messages(
155-
formatter("$sharedMessages", getPackageVersionFor("_fe_analyzer_shared")),
156-
formatter("$cfeMessages", getPackageVersionFor("front_end")),
157-
);
148+
return new Messages("$sharedMessages", "$cfeMessages");
158149
}
159150

160151
final RegExp placeholderPattern = new RegExp(

0 commit comments

Comments
 (0)