Skip to content

Commit 9293d2f

Browse files
johnniwintherCommit Queue
johnniwinther
authored and
Commit Queue
committed
[cfe] Report errors on macro declaration/annotation in the same library cycle
Change-Id: Ia8b7b572f8947e7b82f3468331cd33e430ef8376 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/356060 Reviewed-by: Jens Johansen <[email protected]> Commit-Queue: Johnni Winther <[email protected]>
1 parent 170e014 commit 9293d2f

File tree

22 files changed

+486
-30
lines changed

22 files changed

+486
-30
lines changed

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11451,6 +11451,41 @@ Message _withArgumentsMacroClassNotDeclaredMacro(String name) {
1145111451
);
1145211452
}
1145311453

11454+
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
11455+
const Template<Message Function(String name)>
11456+
templateMacroDefinitionApplicationSameLibraryCycle =
11457+
const Template<Message Function(String name)>(
11458+
"MacroDefinitionApplicationSameLibraryCycle",
11459+
problemMessageTemplate:
11460+
r"""The macro '#name' can't be applied in the same library cycle where it is defined.""",
11461+
correctionMessageTemplate:
11462+
r"""Try moving it to a different library that does not import the one where it is applied.""",
11463+
withArguments: _withArgumentsMacroDefinitionApplicationSameLibraryCycle,
11464+
);
11465+
11466+
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
11467+
const Code<Message Function(String name)>
11468+
codeMacroDefinitionApplicationSameLibraryCycle =
11469+
const Code<Message Function(String name)>(
11470+
"MacroDefinitionApplicationSameLibraryCycle",
11471+
);
11472+
11473+
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
11474+
Message _withArgumentsMacroDefinitionApplicationSameLibraryCycle(String name) {
11475+
if (name.isEmpty) throw 'No name provided';
11476+
name = demangleMixinApplicationName(name);
11477+
return new Message(
11478+
codeMacroDefinitionApplicationSameLibraryCycle,
11479+
problemMessage:
11480+
"""The macro '${name}' can't be applied in the same library cycle where it is defined.""",
11481+
correctionMessage:
11482+
"""Try moving it to a different library that does not import the one where it is applied.""",
11483+
arguments: {
11484+
'name': name,
11485+
},
11486+
);
11487+
}
11488+
1145411489
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
1145511490
const Code<Null> codeMainNotFunctionDeclaration =
1145611491
messageMainNotFunctionDeclaration;

