Skip to content

Commit 66f19e6

Browse files
johnniwintherjohnniwinther
committed
More equivalence testing
- including a fix in checkLists! [email protected] Review-Url: https://codereview.chromium.org/2979783003 .
1 parent c894d8b commit 66f19e6

File tree

6 files changed

+153
-23
lines changed

6 files changed

+153
-23
lines changed

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

Lines changed: 50 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,10 @@ class Holder {
8989

9090
Holder(this.name, this.index,
9191
{this.isStaticStateHolder: false, this.isConstantsHolder: false});
92+
93+
String toString() {
94+
return 'Holder(name=${name})';
95+
}
9296
}
9397

9498
/**
@@ -144,6 +148,10 @@ class MainFragment extends Fragment {
144148
staticLazilyInitializedFields, constants);
145149

146150
bool get isMainFragment => true;
151+
152+
String toString() {
153+
return 'MainFragment()';
154+
}
147155
}
148156

149157
/**
@@ -164,6 +172,10 @@ class DeferredFragment extends Fragment {
164172
staticLazilyInitializedFields, constants);
165173

166174
bool get isMainFragment => false;
175+
176+
String toString() {
177+
return 'DeferredFragment(name=${name})';
178+
}
167179
}
168180

169181
class Constant {
@@ -172,6 +184,10 @@ class Constant {
172184
final ConstantValue value;
173185

174186
Constant(this.name, this.holder, this.value);
187+
188+
String toString() {
189+
return 'Constant(name=${name},value=${value.toStructuredText()})';
190+
}
175191
}
176192

177193
abstract class FieldContainer {
@@ -191,6 +207,10 @@ class Library implements FieldContainer {
191207

192208
Library(this.element, this.uri, this.statics, this.classes,
193209
this.staticFieldsForReflection);
210+
211+
String toString() {
212+
return 'Library(uri=${uri},element=${element})';
213+
}
194214
}
195215

196216
class StaticField {
@@ -208,6 +228,10 @@ class StaticField {
208228

209229
StaticField(this.element, this.name, this.holder, this.code, this.isFinal,
210230
this.isLazy);
231+
232+
String toString() {
233+
return 'StaticField(name=${name},element=${element})';
234+
}
211235
}
212236

213237
class Class implements FieldContainer {
@@ -292,7 +316,7 @@ class Class implements FieldContainer {
292316
int get superclassHolderIndex =>
293317
(superclass == null) ? 0 : superclass.holder.index;
294318

295-
String toString() => 'Class(${element.name})';
319+
String toString() => 'Class(name=${name},element=$element)';
296320
}
297321

298322
class MixinApplication extends Class {
@@ -335,6 +359,8 @@ class MixinApplication extends Class {
335359
void setMixinClass(Class mixinClass) {
336360
_mixinClass = mixinClass;
337361
}
362+
363+
String toString() => 'Mixin(name=${name},element=$element)';
338364
}
339365

340366
/// A field.
@@ -378,6 +404,10 @@ class Field {
378404

379405
bool get needsInterceptedGetterOnThis => getterFlags == 3;
380406
bool get needsInterceptedSetterOnThis => setterFlags == 3;
407+
408+
String toString() {
409+
return 'Field(name=${name},element=${element})';
410+
}
381411
}
382412

383413
abstract class Method {
@@ -392,11 +422,6 @@ abstract class Method {
392422
final js.Expression code;
393423

394424
Method(this.element, this.name, this.code);
395-
396-
String toString() {
397-
return 'method[name=${name},element=${element}'
398-
',code=${js.nodeToString(code)}]';
399-
}
400425
}
401426

402427
/// A method that corresponds to a method in the original Dart program.
@@ -481,6 +506,11 @@ class InstanceMethod extends DartMethod {
481506
}
482507

483508
bool get isStatic => false;
509+
510+
String toString() {
511+
return 'InstanceMethod(name=${name},element=${element}'
512+
',code=${js.nodeToString(code)})';
513+
}
484514
}
485515

486516
/// A method that is generated by the backend and has not direct correspondence
@@ -491,7 +521,8 @@ class StubMethod extends Method {
491521
: super(element, name, code);
492522

493523
String toString() {
494-
return 'stub[name=${name},element=${element},code=${js.nodeToString(code)}]';
524+
return 'StubMethod(name=${name},element=${element}'
525+
',code=${js.nodeToString(code)})';
495526
}
496527
}
497528

@@ -514,6 +545,11 @@ class ParameterStubMethod extends StubMethod {
514545

515546
ParameterStubMethod(js.Name name, this.callName, js.Expression code)
516547
: super(name, code);
548+
549+
String toString() {
550+
return 'ParameterStubMethod(name=${name},element=${element}'
551+
',code=${js.nodeToString(code)})';
552+
}
517553
}
518554

519555
abstract class StaticMethod implements Method {
@@ -549,12 +585,18 @@ class StaticDartMethod extends DartMethod implements StaticMethod {
549585
bool get isStatic => true;
550586

551587
String toString() {
552-
return 'static_method[name=${name},element=${element}}]';
588+
return 'StaticDartMethod(name=${name},element=${element}'
589+
',code=${js.nodeToString(code)})';
553590
}
554591
}
555592

556593
class StaticStubMethod extends StubMethod implements StaticMethod {
557594
Holder holder;
558595
StaticStubMethod(js.Name name, this.holder, js.Expression code)
559596
: super(name, code);
597+
598+
String toString() {
599+
return 'StaticStubMethod(name=${name},element=${element}}'
600+
',code=${js.nodeToString(code)})';
601+
}
560602
}

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1085,7 +1085,6 @@ class ProgramBuilder {
10851085
}
10861086
}
10871087
}
1088-
10891088
fields.add(new Field(field, name, accessorName, getterFlags, setterFlags,
10901089
needsCheckedSetter));
10911090
}

‎pkg/compiler/lib/src/kernel/element_map_impl.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,8 +319,8 @@ abstract class KernelToElementMapBase extends KernelToElementMapBaseMixin {
319319
Link<InterfaceType> interfaces = linkBuilder.toLink();
320320
OrderedTypeSetBuilder setBuilder =
321321
new _KernelOrderedTypeSetBuilder(this, cls);
322-
data.orderedTypeSet =
323-
setBuilder.createOrderedTypeSet(data.supertype, interfaces);
322+
data.orderedTypeSet = setBuilder.createOrderedTypeSet(
323+
data.supertype, interfaces.reverse());
324324
data.interfaces = new List<InterfaceType>.from(interfaces.toList());
325325
}
326326
}

‎pkg/compiler/lib/src/kernel/env.dart

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,10 @@ class LibraryEnv {
108108
_memberMap = <String, ir.Member>{};
109109
_setterMap = <String, ir.Member>{};
110110
for (ir.Member member in library.members) {
111+
if (member.name.name.contains('#')) {
112+
// Skip synthetic .dill members.
113+
continue;
114+
}
111115
if (member is ir.Procedure) {
112116
if (member.kind == ir.ProcedureKind.Setter) {
113117
_setterMap[member.name.name] = member;
@@ -224,6 +228,10 @@ class ClassEnv {
224228

225229
void addMembers(ir.Class c, {bool includeStatic}) {
226230
for (ir.Member member in c.members) {
231+
if (member.name.name.contains('#')) {
232+
// Skip synthetic .dill members.
233+
continue;
234+
}
227235
if (member is ir.Constructor ||
228236
member is ir.Procedure &&
229237
member.kind == ir.ProcedureKind.Factory) {

‎tests/compiler/dart2js/equivalence/check_functions.dart

Lines changed: 92 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,15 @@ void checkNativeBasicData(NativeBasicDataImpl data1, NativeBasicDataImpl data2,
382382
data2.nativeClassTagInfo,
383383
strategy.elementEquivalence,
384384
(a, b) => a == b);
385-
// TODO(johnniwinther): Check the remaining properties.
385+
checkSetEquivalence(
386+
data1,
387+
data2,
388+
'jsInteropLibraries',
389+
data1.jsInteropLibraries,
390+
data2.jsInteropLibraries,
391+
strategy.elementEquivalence);
392+
checkSetEquivalence(data1, data2, 'jsInteropClasses', data1.jsInteropClasses,
393+
data2.jsInteropClasses, strategy.elementEquivalence);
386394
}
387395

388396
void checkBackendUsage(
@@ -442,7 +450,8 @@ void checkBackendUsage(
442450
}
443451

444452
checkElementEnvironment(
445-
ElementEnvironment env1, ElementEnvironment env2, TestStrategy strategy) {
453+
ElementEnvironment env1, ElementEnvironment env2, TestStrategy strategy,
454+
{bool checkConstructorBodies: false}) {
446455
strategy.testElements(
447456
env1, env2, 'mainLibrary', env1.mainLibrary, env2.mainLibrary);
448457
strategy.testElements(
@@ -459,6 +468,14 @@ checkElementEnvironment(
459468
env1.getMemberMetadata(member1),
460469
env2.getMemberMetadata(member2),
461470
strategy.testConstantValues);
471+
472+
// TODO(johnniwinther): Check function type. Currently hitting problems with
473+
// return types of generative constructors, function types referring to
474+
// typedefs and forwarding constructors.
475+
/*if (member1 is FunctionEntity && member2 is FunctionEntity) {
476+
check(member1, member2, 'getFunctionType', env1.getFunctionType(member1),
477+
env2.getFunctionType(member2), strategy.typeEquivalence);
478+
}*/
462479
}
463480

