This CL adds more of the type operations required in the subtype
constraint gathering algorithm into the shared type operation
class. The added operations are used in the constraint gathering
algorithms in the Analyzer and the CFE.
Part of https://github.com/dart-lang/sdk/issues/54902
This is a reland of
https://dart-review.googlesource.com/c/sdk/+/346840
Change-Id: I99086cfc3cd56db40055e9bb8e23acec8bdf830b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/354622
Reviewed-by: Paul Berry <paulberry@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Chloe Stefantsova <cstefantsova@google.com>
Issue https://github.com/dart-lang/sdk/issues/54628 raised the issue
that the static analysis of `await e` behaves in unexpected ways when
the static type of `e` is a subtype of a future related type, rather
than just a plain `Future<T>` or `FutureOr<T>` for some `T`.
This CL adds a test that assumes the update to the language
specification which is performed by the PR
https://github.com/dart-lang/language/pull/3574. Hence, this CL
should ONLY BE LANDED if and when that PR is landed.
Change-Id: Ib8acb77e24ffbceb0b4034d3b23a0f4fef8e3d1c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/354020
Reviewed-by: Lasse Nielsen <lrn@google.com>
Commit-Queue: Erik Ernst <eernst@google.com>
The old version of language/void/void_type_usage_test.dart had some
faults (compile-time errors were expected, but they weren't reported
by tools and yet the test run succeeded). The failures were caused by some amount of duplication in the labels (a handful of duplicate labels from a set of about 450 labels, that is, it wasn't immediately obvious). This CL updates the test to use the new test expectation syntax (`// ^^^^`) and adjusts the expectations to match the currently reported error messages, plus the ones about `void` that are missing.
Note that this test went through a phase where I believed that we had about 260 failures (130 CFE and 130 analyzer), but they turned out to be a consequence of migrating this test incorrectly from being a multi test into a form where it uses the current test expectation comments (`// ^^^` and such). At this point we just have 3 failures (all on expressions of the form `e += 1`), all with the CFE.
Bug: https://github.com/dart-lang/sdk/issues/31883
Change-Id: Ib1ceb56326a5847e3ca23ac0ee655eee65f0d76f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/350921
Reviewed-by: Nate Bosch <nbosch@google.com>
Commit-Queue: Erik Ernst <eernst@google.com>
Those timing out tests cause CI infra issues (purple bots)
on deflaking.
Issue https://github.com/dart-lang/sdk/issues/55025
Change-Id: Ibdc3766e0e482f180c03acc987c3323a3574647c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/354621
Commit-Queue: Martin Kustermann <kustermann@google.com>
Reviewed-by: William Hesse <whesse@google.com>
Auto-Submit: Martin Kustermann <kustermann@google.com>
This covers a failure in language/macros/json/json_serializable_diagnostics_test.dart
Change-Id: I751a58db6316ef2d9bb42533e279ab5fa49d1fd0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/354042
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Jake Macdonald <jakemac@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
The only expected exceptions at this point are DiagnosticException and MacroException subtypes; everything else is a bug in the implementation.
Tweak the diagnostic to explain that it's due to a bug in the macro. Add the exception message and stack trace as a context message, and a fix hint to contact the author.
Add a language test.
Retry landing with updates to `pkg/analyzer/test/summary/macro_test`.
R=jensj@google.com, scheglov@google.com, sigmund@google.com
Change-Id: Iebc3df784922dcb6ef112ba8d023d5388516373b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/352361
Reviewed-by: Jake Macdonald <jakemac@google.com>
Auto-Submit: Morgan :) <davidmorgan@google.com>
Commit-Queue: Morgan :) <davidmorgan@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
This looks like it was an oversight--previous to this change, there
was zero test coverage for this case.
Change-Id: I4301a3ba90aedce3b0fcd901649c370cba522f4e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/353280
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
This looks like it was an oversight--previous to this change, there
was zero test coverage for this case.
Change-Id: Icba1381c389af37ab4fe38f345cc78fe6c01d7f9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/353226
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
This CL contains several small bug fixes:
- The context is only used for type inference if it is a record type
with the same shape as the record literal. Previously, if the
context was a record type with a different shape than the record
literal, the analyzer would attempt to do an approximate match
(using the context from any matching named fields, and from all the
positional fields that were in common between the context and the
literal). At first glance, it might seem like this would only matter
for erroneous code (since record shape mismatches typically lead to
compile-time errors). But if the context arises from a local
variable promotion, then a mismatch doesn't lead to a compile-time
error; it simply leads to a demotion. So the difference is
observable for non-erroneous code.
- If one of the fields is implicitly downcast from `dynamic`, the
static type of the field's expression remains `dynamic`. This makes
the behavior of dynamic downcasts inside field literals consistent
with all other implicit dynamic downcasts.
- If one of the fields is implicitly downcast from `dynamic`, the
downcast is made to the greatest closure of the context. Previously,
the downcast was made to the context itself, which meant that it was
possible to create static types containing the unknown type,
violating one of the key assumptions of the Dart type system.
- If one of the fields has a static type of `dynamic`, and `dynamic`
is a subtype of the greatest closure of the context (e.g. because
the context is `Object?`), no dynamic downcast is
performed. Previously, a dynamic downcast _was_ performed, meaning
that the static type of the resulting record literal would have
`dynamic` in a spot where `Object?` should be.
This brings the analyzer behavior into line with the spec and the
front end, with one minor exception:
- When the front end uses the greatest closure of the context to
implicitly downcast a field from dynamic, it uses a modified
greatest closure algorithm where covariant instances of `_` are
replaced with `dynamic` instead of `Object?`.
The front end's behavior in this rare case doesn't agree with the
spec; I'll address this in a future CL.
Change-Id: Ib1ab7ee4d0f63a152480704e2c0d5332446a613c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/350983
Reviewed-by: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
PR https://github.com/dart-lang/language/pull/3560 and
https://github.com/dart-lang/language/pull/3598 updated the feature
specification of extension types to define a notion of types being
'incompatible with await'. This CL adds a test to verify that various
types are being classified correctly, and errors are hence reported
as expected for expressions of the form `await e`.
Change-Id: I39e992b4317ef49fbc2267819481d71d87b56aca
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/351143
Reviewed-by: Lasse Nielsen <lrn@google.com>
Commit-Queue: Erik Ernst <eernst@google.com>
Based on abandoned change #264261.
Changes to that PR: compare and show file paths, improve part parsing, include analyzer expectations from other files as well as CFE expectations, update unit tests.
Add end to end tests. The simplest case "library_failure_test.dart" passes before this PR, all others require the change in this PR to pick up expectations in other files.
Change-Id: Ia71b78a8f0dced83f603309877132f261b47c5a3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/345541
Reviewed-by: Erik Ernst <eernst@google.com>
Reviewed-by: Bob Nystrom <rnystrom@google.com>
Commit-Queue: Morgan :) <davidmorgan@google.com>
Nested macros: check that macro impls can themselves use macros.
Disallowed use: check that macros can't be used in the library in which they are declared, or in the same library cycle.
R=jakemac@google.com
Change-Id: I3b12afc27e5784edc217508b7763a9b448229877
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/352522
Reviewed-by: Jake Macdonald <jakemac@google.com>
Commit-Queue: Morgan :) <davidmorgan@google.com>
Auto-Submit: Morgan :) <davidmorgan@google.com>
- These tests rely on the internals of the old type system.
- Copy some expectations to the language suite when it looks like
there isn't already coverage for the same test.
Change-Id: I63bfa2bc94fb29b4e4f90c3c02cf0943d19764b6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/352463
Commit-Queue: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Kallen Tu <kallentu@google.com>
This introduces an asyncExpectThrowsWhen to async_helper, similar to
Expect.throwsWhen, to allow specifying semantics under behavior
variations in this test.
We also update await_for_test to no longer skip expectations in dart2js.
Change-Id: Ie147f74f384a0e196e40b75c59fe585f011ede49
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/351721
Reviewed-by: Lasse Nielsen <lrn@google.com>
Reviewed-by: Stephen Adams <sra@google.com>
Commit-Queue: Sigmund Cherem <sigmund@google.com>
Today, most tests that touch on a behavior variation end up
skipping expectations or the entirety of a test for some
testing configurations. Moving forward, we'd like skip less
and try to account for the behavior variations if that's
reasonable.
This CL shows an approach to improve our test coverage for
behavior variations. We introduce two new methods to
[Expect] that allow us to conditionally check that a
function throws, depending on variation predicates.
The CL changes expectations for errors that don't occur
when dart2js omits parameter type checks or implicit
downcasts.
Note: originally I had the intention to introduce a name
parameter to `Expect.throws` and `Expect.throwsTypeError` to
avoid introducing a new API. However, because these APIs are
used for testing core language features, such as function
parameters themselves, we decided to keep the use of
features in these APIs as simple as it can be.
CoreLibraryReviewExempt: no public library semantic change - only improving test coverage under variations
Change-Id: I531657622655778491eaca8b37ba69ffaab559fc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/351340
Reviewed-by: Lasse Nielsen <lrn@google.com>
Commit-Queue: Sigmund Cherem <sigmund@google.com>
When a local variable is promoted to a record type, and then an
assignment statement is used to assign a record literal to that local
variable, if the fields of the new record literal are not assignable
to the fields of the promoted record type, that's not a problem; both
the analyzer and front end agree that the local variable is simply
demoted.
But the spec implies that if the old and new record _shapes_ are the
same, then a compile-time error will occur instead of a demotion. I've
created https://github.com/dart-lang/language/pull/3613 to bring the
spec in line with the implementations. This test demonstrates the
current behavior of the implementations, and makes sure that it
doesn't regress.
Change-Id: I0eacd7ca7f6579a35dbc34687113a2112418f368
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/352462
Reviewed-by: Bob Nystrom <rnystrom@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Prior to this CL, the CFE used a nullable type schema for null-aware
spread operators in list literals, but a non-nullable type schema for
null-aware spread operators in set and map literals. This was clearly
an oversight; a nullable type schema should be used for for null-aware
spread operators in all kinds of collection literals.
This change brings the CFE into alignment with the analyzer.
Fixes https://github.com/dart-lang/sdk/issues/54828.
Change-Id: I0d5aa128656c22211228f0dd35ccee40925b4ef0
Bug: https://github.com/dart-lang/sdk/issues/54828
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/349921
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
This reverts commit 093271880a.
Reason for revert: Breaks some analyzer tests.
Original change's description:
> [macros] Tweak diagnostic for unexpected macro implementation exception.
>
> The only expected exceptions at this point are DiagnosticException and MacroException subtypes; everything else is a bug in the implementation.
>
> Tweak the diagnostic to explain that it's due to a bug in the macro. Add the exception message and stack trace as a context message, and a fix hint to contact the author.
>
> Add a language test.
>
> R=jakemac@google.com
>
> Change-Id: Ieaa6bedb3bc1646e4be61b384619152cc173b7e0
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/351040
> Auto-Submit: Morgan :) <davidmorgan@google.com>
> Reviewed-by: Jens Johansen <jensj@google.com>
> Commit-Queue: Morgan :) <davidmorgan@google.com>
> Reviewed-by: Jake Macdonald <jakemac@google.com>
Change-Id: Ia64c112aad79fdcdd1f6e1a4b800616632107fc2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/352260
Auto-Submit: Morgan :) <davidmorgan@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Erik Ernst <eernst@google.com>
Reviewed-by: Erik Ernst <eernst@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
The only expected exceptions at this point are DiagnosticException and MacroException subtypes; everything else is a bug in the implementation.
Tweak the diagnostic to explain that it's due to a bug in the macro. Add the exception message and stack trace as a context message, and a fix hint to contact the author.
Add a language test.
R=jakemac@google.com
Change-Id: Ieaa6bedb3bc1646e4be61b384619152cc173b7e0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/351040
Auto-Submit: Morgan :) <davidmorgan@google.com>
Reviewed-by: Jens Johansen <jensj@google.com>
Commit-Queue: Morgan :) <davidmorgan@google.com>
Reviewed-by: Jake Macdonald <jakemac@google.com>
Replaces uses of `webNumbers` with `jsNumbers`.
CoreLibraryReviewExempt: no semantic changes - only updates to use new pkg/expect variations properties
Change-Id: I8072b0b1f92d37e756c211b5d0de106a59d0960b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/346911
Reviewed-by: Mayank Patke <fishythefish@google.com>
This updates tests to start using `checkedParamters` and
`checkedImplicitDowncasts` instead of `dart2jsProductionMode`
CoreLibraryReviewExempt: no semantic changes - only updates to use new pkg/expect variations properties
Change-Id: Ie717b4fd44590ff1f977142cf5802c4aace34dea
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/346910
Reviewed-by: Mayank Patke <fishythefish@google.com>
Once available, I will throw these diagnostics instead of just returning.
Change-Id: I64d9a1657f876328a5dce9a7e3e3ea5035a9f69e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/350741
Auto-Submit: Jake Macdonald <jakemac@google.com>
Commit-Queue: Jake Macdonald <jakemac@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
I also formatted this file, since you can do that now :)
Change-Id: I7e7ad586cb4ba31f42a873a362d8099d1576e608
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/350480
Commit-Queue: Morgan :) <davidmorgan@google.com>
Auto-Submit: Jake Macdonald <jakemac@google.com>
Reviewed-by: Morgan :) <davidmorgan@google.com>
According to the patterns spec, the pattern context type schema for a
cast pattern should be `_`. What was actually implemented was
`Object?`.
This is unlikely to make a difference in practice, since (a) cast
patterns are unlikely to be used in circumstances where the pattern
context type schema makes a difference, and (b) it takes some effort
to come up with expressions whose type inference behavior is differenc
between a schema of `Object?` and a schema of `_`.
Change-Id: I695752c8c163621a34faaa8d62b2b076c8152eb0
Bug: https://github.com/dart-lang/sdk/issues/54640
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/346383
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
The data structure in the front end for recording which getter names
can be type promoted via "field promotion",
`fieldNonPromotabilityInfo`, is a field in the `SourceLibraryBuilder`
class. This means that each library tracks its own notion of which
getter names are promotable. This makes sense because only private
getter names are eligible for field promotion, and private getter
names can only be used to access declarations in the same library.
Prior to this commit, if the user tried to perform type promotion on a
_public_ getter name, the shared flow analysis logic would correctly
deem that public name non-promotable and reject the promotion. If the
front end then called `FlowAnalysis.whyNotPromoted` (which it
typically does to get details about failed type promotions that lead
to compile-time errors), flow analysis would then use
`FlowAnalysisOperations.whyPropertyIsNotPromotable` to query the
`fieldNonPromotabilityInfo` data structure.
Since `fieldNonPromotabilityInfo` is tracked separately for each
library, if the public getter referred to a field declared in some
other library, no information would be found, so
`FlowAnalysisOperations.whyPropertyIsNotPromotable` would return
`null`. This would lead flow analysis to incorrectly conclude that the
reason for the getter name being non-promotable was due to a conflict
with some other getter with the same name in the same library, so it
would return `PropertyNotPromotedForNonInherentReason` to the front
end. The front end would then iterate through
`fieldNonPromotabilityInfo` looking for conflicting getters, again
finding nothing. This led to an assertion failure, because it doesn't
make logical sense for a getter name to be non-promotale due to
conflicts if there are no conflicts. In production builds of the front
end (with assertions disabled), the behavior was that the compile-time
error would simply have no "why not promoted" context message (which
is fairly benign, but still undesirable).
This commit fixes the bug by modifying the shared logic for
`FlowAnalysis.whyNotPromoted` so that if the getter name in question
doesn't begin with `_` (meaning it's public), then it doesn't bother
calling `FlowAnalysisOperations.whyPropertyIsNotPromotable` at all,
because it knows the non-promotion reason is because the name is
public. As a result, the correct context message gets generated, and
there is no assertion failure.
Note that the language test included in this commit doesn't check that
the correct context message is generated, because the test
infrastructure isn't capable of testing context messages that point to
other files. But it does still serve to validate that the assertion no
longer fires.
Fixes https://github.com/dart-lang/sdk/issues/54777.
Change-Id: I697f55acad7c162bc5f49f2824c91d172497f344
Bug: https://github.com/dart-lang/sdk/issues/54777
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/349405
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
In CSP mode we cannot use `eval()`, so use static interop mechanism to
set stack trace limit. It seems `window` is available in Chrome and
others have higher limit anyway.
Change-Id: Id57dc86936d0568338a8ee4436e69e39be8eb32c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/349483
Reviewed-by: Slava Egorov <vegorov@google.com>
Commit-Queue: Martin Kustermann <kustermann@google.com>
Note that this crashes in the CFE currently.
Also, in the analyzer, the diagnostics are being attached to the macro application and not the type annotation which is provided as a target.
Change-Id: Id5fc36eb862314015fd6839167f8ba043cd7d34e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/349181
Reviewed-by: Morgan :) <davidmorgan@google.com>
Commit-Queue: Jake Macdonald <jakemac@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Small changes so it passes on the analyzer, the CFE does not yet detect cycles.
R=jakemac@google.com, scheglov@google.com
Change-Id: Iaeb586da7066e50b5b90c9793bc34b51e0eabbbc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/345062
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Jake Macdonald <jakemac@google.com>
Auto-Submit: Morgan :) <davidmorgan@google.com>
Commit-Queue: Morgan :) <davidmorgan@google.com>
This was a mistake in the shared logic implementation during the
implementation of patterns; instead of verifying that the for-in
expression is a subtype of `Iterable<dynamic>` (or `Stream<dynamic>`
in the case of an `await for`), it merely verifies that it's a subtype
of `Iterable<dynamic>?` (or `Stream<dynamic>?`). At some point, the
requisite error checking was added to the front end, but not to the
analyzer.
In an ideal world, the solution would be to remove the error check
from the front end and add it to the shared logic; however, doing so
is not easy, because what needs to be done is an assignability check
(as well as insertion of implicit downcasts and other coercions). The
shared logic doesn't currently have a good hook for doing that. So,
for now, this CL simply adds the requisite check to the analyzer. I
plan to try to find a clean way to share some of this logic in a
follow-up CL.
Fixes https://github.com/dart-lang/sdk/issues/54671.
Change-Id: Ib2fa8bce758dfe12401d85850666674ca1317e3f
Bug: https://github.com/dart-lang/sdk/issues/54671
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/347360
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Whenever a pattern performs an implicit `is` test, flow analysis
attempts to determine whether the `is` test is guaranteed to succeed;
if it is, then flow analysis considers the code path in which the `is`
test fails to be unreachable. This allows flow analysis to recognize
switch statements that are trivially exhaustive (because one of the
cases is guaranteed to match), avoiding spurious errors such as
"variable must be assigned before use" or "missing return statement".
This change upgrades the logic for computing when an `is` test is
guaranteed to succeed, so that it accounts for type erasure of
extension types. This brings flow analysis's treatment of switch
statements into closer alignment with the exhaustiveness checker,
which should reduce the risk of confusing error messages. For more
information see
https://github.com/dart-lang/language/issues/3534#issuecomment-1885839268.
Fixes https://github.com/dart-lang/language/issues/3534.
Bug: https://github.com/dart-lang/language/issues/3534
Change-Id: Ib73d191e04e7fa8c0e6888c2733dae73d8f389da
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/345822
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
I discovered recently that when we implemented the "patterns" feature,
we got the pattern type schema for cast patterns wrong--it was
specified to be `_`, but what was actually implemented was
`Object?`. I've filed a breaking change request to address that:
https://github.com/dart-lang/sdk/issues/54640.
In the course of prototyping a fix, I was surprised to find that no
language tests or co19 tests were affected; it appears that we don't
have adequate test coverage of pattern type schemas. This CL addresses
the lack of test coverage by adding a language test.
The test case for cast patterns is currently commented out, pending
resolution of the breaking change request; after the breaking change
lands, I will update the test accordingly.
Change-Id: I0d27fd8ac65f16d21a8597586e6f76fd9a5ba86e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/346621
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Bob Nystrom <rnystrom@google.com>
This moves the hoisted declaration of pattern variables till after the
variable cache declarations, such that any use of variables in the
matched expression, shadowed by the pattern variables, occurs before the
declaration of the shadowing variables.
Closes#54559
Change-Id: Id8ca8e7a499b9d71a50cd9987808d159f26bbd24
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/345942
Reviewed-by: Erik Ernst <eernst@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
In https://dart-review.googlesource.com/c/sdk/+/345082, type erasure
was added to the handling of relational patterns, to address some co19
failures, e.g.:
extension type const BoolET1(bool _) {}
const True1 = BoolET1(true);
String testStatement1(bool b) {
switch (b) {
case == True1:
...
}
}
This was failing because the type of `True1` (`BoolET1`) is not
assignable to the argument type of `operator==`, which is
`Object`. (This is because extension types do not, by default, extend
`Object`; they extend `Object?`).
Adding type erasure elimited the co19 failure, but it caused other
code to be allowed that shouldn't be allowed, such as:
extension type const E(int representation) implements Object {}
class A {
bool operator <(int other) => ...;
}
const E0 = E(0);
test(A a) {
if (a case < E0) ...;
}
This shouldn't be allowed because the type expected by `A.<` is `int`;
allowing `E0` to be passed to this operator breaks extension type
encapsulation.
The correct fix is for assignability checks for `operator==` to use
`S?` rather than `S`, where `S` is the argument type of
`operator==`. This is consistent with the patterns specification, and
it ensures that `== null` and `!= null` are allowed, while continuing
to prohibit relational patterns that break extension type
encapsulation.
Fixes https://github.com/dart-lang/sdk/issues/54594.
Bug: https://github.com/dart-lang/sdk/issues/54594.
Change-Id: Id090f432500d75ba694383f1788d58353cd1fc72
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/345860
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
- Rename some compile time errors tests to end with "error_test.dart"
- Delete runtime tests that no longer perform the operations they
claim to be testing. In many cases they do nothing at all.
Change-Id: I49c7b50d6628f56641b878635b32dbc0bc016bbb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/345345
Commit-Queue: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Erik Ernst <eernst@google.com>
The direct algorithm for solving for equality replaces the more
general constraint solving algorithm. It reduces complexity and
removes the requirement on the constraint gathering and solving engine
to be applicable at the earlier stages of compilation.
Change-Id: I73dfa39bb1534803c6985578708ecab198c26e25
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/343920
Reviewed-by: Erik Ernst <eernst@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Chloe Stefantsova <cstefantsova@google.com>
Today we make a half-hearted attempt to generate the correct default type arguments for parameter stubs. By the time we create them in the emitter we don't have enough information to fully replace all the type variables that might appear in a type parameter's bound. So instead we erase all type variables to "any".
By generating parameter stubs in SSA we can use existing RTI infrastructure to generate the correct recipes/bindings/evals. Instead of directly creating JS we create the appropriate SSA graph and let the normal function compiler do the heavy lifting.
There are benefits and drawbacks to this new approach:
1) The stub bodies go through the SSA optimizer so some parameters are abbreviated, some recurring nulls get replaced by a variable, etc. The net result is slightly more compact code.
2) We are now compiling code during the emitter phase. This is relatively fast (adds ~2% duration to the phase) but also requires deserializing some data we weren't previously using. This results in ~200MB more memory being used. I've changed the serialization layout a bit to try to mitigate this but some of it is unavoidable.
Change-Id: Ie786da933accc039716e1c8481dbb8a09f3cc9b2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/340700
Reviewed-by: Stephen Adams <sra@google.com>
Reviewed-by: Kallen Tu <kallentu@google.com>
Commit-Queue: Nate Biggs <natebiggs@google.com>
Provides consistent behavior for identical and equals across all
backends. The test was failing on JavaScript backends because
Strings that are equal are also identical.
Change-Id: I85929f37746d8f7e192410e3d2f6530ce05176c5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/343860
Reviewed-by: Nate Bosch <nbosch@google.com>
Auto-Submit: Nicholas Shahan <nshahan@google.com>
Commit-Queue: Nate Bosch <nbosch@google.com>
Allows test to succeed when calling .toString on a type and expecting
a specific string.
Change-Id: Icf50d324c59e7432a6a9af3d722db3e9e3d8545e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/343684
Reviewed-by: Nate Biggs <natebiggs@google.com>
Reviewed-by: Nate Bosch <nbosch@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
Some named elements were colliding with existing properties on
record object instances.
Use a symbol in the runtime library for the shape and values
properties.
Ensure that the `.constructor` and `.prototype` getters are renamed
to match the expectation when compiling the access.
Add regression test for all 4 named elements.
Fixes: https://github.com/dart-lang/sdk/issues/54375
Change-Id: I7f3577455bfcff7dece6350e8c7e3ee7ffdbbac6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/342703
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Mark Zhou <markzipan@google.com>
This adds a new rule to exhaustiveness handling of the as-pattern. The
new rule checks whether the cast type is fully contained in the space
of the subpattern. If so, we can assume that the whole context type has
been matched because there is no value that passes the as-test that
would be rejected by the subpattern.
Closes#51986Closes#54125
Change-Id: I0dc7fb072395aa3bc9f0b143afb320966f9d64c5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/338760
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
This adds reporting of errors on initializing formals and
function formal parameters in primary constructors of
extension types. These are not supported by the current grammar.
Change-Id: I9e09ea599b9a455398a20c34b79c375fd884423d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/342583
Reviewed-by: Erik Ernst <eernst@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
This change reverts https://dart-review.googlesource.com/c/sdk/+/341020.
The reverted CL removed tests from `tests/language` that contained
legacy code (code with a language version less than 2.12). The primary
purpose of the removal was to unblock removal of legacy support from
the analyzer. It seemed safe to do because legacy code isn't supported
anymore, and no legacy code exists in google3 anymore. However,
several of those tests exercised important "weak mode" runtime
semantics of the web and VM platforms. See discussion at
https://dart-review.googlesource.com/c/sdk/+/341020/comments/bce31aa1_f5392ce1
for details.
This CL restores the deleted tests. Fortunately, since all the
restored tests are annotated with `Requirements=nnbd-weak`, they will
be skipped on the analyzer (thanks to
https://dart-review.googlesource.com/c/sdk/+/342080), so restoring
them no longer gets in the way of removing legacy support from the
analyzer.
Change-Id: Ib828be76f5c7eaeecaad0b7e7f7b0e3ff2f4bdb2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/342090
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
The annotation `Requirements=nnbd-weak` controls prevents a test from
being run on a platform that doesn't have weak mode runtime semantics
(e.g. a program running with sound null safety).
In a previous CL
(https://dart-review.googlesource.com/c/sdk/+/341020), I removed
several tests from `tests/language` that contained legacy code, in
order to unblock removal of legacy support from the analyzer; those
tests all contained this annotation. However, after discussion with
Siggi, we've decided that it would be better to keep those tests
around for a while longer, because they exercise important runtime
behaviors of unsound null safety mode on the web and VM platforms, and
not all google3 code has been migrated off of unsound null safety mode
yet.
So, I intend to take a different approach to unblocking removal of
legacy support from the analyzer: stop running tests with the
annotation `Requirements=nnbd-weak` through the analyzer. This will
allow the tests removed in
https://dart-review.googlesource.com/c/sdk/+/341020 to be restored,
but it will have a side effect of causing a small number of additional
analyzer tests to be skipped. All those tests are benign to skip on
the analyzer (because they are purely concerned with runtime
semantics) except for one:
`superinterfaces_out_of_order_error_test.dart`.
This test doesn't have any runtime semantics at all; it merely checks
that the analyzer and CFE produce the correct errors for certain
compile-time type checks that have nothing to do with weak mode
runtime semantics.
So, to prevent a gap in test coverage when the analyzer begins
skipping tests annotated with `Requirements=nnbd-weak`, we need to
remove this annotation from
`superinterfaces_out_of_order_error_test.dart`.
Change-Id: I7932a0200be750116c41a303b48aaef50bc952ec
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/341980
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Change-Id: Iba7bb2b1e2a1f63a228154be2a9ce23d46da54cf
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/341382
Reviewed-by: Erik Ernst <eernst@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Using `expectStaticType` is a more effective test, because it verifies
that the type is _exactly_ what is expected. It also makes the test
easier to read.
Change-Id: I4b0a22f32601a5dca830a182ff4d37cddd657776
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/340580
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Lasse Nielsen <lrn@google.com>
This CL breaks the dependency cycles in typedefs, allowing the
subsequent phases of compilation to rely on typdefs no longer being cyclic.
Closes https://github.com/dart-lang/sdk/issues/54269
Change-Id: Id5e2d579c65019ad7072913ccf586ea2a1963ef9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/341180
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Chloe Stefantsova <cstefantsova@google.com>
Reviewed-by: Erik Ernst <eernst@google.com>
Now that we no longer need to support legacy (pre-2.12) code in
google3, we don't need to test our support for it either.
Most of our language tests for legacy code were in `tests/language_2`
and have already been deleted; this CL addresses the few tests that
remain in `tests/language`, mostly dealing with "mixed mode" scenarios
(i.e., interactions between legacy code and null safe code).
Change-Id: I023d84547fdf17fb0599c82fbeb4fd07252ccf0f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/341020
Reviewed-by: Erik Ernst <eernst@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This adds support for extension types in exhaustiveness checking by
performing extension type erasure on the type before computing the
corresponding static type/space.
Change-Id: Ie75c903aec52a7c34410695787a9bea6008d637d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/340442
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
A quirk of analyzer and CFE implementations is that they resolve
property gets such as `foo.bar` to specific field or getter
declarations that may be not be directly defined on the target type;
for example if `foo` has type `B`, and `B` extends `A`, and `A`
contains a field called `bar`, then `foo.bar` is considered to refer
to `A.bar`, for example:
class A {
int? bar;
}
class B extends A {}
f(B foo) {
print(foo.bar); // Resolved to `A.bar`.
}
This is in contrast with the language specification, which makes a
clean distinction between class _declarations_ and the _interfaces_
implied by those declarations. While a class declaration can contain
(among other things) both getters and fields, which might be concrete
or abstract, an interface doesn't distinguish between getters and
fields, and is inherently abstract.
The advantage of the analyzer/CFE approach is that it allows more
intuitive error messages and "go to definition" behavior, which
benefits users. But it has some ill-defined corner cases involving
complex class hierarchies, because not every property access can be
resolved to a unique declaration (sometimes a getter is multiply
inherited from multiple interfaces, for example). The language spec
approach has the advantage of being well-defined and consistent even
in situations involving complex class hierarchies.
When I initially implemented field promotion, I took advantage of this
quirk of the analyzer and CFE implementations, so that I could make
property gets that refer to field declarations promotable, while
keeping property gets that refer to abstract getter declarations
non-promotable. This caused unpredictable behaviors in the ill-defined
corner cases. It also meant that in certain rare cases, a property
access might not be promoted even when it would be sound to do so
(e.g. a property access might refer to a private abstract getter
declaration, but the only concrete _implementation_ of that abstract
getter was a final field).
This CL changes the rule for promotability so that any get of a
private property is considered promotable, provided that the
containing library doesn't contain any concrete getters, non-final
fields, or external fields with the same name. It no longer matters
whether the private property refers to a field or a getter. This rule
is simpler than the old rule, restores the spec's clean distinction
between class declarations and interfaces, and allows more promotions
without sacrificing soundness.
For additional details please see the breaking change announcement at
https://github.com/dart-lang/sdk/issues/54056, as well as the original
change proposal at https://github.com/dart-lang/language/issues/3328.
Fixes https://github.com/dart-lang/sdk/issues/54056.
Fixes https://github.com/dart-lang/language/issues/3328.
Change-Id: I38ffcb9ecce8bccb93b1b2586a1222a0fb1005a7
Bug: https://github.com/dart-lang/sdk/issues/54056
Bug: https://github.com/dart-lang/language/issues/3328
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/337609
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Lasse Nielsen <lrn@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
This change implements one of the features of experimental feature
`inference-update-3`: with the experimental feature enabled,
assignments to local variables use the declared (or inferred) type of
the local variable as the context for evaluating the RHS of the
assignment, regardless of whether the local variable is promoted. With
the experimental feature disabled, assignments to local variables
continue to use the promoted type of the local variable as the context
for evaluating the RHS of the assignment.
This eliminates one of the scenarios in which the context type of an
assignment is "aspirational" (i.e., not required to be met in order to
prevent a compile time error). Once all aspirational context types
have been removed from the language, we will be able to re-work
coercions to be based on context type, which fixes a number of
usability footguns in the language. See
https://github.com/dart-lang/language/issues/3471 for details.
Bug: https://github.com/dart-lang/language/issues/3471
Change-Id: Ic07ac1810b641a9208c168846cd5fd912088d62b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/338802
Reviewed-by: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Named parameters can't start with underscores whether they are optional or required.
Change-Id: Iaaeab80f74a691b4981148f06412a60ca115f73e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/337883
Auto-Submit: Parker Lougheed <parlough@gmail.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Was failing to compile because `array` was a List<A> so indexSet on line 32 was invalid.
Change-Id: I555a2c341e0d9dec5e85d92ecc855d073f134694
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/339060
Commit-Queue: Nate Biggs <natebiggs@google.com>
Reviewed-by: Kallen Tu <kallentu@google.com>
These tests were just failing on the VM for a very long time.
Rewrite them into a simpler version that just tests conditional
importing rather than trying to do clever things with HttpRequest.
R=eernst@google.com
Change-Id: Iebb7f6210ae4f2215c9445695189533116ffb5b7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/338160
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Erik Ernst <eernst@google.com>
This test always fails on the VM because unlike other
targets VM allows to invoke this constructor with `new`.
Instead of having an always failing test change this test
to test VM's current behavior. This will also allow us to
catch breaking changes in the future.
Closes https://github.com/dart-lang/sdk/issues/53183R=eernst@google.com
Change-Id: I4ba63dc94d2bd824df44c604b0ca1c57eb8bd559
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/338124
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Erik Ernst <eernst@google.com>
Use generic types in type tests. This causes types to be retained.
Fixes:
language/control_flow_collections/if_inference_test
language/control_flow_collections/map_set_ambiguity_test
Change-Id: Ie8fe93320ec623cc3812d43bd36550a0fe32f613
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/338424
Commit-Queue: Stephen Adams <sra@google.com>
Reviewed-by: Bob Nystrom <rnystrom@google.com>
