Summary:
Previously, when a function parameter was captured both in an initializer and in
the body of a constructor, we would create two contexts: one created in a local
initializer and used by other initializers, and another in the body of the
function. This is incorrect, as it means that changes to the parameter in the
initializer's closure won't be visible in the body.
Now, to work around this problem we re-use the context created for the
initializers in the body of the constructor by moving the body into a new
constructor, and redirecting the original constructor to that one, passing the
context as an additional argument. This dance is necessary because local
initializers aren't visible in the body of a constructor.
Test Plan:
A few of the existing closure conversion tests were changed or fixed by this
revision. We also modify the 'closure_in_initializer.dart' test to hit this case
directly.
R=dmitryas@google.com
Reviewers: dmitryas@google.com
Review-Url: https://codereview.chromium.org/2991853002 .
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 .
It is informative in the same way as documentation.
Yesterday I tried to make Analyzer and Analysis Server stop using
FieldFormalParameterElement(s) and found one important use case requested
by the Flutter team. We need to show documentation of a field when
user requests documentation on the corresponding field formal named
parameter. This happens outside of the file that declares the parameter
and the field, so we don't have AST available. Of course we could
resolve the unit, but it would cost us something. And if we decide
that it is OK, then maybe we don't need to have documenation comments
in elements at all.
This is of course more convenience, and we could store documentation
and parameter to field mappings outside, like we store index. Just a
compromise - convenience vs. purity.
R=ahe@google.com, kmillikin@google.com, paulberry@google.com, sigmund@google.com
BUG=
Review-Url: https://codereview.chromium.org/2983173002 .
This review scraps the (currently disabled) code for converting tearoffs in the
closure conversion pass.
The closure conversion pass can only ever do a partial job with tearoffs, due to
the possibility of an unconverted library tearing off a method from any object
it likes. Partially converting [PropertyGet]s makes the closure conversion pass
slower and introduces a new method for any field or method anywhere with a name
used in any [PropertyGet], inflating code size and potentially regressing
performance. As it provides no concrete value in return we've decided to scrap
this aspect of the transformation. Anyway, creating closures for tearoffs is
much easier for a backend than converting anonymous or nested functions, since
there is only one object ("this") captured. Thus ignoring tear-offs does not
undermine the value of the transformation.
BUG=
R=dmitryas@google.com
Review-Url: https://codereview.chromium.org/2986553002 .
Proper sequencing of _asyncStackTraceHelper in Kernel
This helper function was being called before its argument was
initialized so it was passing null. Instead, it should be called
after its argument is initialized.
Because the initialization happens in Kernel code, it is simplest to
insert the call explicitly in Kernel code as well as part of the async
transformation. This has the consequence that we now call the helper
function even when the flag causal_async_stacks is false.
Fixes issue #29771.
Fixes issue #30178
Fixes issue #30058
BUG=
R=aam@google.com, asiva@google.com
Review-Url: https://codereview.chromium.org/2936793003 .
Review-Url: https://codereview.chromium.org/2982943002 .
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 .
This CL tweaks the public APIs in package:front_end, and
starts using those APIs outside the package. For example, this
removes 9 uses of DillTarget, so it is not longer mentioned
outside pkg/front_end and the analyzer_target.
Actual changes:
- in package:front_end
* added kernel_generator_impl: new file contains code that
used to be in kernel_generator. Code has some modifications:
it uses a single canonical-root when loading summaries, and
it supports generating both outlines and kernel in one go.
* removed code that didn't belong here:
a. most of calculating deps for .GN moved to patch_sdk
b. vm-specific outcomes moved to kernel-service
* updated how `native` is implemented, so we can more easily
support dart2js and ddc
* updated how we check where `int`, `bool`, etc can be implemented.
* added support "hermetic mode" in modular builds
('chaseDependencies = false' option)
* moved `trim` step out of fasta, and for now call it only within
the public API. This is not yet exposed, and I stopped covering it in
most tests (now only covered in shaker tests). The plan is to add
tests for the public API covering this in the future.
* removed `uriToSource` when serializing outlines
* added unit tests for public APIs
- patch_sdk
* use the public API to craete platform.dill, outline.dill (now
500K insted of 3Mb because it excludes sources), and vmservice_io.dill
* moved here logic internal to .GN
- kernel service
* use the public API
* moved here logic that depends on VM internals (e.g. status enum,
compilation results)
- package:compiler
* use the public API in tools and unit tests
* simplified patched-sdk generation: no more extending fasta's internals
- package:kernel
* fix bug in deserialization: initializers and other lists were
overwritten accidentally with external definitions.
* updated unit tests, moved shared logic to frontend/src/fasta/testing
R=johnniwinther@google.com, paulberry@google.com
Review-Url: https://codereview.chromium.org/2953703002 .
