Skip to content

Commit 33bc628

Browse files
johnniwinthercommit-bot@chromium.org
authored andcommitted
Treeshake unread fields
Change-Id: I188df5fd18163e5b32213a364d0fc163300db7b9 Reviewed-on: https://dart-review.googlesource.com/c/92130 Commit-Queue: Johnni Winther <[email protected]> Reviewed-by: Sigmund Cherem <[email protected]>
1 parent b040d3c commit 33bc628

40 files changed

+863
-147
lines changed

pkg/compiler/lib/src/js_backend/annotations.dart

Lines changed: 113 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ enum PragmaAnnotation {
3838
noInline,
3939
tryInline,
4040
disableFinal,
41+
noElision,
4142
noThrows,
4243
noSideEffects,
4344
trustTypeAnnotations,
@@ -54,18 +55,13 @@ Set<PragmaAnnotation> processMemberAnnotations(
5455
bool hasNoInline = false;
5556
bool hasTryInline = false;
5657
bool disableFinal = false;
58+
bool noElision = false;
5759

5860
if (_assumeDynamic(elementEnvironment, commonElements, element)) {
5961
values.add(PragmaAnnotation.assumeDynamic);
6062
annotationsDataBuilder.registerAssumeDynamic(element);
6163
}
6264

63-
// TODO(sra): Check for inappropriate annotations on fields.
64-
if (element.isField) {
65-
return values;
66-
}
67-
68-
FunctionEntity method = element;
6965
LibraryEntity library = element.library;
7066
bool platformAnnotationsAllowed = library.canonicalUri.scheme == 'dart' ||
7167
maybeEnableNative(library.canonicalUri);
@@ -74,7 +70,7 @@ Set<PragmaAnnotation> processMemberAnnotations(
7470
bool hasNoSideEffects = false;
7571

7672
for (ConstantValue constantValue
77-
in elementEnvironment.getMemberMetadata(method)) {
73+
in elementEnvironment.getMemberMetadata(element)) {
7874
if (!constantValue.isConstructedObject) continue;
7975
ConstructedConstantValue value = constantValue;
8076
ClassEntity cls = value.type.element;
@@ -83,37 +79,69 @@ Set<PragmaAnnotation> processMemberAnnotations(
8379
if (platformAnnotationsAllowed) {
8480
if (cls == commonElements.forceInlineClass) {
8581
hasTryInline = true;
82+
if (element is! FunctionEntity) {
83+
reporter.internalError(element,
84+
"@TryInline() is only allowed in methods and constructors.");
85+
}
8686
} else if (cls == commonElements.noInlineClass) {
8787
hasNoInline = true;
88+
if (element is! FunctionEntity) {
89+
reporter.internalError(element,
90+
"@NoInline() is only allowed in methods and constructors.");
91+
}
8892
} else if (cls == commonElements.noThrowsClass) {
8993
hasNoThrows = true;
9094
bool isValid = true;
91-
if (method.isTopLevel) {
92-
isValid = true;
93-
} else if (method.isStatic) {
94-
isValid = true;
95-
} else if (method is ConstructorEntity && method.isFactoryConstructor) {
96-
isValid = true;
95+
if (element is FunctionEntity) {
96+
if (element.isTopLevel) {
97+
isValid = true;
98+
} else if (element.isStatic) {
99+
isValid = true;
100+
} else if (element is ConstructorEntity &&
101+
element.isFactoryConstructor) {
102+
isValid = true;
103+
}
104+
} else {
105+
isValid = false;
97106
}
98107
if (!isValid) {
99108
reporter.internalError(
100-
method,
109+
element,
101110
"@NoThrows() is currently limited to top-level"
102111
" or static functions and factory constructors.");
103112
}
104-
annotationsDataBuilder.registerCannotThrow(method);
113+
if (element is FunctionEntity) {
114+
annotationsDataBuilder.registerCannotThrow(element);
115+
}
105116
} else if (cls == commonElements.noSideEffectsClass) {
106117
hasNoSideEffects = true;
107-
annotationsDataBuilder.registerSideEffectsFree(method);
118+
if (element is FunctionEntity) {
119+
annotationsDataBuilder.registerSideEffectsFree(element);
120+
} else {
121+
reporter.internalError(element,
122+
"@NoSideEffects() is only allowed in methods and constructors.");
123+
}
108124
}
109125
}
110126

111127
if (cls == commonElements.expectNoInlineClass) {
112128
hasNoInline = true;
129+
if (element is! FunctionEntity) {
130+
reporter.internalError(element,
131+
"@NoInline() is only allowed in methods and constructors.");
132+
}
113133
} else if (cls == commonElements.metaNoInlineClass) {
114134
hasNoInline = true;
135+
if (element is! FunctionEntity) {
136+
reporter.internalError(
137+
element, "@noInline is only allowed in methods and constructors.");
138+
}
115139
} else if (cls == commonElements.metaTryInlineClass) {
116140
hasTryInline = true;
141+
if (element is! FunctionEntity) {
142+
reporter.internalError(
143+
element, "@tryInline is only allowed in methods and constructors.");
144+
}
117145
} else if (cls == commonElements.pragmaClass) {
118146
// Recognize:
119147
//
@@ -133,12 +161,24 @@ Set<PragmaAnnotation> processMemberAnnotations(
133161
reporter.reportErrorMessage(element, MessageKind.GENERIC,
134162
{'text': "@pragma('$name') annotation does not take options"});
135163
}
164+
if (element is! FunctionEntity) {
165+
reporter.reportErrorMessage(element, MessageKind.GENERIC, {
166+
'text': "@pragma('$name') annotation is only supported "
167+
"for methods and constructors."
168+
});
169+
}
136170
hasNoInline = true;
137171
} else if (name == 'dart2js:tryInline') {
138172
if (!optionsValue.isNull) {
139173
reporter.reportErrorMessage(element, MessageKind.GENERIC,
140174
{'text': "@pragma('$name') annotation does not take options"});
141175
}
176+
if (element is! FunctionEntity) {
177+
reporter.reportErrorMessage(element, MessageKind.GENERIC, {
178+
'text': "@pragma('$name') annotation is only supported "
179+
"for methods and constructors."
180+
});
181+
}
142182
hasTryInline = true;
143183
} else if (!platformAnnotationsAllowed) {
144184
reporter.reportErrorMessage(element, MessageKind.GENERIC,
@@ -150,7 +190,28 @@ Set<PragmaAnnotation> processMemberAnnotations(
150190
reporter.reportErrorMessage(element, MessageKind.GENERIC,
151191
{'text': "@pragma('$name') annotation does not take options"});
152192
}
193+
if (element is! FunctionEntity) {
194+
reporter.reportErrorMessage(element, MessageKind.GENERIC, {
195+
'text': "@pragma('$name') annotation is only supported "
196+
"for methods and constructors."
197+
});
198+
}
153199
disableFinal = true;
200+
} else if (name == 'dart2js:noElision') {
201+
if (!optionsValue.isNull) {
202+
reporter.reportErrorMessage(element, MessageKind.GENERIC,
203+
{'text': "@pragma('$name') annotation does not take options"});
204+
}
205+
if (element is! FieldEntity) {
206+
reporter.reportErrorMessage(element, MessageKind.GENERIC, {
207+
'text': "@pragma('$name') annotation is only supported "
208+
"for fields."
209+
});
210+
}
211+
noElision = true;
212+
} else {
213+
reporter.reportErrorMessage(element, MessageKind.GENERIC,
214+
{'text': "Unknown dart2js pragma @pragma('$name')"});
154215
}
155216
}
156217
}
@@ -163,23 +224,35 @@ Set<PragmaAnnotation> processMemberAnnotations(
163224
}
164225
if (hasNoInline) {
165226
values.add(PragmaAnnotation.noInline);
166-
annotationsDataBuilder.markAsNonInlinable(method);
227+
if (element is FunctionEntity) {
228+
annotationsDataBuilder.markAsNonInlinable(element);
229+
}
167230
}
168231
if (hasTryInline) {
169232
values.add(PragmaAnnotation.tryInline);
170-
annotationsDataBuilder.markAsTryInline(method);
233+
if (element is FunctionEntity) {
234+
annotationsDataBuilder.markAsTryInline(element);
235+
}
171236
}
172237
if (disableFinal) {
173238
values.add(PragmaAnnotation.disableFinal);
174-
annotationsDataBuilder.markAsDisableFinal(method);
239+
if (element is FunctionEntity) {
240+
annotationsDataBuilder.markAsDisableFinal(element);
241+
}
242+
}
243+
if (noElision) {
244+
values.add(PragmaAnnotation.noElision);
245+
if (element is FieldEntity) {
246+
annotationsDataBuilder.markAsNoElision(element);
247+
}
175248
}
176249
if (hasNoThrows && !hasNoInline) {
177250
reporter.internalError(
178-
method, "@NoThrows() should always be combined with @noInline.");
251+
element, "@NoThrows() should always be combined with @noInline.");
179252
}
180253
if (hasNoSideEffects && !hasNoInline) {
181254
reporter.internalError(
182-
method, "@NoSideEffects() should always be combined with @noInline.");
255+
element, "@NoSideEffects() should always be combined with @noInline.");
183256
}
184257
return values;
185258
}
@@ -200,9 +273,12 @@ abstract class AnnotationsData {
200273
/// `@pragma('dart2js:tryInline')` annotation.
201274
Iterable<FunctionEntity> get tryInlineFunctions;
202275

203-
/// Functions with a `@pragma('dart2js:disable-final')` annotation.
276+
/// Functions with a `@pragma('dart2js:disableFinal')` annotation.
204277
Iterable<FunctionEntity> get disableFinalFunctions;
205278

279+
/// Fields with a `@pragma('dart2js:noElision')` annotation.
280+
Iterable<FieldEntity> get noElisionFields;
281+
206282
/// Functions with a `@NoThrows()` annotation.
207283
Iterable<FunctionEntity> get cannotThrowFunctions;
208284

@@ -221,6 +297,7 @@ class AnnotationsDataImpl implements AnnotationsData {
221297
final Iterable<FunctionEntity> nonInlinableFunctions;
222298
final Iterable<FunctionEntity> tryInlineFunctions;
223299
final Iterable<FunctionEntity> disableFinalFunctions;
300+
final Iterable<FieldEntity> noElisionFields;
224301
final Iterable<FunctionEntity> cannotThrowFunctions;
225302
final Iterable<FunctionEntity> sideEffectFreeFunctions;
226303
final Iterable<MemberEntity> assumeDynamicMembers;
@@ -229,6 +306,7 @@ class AnnotationsDataImpl implements AnnotationsData {
229306
this.nonInlinableFunctions,
230307
this.tryInlineFunctions,
231308
this.disableFinalFunctions,
309+
this.noElisionFields,
232310
this.cannotThrowFunctions,
233311
this.sideEffectFreeFunctions,
234312
this.assumeDynamicMembers);
@@ -244,6 +322,9 @@ class AnnotationsDataImpl implements AnnotationsData {
244322
Iterable<FunctionEntity> disableFinalFunctions =
245323
source.readMembers<FunctionEntity>(emptyAsNull: true) ??
246324
const <FunctionEntity>[];
325+
Iterable<FieldEntity> noElisionFields =
326+
source.readMembers<FieldEntity>(emptyAsNull: true) ??
327+
const <FieldEntity>[];
247328
Iterable<FunctionEntity> cannotThrowFunctions =
248329
source.readMembers<FunctionEntity>(emptyAsNull: true) ??
249330
const <FunctionEntity>[];
@@ -258,6 +339,7 @@ class AnnotationsDataImpl implements AnnotationsData {
258339
nonInlinableFunctions,
259340
tryInlineFunctions,
260341
disableFinalFunctions,
342+
noElisionFields,
261343
cannotThrowFunctions,
262344
sideEffectFreeFunctions,
263345
assumeDynamicMembers);
@@ -268,6 +350,7 @@ class AnnotationsDataImpl implements AnnotationsData {
268350
sink.writeMembers(nonInlinableFunctions);
269351
sink.writeMembers(tryInlineFunctions);
270352
sink.writeMembers(disableFinalFunctions);
353+
sink.writeMembers(noElisionFields);
271354
sink.writeMembers(cannotThrowFunctions);
272355
sink.writeMembers(sideEffectFreeFunctions);
273356
sink.writeMembers(assumeDynamicMembers);
@@ -279,6 +362,7 @@ class AnnotationsDataBuilder implements AnnotationsData {
279362
List<FunctionEntity> _nonInlinableFunctions;
280363
List<FunctionEntity> _tryInlinableFunctions;
281364
List<FunctionEntity> _disableFinalFunctions;
365+
List<FieldEntity> _noElisionFields;
282366
List<FunctionEntity> _cannotThrowFunctions;
283367
List<FunctionEntity> _sideEffectFreeFunctions;
284368
List<MemberEntity> _trustTypeAnnotationsMembers;
@@ -299,6 +383,11 @@ class AnnotationsDataBuilder implements AnnotationsData {
299383
_disableFinalFunctions.add(function);
300384
}
301385

386+
void markAsNoElision(FieldEntity field) {
387+
_noElisionFields ??= <FieldEntity>[];
388+
_noElisionFields.add(field);
389+
}
390+
302391
void registerCannotThrow(FunctionEntity function) {
303392
_cannotThrowFunctions ??= <FunctionEntity>[];
304393
_cannotThrowFunctions.add(function);
@@ -320,6 +409,8 @@ class AnnotationsDataBuilder implements AnnotationsData {
320409
_tryInlinableFunctions ?? const <FunctionEntity>[];
321410
Iterable<FunctionEntity> get disableFinalFunctions =>
322411
_disableFinalFunctions ?? const <FunctionEntity>[];
412+
Iterable<FieldEntity> get noElisionFields =>
413+
_noElisionFields ?? const <FieldEntity>[];
323414
Iterable<FunctionEntity> get cannotThrowFunctions =>
324415
_cannotThrowFunctions ?? const <FunctionEntity>[];
325416
Iterable<FunctionEntity> get sideEffectFreeFunctions =>

pkg/compiler/lib/src/js_backend/constant_emitter.dart

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,8 @@ class ConstantEmitter implements ConstantValueVisitor<jsAst.Expression, Null> {
224224
// are in the same order as the members of the class element.
225225
int emittedArgumentCount = 0;
226226
_worldBuilder.forEachInstanceField(classElement,
227-
(ClassEntity enclosing, FieldEntity field) {
227+
(ClassEntity enclosing, FieldEntity field, {bool isElided}) {
228+
if (isElided) return;
228229
if (field.name == JavaScriptMapConstant.LENGTH_NAME) {
229230
arguments
230231
.add(new jsAst.LiteralNumber('${constant.keyList.entries.length}'));
@@ -318,7 +319,9 @@ class ConstantEmitter implements ConstantValueVisitor<jsAst.Expression, Null> {
318319
jsAst.Expression constructor =
319320
_emitter.constructorAccess(constant.type.element);
320321
List<jsAst.Expression> fields = <jsAst.Expression>[];
321-
_worldBuilder.forEachInstanceField(element, (_, FieldEntity field) {
322+
_worldBuilder.forEachInstanceField(element, (_, FieldEntity field,
323+
{bool isElided}) {
324+
if (isElided) return;
322325
if (!_allocatorAnalysis.isInitializedInAllocator(field)) {
323326
fields.add(constantReferenceGenerator(constant.fields[field]));
324327
}

pkg/compiler/lib/src/js_backend/namer.dart

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2008,8 +2008,9 @@ class ConstantNamingVisitor implements ConstantValueVisitor {
20082008

20092009
// TODO(johnniwinther): This should be accessed from a codegen closed world.
20102010
codegenWorldBuilder.forEachInstanceField(constant.type.element,
2011-
(_, FieldEntity field) {
2011+
(_, FieldEntity field, {bool isElided}) {
20122012
if (failed) return;
2013+
if (isElided) return;
20132014
_visit(constant.fields[field]);
20142015
});
20152016
}
@@ -2147,7 +2148,8 @@ class ConstantCanonicalHasher implements ConstantValueVisitor<int, Null> {
21472148
int visitConstructed(ConstructedConstantValue constant, [_]) {
21482149
int hash = _hashString(3, constant.type.element.name);
21492150
codegenWorldBuilder.forEachInstanceField(constant.type.element,
2150-
(_, FieldEntity field) {
2151+
(_, FieldEntity field, {bool isElided}) {
2152+
if (isElided) return;
21512153
hash = _combine(hash, _visit(constant.fields[field]));
21522154
});
21532155
return hash;

pkg/compiler/lib/src/js_emitter/instantiation_stub_generator.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,8 @@ class InstantiationStubGenerator {
139139
// 2. Find the function field access path.
140140
FieldEntity functionField;
141141
_codegenWorldBuilder.forEachInstanceField(instantiationClass,
142-
(ClassEntity enclosing, FieldEntity field) {
142+
(ClassEntity enclosing, FieldEntity field, {bool isElided}) {
143+
if (isElided) return;
143144
if (field.name == '_genericClosure') functionField = field;
144145
});
145146
assert(functionField != null,

pkg/compiler/lib/src/js_emitter/model.dart

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -384,9 +384,18 @@ class Field {
384384

385385
final ConstantValue initializerInAllocator;
386386

387+
final bool isElided;
388+
387389
// TODO(floitsch): support renamed fields.
388-
Field(this.element, this.name, this.accessorName, this.getterFlags,
389-
this.setterFlags, this.needsCheckedSetter, this.initializerInAllocator);
390+
Field(
391+
this.element,
392+
this.name,
393+
this.accessorName,
394+
this.getterFlags,
395+
this.setterFlags,
396+
this.needsCheckedSetter,
397+
this.initializerInAllocator,
398+
this.isElided);
390399

391400
bool get needsGetter => getterFlags != 0;
392401
bool get needsUncheckedSetter => setterFlags != 0;

pkg/compiler/lib/src/js_emitter/program_builder/program_builder.dart

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1072,8 +1072,15 @@ class ProgramBuilder {
10721072
initializerInAllocator = _allocatorAnalysis.initializerValue(field);
10731073
}
10741074

1075-
fields.add(new Field(field, name, accessorName, getterFlags, setterFlags,
1076-
needsCheckedSetter, initializerInAllocator));
1075+
fields.add(new Field(
1076+
field,
1077+
name,
1078+
accessorName,
1079+
getterFlags,
1080+
setterFlags,
1081+
needsCheckedSetter,
1082+
initializerInAllocator,
1083+
_closedWorld.elidedFields.contains(field)));
10771084
}
10781085

10791086
FieldVisitor visitor = new FieldVisitor(_options, _elementEnvironment,

0 commit comments

Comments
 (0)