Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(343)

Unified Diff: pkg/compiler/lib/src/js_backend/lookup_map_analysis.dart

Issue 1312943007: Add version validation for LookupMap, also add unittest directly in LookupMap. (Closed) Base URL: git@github.com:dart-lang/sdk.git@master
Patch Set: Created 5 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: pkg/compiler/lib/src/js_backend/lookup_map_analysis.dart
diff --git a/pkg/compiler/lib/src/js_backend/lookup_map_analysis.dart b/pkg/compiler/lib/src/js_backend/lookup_map_analysis.dart
index eeea65063e702941c21001c6340acc17cc68dc3c..06d8a8d91eab9ddb1a6dc78557ccbd5728ff9a15 100644
--- a/pkg/compiler/lib/src/js_backend/lookup_map_analysis.dart
+++ b/pkg/compiler/lib/src/js_backend/lookup_map_analysis.dart
@@ -7,6 +7,7 @@ library compiler.src.js_backend.lookup_map_analysis;
import '../common/registry.dart' show Registry;
import '../compiler.dart' show Compiler;
+import '../diagnostics/messages.dart' show MessageKind;
import '../constants/values.dart' show
ConstantValue,
ConstructedConstantValue,
@@ -14,8 +15,15 @@ import '../constants/values.dart' show
NullConstantValue,
TypeConstantValue;
import '../dart_types.dart' show DartType;
-import '../elements/elements.dart' show Elements, Element, ClassElement,
- FieldElement, FunctionElement, FunctionSignature;
+import '../elements/elements.dart' show
+ ClassElement,
+ Element,
+ Elements,
+ FieldElement,
+ FunctionElement,
+ FunctionSignature,
+ LibraryElement,
+ VariableElement;
import '../enqueue.dart' show Enqueuer;
import 'js_backend.dart' show JavaScriptBackend;
import '../dart_types.dart' show DynamicType, InterfaceType;
@@ -63,6 +71,13 @@ class LookupMapAnalysis {
/// discover that a key in a map is potentially used.
final JavaScriptBackend backend;
+ /// The resolved [VariableElement] associated with the top-level `_version`.
+ VariableElement lookupMapVersionVariable;
+
+ /// The resolved [LibraryElement] associated with
+ /// `package:lookup_map/lookup_map.dart`.
+ LibraryElement lookupMapLibrary;
+
/// The resolved [ClassElement] associated with `LookupMap`.
ClassElement typeLookupMapClass;
@@ -101,9 +116,7 @@ class LookupMapAnalysis {
final _pending = <ConstantValue, List<_LookupMapInfo>>{};
/// Whether the backend is currently processing the codegen queue.
- // TODO(sigmund): is there a better way to do this. Do we need to plumb the
- // enqueuer on each callback?
- bool get _inCodegen => backend.compiler.phase == Compiler.PHASE_COMPILING;
+ bool _inCodegen = false;
LookupMapAnalysis(this.backend);
@@ -114,14 +127,51 @@ class LookupMapAnalysis {
return typeLookupMapClass != null;
}
- /// Initializes this analysis by providing the resolver information of
- /// `LookupMap`.
- void initRuntimeClass(ClassElement cls) {
+ /// Initializes this analysis by providing the resolved library. This is
+ /// invoked during resolution when the `lookup_map` library is discovered.
+ void init(LibraryElement library) {
+ lookupMapLibrary = library;
+ // We will enable the lookupMapAnalysis as long as we get a known version of
+ // the lookup_map package. We otherwise produce a warning.
+ lookupMapVersionVariable = library.implementation.findLocal('_version');
floitsch 2015/09/07 10:48:02 I think I would have gone with an annotation, but
+ if (lookupMapVersionVariable == null) {
+ backend.compiler.reportInfo(library,
+ MessageKind.UNRECOGNIZED_VERSION_OF_LOOKUP_MAP);
+ } else {
+ backend.compiler.enqueuer.resolution.addToWorkList(
+ lookupMapVersionVariable);
+ }
+ }
+
+ /// Checks if the version of lookup_map is valid, and if so, enable this
+ /// analysis during codegen.
+ void onCodegenStart() {
+ _inCodegen = true;
+ // At this point, the lookupMapVersionVariable should be resolved and it's
+ // constant value should be available.
+ ConstantValue value =
+ backend.constants.getConstantValueForVariable(lookupMapVersionVariable);
+ if (value == null) {
+ backend.compiler.reportInfo(lookupMapVersionVariable,
+ MessageKind.UNRECOGNIZED_VERSION_OF_LOOKUP_MAP);
+ return;
+ }
+
+ // TODO(sigmund): add proper version resolution using the pub_semver package
+ // when we introduce the next version.
+ String version = value.primitiveValue.slowToString();
+ if (version != '0.0.1') {
+ backend.compiler.reportInfo(lookupMapVersionVariable,
+ MessageKind.UNRECOGNIZED_VERSION_OF_LOOKUP_MAP);
+ return;
+ }
+
+ ClassElement cls = lookupMapLibrary.findLocal('LookupMap');
cls.computeType(backend.compiler);
entriesField = cls.lookupMember('_entries');
keyField = cls.lookupMember('_key');
valueField = cls.lookupMember('_value');
- // TODO(sigmund): Maybe inline nested maps make the output code smaller?
+ // TODO(sigmund): Maybe inline nested maps to make the output code smaller?
typeLookupMapClass = cls;
}

Powered by Google App Engine
This is Rietveld 408576698