I'm still chasing down a couple of google3 failures, but this is ready for review so that I can get it approved and land it once the last couple of failures are fixed. Here is who I think should review what:
athom@ for:
- DEPS
johnniwinther@ for:
- pkg/_fe_analyzer_shared
- pkg/front_end
- pkg/kernel
brianwilkerson@ for:
- pkg/analysis_server
- pkg/analyzer_plugin
- pkg/analyzer_utilities
kenzieschmoll@ for:
- pkg/dartdev
pquitslund@ for:
- pkg/linter/lib/src/lint_codes.g.dart
If you're not the right person, feel free to summon someone else.
Most of the changes here are re-running code generators that auto-format the code they generate. The changes are:
Roll in the latest dart_style commit:
- DEPS
Manually updated these test expectations to expect the new style:
- pkg/analysis_server/test/lsp/format_test.dart
- pkg/analysis_server/test/services/refactoring/agnostic/change_method_signature_test.dart
- pkg/analysis_server/test/src/domains/flutter/set_property_value_test.dart
- pkg/analysis_server/test/src/services/flutter/container_properties_test.dart
- pkg/analysis_server/test/src/services/flutter/widget_descriptions_test.dart
- pkg/analysis_server/test/src/services/refactoring/convert_selected_formal_parameters_to_named_test.dart
- pkg/analysis_server/test/src/services/refactoring/move_selected_formal_parameters_left_test.dart
This was auto-generated by something, but I'm not sure why it thinks there is a diff:
- pkg/analyzer_plugin/doc/api.html
Updated to require a version of dart_style that accepts a language version in DartFormatter():
- pkg/analyzer_plugin/pubspec.yaml
Updated to always use the latest language version (and thus new style) when invoking the formatter:
- pkg/analyzer_utilities/lib/tools.dart
Manually updated the formatting in the code templates so that they matched the formatting expected by the tests, which implicitly format using the latest language version:
- pkg/dartdev/lib/src/templates/...
The rest are all generated files that are formatted so re-generated using the new style:
- pkg/_fe_analyzer_shared/lib/src/experiments/flags.dart
- pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart
- pkg/analysis_server_client/lib/handler/notification_handler.dart
- pkg/analysis_server_client/lib/src/protocol/...
- pkg/analysis_server/lib/protocol/protocol_generated.dart
- pkg/analysis_server/test/integration/support/integration_test_methods.dart
- pkg/analysis_server/test/integration/support/protocol_matchers.dart
- pkg/analyzer_plugin/lib/protocol/...
- pkg/analyzer_plugin/test/integration/support/integration_test_methods.dart
- pkg/analyzer_plugin/test/integration/support/protocol_matchers.dart
- pkg/analyzer/lib/src/analysis_options/error/option_codes.g.dart
- pkg/analyzer/lib/src/dart/error/...
- pkg/analyzer/lib/src/error/codes.g.dart
- pkg/analyzer/lib/src/manifest/manifest_warning_code.g.dart
- pkg/analyzer/lib/src/pubspec/pubspec_warning_code.g.dart
- pkg/analyzer/lib/src/summary/format.dart
- pkg/analyzer/lib/src/wolf/ir/ir.g.dart
- pkg/compiler/test/tool/graph_isomorphizer/golden/less_simple/lib_000_1.dart
- pkg/compiler/test/tool/graph_isomorphizer/golden/less_simple/lib_001_0.dart
- pkg/compiler/test/tool/graph_isomorphizer/golden/less_simple/lib_010_0.dart
- pkg/compiler/test/tool/graph_isomorphizer/golden/less_simple/lib_100_0.dart
- pkg/compiler/test/tool/graph_isomorphizer/golden/simple/...
- pkg/front_end/lib/src/api_prototype/experimental_flags_generated.dart
- pkg/front_end/lib/src/codes/cfe_codes_generated.dart
- pkg/front_end/lib/src/util/parser_ast_helper.dart
- pkg/front_end/test/parser_test_listener.dart
- pkg/front_end/test/parser_test_parser.dart
- pkg/front_end/testcases/...
- pkg/kernel/lib/src/coverage.dart generated
- pkg/kernel/lib/src/equivalence.dart generated
- pkg/linter/lib/src/lint_codes.g.dart
Change-Id: Ice0141b763e63b84f54692cd19a442a0719673c6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/391263
Commit-Queue: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Alexander Thomas <athom@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
This reverts commit af2bea8162.
Reason for revert: Caused unexpected performance regressions.
Original change's description:
> [dart2wasm] Make int, double, bool box fields immutable
>
> Make the `value` fields of `BoxedInt`, `BoxedDouble` and `BoxedBool`
> `final`, to generate immutable Wasm fields for them.
>
> Immutable fields can potentially generate better code, as loads from the
> same object will always generate the same value.
>
> To allow making the `value` fields `final`, add a constructor.
>
> To allow adding a constructor, "implement` base classes instead of
> extending them. With `extends` the front-end wants us to call the
> superclass constructors, even though they don't have any constructors.
>
> Implementing `bool` (instead of extending) causes issues in TFA as it
> currently assumes bool literals are compiled as the `bool` class. Update
> TFA to treat bool literals the same way as `int` and `double` literals.
>
> Tested: existing tests
> Change-Id: I3282e188d784fa7a22421edc79ed47f9d85faf19
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/393320
> Commit-Queue: Ömer Ağacan <omersa@google.com>
> Reviewed-by: Martin Kustermann <kustermann@google.com>
Change-Id: I8bb49bcc4bd9cd01f3e8a692d7ba1bef889ab555
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/393840
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
Auto-Submit: Ömer Ağacan <omersa@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Make the `value` fields of `BoxedInt`, `BoxedDouble` and `BoxedBool`
`final`, to generate immutable Wasm fields for them.
Immutable fields can potentially generate better code, as loads from the
same object will always generate the same value.
To allow making the `value` fields `final`, add a constructor.
To allow adding a constructor, "implement` base classes instead of
extending them. With `extends` the front-end wants us to call the
superclass constructors, even though they don't have any constructors.
Implementing `bool` (instead of extending) causes issues in TFA as it
currently assumes bool literals are compiled as the `bool` class. Update
TFA to treat bool literals the same way as `int` and `double` literals.
Tested: existing tests
Change-Id: I3282e188d784fa7a22421edc79ed47f9d85faf19
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/393320
Commit-Queue: Ömer Ağacan <omersa@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
This optimization is based on the assumption (which at least holds true
for the dill file extracted from the mentioned bug) that only very few
nodes actually have metadata associated with it. For the test file, more
than two million node-offsets is looked up, and only ~7.5 thousand nodes
actually have metadata on it.
What is done here is to assume that most often we ask offsets in order
and that most often the answer is that there's no metadata (when we fall
into this case we can return `false` quickly) and we can skip looking up
the actual metadata. This furthermore - although not benchmark here -
should make it (a lot) faster in the case where we have multiple
subsections as we here only lookup in in one index. Only upon a `true`
response from that we have to lookup in all subsections to find and read
the actual metadata to associate.
Reading the test file without metadata, taking an average of averages
(average of 5 runs of the binary benchmarks output for
"AstFromBinaryEager") takes: 374481.4 us.
Before https://dart-review.googlesource.com/c/sdk/+/391761 reading the
test file with metadata takes: 627120.68 us.
This was about a quarter of a second, or 67+% slower.
With https://dart-review.googlesource.com/c/sdk/+/391761 this number
became: 441784.44 us.
A reduction of ~185 ms, or almost 30%, making the tax of reading
metadata "only" ~18%.
With this CL this number goes to 398011.44 us.
This is a further reduction of ~43 ms, or almost 10% (for a combined
reduction of ~230 ms or 36+%).
The tax of reading metadata is now ~6.3%.
Bug: https://github.com/flutter/flutter/issues/156713
Change-Id: I20bbabecfc8976293fe897528d05fc2145e2a8c0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/392021
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
On a flutter dill extracted via the mentioned bug I get these timings
when (once, i.e. I haven't done extra statistics) running
```
out/ReleaseX64/dart pkg/kernel/test/binary_bench2.dart --warmups=10 \
--iterations=5 --metadata AstFromBinaryEager <dill from flutter>
```
(and without `--metadata` for without metadata):
Without reading metadata: ~378 ms
With reading metadata before this CL: ~627 ms
With reading metadata with this CL: ~435 ms
Bug: https://github.com/flutter/flutter/issues/156713
Change-Id: Id6cb27bc00526ff61c48eeb66ebb86dff1b971a2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/391761
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
This CL:
* Makes the incremental compiler copy over the metadata just created for
the libraries compiled into the Component returned.
* Preserves metadata when serializing via the incremental serializer.
* Fixes loading a multi-component with metadata.
* Lets the VM copy all metadata in "_combinePendingDeltas"
when there is only one component to "combine".
It is generally my view, that to do better, metadata has to be redone
(among other things likely has to live on the Library instead).
https://github.com/dart-lang/sdk/issues/39302
With this CL:
```
rm foo.dill ; tools/sdks/dart-sdk/bin/dart pkg/frontend_server/bin/frontend_server_starter.dart --no-print-incremental-dependencies --target=vm --sdk-root=out/ReleaseX64 --platform=vm_platform_strong.dill --output-dill=foo.dill --packages=.dart_tool/package_config.json pkg/compiler/lib/src/dart2js.dart
tools/sdks/dart-sdk/bin/dart pkg/vm/bin/dump_kernel.dart foo.dill foo.dump
grep "@vm.call-site-attributes" foo.dump | wc -l
```
(i.e. a one-shot compile) prints 9500.
```
rm foo.dill ; tools/sdks/dart-sdk/bin/dart pkg/frontend_server/bin/frontend_server_starter.dart --incremental --no-incremental-serialization --no-print-incremental-dependencies --target=vm --sdk-root=out/ReleaseX64 --platform=vm_platform_strong.dill --output-dill=foo.dill --packages=.dart_tool/package_config.json pkg/compiler/lib/src/dart2js.dart
tools/sdks/dart-sdk/bin/dart pkg/vm/bin/dump_kernel.dart foo.dill foo.dump
grep "@vm.call-site-attributes" foo.dump | wc -l
```
(i.e. an incremental compile but without incremental serialization) prints 9500.
and
```
rm foo.dill ; tools/sdks/dart-sdk/bin/dart pkg/frontend_server/bin/frontend_server_starter.dart --incremental --no-print-incremental-dependencies --target=vm --sdk-root=out/ReleaseX64 --platform=vm_platform_strong.dill --output-dill=foo.dill --packages=.dart_tool/package_config.json pkg/compiler/lib/src/dart2js.dart
tools/sdks/dart-sdk/bin/dart pkg/vm/bin/dump_kernel.dart foo.dill foo.dump
grep "@vm.call-site-attributes" foo.dump | wc -l
```
(i.e. an incremental compile with incremental serialization) prints 9500.
If not deleting the dill file first (which will then initialize from it)
metadata might be different though.
If there are no changes and the incremental serializer was and is used
we should still get `9530` (because the bytes are just copied), but
otherwise we might get different results because it's not really
possible to otherwise pick-and-choose and/or merge the metadata.
TEST=Existing tests that it didn't get worse.
Change-Id: I19d530357e7e2a174d7408dbbf0ed7e0248a5b70
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/358444
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
This change combines function-handling logic from the analyzer's
`TypeConstraintGatherer._functionType0` and the CFE's
`TypeConstraintGatherer._isNullabilityAwareSubtypeMatch` methods into
`TypeConstraintGenerator.performSubtypeConstraintGenerationForFunctionTypes`,
which is in `_fe_analyzer_shared`.
The CFE and the analyzer have some pretty significant differences in
how they represent function types:
- In the analyzer, all function parameters are in a single
`parameters` list; each element of this list (of type
`ParameterElement`) can be queried to find out if it is named or
unnamed, and if it is required or optional. A convention enforced
partially by the `FunctionType` constructor is that the `parameters`
list stores reqired unnamed parameters first, then either optional
unnamed parameters or named parameters; named parameters are sorted
by name. The analyzer provides additional getters
`namedParameterTypes`, `normalParameterNames`,
`normalParameterTypes`, `optionalParameterNames`, and
`optionalParameterTypes`, which provide other views of this
information (for example, `namedParameterTypes` contains just the
named parameters, as a map from name to `ParameterElement`).
- In the CFE, unnamed and named parameters are in two separate lists
(`positionalParameters`, of type `List<DartType>`, and
`namedParameters`, of type `List<NamedType>`); in
`positionalParameters`, required parameters come before optional
ones. A single integer (`requiredParameterCount`) indicates how many
elements of `positionalParameters` are required, and by convention,
`namedParameters` is sorted by name.
In order to share logic between these representations, I had to come
up with a common API that these two representations could be easily
adapted to. The analyzer's representation proved to be easier to
adapt, so I based the common API mostly on the CFE's representation,
but with some name changes for clarity. The shared API is:
- `positionalParameterTypes` gets a list of positional parameter types
- `requiredPositionalParameterCount` tells how many entries in
`positionalParameterTypes` are required.
- `returnType` gets the function type's return type.
- `sortedNamedParameters` gets a list of information about named
parameters. The list elements are sorted by name, and each element
of this list is of type `FunctionParameterStructure` (a common
interface implemented both by the analyzer's `ParameterElement` and
the CFE's `NamedType`).
- `typeFormals` gets a list of the function type's formal type
parameters.
To minimize the performance impact of adapting the analyzer to this
API, the analyzer computes `positionalParameterTypes`,
`requiredPositionalParameterCount`, and `sortedNamedParameters` at the
time a `FunctionType` is constructed. Hopefully this should not be too
much of a performance hit, since doing so does not take too much more
effort than checking that the named parameters are sorted (which the
`FunctionType` constructor was already doing).
This is based on previous work by Chloe Stefantsova in
https://dart-review.googlesource.com/c/sdk/+/386480.
Change-Id: Iefe18d72771146399d81747ceab9c929516b0523
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/386322
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
This CL primarily adds a lint for fields with `StackTrace.current`
which is often useful for debugging, but shouldn't be left in.
* Add an ast visitor after the "explicit creation test",
finding - for now - `StackTrace.current` calls in fields.
This should have caught the left-in debug such in
https://dart-review.googlesource.com/c/sdk/+/379281.
* Renamed explicit_creation to compile_and_lint
* Remove (for some time) unused "smoke_test_quick" files.
* Add kernel to the compile_and_lint suite, but ignore it for explicit
creation (i.e. kernel will also be checked for fields
with `StackTrace.current`).
Change-Id: Ib886d23a8945e7063dc673a7f99cbb3a6adc1139
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/382361
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
This CL uses exhaustive switch statement instead of chained if
statements in the implementation of isSubtypeOf. In addition to that,
all of the individual methods for computing isSubtypeOf for specific
pairs of DartTypes are inlined into the switch statement.
Change-Id: I425e8d415d07767313c6f1ff796f7715e35f10bf
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/382000
Commit-Queue: Chloe Stefantsova <cstefantsova@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
This CL removes Type and TypeSchema type variables from the abstract
classes with shared code between the CFE and the analyzer. Extension
types SharedTypeView and SharedTypeSchemaView are declared to replace
the type variables.
The update propagates the discipline of distinguishing between types
and type schemas into the clients of the shared code. Now the code in
the CFE and the Analyzer that uses the shared code needs to statically
specify the interpretation of their type objects as either types or
type schemas.
Another benefit of the update is SharedTypeView and
SharedTypeSchemaView being less opaque than the Type and TypeSchema
type variables, which removes the necessity for some code duplication
in abstract methods for types and type schemas.
Finally, the update enables some further changes in the shared code
between the Analyzer and the CFE.
Change-Id: I88e8cfcd47d4f721974b4f2612521e85bb54c30f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/379302
Commit-Queue: Chloe Stefantsova <cstefantsova@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
This adds an explicit computation for the nullability of type variables
while taking cyclic dependencies into account. This removes the need
for post-processing of pending nullabilities.
Change-Id: Ic7c42eef8270610d3b4f1d27ea0067c80df88daa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/381242
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
SharedType and its subtypes now all declare one recursive type
variable Type with the bound SharedType<Type>. It allows to treate
SharedType and its subtypes as an abstract familty of types that has a
specific structure.
More specifically, the type variables Type and TypeSchema in the
abstract classes for shared algorithms between the Analyzer and the
CFE can now be defined recursively as extending SharedType<Type> and
SharedType<TypeSchema>, giving both types and type schemas the
structure of the family of types with the root at SharedType.
One of the benefits for that is that some abstract members become
unnecessary. For example, a type or type schema can now be tested for
having the shape of the type 'dynamic' with a direct is-check,
comparing them, correspondingly, with SharedType<Type> and
SharedType<TypeSchema>.
More importantly, having the family of recursive types lays the
foundation for further work on sharing the type structure between the
Analyzer and the CFE.
Change-Id: I67904f878668c035702092e8c21d3ce66f5ca469
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/378700
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Chloe Stefantsova <cstefantsova@google.com>
Turns out most things calling `Substitution.fromMap` first creates a map
of size 1, in which case a map doesn't seem like the right thing to use.
Also replace a few `growable: true` with `growable: false`.
When compiling the CFE with the CFE with an AOT snapshot, this CL:
instructions:u: -0.4582% +/- 0.0019% (-97936268.60 +/- 405215.88)
Change-Id: Ia1f5221cf84cd763139fcc5bd8518cbe7e575dfd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/375503
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
Dynamic interface annotator now automatically annotates classes and
members which are used implicitly:
* Targets of redirecting factories.
* Classes and members used in constants.
* Instance fields of classes used in instance constants or
having a callable const constructor.
* Everything used in the bodies of extendable mixin declarations and
mixin classes.
* Possible dispatch targets of instance calls
(overrides/implementations).
Front-end server now also exposes '--dynamic-interface' option,
similar to gen_kernel tool.
Also, this change includes a couple of minor fixes:
* Typo in ast.dart.
* In the front-end server, additional dill file specified with
'--import-dill' option is not ignored when '--link-platform' is
also used.
TEST=pkg/vm/testcases/transformations/dynamic_interface_annotator
Change-Id: I705b64efb1834dec6e0bdc5873025607f3472139
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/377761
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
This a LocalStack extension type for using a list as a typed stack and
uses this for local scopes in the body builder, instead of passing the
local scopes through the listener stack.
This also removes the need for LocalScope.parent
TEST=existing
Change-Id: I536c63258e4196a1582e9a1d73489adcfdaa6698
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/376400
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
When compiling the CFE with the CFE with an AOT snapshot, this CL:
instructions:u: -0.5575% +/- 0.0037% (-119851086.80 +/- 799956.68)
Though probably mostly because
Scavenge( new space) goes from 62 to 61
Change-Id: Ida0a834d6da34c8e09f521df3ff1490c6b6a875b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/375522
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
The agnostic mode was added to allow the platform dill embedded in
the VM to support both weak and strong mode. Since weak mode is no
longer supported in the VM, the agnostic mode can new be deleted.
All uses of the agnostic in Dart and Flutter have been removed prior
to this change.
Change-Id: Iff0f69d9cd64e887e01cd7e7d336a97761bd6d4b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/366801
Reviewed-by: Alexander Thomas <athom@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
This allows TypeAnalyzerOperations.getNullabilitySuffix to be removed,
and allows shared code to access nullability suffixes directly as
properties of a type, in the same way that the analyzer does.
Change-Id: I33ab90798b779534b5b0b111ec5137845b32f23b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/368443
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Robert Nystrom