464481
checkSetEquivalence(env1, env2, 'libraries', env1.libraries, env2.libraries,
@@ -467,6 +484,8 @@ checkElementEnvironment(
467484
Expect.identical(lib1, env1.lookupLibrary(lib1.canonicalUri));
468485
Expect.identical(lib2, env2.lookupLibrary(lib2.canonicalUri));
469486

487+
// TODO(johnniwinther): Check libraryName.
488+
470489
List<ClassEntity> classes2 = <ClassEntity>[];
471490
env1.forEachClass(lib1, (ClassEntity cls1) {
472491
Expect.identical(cls1, env1.lookupClass(lib1, cls1.name));
@@ -496,19 +515,53 @@ checkElementEnvironment(
496515
env2.getSuperClass(cls2, skipUnnamedMixinApplications: true),
497516
strategy.elementEquivalence);
498517

518+
InterfaceType thisType1 = env1.getThisType(cls1);
519+
InterfaceType thisType2 = env2.getThisType(cls2);
520+
check(cls1, cls2, 'thisType', thisType1, thisType2,
521+
strategy.typeEquivalence);
522+
check(cls1, cls2, 'rawType', env1.getRawType(cls1), env2.getRawType(cls2),
523+
strategy.typeEquivalence);
524+
check(
525+
cls1,
526+
cls2,
527+
'createInterfaceType',
528+
env1.createInterfaceType(cls1, thisType1.typeArguments),
529+
env2.createInterfaceType(cls2, thisType2.typeArguments),
530+
strategy.typeEquivalence);
531+
532+
check(cls1, cls2, 'isGenericClass', env1.isGenericClass(cls1),
533+
env2.isGenericClass(cls2));
534+
check(cls1, cls2, 'isMixinApplication', env1.isMixinApplication(cls1),
535+
env2.isMixinApplication(cls2));
536+
check(
537+
cls1,
538+
cls2,
539+
'isUnnamedMixinApplication',
540+
env1.isUnnamedMixinApplication(cls1),
541+
env2.isUnnamedMixinApplication(cls2));
542+
check(
543+
cls1,
544+
cls2,
545+
'getEffectiveMixinClass',
546+
env1.getEffectiveMixinClass(cls1),
547+
env2.getEffectiveMixinClass(cls2),
548+
strategy.elementEquivalence);
549+
550+
// TODO(johnniwinther): Check type variable bounds.
551+
499552
List<InterfaceType> supertypes1 = <InterfaceType>[];
500553
env1.forEachSupertype(cls1, supertypes1.add);
501554
List<InterfaceType> supertypes2 = <InterfaceType>[];
502-
env2.forEachSupertype(cls2, supertypes1.add);
503-
strategy.testTypeLists(
504-
cls1, cls2, 'supertypes', supertypes1, supertypes2);
555+
env2.forEachSupertype(cls2, supertypes2.add);
556+
checkLists(supertypes1, supertypes2, 'supertypes on $cls1, $cls2',
557+
strategy.typeEquivalence);
505558

506559
List<ClassEntity> mixins1 = <ClassEntity>[];
507560
env1.forEachMixin(cls1, mixins1.add);
508561
List<ClassEntity> mixins2 = <ClassEntity>[];
509562
env2.forEachMixin(cls2, mixins2.add);
510-
strategy.testLists(
511-
cls1, cls2, 'mixins', mixins1, mixins2, strategy.elementEquivalence);
563+
checkLists(mixins1, mixins2, 'mixins on $cls1, $cls2',
564+
strategy.elementEquivalence);
512565

513566
Map<MemberEntity, ClassEntity> members1 = <MemberEntity, ClassEntity>{};
514567
Map<MemberEntity, ClassEntity> members2 = <MemberEntity, ClassEntity>{};
@@ -563,6 +616,29 @@ checkElementEnvironment(
563616
"Extra constructor $constructor2 in $cls2");
564617
});
565618

619+
if (checkConstructorBodies) {
620+
Set<ConstructorBodyEntity> constructorBodies1 =
621+
new Set<ConstructorBodyEntity>();
622+
Set<ConstructorBodyEntity> constructorBodies2 =
623+
new Set<ConstructorBodyEntity>();
624+
env1.forEachConstructorBody(cls1,
625+
(ConstructorBodyEntity constructorBody1) {
626+
constructorBodies1.add(constructorBody1);
627+
});
628+
env2.forEachConstructorBody(cls2,
629+
(ConstructorBodyEntity constructorBody2) {
630+
constructorBodies2.add(constructorBody2);
631+
});
632+
checkSetEquivalence(cls1, cls2, 'constructor-bodies',
633+
constructorBodies1, constructorBodies2, strategy.elementEquivalence,
634+
onSameElement: (ConstructorBodyEntity constructorBody1,
635+
ConstructorBodyEntity constructorBody2) {
636+
check(constructorBody1, constructorBody2, 'name',
637+
constructorBody1.name, constructorBody2.name);
638+
639+
checkMembers(constructorBody1, constructorBody2);
640+
});
641+
}
566642
classes2.add(cls2);
567643
});
568644
env2.forEachClass(lib2, (ClassEntity cls2) {
@@ -597,7 +673,7 @@ checkElementEnvironment(
597673
members2.contains(member2), "Extra member $member2 in $lib2");
598674
});
599675
});
600-
// TODO(johnniwinther): Test the remaining properties of [ElementEnvironment].
676+
// TODO(johnniwinther): Check getLocalFunctionType and getUnaliasedType ?
601677
}
602678

