Since 'DartType's overload equality with a structural comparison, we cannot
store metadata for them in a 'Map', as is done for 'TreeNode's. Since there is
no use-case yet for storing metadata on types, we remove this ability and the
restrict the metadata test.
Bug:
Change-Id: I825f55a5fd48da2c615b3154f909dd666baf5a33
Reviewed-on: https://dart-review.googlesource.com/14640
Commit-Queue: Samir Jindel <sjindel@google.com>
Reviewed-by: Kevin Millikin <kmillikin@google.com>
This method is no longer needed, since the front end iterates through
members directly to find cross overrides.
Change-Id: Ibc5180962a097c4d6194dbf4cbd1a0dea15050c2
Reviewed-on: https://dart-review.googlesource.com/13582
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This separates compiling platform.dill files from the patch_sdk.dart
script. The motivation for that is that I'm working on reading patch
files directly from Fasta, so we can completely remove the build step
for generating patched_sdk and dart2js_patched_sdk.
Short-term this should allow Paul to add a strong-mode version of
platform.dill without causing to many conflicts with my work on
patches.
Change-Id: I1150845b2986348d4fffe27092701d8a9b57ea54
Reviewed-on: https://dart-review.googlesource.com/11506
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
This allows creating strong mode TypeEnvironments without actually subclassing
it.
Make SubtypeTester.isBottom treat Null as Bottom in strong mode.
Bug:
Change-Id: Icdd1a4b736ce0fe839a6ef30cf24487111f32d25
Reviewed-on: https://dart-review.googlesource.com/11882
Reviewed-by: Paul Berry <paulberry@google.com>
This section contains a linear mapping between offsets of serialized nodes
and associated opaque binary metadata.
Note: this CL does not yet update C++ reader and only implements metadata in Kernel package.
C++ implementation will be updated if we agree that format seems flexible enough.
Bug:
Change-Id: Id433458afc6c2ea76c72f6a1901b9dc55b8f1696
Reviewed-on: https://dart-review.googlesource.com/9340
Reviewed-by: Jens Johansen <jensj@google.com>
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
1. We no longer remove type parameters on functions.
2. We no longer remove type arguments at call sites.
3. We allow non-captured function type parameters to be used outside closures.
Bug:
Change-Id: I116ec54c90b04be90e1157042c22797e57a7c51c
Reviewed-on: https://dart-review.googlesource.com/9342
Commit-Queue: Samir Jindel <sjindel@google.com>
Reviewed-by: Peter von der Ahé <ahe@google.com>
This CL adds the ability to create a list of "forwarding nodes" for a
source class. A forwarding node is a data structure that will later
be resolved to either an explicitly declared member in the class or a
superclass, or to a forwarding stub. The idea is that we will create
the forwarding nodes at the time of outline building, and later,
during type inference, we will resolve each forwarding node as it is
encountered.
The reason we need to defer resolution of the forwarding nodes until
inference is because we may need to use the results of type inference
to determine which member a given forwarding node resolves to. For
example:
num f() => 1;
class A {
final x = 1; // Inferred type: int
}
class B {
final x = f(); // Inferred type: num
}
abstract class C implements A, B {}
We cannot determine at the time of building the outline for C whether
it inherits its x from A or B, because we need the results of type
inference to determine which of the two x's has a more specific type.
Note that some refactoring of ClassHierarchy was necessary in order to
allow the front end to maintain member lists in the same order used
internally by ClassHierarchy. This will let us avoid unnecessary
redundant sorting of methods.
Change-Id: Iee754957e0ad3b16c4b60608e17a4a7b0006dfb4
Reviewed-on: https://dart-review.googlesource.com/7851
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
This will allow tests that use batch_util.dart to be run inside
google3, where package layout conventions are more strictly enforced
(files in test/ cannot import files in bin/ or vice versa).
Change-Id: I046b864bc3b1c4e78b984b0047b7567873146c52
Reviewed-on: https://dart-review.googlesource.com/7340
Reviewed-by: Samir Jindel <sjindel@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Note that the parser used for unit tests in kernel didn't have a
notation for promoted bounds. I chose `T & A` to mean "type parameter
T with promoted bound A", following Leaf's convention (the rationale
is that semantically, a promoted bound functions like an intersection
type).
It was also necessary to change the test infrastructure so that we
could specify the (non-promoted) bounds of type parameters when
running a subtype test.
Fixes#30833.
Change-Id: Ic62722df63c5ed5288b23560745a1cd0869b63fb
Reviewed-on: https://dart-review.googlesource.com/7628
Reviewed-by: Peter von der Ahé <ahe@google.com>
Reviewed-by: Leaf Petersen <leafp@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This CL adds a new target for kernel front-end, vm_precompiler. This is
an experimental target for new Dart VM precompiler pipeline which
will fully exploit strong mode type system and perform whole-program
optimizations.
As an example of such whole-program optimization, this CL adds
draft implementation of devirtualization of method invocations, which
uses single target computed by closed-world class hierarchy analysis.
Corresponding null checks (required for correctness) are not generated
yet.
Issue: https://github.com/dart-lang/sdk/issues/30480
Change-Id: I704cd16843a08f036a188b1188c80ee4dfbeab3b
Reviewed-on: https://dart-review.googlesource.com/3402
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
This reverts commit e81deebfd8.
My reasoning in the above commit was wrong. Consider the following code:
class A {
void foo() {}
}
abstract class B extends A {
void foo([x]);
}
class C extends B {}
main() {
B b = new C();
b.foo(42); // BAD: A.foo can't accept arguments.
}
To ensure soundness, this code needs to be disallowed, and the current
mechanism for doing that is to use forEachOverridePair. Note that
forEachOverridePair is a bit of a misnomer; in addition to yielding
all pairs of methods (M1, M2) for which M1 overrides M2, it also
yields pairs of methods (M1, M2) for which the target class inherits
the concrete implementation M1, and M2 is part of the interface.
Technically this latter case is not an "override" but rather an
"implementation" (thanks to Lasse for pointing out this distinction).
However in both cases we need to do the same compile-time check to
ensure soundness: we need to check that the type of M1 is a subtype of
M2 (unless the check is suppressed by a "covariant" keyword). Hence
it makes sense for forEachOverridePair to cover both cases.
To ensure that the above example is properly rejected, it is crucial
that some invocation of forEachOverridePair yield the pair (A.foo,
B.foo). Prior to e81deebfd8,
forEachOverridePair(B) would not yield this pair, but
forEachOverridePair(C) would. After
e81deebfd8, both calls yield this pair.
When I made e81deebfd8, I failed to
notice that forEachOverridePair(C) would yield the pair, so I thought
there was a problem. So my "fix" was unnecessary. And it created a
fresh problem: it meant that the following code would be disallowed:
class A {
void foo() {}
}
abstract class B extends A {
void foo([x]);
}
class C extends B {
void foo([x]) {}
}
main() {
B b = new C();
b.foo(42); // OK: C.foo can accept an argument.
}
There is no a priori soundness reason for rejecting this code, and
according to Lasse, it has not yet been decided whether Dart 2.0 will
allow it.
This CL restores the old behavior. Rather than remove the test case
in e81deebfd8, it modifies it to
demonstrate why the old behavior was correct.
R=scheglov@google.com
Review-Url: https://codereview.chromium.org/3004023002 .
ClassHierarchy.forEachOverridePair contains special logic for unusual
cases like this one:
class A {
void foo() {}
}
class B extends A {
void foo();
}
main() {
B b = new B();
b.foo();
}
In this case, A.foo is considered to override B.foo (contrary to the
usual situation where the derived class method overrides the
superclass method). The reasoning is that calling foo on a concrete
instance of B will cause A.foo to be executed (as illustrated in
main); therefore A.foo is callable via the interface of B.foo, thus in
a sense A.foo "overrides" B.foo.
The code contained a questionable optimization, however; it only
executed this special logic if the derived class was concrete.
Presuambly the reasoning was that if B were abstract, then a concrete
instance of B could never be created, so this situation could never
arise.
However, there is nothing to stop a concrete class from being derived
from B, e.g.:
class A {
void foo() {}
}
abstract class B extends A {
void foo();
}
class C extends B {}
main() {
B b = new C();
b.foo();
}
Now, calling foo on a concrete instance of C will cause A.foo to be
executed (as illustrated in main); therefore A.foo is callable via the
interface of B.foo, as before. So we still need to report this as an
override pair even though B is abstract.
R=ahe@google.com, scheglov@google.com
Review-Url: https://codereview.chromium.org/2998383002 .
Summary:
Previously, we use the "Vector" type in the kernel tree for the result of the
"VectorCreation" operation as as the parameter type for converted closure
functions. In the VM, we use the "Context" type instead, which the VM treats a
little differently than normal Dart-visible types, and it doesn't not handle
type checks against it correctly, breaking all closure converted code running in
checked mode.
Now, Since we are forced to use dynamic to represent the types of elements of
the context, we may as well just use dynamic for the context type itself.
Previously, we did not correct handle converted closure type checks for closures
that capture type parameters. The way we handled these type checks also had
several latent bugs that prevented type parameters being handled properly.
Now, we handle type parameters for converted closures similarly to normal
closures, and the places we treat them specially are fewer and more integrated
with the rest of the closure type checking code.
There is still a problem where assignments to captured variables are not
checked, because they are transformed to assignments into the context, whose
elements are necessarily untyped. This breaks many co19 tests, which expect type
errors on these assignments. The example below should error in checked mode, but
after closure conversion is runs with no errors.
int b;
bool c;
(() { b = c; })()
Test Plan:
- All test cases in "pkg/kernel/testcases/closures" now run in checked mode.
- Added a test "closures_types.dart" to check that captured type parameters are
handled correctly in the converted closures' signature types.
R=dmitryas@google.com
Review-Url: https://codereview.chromium.org/3007623002 .
The front end will need to use this method to iterate through the
interface of a class in order to determine when to create forwarding
stubs.
The functionality already exists; this CL merely exposes it and adds
tests.
R=scheglov@google.com
Review-Url: https://codereview.chromium.org/3003913002 .
Summary:
Previously, there was no support for generic methods in kernel. This prevented
us from being able to pass captured type arguments to the target top-level
function in converted closures, so these type arguments were always instantiated
to 'dynamic'.
Now, we save the type arguments to the closure creation operation in the
context, and read them out and forward them appropriately in closure wrapper
function. Since fasta doesn't currently support generic methods (their type
parameters are replaced by 'dynamic'), only top-level generic functions can
surface in kernel, as they are generated by closure conversion of closures that
capture type parameters of a class.
My focus here is enabling closure conversion to work in only these cases, and as
such, the code has some temporary "hacks" in the VM that may not work for
generic member functions or generic closures when they are enabled in fasta.
Test Plan:
I ran all the tests in closures/, and those which were previously expected to
crash due to missing VM support now pass and produce correct results.
Further testing is paused until we understand why the recent commit "[kernel]
Insert kernel bodies into VM heap" has broken all these tests.
Reviewers: regis@google.com, jensj@google.com, dmitryas@google.com
BUG=
R=dmitryas@google.com, jensj@google.com
Review-Url: https://codereview.chromium.org/2998803002 .
Summary:
Previously, the VM would crash when it encountered a ClosureCreation node
because it was not aware of the new type arguments field.
Now, it skips the type arguemnts field, which allows many tests to pass again,
even though it doesn't correct forward the type arguments at runtime.
Test Plan:
Removed expected failure lines for all the tests added in my prior
CL (introducing the new field).
BUG=
R=dmitryas@google.com, jensj@google.com
Reviewers: dmitryas@google.com
Review-Url: https://codereview.chromium.org/2987143002 .
Summary:
Previously, we filled in all occurrences of captured type variables with either
"dynamic" or their bound, if they had one.
Now, we add extra type parameters to the top-level function corresponding to the
closure, and pass in the corresponding arguments as type arguments to the
"MakeClosure" operation.
Test Plan:
Updated [type_variables.dart] and added a new test case to it.
R=dmitryas@google.com
Review-Url: https://codereview.chromium.org/2989563002 .
The closure-conversion transformation is not enabled yet. This commit
only adds the support for it to FlowGraphBuilder and
StreamingFlowGraphBuilder. More work should be done before enabling the
transformation; most mportantly, the 'platform.dill' file that is used
in the Kernel isolate and is loaded by VM for linking with executed
programs should be separated. The former should receive a file not
touched by the transformation, and the latter should receive a
transformed one.
BUG=
R=jensj@google.com, karlklose@google.com, kustermann@google.com
Review-Url: https://codereview.chromium.org/2891053003 .
Note the suite seemed to have a lot of tests crashing in verification.
With my change I'm noting other crashes, but I haven't investigated where they are from.
Also, to run the suite, before it used to be run as:
dart pkg/kernel/test/reify/suite.dart
Now it needs to be run as:
DART_CONFIGURATION=ReleaseX64 out/ReleaseX64/dart pkg/kernel/test/reify/suite.dart
BUG=
R=paulberry@google.com
Review-Url: https://codereview.chromium.org/2981813002 .
Summary:
Previously, we only handled `FieldInitializer` and `LocalInitializer`.
Now we handle all initializers.
Previously, we would create separate contexts for each initializers, which was
incorrect because it changes made to an argument from a closure within one
initializer would not be seen by a closure within another.
Now, we create the context in a `LocalInitializer` so all initializers will see
the same copy of the argument variables.
There is still an outstanding issue where variables introduced as local
initializers and later captured by closures in subsequent initializers are not
placed into the context. However, this will at least trigger an assert in the closure conversion pass.
Test Plan:
'closures_initializers/initializers.dart(.expect)' has been updated with very
simple test cases for super and redirecting initializers. The second bug
mentioned (captured local initializers) has not been reproduced yet.
BUG=
R=dmitryas@google.com
Review-Url: https://codereview.chromium.org/2981603002 .
Original CL had a bug that wasn't visible unless you delete your
out/ReleaseX64/patched_sdk folder.
Patchset #1 is the original CL, patchset #2 shows the fix.
This reverts commit 4aadfe09df.
BUG=
Review-Url: https://codereview.chromium.org/2976543002 .
Vyacheslav Egorov