The new annotation is intended to be used on members of class, enum or mixin to opt out the @visibleForTemplate visibility restriction cascaded from class- / enum- / mixin- level.
1. Throw warning if the annotation is added to a invalid target.
2. Update @visibleForTemplate diagnostics logic to opt out members annotated with @visibleOutsideTemplate.
Change-Id: Iec546fc7785cd45f39a1b2a2cc8849ef1cf9d04a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/304825
Reviewed-by: Marya Belanger <mbelanger@google.com>
Auto-Submit: Ludi Zhan <ludizhan@google.com>
Commit-Queue: Ludi Zhan <ludizhan@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Moved a test from `constant_test.dart` to slowly migrate it over.
Change-Id: Id7d0ef0edcfa4fced056ec8fc937b060a74f4468
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/306919
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Kallen Tu <kallentu@google.com>
Changed the signature of `_getConstant` as a smaller change.
Change-Id: Iac258558a32f11030db53edd658371bacbcc5ee6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/307182
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Kallen Tu <kallentu@google.com>
If a user promotes a field and then subsequently changes the variable
used to access it, the promotion must be discarded. For example, in
this code:
class C {
int? _i;
}
test(C c1, C c2) {
C c = c1; // (1)
if (c._i != null) { // (2)
print(c._i + 1); // (3)
c = c2; // (4)
print(c._i + 1); // (5)
}
}
The test at (2) promotes `c._i` to non-null, so (3) is ok. But since
`c` is reassigned at (4), (5) should be a compile-time error.
Previously, flow analysis used one promotion key to track `c` and one
promotion key to track `c._i`. The `PromotionKeyStore` associated each
promotion key with a map containing the promotion keys of all of its
properties. So, for example, if the promotion key for `c` was 10 and
the promotion key for `c._i` was 11, the `PromotionKeyStore` would
associate promotion key 10 with the map `{'_i': 11}`, so that each
time `c._i` was accessed, promotion key 11 would be found. In order to
detect the compile-time error at (5), it had to keep track of the fact
that keys 10 and 11 were related, so that the assignment to `c` at (4)
could invalidate the promotion of `c._i`. It accomplished this by
linking together all the related promotion keys in a circularly linked
list, which it would walk at the time of any variable assignment.
This worked, but it required a lot of complex bookkeeping. Also, it
posed problems for integrating field promotion with cascades, for
example, in the following code:
class B {
void f([_]) { ... }
}
class C {
B? _b;
}
test(C c1, C c2) {
C c = c1; // (6)
if (c._b != null) { // (7)
c.._b.f( // (8)
[
c = c2, // (9)
c._b.f(), // (10)
])
.._b.f(); // (11)
}
}
The cascaded access `.._b.f` at (8) should be ok, since `c._b` has
been promoted. But since there is an assignment to `c` at (9), the
promotion should not carry over to (10), and an error should be
reported. However, no error should be reported at (11) because the
cascaded access to `.._b.f` at that location is using the old value of
`c` that was captured at the beginning of the cascade, prior to the
assignment. There's no way to achieve this by invalidation alone,
since the code locations at which the promotion is valid ((8) and
(11), but not (10)) aren't even contiguous.
The solution to the problem is to store property promotion keys in
`SsaNode`s used by flow analysis, rather than in the
`PromotionKeyStore`. Since a fresh `SsaNode` is allocated each time a
variable is assigned, this automatically invalidates any previous
property promotions without the need for any extra bookkeeping. So, in
the first example, at (1), an `SsaNode` is allocated and associated
with the promotion key for `c`. At (2), a promotion key is created for
`c._i` and stored in `c`'s `SsaNode`, and the flow model is updated to
indicate that that key has been promoted to non-null. At (3), that
promotion key is reacalled from the `c`'s `SsaNode`, so the promotion
is still in effect. At (4), a fresh `SsaNode` is associated with
`c`. Since that `SsaNode` doesn't contain any promotion keys yet, at
(5), the access to `c._i` causes a fresh promotion key to be
allocated, with no associated promotions. So the invalidation happens
automatically due to the fact that a new `SsaNode` was created.
Flow analysis doesn't yet support cascades, but here's how the second
example is intended to work: as before, at (6), an `SsaNode` is
allocated and associated with the promotion key for `c`. At (7), a
promotion key is created for `c._b` and stored in `c`'s `SsaNode`, and
the flow model is updated to indicate that `c._b` has been promoted to
non-null. At (8), the `SsaNode` for `c` is captured and saved for
later use. At (9), a fresh `SsaNode` is created and associated with
`c`. That fresh `SsaNode` is consulted at (10), so `c._b` is not
promoted at this point. However, at (11), the previously stored
`SsaNode` is used, so the promotion of `._b` still works.
Change-Id: I64519fbcb2368a37aa18adf35cee0ffd290db9b9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/307140
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
When I was first developing the shared analysis logic for patterns, I
imagined that ordinary variable declaration statements would
eventually be re-interpreted as pattern variable declarations, where
the pattern was simply a variable pattern. In order to facilitate
this, I added support for late variable patterns, so that a
decalration of a late variable could be interpreted as a pattern
variable declaration where the pattern was a "late variable pattern".
In the final implementation, however, this functionality never got
used, except in flow analysis unit tests. Both the analyzer and the
CFE continue to use their old logic for analyzing ordinary variable
declarations (including late variable declarations), and only invoke
the shared logic when the declaration in question is truly a pattern
variable declaration.
In retrospect I believe this is the right approach; ordinary
(non-patterned) variable declarations are common enough that it makes
sense to have separate logic for analyzing theem; invoking the full
generality of pattern variable declarations would simply waste CPU
cycles.
So this CL removes support for late variable patterns from the shared
implementation; this will make future refactoring of flow analysis
logic easier.
Since the flow analysis unit tests *were* taking advantage of the
shared logic for late variable patterns, I had to add some separate
logic to the tests to replicate the old functionality when analyzing
an ordinary late variable declaration. This new logic is only used in
tests.
Change-Id: If51efab3c63bdc20d1f2772c2d2dd1e3f487bb00
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/307183
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Added `_getConstant` to handle the unfortunate `null`s returned by the ConstantVisitor. Refactored other code with this new helper too.
Change-Id: Ic2ce009b78fee151a1a6fa5b2d2621646cbf2c3c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/306672
Commit-Queue: Kallen Tu <kallentu@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This allows more flexibility as we can configure the redirect overtime, compared to the GitHub hosted site which we have less control over. It could even redirect to the old linter site in the meantime if desired.
Contributes to https://github.com/dart-lang/linter/issues/4411 and https://github.com/dart-lang/site-www/issues/4498
Change-Id: I3512002cdf7f62c0338d67cec0d7091f1166479d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/307000
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Commit-Queue: Phil Quitslund <pquitslund@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
make Code a sealed type, and all subtypes base classes
Change-Id: I328b6a25446dc89e88ae2f1c7e137b969f1b86ce
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/306682
Reviewed-by: Bob Nystrom <rnystrom@google.com>
Commit-Queue: Jake Macdonald <jakemac@google.com>
In doing so, made Constant sealed to use in switches.
We want to avoid passing back null as much as possible, so replacing those with InvalidConstant errors.
Change-Id: I5271c15961679316f7cbee9159150e3c7350926f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/306671
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Bob Nystrom <rnystrom@google.com>
Commit-Queue: Kallen Tu <kallentu@google.com>
Previously, flow analysis used three classes to keep track of
information about expressions that have been visited:
- ExpressionInfo, to keep track of information about expressions whose
truth or falsity has an effect on flow analysis (such as `== null`
checks, `is` checks, and combinations thereof those using `&&` and
`||`), as well as identifying the literal expression `Null`.
- ReferenceWithType, to keep track of information about expressions
that represent something that can be promoted (references to
variables and fields), as well as the static type of the expression.
- EqualityInfo, which wrapped ExpressionInfo and ReferenceWithType,
and also redundantly stored the static type of the expression.
These have now been combined into a single class hierarchy with a base
class called `ExpressionInfo`. This makes the code easier to reason
about, because it is no longer necessary to think about which of the
three above classes is needed in a given situation. Also, it helps
prepare for a follow-up CL in which I plan to refactor how flow
analysis gathers this information; the refactor will be easier with
just a single class hierarchy of information to be gathered.
This required a modest expansion of the API to flow analysis to
include more static types, since previously, static types weren't
needed by the ExpressionInfo class.
Change-Id: Id3de8b19049f8d920ebe85ab58c624ae3e55f226
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/304211
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This CL caches `FeatureSet`s (really `ExperimentStatus`s) in the
FeatureSetProvider.
When analyzing rwf-materials (562 contexts) with a filled cache I get
these numbers:
Before:
509,901 instances of `ExperimentStatus`.
Current memory: 3.21GB.
Peak memory: 3.24GB.
Heap: 2.61GB of 2.68GB.
After:
2,246 instances of `ExperimentStatus`.
Current memory: 2.80GB
Peak memory: 2.85GB
Heap: 2.21GB of 2.28GB
Difference:
Instances: -99.55%.
Current memory: -12.77%.
Peak memory: -12.03%.
Heap: -15.32% (used); -14.92% (size).
Change-Id: I3901310eed70733f8bd06ad81bf940b848734723
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/306460
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
Changes the return type of ConstantVisitor and make sure everything works
by unwrapping Constants. Added a bunch of TODOs to slowly go through
in follow up CLs.
Change-Id: I5dbb3f4eb6c72daa1fb649b746c8fdc4fb83dadb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/305847
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Kallen Tu <kallentu@google.com>
Two of these tests (`recordPattern_nullable_beforeAs` and
`recordPattern_nullable_beforeWhen`) verify that the fix for #52439
(Destructuring with explicit type of nullable record is a parsing
error), which causes `(...)? identifier` to be recognized as a
variable pattern, doesn't get confused by the pseudo-identifiers `as`
and `when`, and so it continues to correctly parse `(...)? as ...` as a
cast pattern and `(...)? when ...` as a guarded pattern.
The other two tests (`recordPattern_nonNullable_beforeAs` and
`recordPattern_nonNullable_beforeWhen`) verify that `(...) as ...` and
`(...) when ...` are parsed correctly. These forms were never broken,
but they were not well tested either.
Bug: https://github.com/dart-lang/sdk/issues/52439
Change-Id: I866e1c7a6a8e47e0cd91a1a77654405f395b50da
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/305844
Reviewed-by: Jens Johansen <jensj@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This CL introduces native assets suport for `dart run` and introduces
`dart build` which is similar to `dart compile` but outputs a folder
instead to that native assets can be bundled with an executable.
Change-Id: Ib6cfb95539f0adee46c99e531e440928c3f72f2b
Cq-Include-Trybots: luci.dart.try:pkg-linux-debug-try,pkg-linux-release-try,pkg-mac-release-arm64-try,pkg-mac-release-try,pkg-win-release-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/267340
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Daco Harkes <dacoharkes@google.com>
It is 2.19 that is different from 3.0, and has different features.
Change-Id: I869ac0de76502f0d2b606e0782fee83f66e86283
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/306127
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Previously, if a record-typed variable pattern lacked a `var` or
`final` keyword, and was followed by one of the tokens `||`, `&&`,
`as`, `?`, `!`, `when`, or `=>`, the parser would fail to recognize
it. This happened because the call to `computeVariablePatternType`
wasn't passing `true` for the optional parameter `required`; that in
turn placed `computeType` in a mode where it believed it was parsing a
potentially ambiguous construct in a top-level declaration, and hence
it would only accept the record type if the variable name was followed
by something that looked like part of a correct declaration (e.g. a
comma).
The fix is to pass `true` for the optional parameter `required` of
`computeVariablePatternType`. This places `computeType` in a mode
where it accepts the record type regardless of what follows it. This
is correct behavior since at the point where a variable pattern is
being parsed, ambiguities have already been taken care of and the
construct being parsed is most definitely a variable pattern.
Fixes#52521.
Change-Id: If20772ad914827a29df45c27eefb382ec1f470d2
Bug: https://github.com/dart-lang/sdk/issues/52521
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/305848
Reviewed-by: Jens Johansen <jensj@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This is the follow-up to
https://dart-review.googlesource.com/c/sdk/+/282280 that I had
forgotten about.
When analyzing rwf-materials (562 contexts) with a filled cache I get
these numbers:
Before this CL (i.e. parent CL):
99671 ms
98499 ms
99678 ms
With this CL:
85764 ms
86204 ms
86548 ms
Difference at 95.0% confidence
-13110.7 +/- 1256.93
-13.2054% +/- 1.26601%
(Student's t, pooled s = 554.547)
Change-Id: Idcde0c9b92eb34467ce69ee7d552f4a8d035286f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/305681
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
When analyzing rwf-materials (562 contexts) with a filled cache I get
these numbers:
Before this CL (i.e. parent CL):
102829 ms
100934 ms
100991 ms
With this CL:
99671 ms
98499 ms
99678 ms
Difference at 95.0% confidence
-2302 +/- 2041.63
-2.26609% +/- 2.00979%
(Student's t, pooled s = 900.749)
Change-Id: I30025b15f7b8c637d83b2cef234c0e7a602535f3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/305680
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
When analyzing rwf-materials (562 contexts) with a filled cache this
CL removes something like ~1.6 million calls to "isDirectorySync" which -
I think - reads from the filesystem.
On Linux I get this change (on Windows I'm guessing the change is bigger
as the filesystem is often slower):
Before the fix:
111662 ms
112053 ms
112176 ms
With this fix:
102829 ms
100934 ms
100991 ms
Difference at 95.0% confidence
-10379 +/- 1780.48
-9.26997% +/- 1.59023%
(Student's t, pooled s = 785.532)
I.e. it makes it ~10 seconds or ~9% faster.
Change-Id: I42f54b2ff7510365c5a683c993b6b759b35bd152
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/305661
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
So, we don't have to recompute it at every symbol search.
For Flutter repository, searching for 'ButtonStyle' is 40% faster.
SHA: 47fe150674
Description: before
[mean: 56.4][stdDev: 1.471][min: 54.0][max: 61.0]
Description: own files
[mean: 35.0][stdDev: 1.308][min: 33.0][max: 38.0]
Change-Id: Id497bf4117bdfafeeccd04601fd0198e22624c0d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/305843
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Adds an abstract type Constant with two subtypes: DartObjectImpl and
InvalidConstant.
Initial set up for changing the return type in the constant evaluator.
Change-Id: Ia713dee6453937f87d1ae72d3bd240e67d6c13eb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/305500
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Kallen Tu <kallentu@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Instead of having separate API methods `propertyGet` (for accessing
the properties of an explicit target) and `thisOrSuperPropertyGet`
(for accessing the properties of `this` or `super`), switch to a
single `propertyGet` method. Make the `target` parameter of
`propertyGet` a sealed class (`PropertyTarget`) with three subtypes
representing the three possible kinds of targets:
- `ExpressionPropertyTarget` for an explicit target expression,
- `SuperPropertyTarget` is the target is `super`, and
- `ThisPropertyTarget` if the target is `this`.
Also adjust the API for `promotedPropertyType` to use the new
`PropertyTarget` class for its `target` parameter, rather than a
nullable `target` expression and boolean `isSuperAccess` parameter.
Change-Id: I90535f6d6741776548bc4d60359fd4c40c0fab90
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/304763
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
In a future CL I'd like to filter out whole libraries depending
on whether they could have a match. At this point the search, filtering,
and matching become entangled too much to bind them with outside
code in the server. Instead, it all will be the analyzer's concern.
Change-Id: Iefdadbf92046218b3a2607fbeadd37494522112e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/304841
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
When promoting fields, in order to avoid unsoundness, we need to
distinguish between field accesses performed through `super` and field
accesses performed through `this`. Otherwise, a user could do
something like this:
class B {
final int? _i;
B(this.i);
}
class C extends B {
final int? _i;
C(this._i, int? superI) : super(superI);
int f() {
if (super._i != null) {
return this._i; // UNSOUND: `this._i` could be `null`
}
}
}
To avoid this problem, flow analysis now uses separate promotion keys
for `super` and `this`, so that promoting a variable through `this`
leaves it unpromoted when accessed via `super`, and vice versa.
Note that in principle the implementations could inspect the enclosing
class, and only distinguish `this.` and `super.` accesses in the case
where it contains a declaration matching the field name. But doing so
would carry a performance and implementation complexity cost, and
would confer very little real-world benefit (since in practice users
don't mix `this.` and `super.` accesses and expect them to refer to
the same field).
Fixes#50138.
Bug: https://github.com/dart-lang/sdk/issues/50138
Change-Id: Ia0fd79b5ed7649d23a28efcbffb59b4c9ad63f70
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/304364
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Avoid reporting an error if the static types are mismatched because
CompileTimeErrorCode.INVALID_ASSIGNMENT would have already been
reported by the ErrorVerifier.
Closes#37238
Bug: https://github.com/dart-lang/sdk/issues/37238
Change-Id: I2248d054ed6bf6a546e998afb9b731a36e197780
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/304362
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Kallen Tu <kallentu@google.com>
I played with it some more, and found that the overhead of measuring
too fine grainly, when we have tens and hundreds thousands of relatively
inexpensive operations, is too high.
Change-Id: I4038e3972efff860d568c02008120092f99b8298
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/304368
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
I saw that the linter roll reported a strange InvalidType related
error. So, I wanted to check if this is something I can reproduce.
I cannot, but the tests might be useful anyway.
Change-Id: I21138685c4fee028f0620d3fcfa3a221a304c951
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/304325
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
The actual removal will happen in (to be rebased on top of this):
https://dart-review.googlesource.com/c/sdk/+/303280
Fixing direct uses of the deprecated `Identifier get name` is apparently
only about a half of the work. I should have followed my own advice to
the clients and prepared better.
These changes are necessary because we will stop creating and
visiting `Identifier`, and so `NamedType` should be handled directly.
Change-Id: Ie07b75b15eab277ed6c9b29a838561a7eb71f588
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/303425
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
...not just the result for the immediately requested file.
Change-Id: I85f74708fb32ff79b3c8a8b68c9cea5db8093000
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/303980
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
This reverts the changes to ResourceProviderMixin that made it possible
for a test to explicitly set the context type of the resource provider.
I don't believe that's necessary because we run all of the tests on the
windows bot with that context type anyway.
It also removed the windows-specific subclasses of the URI suggestion
tests. They were duplicates of the non-windows-specific tests that were
probably added before we had a windows bot.
Change-Id: I4547cc768992c4d272871032a6939cd216dc5e96
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/303422
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
The removal itself will happen in a separate CL, as a breaking change.
But we still had a few places that relied on `SimpleIdentifier` being visited.
Change-Id: I8f554cec77858cb8bc7deb29d98f0d6def8685ee
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/303008
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
In rare circumstances involving syntax errors, the `parsePattern`
method inserts a synthetic token but does not consume any
tokens. Usually when this happens it's not a problem, because whatever
method is calling `parsePattern` consumes some tokens, so the parser
always makes progress. However, when parsing list patterns, after
calling `parsePattern`, the parser would look for a `,`, and if it
didn't find one, it would supply a synthetic `,` and call
`parsePattern` again, resulting in an infinite loop. A similar
situation happened with map patterns, though the situation was more
complex because in between the calls to `parsePattern`, the parser
would also create synthetic key expressions and `:`s.
To fix the problem, when parsing a list or map pattern, after the call
to `parsePattern`, the parser checks whether any tokens were
consumed. If no tokens were consumed, it ignores the next token from
the input stream in order to make progress.
I also investigated whether there were similar issues with
parenthesized/record patterns and switch expressions, since those
constructs also consist of a sequence of patterns separated by tokens
and other things that could in principle be supplied
synthetically. Fortunately, parser recovery doesn't get into an
infinite loop in those cases, so I didn't make any further
changes. But I did include test cases to make sure.
Fixes#52352.
Bug: https://github.com/dart-lang/sdk/issues/52352
Change-Id: Idc8140236f6054deb1fd3c862036fe47dd84f30b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302803
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
I am concerned about the tests marked with TODOs because I don't know
why the behavior changed. It's possible that the contributor is still
producing the suggestions but that they're being filtered out later.
I checked using the debugger, and they're not in the list of results
that we're getting back, so it seems likely that they represent real
bugs. If we just weren't detecting the bug before then we're good.
But I can't think how rewriting the tests could effect what the user
is seeing, so I think it's safe to make these changes.
Change-Id: If72042b7ec24b97994a12449c811213b8f708f72
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/303000
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Also includes https://dart-review.googlesource.com/c/sdk/+/302680
that was reverted because of adding extre diagnostics for Flutter.
Change-Id: I4cf7cedfcb67a5af226e609d279551fea0b0dee1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302844
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
These were introduced in
https://dart-review.googlesource.com/c/sdk/+/287601 to allow the
common front end to report back to the shared analysis logic when it
desugars a guard expression, so that flow analysis will properly
account for any promotions made by the guard expression; previously,
if a guard expression got desugared by the front end, promotions
performed by the guard would be lost.
In https://dart-review.googlesource.com/c/sdk/+/298840, we've switched
to a more general technique for ensuring that promotions performed by
the guard will not be lost: now the `dispatchExpression` method uses
`FlowAnalysis.forwardExpression` to compensate for the differences in
the conventions of the CFE and the shared logic, so that the shared
logic no longer needs access to the desugared expression. So the
`head` parameter (and the associated return value) are no longer
needed.
In a future change, I plan to adjust the CFE conventions to match
those of the shared logic, so that `FlowAnalysis.forwardExpression`
won't be needed at all (see
https://github.com/dart-lang/sdk/issues/52189).
Change-Id: I686d1d5b7e4c9db353f583c38792274451f9f8bb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302366
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
When a post-feature library implements a pre-feature library declaration that has a final core library class as a super declaration, it should not produce a base/final/sealed transitivity error. Subclasses of a base core library class will still emit this error.
Otherwise, if base/sealed/final is omitted in a non-implement case, we want to report these errors.
Bug: https://github.com/dart-lang/sdk/issues/52315
Change-Id: Icdd4f63f69b061be5eabc7fd30b703d0358018a7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302365
Commit-Queue: Kallen Tu <kallentu@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
This reverts commit d2df7ce24a.
Reason for revert: https://dart-review.googlesource.com/c/sdk/+/291500 was reverted
Original change's description:
> Update comments and readmes to specify `dart run --no-pub` when necessary.
>
> Now that https://dart-review.googlesource.com/c/sdk/+/291500 has
> landed, `dart run` now tries to invoke `dart pub` before execution.
> This doesn't work when running scripts inside the SDK, because the SDK
> doesn't use pub to get its dependencies. So scripts in the SDK now
> need to be run with `--no-pub`.
>
> Change-Id: Ic320b717b2d45278cd26d373ae0823c2935ce102
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/292720
> Reviewed-by: Johnni Winther <johnniwinther@google.com>
Change-Id: I6c13510abde17f71694f362a8071c9c7fa519a88
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302364
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Checks an interface's supertypes for final classes and produces an error if the implementing bypasses a legacy library.
This behaviour should only happen when a post-feature library implements
a pre-feature library declaration that has a final class as a super
declaration.
Similar error to https://dart-review.googlesource.com/c/sdk/+/298320
Bug: https://github.com/dart-lang/sdk/issues/52078
Change-Id: Ie16edb2b231957dad7502fdab3d5faba93bc6773
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/300861
Commit-Queue: Kallen Tu <kallentu@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
I ran the CL that does resolve with it in google3, it is mostly
green, but there are a few clients that extend TypeVisitor directly.
So, I want to publish the interface `InvalidType` first, update the
clients, and only then update the resolution.
Bug: https://github.com/dart-lang/sdk/issues/36697
Change-Id: I1f1c995a36849ebbde2c526bb8761d86be87b4ea
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/301481
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
In https://dart-review.googlesource.com/c/sdk/+/299400, the parser was
adjusted so that it no longer accepts `when` and `as` as the names for
variable patterns in cases where there is a possible ambiguity
(e.g. `int when` is not accepted as a pattern because `int` is a
legitimate pattern, therefore `when` could introduce a guard
clause). This change further prohibits `when` and `as` from being the
names of variable patterns or identifier patterns even in the case
where there is no ambiguity. This is in line with the discussion at
https://github.com/dart-lang/sdk/issues/52199#issuecomment-1526297771,
and the spec change at
https://github.com/dart-lang/language/pull/3033.
Three new error codes are introduced, to cover the three circumstances
in which `when` or `as` might be used illegally: in a declared
variable pattern, in an assigned variable pattern, or in an identifier
pattern. I've also added analyzer tests to ensure that the parser
recovers from these errors nicely. Unfortunately, nice error recovery
is only feasible in the non-ambiguous cases.
I've also updated the language test expectations in
`tests/language/patterns/version_2_32_changes_error_test.dart` to
reflect the new error messages, and added a few more examples of uses
of `when` and `as` that are still permitted.
Fixes#52260.
Bug: https://github.com/dart-lang/sdk/issues/52260
Change-Id: I229f627aa639659c30b83c74895759207da279f7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/301482
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This reverts commit 4919729f00.
This CL also adds back in logic to handle older package:js versions to avoid
failures in our static checking. It also supports dart:js_interop's @JS
annotation since it can now be used for @staticInterop classes.
CoreLibraryReviewExempt: Reland of backend-specific library changes.
Change-Id: I104653a9a6b2593f6bab658808287e2074c18550
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/294130
Reviewed-by: Joshua Litt <joshualitt@google.com>
Commit-Queue: Srujan Gaddam <srujzs@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Also standardize on arrow functions vs not, to what seems idomatic in
this file.
Change-Id: I73f9a71f928e8d02cdabbc0500c8dc150b45172e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/300163
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Using Identifier expression inside NamedType does not make sense.
Change-Id: I4a61d2b472fd66fb7c5e6e92a80cccb391b06d49
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/294920
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This change fixes parsing of case clauses such as:
case foo when !flag:
Constructions like this require some lookahead in order to parse
correctly, because the token `when` is valid both as an identifier and
as a part of the grammar for a case clause. Therefore, at the time
`foo` is encountered, the parser must decide whether it is looking at
a variable pattern (`foo when`, where `when` is the name of the
variable) or an identifier pattern (`foo`, where `when` begins the
case's guard clause). Previous to this fix, the algorithm for
disambiguating these two choices was as follows:
- If the token sequence starting at `foo` looked like a type, and the
token that follows was an identifier, the parser assumed it was
looking at a variable pattern with a type; otherwise it assumed it
was looking at an identifier pattern.
- EXCEPT that if the token that followed the supposed type was `when`
or `as` (both of which are valid identifiers), then it probed
further:
- If the token that followed `when` or `as` was a token that could
legitimately follow a pattern, then it assumed that it was looking
at a variable pattern with a type. (The tokens that could
legitimately follow a pattern are `,`, `:`, `||`, `&&`, `)`, `}`,
`]`, `as`, `when`, `?`, `!`).
- Otherwise it assumed that it was looking at an identifier pattern.
This didn't fully disambiguate, because the third bullet didn't
account for the fact that the tokens `as`, `when`, and `!` could
_either_ legitimately follow a pattern _or_ legitimately begin an
expression (or, in the case of `when`, a type), therefore constructs
like the following were incorrectly parsed:
- `case foo when as:` (where `as` is a local boolean variable)
- `case foo when when:` (where `when` is a local boolean variable)
- `case foo when !flag:` (where `flag` is a local boolean variable)
- `case foo as when:` (where `when` is the name of a type)
The solution is to simplify the disambiguation logic so that if if the
token that follows the supposed type is `when` or `as`, then the
parser assumes that it's looking at an identifier pattern, _not_ a
typed variable pattern.
The consequence of this is that the above four constructions are
parsed correctly; however it is no longer possible for a typed
variable pattern to name a variable `when` or `as`.
For consistency we would like to prohibit _any_ variable pattern from
naming a variable `when` or `as`, however to keep this change as small
as possible (and reduce the risk involved in a possible cherry-pick)
that will be postponed until a later CL.
Fixes#52199.
Bug: https://github.com/dart-lang/sdk/issues/52199
Change-Id: Ibab9b92f01e3e4020d7d64f1ff000a9b964a4564
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/299400
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
There is a quite complicated logic in resolving directives and walking
augmentations and parts. And we don't expect users write a lot of code
in directives, so there is less need for optimizations than when typing
in method bodies.
Change-Id: Id38e90f09eb5c1e84be8fc28ed4211d03762d2e0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/298220
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
This is a reland of commit ccb02fa8fc
Original change's description:
> [analysis_server] Use fixDataYamlFolder constant instead of string literals
>
> Follow-up from https://dart-review.googlesource.com/c/sdk/+/296801.
>
> Change-Id: I72ea8a42ec64c4bd6c6cee021a1464167ec368e6
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296862
> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
> Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Change-Id: I202392de3444556dc2baa9ed9e1b82c3a1ede4aa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/298060
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
This is a reland of commit bbacf39e9c
Original change's description:
> [analysis_server] Analyze fix data in fix_data folder
>
> Fixes part of https://github.com/dart-lang/sdk/issues/52126.
>
> Change-Id: Ib4bd7830a2f644eacedccd375c7c8dc60f040d33
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296801
> Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Change-Id: I571c1e4f87fdf0095d003d496f3c5d88e5cf0ff8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/297940
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This reverts commit bbacf39e9c.
Reason for revert: Causing flutter/engine to fail to roll into flutter/flutter. See https://github.com/flutter/flutter/pull/125363.
Original change's description:
> [analysis_server] Analyze fix data in fix_data folder
>
> Fixes part of https://github.com/dart-lang/sdk/issues/52126.
>
> Change-Id: Ib4bd7830a2f644eacedccd375c7c8dc60f040d33
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296801
> Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Change-Id: I109a4b2c596ad22d73eaf0ac3e25f53a35ba5e28
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/297320
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Zach Anderson <zra@google.com>
The analyzer team has decided to adopt the convention of using `TODO`
comments to document long term issues that should persist in the
codebase, and `FIXME` comments to document short term issues that need
immediate attention. They may even consider adding a presubmit hook
to ensure that `FIXME` comments are only used during local
development.
Accordingly, it makes sense to suppress `TODO` comments from being
surfaced to the IDE "problems" view (since there are hundreds of them,
and they're not immediately actionable). This makes VSCode's
"problems" view much more usable in "tree" mode.
Change-Id: I11a0c59132fb98c1c86fb4adf22d1fdf3b547c80
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/295662
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
In order for an analyzer quick fix to aid the user in correcting
exhaustiveness errors, it needs to know not just the text of the
witness pattern that needs to be inserted, but also which elements are
referenced by the identifiers in that pattern. This will allow it to
add in imports if the user's code doesn't yet import the necessary
library, or insert import prefixes in the case where the user is
already importing the necessary library in a prefixed fashion.
To reduce the effect on the exhaustiveness logic (which was designed
with the intention of building up the witness pattern into a
StringBuffer), we introduce a new type, `DartTemplateBuffer`, which
behaves similarly to StringBuffer but also captures the necessary
semantic information so that the analyzer will be able to fix up
imports in an appropriate way.
This change is mostly isolated to the _fe_analyzer_shared package; it
doesn't add the necessary logic to the analyzer to capture the
elements; it simply plumbs them up to the API boundary between the
packages. Future CLs will add the necessary analyzer logic to make
use of the information.
Change-Id: I7c030fc49a2510acbefd6aeb567e0e7b7102855a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296385
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Nate Bosch <nbosch@google.com>
State more explicitly that sealed classes are abstract and lead the
user to a better fix with the error message.
Bug: https://github.com/dart-lang/sdk/issues/52073
Change-Id: Id24c6cb187ee5497ca2819f930c48ff5aa8d07fc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296025
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Kallen Tu <kallentu@google.com>
(The last change was prematurely merged.)
Change-Id: Idc161ef596fa43927cf7eda223635a798d3292af
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296402
Commit-Queue: Phil Quitslund <pquitslund@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