603679
bool areInstantiationInfosEquivalent(
@@ -894,6 +970,8 @@ void checkEmitterFragments(
894970
if (fragment1 is MainFragment && fragment2 is MainFragment) {
895971
check(fragment1, fragment2, 'invokeMain', fragment1.invokeMain,
896972
fragment2.invokeMain, areJsNodesEquivalent);
973+
} else if (fragment1 is DeferredFragment && fragment2 is DeferredFragment) {
974+
check(fragment1, fragment2, 'name', fragment1.name, fragment2.name);
897975
}
898976
}
899977

@@ -909,7 +987,7 @@ void checkEmitterLibraries(
909987
checkLists(
910988
library1.staticFieldsForReflection,
911989
library2.staticFieldsForReflection,
912-
'staticFieldsForReflection',
990+
'staticFieldsForReflection on $library1/$library2',
913991
(a, b) => a.name.key == b.name.key,
914992
onSameElement: checkEmitterFields);
915993
}
@@ -933,8 +1011,11 @@ void checkEmitterClasses(Class class1, Class class2, TestStrategy strategy) {
9331011
checkLists(class1.noSuchMethodStubs, class2.noSuchMethodStubs,
9341012
'noSuchMethodStubs', (a, b) => a.name.key == b.name.key,
9351013
onSameElement: (a, b) => checkEmitterMethods(a, b, strategy));
936-
checkLists(class1.staticFieldsForReflection, class2.staticFieldsForReflection,
937-
'staticFieldsForReflection', (a, b) => a.name.key == b.name.key,
1014+
checkLists(
1015+
class1.staticFieldsForReflection,
1016+
class2.staticFieldsForReflection,
1017+
'staticFieldsForReflection on $class1/$class2',
1018+
(a, b) => a.name.key == b.name.key,
9381019
onSameElement: checkEmitterFields);
9391020

9401021
check(class1, class2, 'superclassName', class1.superclassName?.key,

‎tests/compiler/dart2js/equivalence/check_helpers.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,7 @@ void checkLists(
406406
sb.write("\n Extra: \n ${extra.map(elementToString).join('\n ')}");
407407
}
408408
String message = sb.toString();
409-
if (mismatch.isNotEmpty) {
409+
if (mismatch.isNotEmpty || unfound.isNotEmpty || extra.isNotEmpty) {
410410
Expect.fail(message);
411411
} else if (verbose) {
412412
print(message);

0 commit comments

Comments
 (0)