‎pkg/front_end/lib/src/fasta/kernel/macro/annotation_parser.dart

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ List<MacroApplication>? prebuildAnnotations(
2525
{required SourceLibraryBuilder enclosingLibrary,
2626
required List<MetadataBuilder>? metadataBuilders,
2727
required Uri fileUri,
28-
required Scope scope}) {
28+
required Scope scope,
29+
required Set<ClassBuilder> currentMacroDeclarations}) {
2930
if (metadataBuilders == null) return null;
3031
List<MacroApplication>? result;
3132
for (MetadataBuilder metadataBuilder in metadataBuilders) {
@@ -38,7 +39,20 @@ List<MacroApplication>? prebuildAnnotations(
3839
MacroApplication? application = listener.popMacroApplication();
3940
if (application != null) {
4041
result ??= [];
41-
result.add(application);
42+
if (currentMacroDeclarations.contains(application.classBuilder)) {
43+
ClassBuilder classBuilder = application.classBuilder;
44+
UriOffset uriOffset = application.uriOffset;
45+
enclosingLibrary.addProblem(
46+
templateMacroDefinitionApplicationSameLibraryCycle
47+
.withArguments(classBuilder.name),
48+
uriOffset.fileOffset,
49+
noLength,
50+
uriOffset.uri);
51+
result.add(
52+
new MacroApplication.invalid(classBuilder, uriOffset: uriOffset));
53+
} else {
54+
result.add(application);
55+
}
4256
}
4357
}
4458
if (result != null && result.length > 1) {
@@ -217,7 +231,7 @@ class _MacroListener implements Listener {
217231
}
218232
} else {
219233
if (_macroClassBuilder != null && _unhandledReason != null) {
220-
_erroneousMacroApplication = new MacroApplication.error(
234+
_erroneousMacroApplication = new MacroApplication.unhandled(
221235
_unhandledReason!, _macroClassBuilder!,
222236
uriOffset: new UriOffset(uri, beginToken.next!.charOffset));
223237
}

‎pkg/front_end/lib/src/fasta/kernel/macro/macro.dart

Lines changed: 47 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -53,18 +53,36 @@ class MacroApplication {
5353
final ClassBuilder classBuilder;
5454
final String constructorName;
5555
final macro.Arguments arguments;
56-
final String? errorReason;
56+
final bool isErroneous;
57+
final String? unhandledReason;
5758

59+
/// Creates a [MacroApplication] for a macro annotation that should be
60+
/// applied.
5861
MacroApplication(this.classBuilder, this.constructorName, this.arguments,
5962
{required this.uriOffset})
60-
: errorReason = null;
61-
62-
MacroApplication.error(String this.errorReason, this.classBuilder,
63+
: isErroneous = false,
64+
unhandledReason = null;
65+
66+
/// Creates an erroneous [MacroApplication] for a macro annotation using
67+
/// syntax that is not unhandled.
68+
// TODO(johnniwinther): Separate this into unhandled (but valid) and
69+
// unsupported (thus invalid) annotations.
70+
MacroApplication.unhandled(String this.unhandledReason, this.classBuilder,
6371
{required this.uriOffset})
72+
: isErroneous = true,
73+
constructorName = '',
74+
arguments = new macro.Arguments(const [], const {});
75+
76+
/// Creates an erroneous [MacroApplication] for an invalid macro application
77+
/// for which an error has been reported, which should _not_ be applied. For
78+
/// instance a macro annotation of a macro declared in the same library cycle.
79+
MacroApplication.invalid(this.classBuilder, {required this.uriOffset})
6480
: constructorName = '',
81+
isErroneous = true,
82+
unhandledReason = null,
6583
arguments = new macro.Arguments(const [], const {});
6684

67-
bool get isErroneous => errorReason != null;
85+
bool get isUnhandled => unhandledReason != null;
6886

6987
late macro.MacroInstanceIdentifier instanceIdentifier;
7088
late Set<macro.Phase> phasesToExecute;
@@ -245,10 +263,10 @@ void checkMacroApplications(
245263
MacroApplication? macroApplication =
246264
applications?.remove(annotation.fileOffset);
247265
if (macroApplication != null) {
248-
if (macroApplication.isErroneous) {
266+
if (macroApplication.isUnhandled) {
249267
libraryBuilder.addProblem(
250268
templateUnhandledMacroApplication
251-
.withArguments(macroApplication.errorReason!),
269+
.withArguments(macroApplication.unhandledReason!),
252270
annotation.fileOffset,
253271
noLength,
254272
fileUri);
@@ -350,14 +368,17 @@ class MacroApplications {
350368
bool get hasLoadableMacroIds => _pendingLibraryData.isNotEmpty;
351369

352370
void computeLibrariesMacroApplicationData(
353-
Iterable<SourceLibraryBuilder> libraryBuilders) {
371+
Iterable<SourceLibraryBuilder> libraryBuilders,
372+
Set<ClassBuilder> currentMacroDeclarations) {
354373
for (SourceLibraryBuilder libraryBuilder in libraryBuilders) {
355-
_computeSourceLibraryMacroApplicationData(libraryBuilder);
374+
_computeSourceLibraryMacroApplicationData(
375+
libraryBuilder, currentMacroDeclarations);
356376
}
357377
}
358378

359379
void _computeSourceLibraryMacroApplicationData(
360-
SourceLibraryBuilder libraryBuilder) {
380+
SourceLibraryBuilder libraryBuilder,
381+
Set<ClassBuilder> currentMacroDeclarations) {
361382
// TODO(johnniwinther): Handle augmentation libraries.
362383
LibraryMacroApplicationData libraryMacroApplicationData =
363384
new LibraryMacroApplicationData();
@@ -366,7 +387,8 @@ class MacroApplications {
366387
enclosingLibrary: libraryBuilder,
367388
scope: libraryBuilder.scope,
368389
fileUri: libraryBuilder.fileUri,
369-
metadataBuilders: libraryBuilder.metadata);
390+
metadataBuilders: libraryBuilder.metadata,
391+
currentMacroDeclarations: currentMacroDeclarations);
370392
if (libraryMacroApplications != null) {
371393
libraryMacroApplicationData.libraryApplications =
372394
new LibraryApplicationData(
@@ -384,7 +406,8 @@ class MacroApplications {
384406
enclosingLibrary: libraryBuilder,
385407
scope: classBuilder.scope,
386408
fileUri: classBuilder.fileUri,
387-
metadataBuilders: classBuilder.metadata);
409+
metadataBuilders: classBuilder.metadata,
410+
currentMacroDeclarations: currentMacroDeclarations);
388411
if (classMacroApplications != null) {
389412
classMacroApplicationData.classApplications =
390413
new ClassApplicationData(_macroIntrospection, libraryBuilder,
@@ -398,7 +421,8 @@ class MacroApplications {
398421
enclosingLibrary: libraryBuilder,
399422
scope: classBuilder.scope,
400423
fileUri: memberBuilder.fileUri,
401-
metadataBuilders: memberBuilder.metadata);
424+
metadataBuilders: memberBuilder.metadata,
425+
currentMacroDeclarations: currentMacroDeclarations);
402426
if (macroApplications != null) {
403427
classMacroApplicationData.memberApplications[memberBuilder] =
404428
new MemberApplicationData(_macroIntrospection, libraryBuilder,
@@ -409,7 +433,8 @@ class MacroApplications {
409433
enclosingLibrary: libraryBuilder,
410434
scope: classBuilder.scope,
411435
fileUri: memberBuilder.fileUri,
412-
metadataBuilders: memberBuilder.metadata);
436+
metadataBuilders: memberBuilder.metadata,
437+
currentMacroDeclarations: currentMacroDeclarations);
413438
if (macroApplications != null) {
414439
classMacroApplicationData.memberApplications[memberBuilder] =
415440
new MemberApplicationData(_macroIntrospection, libraryBuilder,
@@ -429,7 +454,8 @@ class MacroApplications {
429454
enclosingLibrary: libraryBuilder,
430455
scope: classBuilder.scope,
431456
fileUri: memberBuilder.fileUri,
432-
metadataBuilders: memberBuilder.metadata);
457+
metadataBuilders: memberBuilder.metadata,
458+
currentMacroDeclarations: currentMacroDeclarations);
433459
if (macroApplications != null) {
434460
classMacroApplicationData.memberApplications[memberBuilder] =
435461
new MemberApplicationData(_macroIntrospection, libraryBuilder,
@@ -440,7 +466,8 @@ class MacroApplications {
440466
enclosingLibrary: libraryBuilder,
441467
scope: classBuilder.scope,
442468
fileUri: memberBuilder.fileUri,
443-
metadataBuilders: memberBuilder.metadata);
469+
metadataBuilders: memberBuilder.metadata,
470+
currentMacroDeclarations: currentMacroDeclarations);
444471
if (macroApplications != null) {
445472
classMacroApplicationData.memberApplications[memberBuilder] =
446473
new MemberApplicationData(_macroIntrospection, libraryBuilder,
@@ -462,7 +489,8 @@ class MacroApplications {
462489
enclosingLibrary: libraryBuilder,
463490
scope: libraryBuilder.scope,
464491
fileUri: builder.fileUri,
465-
metadataBuilders: builder.metadata);
492+
metadataBuilders: builder.metadata,
493+
currentMacroDeclarations: currentMacroDeclarations);
466494
if (macroApplications != null) {
467495
libraryMacroApplicationData.memberApplications[builder] =
468496
new MemberApplicationData(_macroIntrospection, libraryBuilder,
@@ -473,7 +501,8 @@ class MacroApplications {
473501
enclosingLibrary: libraryBuilder,
474502
scope: libraryBuilder.scope,
475503
fileUri: builder.fileUri,
476-
metadataBuilders: builder.metadata);
504+
metadataBuilders: builder.metadata,
505+
currentMacroDeclarations: currentMacroDeclarations);
477506
if (macroApplications != null) {
478507
libraryMacroApplicationData.memberApplications[builder] =
479508
new MemberApplicationData(_macroIntrospection, libraryBuilder,

‎pkg/front_end/lib/src/fasta/source/source_loader.dart

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,9 @@ class SourceLoader extends Loader {
217217

218218
ClassBuilder? _macroClassBuilder;
219219

220+
/// The macro declarations that are currently being compiled.
221+
Set<ClassBuilder> _macroDeclarations = {};
222+
220223
SourceLoader(this.fileSystem, this.includeComments, this.target)
221224
: dataForTesting =
222225
retainDataForTesting ? new SourceLoaderDataForTesting() : null;
@@ -1544,10 +1547,12 @@ severity: $severity
15441547
ClassBuilder builder = iterator.current;
15451548
if (builder.isMacro) {
15461549
Uri libraryUri = builder.libraryBuilder.importUri;
1547-
if (!target.context.options.runningPrecompilations
1548-
.contains(libraryUri) &&
1549-
!target.context.options.macroExecutor
1550-
.libraryIsRegistered(libraryUri)) {
1550+
if (target.context.options.runningPrecompilations
1551+
.contains(libraryUri)) {
1552+
// We are explicitly compiling this macro.
1553+
_macroDeclarations.add(builder);
1554+
} else if (!target.context.options.macroExecutor
1555+
.libraryIsRegistered(libraryUri)) {
15511556
(macroLibraries[libraryUri] ??= []).add(builder);
15521557
if (retainDataForTesting) {
15531558
(dataForTesting!.macroDeclarationData
@@ -1681,11 +1686,22 @@ severity: $severity
16811686
// We have found the first needed layer of precompilation. There might
16821687
// be more layers but we'll compute these at the next attempt at
16831688
// compilation, when this layer has been precompiled.
1684-
// TODO(johnniwinther): Use this to trigger a precompile step.
16851689
return new NeededPrecompilations(neededPrecompilations);
16861690
}
16871691
}
16881692
}
1693+
if (compilationSteps.isNotEmpty) {
1694+
for (List<Uri> compilationStep in compilationSteps) {
1695+
for (Uri uri in compilationStep) {
1696+
List<ClassBuilder>? macroClasses = macroLibraries[uri];
1697+
if (macroClasses != null) {
1698+
// These macros are to be compiled during this (last) compilation
1699+
// step.
1700+
_macroDeclarations.addAll(macroClasses);
1701+
}
1702+
}
1703+
}
1704+
}
16891705
return null;
16901706
}
16911707

@@ -1700,8 +1716,8 @@ severity: $severity
17001716
this,
17011717
target.context.options.macroExecutor,
17021718
dataForTesting?.macroApplicationData);
1703-
macroApplications
1704-
.computeLibrariesMacroApplicationData(sourceLibraryBuilders);
1719+
macroApplications.computeLibrariesMacroApplicationData(
1720+
sourceLibraryBuilders, _macroDeclarations);
17051721
if (macroApplications.hasLoadableMacroIds) {
17061722
target.benchmarker?.beginSubdivide(
17071723
BenchmarkSubdivides.computeMacroApplications_macroExecutorProvider);
@@ -1715,8 +1731,8 @@ severity: $severity
17151731
Future<void> computeAdditionalMacroApplications(
17161732
MacroApplications macroApplications,
17171733
Iterable<SourceLibraryBuilder> sourceLibraryBuilders) async {
1718-
macroApplications
1719-
.computeLibrariesMacroApplicationData(sourceLibraryBuilders);
1734+
macroApplications.computeLibrariesMacroApplicationData(
1735+
sourceLibraryBuilders, _macroDeclarations);
17201736
if (macroApplications.hasLoadableMacroIds) {
17211737
target.benchmarker?.beginSubdivide(
17221738
BenchmarkSubdivides.computeMacroApplications_macroExecutorProvider);

‎pkg/front_end/messages.status

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -711,6 +711,8 @@ ListLiteralTooManyTypeArguments/example: Fail
711711
LoadLibraryTakesNoArguments/example: Fail
712712
MacroClassNotDeclaredMacro/analyzerCode: Fail
713713
MacroClassNotDeclaredMacro/example: Fail
714+
MacroDefinitionApplicationSameLibraryCycle/analyzerCode: Fail
715+
MacroDefinitionApplicationSameLibraryCycle/example: Fail
714716
MainNotFunctionDeclaration/analyzerCode: Fail
715717
MainNotFunctionDeclarationExported/analyzerCode: Fail
716718
MainNotFunctionDeclarationExported/part_wrapped_script: Fail

‎pkg/front_end/messages.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7581,3 +7581,7 @@ InvalidMacroApplicationTarget:
75817581

75827582
NoMacroApplicationTarget:
75837583
problemMessage: "The macro can not be applied to this declaration."
7584+
7585+
MacroDefinitionApplicationSameLibraryCycle:
7586+
problemMessage: "The macro '#name' can't be applied in the same library cycle where it is defined."
7587+
correctionMessage: Try moving it to a different library that does not import the one where it is applied.

‎pkg/front_end/test/macros/application/data/tests/cycle1.expect

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
library;
2+
//
3+
// Problems in library:
4+
//
5+
// org-dartlang-test:///a/b/c/main.dart:8:2: Error: The macro 'Macro' can't be applied in the same library cycle where it is defined.
6+
// Try moving it to a different library that does not import the one where it is applied.
7+
// @Macro() // Error
8+
// ^
9+
//
10+
import self as self;
11+
import "main_lib.dart" as mai;
12+
13+
import "dart:async";
14+
import "org-dartlang-test:///a/b/c/main_lib.dart";
15+
16+
@#C1
17+
static method method() → dynamic {}
18+
19+
library;
20+
import self as mai;
21+
import "dart:core" as core;
22+
import "package:_fe_analyzer_shared/src/macros/api.dart" as api;
23+
24+
import "dart:async";
25+
import "package:_fe_analyzer_shared/src/macros/api.dart";
26+
import "org-dartlang-test:///a/b/c/main.dart";
27+
28+
macro class Macro extends core::Object implements api::FunctionDeclarationsMacro /*hasConstConstructor*/ {
29+
const constructor •() → mai::Macro
30+
: super core::Object::•()
31+
;
32+
method buildDeclarationsForFunction(api::FunctionDeclaration function, api::DeclarationBuilder builder) → FutureOr<void> {}
33+
}
34+
35+
constants {
36+
#C1 = mai::Macro {}
37+
}

‎pkg/front_end/test/macros/application/data/tests/cycle1/main.dart

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'dart:async';
6+
import 'main_lib.dart';
7+
8+
@Macro() // Error
9+
method() {}

‎pkg/front_end/test/macros/application/data/tests/cycle1/main_lib.dart

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'dart:async';
6+
import 'package:_fe_analyzer_shared/src/macros/api.dart';
7+
import 'main.dart';
8+
9+
macro class Macro implements FunctionDeclarationsMacro {
10+
const Macro();
11+
12+
FutureOr<void> buildDeclarationsForFunction(
13+
FunctionDeclaration function, DeclarationBuilder builder) {}
14+
}

0 commit comments

Comments
 (0)