Some recent changes to the parser looked like they should have cost
basically nothing but ended up costing a little bit anyway.
This CL marks the new functions for inlining recovering at least some of
the cost.
Change-Id: I599d557eb25bee892c8975d794618e837fec0caa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/510320
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
The analyzer makes a copy of `lineStarts` without the last element,
and copying it in to a `Uint16List` or an `Uint32List` depending on
what's needed (to avoid using more ram than necessary).
Because the (actual) scanner (`AbstractScanner`) does the same trick
with `Uint16List`/`Uint32List` the linestarts output from the scanner,
while a `List<int>`, is actually a `LineStarts` class, and copying that
into a `Uint16List` (or `Uint32List`) goes into
`_TypedIntListMixin._SlowSetRange` which as the name suggests is slower
than it has to be.
This CL puts the copying into the `LineStarts` class where it actually
has a `Uint16List`/`Uint32List` already, and doing the copy from that
instead avoids the slowdown.
Numbers from `perf stat`:
Before:
```
39,552,191,101 instructions:u
39,552,058,132 instructions:u
39,552,065,649 instructions:u
```
After:
```
39,419,426,977 instructions:u
39,419,429,914 instructions:u
39,419,441,613 instructions:u
```
Saving: about 132 million instructions.
Change-Id: Ibf6c7f6cc9324a54d2bd99c37058103af106164c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/508580
Reviewed-by: Johnni Winther <johnniwinther@google.com>
This updates the handling of initializers in primary constructor body declarations. The parser is updated to pass the `:` token, similar to what is done for the regular constructors. The CFE is updated to use a boolean, rather that the `:` token (or a synthetic token), to determine whether initializers should be processed as part of building the outline. The parsing of initializers is done using the `:` token, if any, now pass both from regular constructors and primary constructor body declarations.
Closes#63468
Change-Id: Ia266dea7946b30b02d56f9f3cedf40848c4ee440
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/507401
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Add a syntactic diagnostic for extension type augmentations that declare
representation fields. Extension type augmentations may augment the
declaration, but they must not redeclare the representation.
Thread an explicit parser option through primary constructor parsing so
extension type augmentations can omit a representation without producing
the usual missing-primary-constructor diagnostics, while still reporting
an error when a representation is present.
Register the new diagnostic in the shared and analyzer generated
diagnostic tables and add it to fix status tracking.
Change-Id: I84815bb0669a76126564e590760e93096a3af046
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/505860
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
SLSA-Policy-Verified: SLSA Policy Verification Service <devtools-gerritcodereview-exitgate@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Report new diagnostics when an introductory function, member, or factory
constructor is still incomplete after applying all augmentations.
Keep the existing missing-body diagnostics for declarations that have no
augmentations, but report augmentation-specific diagnostics when an
augmentation chain exists and none of the fragments provides a body or
factory redirection.
Move the checks into error verification so that extension and extension
type members can participate in augmentation completion before reporting
the existing abstract-member diagnostics. Also suppress the
corresponding shared parser diagnostic when it is reported by the
verifier.
Consolidate body-related tests by declaration shape instead of by
individual diagnostic. This keeps missing bodies, external bodies,
augmentation completion, and already-complete checks side by side,
making the interaction between these rules easier to review and extend.
Move the constructorAlreadyComplete coverage from its dedicated test
file into constructor_body_test.dart, and add the factory body
completeness cases there as well. Add executable_body_test.dart for
function and member body coverage, including top-level declarations,
static members, extension members, and extension type members.
Add diagnostic definitions and fix-status entries for the new
diagnostics.
Change-Id: I9ee803c1e767ff47a608c86413ef7ffc71518cfd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/503540
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
https://dart-review.googlesource.com/c/sdk/+/498840 added support for
promotion of properties (private, final instance variables with a name
which isn't used much for other purposes) in the context of anonymous
methods.
This CL reduces the set of situations where this feature is enabled such
that only `this` will allow property promotions to be carried in (such
that `this._x` is promoted in `v.=> this._x` when `v` is such that
`v._x` has been promoted before the anonymous method occurs). It also
generalizes the mechanism such that property promotions are carried out
(so we can do `if (v.=> _x is int) v._x.isEven;`).
Tests has been adjusted accordingly.
Change-Id: Ibe70713d3d9c89a6d95f9c3dd28df8f147cb518d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/502660
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Erik Ernst <eernst@google.com>
Move the analyzer-only Diagnostic, DiagnosticMessage, Severity, and
locatable diagnostic helper types out of _fe_analyzer_shared and into
package:analyzer.
I paln to make changes outlined in
https://github.com/dart-lang/sdk/issues/63311 and chat discussion.
Keeping these classes in the analyzer simplifies the migration and
avoids introducing a shared abstraction before there is a concrete need
for one.
If we decide later need to have a shared abstraction, we can always
extract one at that point. With coding agents internal code motion is
cheap.
Update analyzer, analysis server plugin, analyzer plugin, linter, and
scanner call sites to import the moved APIs from analyzer libraries, and
refresh API baselines to reflect the new public owner.
Change-Id: Ie0ef0f01c6e4be7ebaac25619ac3e3fe991a44d9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/501000
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
This CL adds support for promotion of certain private final instance
variables along with anonymous methods. The promotions do not differ
from the ones which are already available in Dart without anonymous
methods, but it requires some generalizations to handle the changing
value of `this` which is made possible by anonymous methods.
Change-Id: I720a5fa6d29a8a7d19bb2e167dc135f97492b525
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/498840
Reviewed-by: Paul Berry <paulberry@google.com>
This CL enables the primary constructors feature by default in Dart 3.13.
The primary constructors feature is a brevity feature. There are no new semantics, but it allows us to express declarations in a less verbose way.
This feature allows one constructor and a set of instance variables to be specified in the header of a declaration.
Currently a declaration with a constructor and some fields is written as:
```dart
// Current syntax.
class Point {
int x;
int y;
Point(this.x, this.y);
}
```
With a primary constructor, we would write the above as:
```
class Point(var int x, var int y);
```
If a primary constructor needs an initializer list or a body, they can be
specified inside the class using the `this` body syntax:
```dart
class Point(var int x, var int y) {
this : assert(x >= 0) {
print('Point created at $x, $y');
}
}
```
As part of this feature, you can also use the `new` and `factory` keywords to
declare constructors in the class body without repeating the class name:
```dart
class Point {
int x, y;
// Equivalent to Point(this.x, this.y)
new(this.x, this.y);
// Equivalent to Point.origin()
new origin() : x = 0, y = 0;
// Equivalent to factory Point.clone(Point other)
factory clone(Point other) => Point(other.x, other.y);
}
```
To learn more about the feature, check out the feature specification located here: https://github.com/dart-lang/language/blob/main/accepted/future-releases/primary-constructors/feature-specification.md
Tested: Has existing language, CFE, analyzer, analysis server tests.
Bug: https://github.com/dart-lang/sdk/issues/61524
Change-Id: I296f2fcd918b87bf2a1dd00256340759866c2423
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/489241
Reviewed-by: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Michael Thomsen <mit@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Kallen Tu <kallentu@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
This reverts commit 3eb697c0af.
Reason for revert: Internal Google3 breakages
Original change's description:
> [flow analysis] Fix unsound type promotion in inner async/generator functions.
>
> An `await` expression or `yield` statement suspends the current
> function and allows other code in the same isolate to execute. In the
> case of nested functions, an `await` or `yield` in the inner function
> can allow the outer function to continue executing. That means that if
> the inner function promotes a local variable belonging to the outer
> function, then it isn't sound to carry that promotion past an `await`
> or `yield`.
>
> This change fixes the unsoundness by adding a flow analysis method
> `suspension`, which the shared type analysis logic uses to tell flow
> analysis that an `await` or `yield` has been found. The `suspension`
> method un-does the promotions of any variables that might be written
> to while the inner function is suspended.
>
> Fixes https://github.com/dart-lang/sdk/issues/62889.
>
> Change-Id: I77eaf997159819a7c50f44b67174d2aa6a6a6964
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/499382
> Reviewed-by: Johnni Winther <johnniwinther@google.com>
> Commit-Queue: Paul Berry <paulberry@google.com>
> Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
> Reviewed-by: Bob Nystrom <rnystrom@google.com>
Change-Id: I187ba9a347394946ecc8749d35dc7914d271b90a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/500540
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Bob Nystrom <rnystrom@google.com>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Paul Berry <paulberry@google.com>
An `await` expression or `yield` statement suspends the current
function and allows other code in the same isolate to execute. In the
case of nested functions, an `await` or `yield` in the inner function
can allow the outer function to continue executing. That means that if
the inner function promotes a local variable belonging to the outer
function, then it isn't sound to carry that promotion past an `await`
or `yield`.
This change fixes the unsoundness by adding a flow analysis method
`suspension`, which the shared type analysis logic uses to tell flow
analysis that an `await` or `yield` has been found. The `suspension`
method un-does the promotions of any variables that might be written
to while the inner function is suspended.
Fixes https://github.com/dart-lang/sdk/issues/62889.
Change-Id: I77eaf997159819a7c50f44b67174d2aa6a6a6964
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/499382
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Bob Nystrom <rnystrom@google.com>
This adds error recovery for mixin and extension declarations with primary constructors. This avoids cascading errors when primary constructors are used with these declarations which do not support primary constructors.
Closes#63157
Change-Id: I17bfab4a7eabac1c75e7cfb580295dcf619c991d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/499180
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
I changed the parsing of const dot shorthands to parse the entire selector chain in `parsePrecedenceExpression` rather than parsing them in `parseConstExpression`. We were originally parsing the initial expression before the `.` and only flagging that as a dot shorthand which prevented any chaining on const constructors. Const expressions with chained methods/property accesses were not being flagged as a dot shorthand so we weren’t saving the context type properly which led to producing the “unknown context type” error.
Before:
```
class C {
const new someConstCtor();
C method() => this;
}
C c = const .someConstCtor.method();
// (const .someConstCtor) is parsed and handled as a const constructor
// (const .someConstCtor) is handled as a dot shorthand, context is
// saved on only this AST and not the outer chain.
// Then we parse the rest of the selector chain (.method())
//
// Error: No context type for (const .someConstCtor.method())
```
After:
```
class C {
const new someConstCtor();
C method() => this;
}
C c = const .someConstCtor.method();
// (const .someConstCtor) is parsed and handled as a const constructor.
// Then we parse the rest of the selector chain (.method())
// (const .someConstCtor.method()) is handled as a dot shorthand, context is saved for this outer node.
//
// OK. No error because we are able to retrieve the type for the
// entire dot shorthand chain.
```
Fixes: https://github.com/dart-lang/sdk/issues/63119
Change-Id: I3308d8eb7ce101466be257aba6b5448921bff136
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/495560
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Kallen Tu <kallentu@google.com>
This CL adds an implementation of anonymous `=>` methods in the CFE
which is sufficiently complete to handle the existing test cases in
language/anonymous_methods/expression.
Coverage is handled by adding magic comments to ignore the fact that new
code is not covered by existing testcases. This will be settled in a
separate CL.
To keep the failures visible, the failures in configurations
dart2js-hostasserts-linux-d8-try, dart2js-linux-chrome-try,
dart2js-minified-linux-d8-try, and ddc-linux-chrome-try have not been
approved. It seems likely to me that those are bugs in dart2js and in
DDC.
Change-Id: Ia70fedd4de6166d6a3bf8a108f49728b6e16c9f6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/494440
Commit-Queue: Erik Ernst <eernst@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
This enables the project to use dot shorthands.
Version 3.11 has been on the stable channel for over two months, so it
should be safe to rely on this language version.
Change-Id: I4a7c8706731a29798185fe806e17c9d26a6a6964
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/495801
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Permit `abstract` on top-level variable declarations when the
augmentations feature is enabled. Previously the parser always reported
`abstract` as extraneous at the top level, which rejected valid syntax
and dropped the modifier before later stages could see it.
Thread the abstract token through top-level field parsing, record it on
`TopLevelVariableDeclaration`, and pass it through the front-end
builders. Reorder the field callback arguments so `augment` precedes
`abstract`, matching the augmentation grammar for incomplete top-level
variables and keeping the parser, listener, and outline plumbing
consistent.
Change-Id: I680414a746b707d483e485702685c95f28a9c9ac
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/494564
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
This uses the previously added `%ignore=` syntax to enable validation of
the diagnostic documentation for several diagnostics that were
previously not being validated.
In the process I also cleaned up a few of the code snippets used in
those docs in order to minimize the number of ignored diagnostics.
I removed the newly validated diagnostics from the list in the validator,
and improved the comments for the ones that are still being ignored to
show why they're ignored and point to some possible future remediations
that would allow them to also be validated.
Change-Id: I46b0eeaab86740ba82784987e84f3e93046d39f0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/494941
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Doesn't change anything in `front_end/*testcases/primary_constructors/`.
(Would have skipped any other file with `test` in its path and
an explicit language version marker, but there weren't any outside
of those `front_end` directories).
Almost no files used as test input were affected, and none testing the actual syntax changed.
The `.../nnbd/required_2.dart` test case was split into a legacy version retaining the `var`/`final` with a language marker, and a new version without the `var`/`final` cases.
The `pkg/analyzer/` tests, and any other tests that have source code
in strings, are not migrated by this CL.
Tested: No change to behavior. One test split into legacy and new.
Change-Id: I7f5aa4cc98001a9adecacd106c0b3be14f96be1c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/480542
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Nate Bosch <nbosch@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Lasse Nielsen <lrn@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
This makes the arguments to `ScannerConfiguration` required and uses the current enabled-by-default state as default.
Furthermore, an extension method is added for `ExperimentalFeatures` (and object already required by the parser) that creates the `ScannerConfiguration` based on the enabled features. This method is used throughout the analyzer and CFE, ensuring that the scanner is set up consistently with the parser.
Change-Id: Id51456d5a547b06d0925865e35e01d9dc7d65381
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/493760
Reviewed-by: Jens Johansen <jensj@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
This removes the remaining support for `augment super` in the parser, analyzer and CFE.
`augment super` will not be part of the augmentations feature.
Change-Id: Ie8a185bba067ab6ecf3b3229ecc20d5c284842bd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/493401
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Moves some of the business logic for type analyzing yield statements
from the analyzer and front_end codebases into the shared
`TypeAnalyzer` class. There is no functional change.
This paves the way for fixing
https://github.com/dart-lang/sdk/issues/62889 (Unsound type promotion
in inner async/generator functions), which will require the type
analysis of yield statements to be integrated more closely with flow
analysis. Sharing the type analysis logic will avoid the need to do
that integration twice.
Change-Id: I36c1f5e45e33d9e46dc934fb71bc634d6a6a6964
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/491260
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Adds the class `StatementTypeAnalysisResult` to the shared type
inference codebase, along with a test method,
`checkStatementTypeAnalysisResult`, which will allow unit tests to
examine it. This class is the base class for the return type of all
statement analysis methods in the shared `TypeAnalyzer`
class. (Previously these methods returned void).
There are no fields in `StatementTypeAnalysisResult`; for now, it is
intended solely as a base class. To avoid incurring an extra runtime
cost, statement analysis methods that don't need to return any data
have been changed to return `const StatementTypeAnalysisResult()`
rather implicitly than returning `null`.
So far there are no derived classes. In a follow-up CL I will add an
`analyzeYieldStatement` method to the shared `TypeAnalyzer` class; it
will use a derived class to communicate information about the operand
to the front end.
Change-Id: I83c80b15d18e88fa80c08999bb78ca2f6a6a6964
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/491240
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
A flag that tells whether constructor is in an abstract class, enum or mixin was not correctly passed for primary constructors. This flag determines whether constructor tear-off lowerings are created for backends that use this, such dart2js and dartdevc.
The parser listener is updated to pass a DeclarationKind for primary constructors, similar to what is already done for regular constructors. The enable the CFE to pass the correct flag.
Change-Id: I114417795ae276dfeab9f10a0b0fcbcd2dbfc00b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/490541
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
When analyzing the CFE with StableAnalysis in AOT with GC disabled:
* This CL reduces the combined cost of `popTypedList2` from ~501 million
instructions to ~150 million instructions according to
`valgrind --tool=callgrind`.
* Total savings, via `valgrind --tool=callgrind`, is reported as
~359 million instructions.
* Total savings, via `perf stat`, is reported as
~367 million instructions (see also below).
A benchmark run with 5 runs each gives this (but note that the normal gc
runs should mostly be ignored):
With normal GC:
```
page-faults:u: 2.5544% +/- 0.0884% (4895.60 +/- 169.43) (191651.60 -> 196547.20)
instructions:u: -1.7106% +/- 0.0039% (-990609348.00 +/- 2286578.79) (57909229892.40 -> 56918620544.40)
branch-misses:u: -4.0211% +/- 2.2322% (-7537436.00 +/- 4184151.25) (187447442.80 -> 179910006.80)
maxRssKbytes: -1.1530% +/- 0.0148% (-7169.60 +/- 91.81) (621801.60 -> 614632.00)
maxRssBytes: -1.1530% +/- 0.0148% (-7341670.40 +/- 94008.95) (636724838.40 -> 629383168.00)
Comparing GC data:
Scavenge( new space) goes from 171 to 169
MarkSweep( promotion) goes from 17 to 16
MarkSweep( old space) goes from 0 to 1
Notice combined GC time goes from 3636 ms to 3507 ms (notice only 1 run each).
```
```
With GC disabled:
page-faults:u: -1.7322% +/- 0.0001% (-21490.20 +/- 1.49) (1240629.60 -> 1219139.40)
instructions:u: -0.9185% +/- 0.0021% (-367119249.20 +/- 829664.80) (39970048410.80 -> 39602929161.60)
maxRssKbytes: -1.7226% +/- 0.0012% (-85733.60 +/- 60.60) (4977076.00 -> 4891342.40)
maxRssBytes: -1.7226% +/- 0.0012% (-87791206.40 +/- 62052.71) (5096525824.00 -> 5008734617.60)
```
Change-Id: I85eae091cc45d5fb2820fbd263d022619c9875f6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/490122
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
Moves the bulk of the business logic for type analyzing await
expressions from the analyzer and front_end codebases into the shared
`TypeAnalyzer` class. There is no functional change.
This paves the way for fixing
https://github.com/dart-lang/sdk/issues/62889 (Unsound type promotion
in inner async/generator functions), which will require the type
analysis of await expressions to be integrated more closely with flow
analysis. Sharing the type analysis logic will avoid the need to do
that integration twice.
Change-Id: I9f8770ce8127a960b746a89ee3e370ed6a6a6964
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/489480
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Konstantin Shcheglov