Fix registration of (un)provided parameters.
Change-Id: I5cf1b7543561a728a219a62c8fa6633d4d25846a
Reviewed-on: https://dart-review.googlesource.com/c/91223
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
diff --git a/pkg/compiler/lib/src/enqueue.dart b/pkg/compiler/lib/src/enqueue.dart
index 499f749..4defca5 100644
--- a/pkg/compiler/lib/src/enqueue.dart
+++ b/pkg/compiler/lib/src/enqueue.dart
@@ -76,6 +76,14 @@
}
abstract class Enqueuer {
+ /// If `true` the checking for unenqueued members is skipped. The current
+ /// implementation registers parameter usages as a side-effect so unit
+ /// testing of member usage we need to test both with and without the
+ /// enqueuer check.
+ // TODO(johnniwinther): [checkEnqueuerConsistency] should not have
+ // side-effects.
+ static bool skipEnqueuerCheckForTesting = false;
+
WorldBuilder get worldBuilder;
void open(ImpactStrategy impactStrategy, FunctionEntity mainMethod,
@@ -277,6 +285,7 @@
bool checkNoEnqueuedInvokedInstanceMethods(
ElementEnvironment elementEnvironment) {
+ if (Enqueuer.skipEnqueuerCheckForTesting) return true;
return checkEnqueuerConsistency(elementEnvironment);
}
diff --git a/pkg/compiler/lib/src/universe/member_usage.dart b/pkg/compiler/lib/src/universe/member_usage.dart
index b7d31e9..57481fb 100644
--- a/pkg/compiler/lib/src/universe/member_usage.dart
+++ b/pkg/compiler/lib/src/universe/member_usage.dart
@@ -332,12 +332,17 @@
return MemberUses.NONE;
}
bool alreadyHasInvoke = hasInvoke;
- _parameterUsage.invoke(callStructure);
+ bool hasPartialChange = _parameterUsage.invoke(callStructure);
+ EnumSet<MemberUse> result;
if (alreadyHasInvoke) {
- return MemberUses.NONE;
+ result = MemberUses.NONE;
+ } else {
+ result = _pendingUse
+ .removeAll(hasRead ? MemberUses.NONE : MemberUses.NORMAL_ONLY);
}
- return _pendingUse
- .removeAll(hasRead ? MemberUses.NONE : MemberUses.NORMAL_ONLY);
+ return hasPartialChange
+ ? result.union(MemberUses.PARTIAL_USE_ONLY)
+ : result;
}
@override
@@ -467,12 +472,17 @@
return MemberUses.NONE;
}
bool alreadyHasInvoke = hasInvoke;
- _parameterUsage.invoke(callStructure);
+ bool hasPartialChange = _parameterUsage.invoke(callStructure);
+ EnumSet<MemberUse> result;
if (alreadyHasInvoke) {
- return MemberUses.NONE;
+ result = MemberUses.NONE;
+ } else {
+ result = _pendingUse
+ .removeAll(hasRead ? MemberUses.NONE : MemberUses.NORMAL_ONLY);
}
- return _pendingUse
- .removeAll(hasRead ? MemberUses.NONE : MemberUses.NORMAL_ONLY);
+ return hasPartialChange
+ ? result.union(MemberUses.PARTIAL_USE_ONLY)
+ : result;
}
@override
@@ -495,7 +505,22 @@
}
/// Enum class for the possible kind of use of [MemberEntity] objects.
-enum MemberUse { NORMAL, CLOSURIZE_INSTANCE, CLOSURIZE_STATIC }
+enum MemberUse {
+ /// Read or write of a field, or invocation of a method.
+ NORMAL,
+
+ /// Tear-off of an instance method.
+ CLOSURIZE_INSTANCE,
+
+ /// Tear-off of a static method.
+ CLOSURIZE_STATIC,
+
+ /// Invocation that provides previously unprovided optional parameters.
+ ///
+ /// This is used to check that no partial use is missed by the enqueuer, as
+ /// asserted through the `Enqueuery.checkEnqueuerConsistency` method.
+ PARTIAL_USE
+}
/// Common [EnumSet]s used for [MemberUse].
class MemberUses {
@@ -510,6 +535,8 @@
const EnumSet<MemberUse>.fixed(3);
static const EnumSet<MemberUse> ALL_STATIC =
const EnumSet<MemberUse>.fixed(5);
+ static const EnumSet<MemberUse> PARTIAL_USE_ONLY =
+ const EnumSet<MemberUse>.fixed(8);
}
typedef void MemberUsedCallback(MemberEntity member, EnumSet<MemberUse> useSet);
@@ -685,11 +712,11 @@
/// invocation of the method or constructor.
bool _areAllTypeParametersProvided;
- /// The set of named parameters that have been provided in at least one
+ /// The set of named parameters that have not yet been provided in any
/// invocation of the method or constructor.
///
/// If all named parameters have been provided this is set to `null`.
- Set<String> _providedNamedParameters;
+ Set<String> _unprovidedNamedParameters;
ParameterUsage(this._parameterStructure) {
_hasInvoke = false;
@@ -699,32 +726,41 @@
? null
: 0;
if (!_parameterStructure.namedParameters.isEmpty) {
- _providedNamedParameters =
+ _unprovidedNamedParameters =
new Set<String>.from(_parameterStructure.namedParameters);
}
}
- void invoke(CallStructure callStructure) {
- if (isFullyUsed) return;
+ bool invoke(CallStructure callStructure) {
+ if (isFullyUsed) return false;
_hasInvoke = true;
+ bool changed = false;
if (_providedPositionalParameters != null) {
- _providedPositionalParameters = Math.max(
+ int newProvidedPositionalParameters = Math.max(
_providedPositionalParameters, callStructure.positionalArgumentCount);
+ changed |=
+ newProvidedPositionalParameters != _providedPositionalParameters;
+ _providedPositionalParameters = newProvidedPositionalParameters;
if (_providedPositionalParameters >=
_parameterStructure.positionalParameters) {
_providedPositionalParameters = null;
}
}
- if (_providedNamedParameters != null &&
+ if (_unprovidedNamedParameters != null &&
callStructure.namedArguments.isNotEmpty) {
- _providedNamedParameters.removeAll(callStructure.namedArguments);
- if (_providedNamedParameters.isEmpty) {
- _providedNamedParameters = null;
+ int _providedNamedParametersCount = _unprovidedNamedParameters.length;
+ _unprovidedNamedParameters.removeAll(callStructure.namedArguments);
+ changed |=
+ _providedNamedParametersCount != _unprovidedNamedParameters.length;
+ if (_unprovidedNamedParameters.isEmpty) {
+ _unprovidedNamedParameters = null;
}
}
if (!_areAllTypeParametersProvided && callStructure.typeArgumentCount > 0) {
_areAllTypeParametersProvided = true;
+ changed = true;
}
+ return changed;
}
bool get hasInvoke => _hasInvoke;
@@ -732,13 +768,13 @@
bool get isFullyUsed =>
_hasInvoke &&
_providedPositionalParameters == null &&
- _providedNamedParameters == null &&
+ _unprovidedNamedParameters == null &&
_areAllTypeParametersProvided;
void fullyUse() {
_hasInvoke = true;
_providedPositionalParameters = null;
- _providedNamedParameters = null;
+ _unprovidedNamedParameters = null;
_areAllTypeParametersProvided = true;
}
@@ -749,10 +785,10 @@
_parameterStructure.requiredParameters,
_providedPositionalParameters ??
_parameterStructure.positionalParameters,
- _providedNamedParameters == null
+ _unprovidedNamedParameters == null
? _parameterStructure.namedParameters
: _parameterStructure.namedParameters
- .where((n) => !_providedNamedParameters.contains(n))
+ .where((n) => !_unprovidedNamedParameters.contains(n))
.toList(),
_areAllTypeParametersProvided ? _parameterStructure.typeParameters : 0);
}
diff --git a/pkg/compiler/lib/src/universe/resolution_world_builder.dart b/pkg/compiler/lib/src/universe/resolution_world_builder.dart
index c3e3aa3..62bce9b 100644
--- a/pkg/compiler/lib/src/universe/resolution_world_builder.dart
+++ b/pkg/compiler/lib/src/universe/resolution_world_builder.dart
@@ -640,13 +640,13 @@
void _process(
Map<String, Set<MemberUsage>> memberMap,
EnumSet<MemberUse> action(MemberUsage usage),
- bool shouldBeRemove(MemberUsage usage)) {
+ bool shouldBeRemoved(MemberUsage usage)) {
_processSet(memberMap, methodName, (MemberUsage usage) {
if (selector.appliesUnnamed(usage.entity) &&
_selectorConstraintsStrategy.appliedUnnamed(
dynamicUse, usage.entity, this)) {
memberUsed(usage.entity, action(usage));
- return true;
+ return shouldBeRemoved(usage);
}
return false;
});
@@ -848,7 +848,9 @@
map[memberName] = new Set<MemberUsage>();
Set<MemberUsage> remaining = new Set<MemberUsage>();
for (MemberUsage usage in members) {
- if (!updateUsage(usage)) remaining.add(usage);
+ if (!updateUsage(usage)) {
+ remaining.add(usage);
+ }
}
map[memberName].addAll(remaining);
}
diff --git a/pkg/compiler/lib/src/util/enumset.dart b/pkg/compiler/lib/src/util/enumset.dart
index 97df1a4..e292aa6 100644
--- a/pkg/compiler/lib/src/util/enumset.dart
+++ b/pkg/compiler/lib/src/util/enumset.dart
@@ -54,6 +54,12 @@
return new EnumSet.fromValue(value & other.value);
}
+ /// Returns a new set containing all values either in this set or in the
+ /// [other] set.
+ EnumSet<E> union(EnumSet<E> other) {
+ return new EnumSet.fromValue(value | other.value);
+ }
+
/// Returns a new set containing all values in this set that are not in the
/// [other] set.
EnumSet<E> minus(EnumSet<E> other) {
diff --git a/tests/compiler/dart2js/member_usage/member_usage_test.dart b/tests/compiler/dart2js/member_usage/member_usage_test.dart
index 0590221..32853b9 100644
--- a/tests/compiler/dart2js/member_usage/member_usage_test.dart
+++ b/tests/compiler/dart2js/member_usage/member_usage_test.dart
@@ -6,6 +6,7 @@
import 'package:async_helper/async_helper.dart';
import 'package:compiler/src/compiler.dart';
import 'package:compiler/src/elements/entities.dart';
+import 'package:compiler/src/enqueue.dart';
import 'package:compiler/src/ir/util.dart';
import 'package:compiler/src/kernel/kernel_strategy.dart';
import 'package:compiler/src/universe/member_usage.dart';
@@ -18,7 +19,15 @@
main(List<String> args) {
asyncTest(() async {
Directory dataDir = new Directory.fromUri(Platform.script.resolve('data'));
- await checkTests(dataDir, const ClosedWorldDataComputer(),
+ print('------------------------------------------------------------------');
+ print(' Test with enqueuer checks');
+ print('------------------------------------------------------------------');
+ await checkTests(dataDir, const ClosedWorldDataComputer(false),
+ args: args, testOmit: false, testFrontend: true);
+ print('------------------------------------------------------------------');
+ print(' Test without enqueuer checks');
+ print('------------------------------------------------------------------');
+ await checkTests(dataDir, const ClosedWorldDataComputer(true),
args: args, testOmit: false, testFrontend: true);
});
}
@@ -30,7 +39,14 @@
}
class ClosedWorldDataComputer extends DataComputer<Features> {
- const ClosedWorldDataComputer();
+ final bool skipEnqueuerCheck;
+
+ const ClosedWorldDataComputer(this.skipEnqueuerCheck);
+
+ @override
+ void setup() {
+ Enqueuer.skipEnqueuerCheckForTesting = skipEnqueuerCheck;
+ }
@override
void computeMemberData(Compiler compiler, MemberEntity member,
