Pull the error handling outwards to _InstanceCreationEvaluator.evaluate. All errors in const constructors should point to the location of the actual exception and provide more context.
Change-Id: Iafcf46182fab698f5546c63780de6dd9a605a51b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/318802
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Kallen Tu <kallentu@google.com>
evaluateConstructorCall now returns a Constant, which means evaluation may be cut early in some places if there's an error. However, some code generators rely on _InstanceCreationEvaluator to complete, and so there's no _major_ changes to the design right now (and some small additions to avoid breaking many tests).
This CL is just the framework for the "return Constant" design and will be the base for reporting better errors in constructor invocations.
These changes already show that we cut some amount of over-reporting in our existing language and unit tests, which is a step in the right direction.
Bug: https://github.com/dart-lang/sdk/issues/47603, https://github.com/dart-lang/sdk/issues/47351, https://github.com/dart-lang/sdk/issues/49389
Change-Id: I5ba7f1282658884c18a32d5e98c7804bbfeac0f6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/312347
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Kallen Tu <kallentu@google.com>
The tests `variable_property_not_promoted_test` and
`this_property_not_promoted_test` verify that fields are _not_
promoted. To prevent them from failing when we enable the language
feature `inference-update-2` (which provides field promotion), they
need `@dart=3.0` directives. This ensures that we will continue to
have coverage of the old (non-promoting) behavior after field
promotion is switched on.
Bug: https://github.com/dart-lang/language/issues/2020
Change-Id: I376da01dfe4223f17ae4863e4460b4f1a0708dc6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/314760
Reviewed-by: Lasse Nielsen <lrn@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
While rereading the analyzer logic that decides whether to suppress
field promotion due to the presence of a `noSuchMethod` forwarder, I
found a few corner cases that aren't handled correctly:
- The logic for deciding which fields are included in a class's
implementation currently doesn't understand that an abstract field
is abstract; it treats it as a concrete field, therefore if a
concrete subclass implements of `noSuchMethod`, but fails to
implement the field, the analyzer fails to detect that there will be
a `noSuchMethod` forwarder (and thus fails to suppress promotion).
- The logic for collecting the set of fields (and getters) that are
included in a class's interface (or implementation) currently stops
at a library boundary, so it doesn't properly handle the situation
where there is a library cycle, and two classes in one library are
related through an intermediate class in some other library.
- The logic for collecting the set of getters that are included in a
class's interface currently ignores the `on` clauses of mixins.
This CL includes tests for all these corner cases; I will fix them in
a follow-up CL.
Change-Id: I92d73c0643f1ab89144feefab68779418a2c7a35
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313840
Reviewed-by: Erik Ernst <eernst@google.com>
Reviewed-by: Lasse Nielsen <lrn@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Prior to this change, the first phase of flow analysis, in which flow
analysis "looks ahead" to see which variables are potentially assigned
in each code block, was failing to properly account for variables that
are assigned by pattern assignment expressions. This "look ahead"
information is used by flow analysis to determine the effects of
nonlinear control flow (e.g. to figure out which variables to demote
at the top of a loop, or to figure out which variables are potentially
assigned inside a closure). As a result, it was possible to construct
correct code that would be rejected by the analyzer and compiler, as
well as incorrect code that would (unsoundly) be accepted.
The fix is to call `AssignedVariables.write` from the analyzer's
`FlowAnalysisVisitor`, and from the CFE's `BodyBuilder`, to let flow
analysis know about variable assignments that occur inside pattern
assignment expressions.
Fixes#52745.
Change-Id: I0e6f426a5c5c36f214d5a206aaf5a2de0bfdaac1
Bug: https://github.com/dart-lang/sdk/issues/52745
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/310502
Auto-Submit: Paul Berry <paulberry@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Constant Sets now have their own classes rather than being constructed as a wrapper over a const Map. The new scheme is similar to Maps - using a ConstantStringSet backed by a JavaScript Object 'dictionary' when the elements are all suitable String values, otherwise using a GeneralConstantSet backed by a list of elements that is converted to a lookup map on demand.
Constant Sets and Maps with suitable String keys are backed by a JavaScript Object literal 'dictionary'. The use of the Object literal has been changed so that the property values are no longer the values of the Dart Map, but the position of the key in the order of the entries. The values are provided as a List (Array), so there is an additional indexing operation to access the value on lookup. This does not seem to affect performance.
The advantage of the two-level lookup using the 'dictionary' is that Maps with the same keys (in the same order) can share a 'dictionary'. In order to achieve the advantage, the JavaScript Object is modeled as a first class ConstantValue - JavaScriptObjectConstantValue.
These changes achieve a code size benefit of -0.90% (~130K) on the main unit of a certain large app, and -0.35% for flute/benchmarks/lib/complex.dart
Change-Id: Icad2e6136218486a439e3c5ed0296462e3c3c4e6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/310020
Commit-Queue: Stephen Adams <sra@google.com>
Reviewed-by: Mayank Patke <fishythefish@google.com>
This change updates the flow analysis support for field promotion
(which is not yet switched on by default) so that it supports field
accesses inside cascade expressions. The key moving parts are:
- The type hierarchy `PropertyTarget` (which is used by the client to
tell flow analysis whether the target of a property access is
`this`, `super`, or an ordinary expression) now has a new class,
`CascadePropertyTarget`, to represent the situation where the target
of the property access is an implicit reference to the target of the
innermost enclosing cascade expression.
- Flow analysis has two new methods on its API:
`cascadeExpression_afterTarget` and `cascadeExpression_end`, so that
the client can inform flow analysis when a cascade expression is
being analyzed.
- Flow analysis uses its `_makeTemporaryReference` method to track the
implicit temporary variable that stores the target of cascade
expressions. (This method was developed as part of flow analysis
support for patterns, where it creates the data structures necessary
to track the implicit variables that are created as part of pattern
desugaring).
- The "mini-AST" pseudo-language used by flow analysis unit tests now
has a way to represent cascade expressions and method invocations.
- In addition to unit tests for `_fe_analyzer_shared`, `analyzer`, and
`front_end`, there are new language tests in
`tests/language/inference_update_2` to test cascaded field
promotions in end-to-end fashion.
Bug: https://github.com/dart-lang/language/issues/2020
Change-Id: I21353bbc884ed599cb1739cecfb68ad1d975d18b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/309220
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This CL moves language/unsorted/flatten_test.dart to language/async,
changes it from a multi-test to a regular test, removes some run-time
errors that aren't relevant to the purpose of the test, and removes
a function literal that triggers a type inference anomaly (language
repo issue 3148) which is also not the purpose of this test.
Change-Id: Ib0c84f79476dafd9b6d763062eef687d7432d424
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/309280
Reviewed-by: Lasse Nielsen <lrn@google.com>
Commit-Queue: Erik Ernst <eernst@google.com>
Change-Id: If31c487e3ebe2c1ae847aff7c8994580b8b6f2f6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/309660
Reviewed-by: Alexander Thomas <athom@google.com>
Commit-Queue: Alexander Thomas <athom@google.com>
This is a reland of commit 6f4bed30cc
Original change's description:
> [vm] Fix CompileType::CanBeFuture()
>
> In certain cases compiler can omit a type check in 'await e' if it can
> prove that expression 'e' cannot be evaluated to Future.
>
> Previously, if actual type of 'e' is unknown, CompileType::CanBeFuture()
> tested if static type of 'e' is a subtype of Future.
>
> This is not correct, as static type could have a subtype which is also
> a subtype of Future, but static type itself is not a subtype of Future:
>
> class A {}
> class B implements A, Future<A> {}
>
> A e = confuse(B()); // 'e' is B, but has a static type A.
> await e; // A is not a subtype of Future, but 'e' is Future
> // and should be awaited.
>
TEST=language/async/await_flatten_test
Fixes https://github.com/dart-lang/sdk/issues/52585
Change-Id: I37add9021261c95dee5930e031187868e821c161
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/309481
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
This reverts commit 6f4bed30cc.
Reason for revert: g3 failure (b/287294480).
Original change's description:
> [vm] Fix CompileType::CanBeFuture()
>
> In certain cases compiler can omit a type check in 'await e' if it can
> prove that expression 'e' cannot be evaluated to Future.
>
> Previously, if actual type of 'e' is unknown, CompileType::CanBeFuture()
> tested if static type of 'e' is a subtype of Future.
>
> This is not correct, as static type could have a subtype which is also
> a subtype of Future, but static type itself is not a subtype of Future:
>
> class A {}
> class B implements A, Future<A> {}
>
> A e = confuse(B()); // 'e' is B, but has a static type A.
> await e; // A is not a subtype of Future, but 'e' is Future
> // and should be awaited.
>
> TEST=language/async/await_flatten_test
> Fixes https://github.com/dart-lang/sdk/issues/52585
>
> Change-Id: I270a8260224246e1f8c16eff57231363a0f25ae6
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/309380
> Commit-Queue: Alexander Markov <alexmarkov@google.com>
> Reviewed-by: Ryan Macnak <rmacnak@google.com>
Change-Id: I87a08e46b4af6c57efa828c0f9041e6b5e51708e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/309480
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Alexander Markov <alexmarkov@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
In certain cases compiler can omit a type check in 'await e' if it can
prove that expression 'e' cannot be evaluated to Future.
Previously, if actual type of 'e' is unknown, CompileType::CanBeFuture()
tested if static type of 'e' is a subtype of Future.
This is not correct, as static type could have a subtype which is also
a subtype of Future, but static type itself is not a subtype of Future:
class A {}
class B implements A, Future<A> {}
A e = confuse(B()); // 'e' is B, but has a static type A.
await e; // A is not a subtype of Future, but 'e' is Future
// and should be awaited.
TEST=language/async/await_flatten_test
Fixes https://github.com/dart-lang/sdk/issues/52585
Change-Id: I270a8260224246e1f8c16eff57231363a0f25ae6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/309380
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Public members added to augmentation libraries where not included
in the export scope of the origin library, making them unavailable
for libraries the import the original library.
Change-Id: I74c7da5e0be98ff15b6ddbf55aa3812c04ded356
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/304884
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Test tests/language/nnbd/syntax/opt_out_nnbd_modifiers_test.dart
has a legacy library, so it cannot run with sound null safety.
This change adds '// Requirements=nnbd-weak' in order to skip
this test in sound null safe mode.
Change-Id: I48eb0d920a1de8e426aa9663ce8338b92e73c9aa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/304207
Commit-Queue: Bob Nystrom <rnystrom@google.com>
Auto-Submit: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Bob Nystrom <rnystrom@google.com>
Null assertion `iq!` throws a runtime error, so the test should not
expect successful completion.
Change-Id: Ic1a59932ecdc0a4d18b1ff0c9a9b335038e0309c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/304206
Commit-Queue: Bob Nystrom <rnystrom@google.com>
Auto-Submit: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Bob Nystrom <rnystrom@google.com>
noSuchMethod() throws a runtime error, so the test should not expect
to complete successfully when calling noSuchMethod().
Change-Id: Ib1b7c3164771939b7c7374f08d86e1b216cab392
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/304204
Reviewed-by: Bob Nystrom <rnystrom@google.com>
Auto-Submit: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Bob Nystrom <rnystrom@google.com>
This test verifies stack traces of certain type errors.
In AOT mode, stack traces only contain line numbers but not columns.
So, the test is updated to accomodate for AOT stack traces.
Also, the test is skipped in 'dwarf' and 'obfuscated' modes as
they don't provide symbolic stack traces.
Change-Id: I5a590b13c9ecaffbd4a495441fc66f807d942a12
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/304208
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Auto-Submit: Alexander Markov <alexmarkov@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>
These tests are based on the unit tests I wrote during development of
the flow analysis support for patterns; as such, they are not
especially uniform in their coverage, but they should be a good
start. I intend to expand coverage in future CLs.
Bug: https://github.com/dart-lang/sdk/issues/50419
Change-Id: I23bd96621a0269c1e642a7fa3b65c851357505c3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/304320
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Bob Nystrom <rnystrom@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>
These tests are flaky or timing out on the stable DDC configurations.
This change adds skips for the canary configurations to avoid crashing
the infra when too many tests that timeout are attempted to be
deflaked.
When the canary mode stabilizes we should manually mark all these
tests as flaky if they are still not passing on the stable DDC
configurations.
Issue: https://github.com/dart-lang/sdk/issues/50666
Change-Id: I9b1cb8fe466624480767fea05610331bf0f4ed87
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302843
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Nicholas Shahan <nshahan@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>
With one exception (noted below), the shared analysis logic uses the
convention that the expressions passed to flow analysis are the
original (pre-lowered) expressions, whereas the expressions passed to
flow analysis by the CFE are the lowered expressions. This difference
caused two problems:
- If a boolean expression appeared on the right hand side of a pattern
assignment or pattern variable declaration, and it was lowered, the
flow analysis implications of that boolean expression would be lost.
- If a boolean expression appeared in a `when` clause, and it was
lowered, the flow analysis implications of that boolean expression
would be lost. Exception: for switch statements, the shared analysis
logic has been passing lowered expressions to flow analysis, so this
problem didn't occur for `when` clauses in switch statements.
Notably, when compiling for the VM, the CFE lowers expressions like
`x != null` to something more like `!(x == null)`.
Fortunately, the first of these two situations shouldn't cause
problems very often, since typically if the right hand side of an
assignment or variable declaration is a boolean expression, there is
no need for the left hand side to involve patterns.
As for the second of these two situations, it's also not too likely to
cause problems, since typically null checks occur inside patterns
rather than in `when` clauses.
As a short term fix, we remove the exception noted above, and we
account for the difference in conventions by adding a call to
`FlowAnalysis.forwardExpression` to the CFE's implementation of
`dispatchExpression`, so that when transitioning between CFE logic and
shared logic, flow analysis will be informed how to match up the
lowered expressions to their pre-lowered counterparts.
Longer term, I would like to switch everything to the convention of
using the original (pre-lowered) expressions; this will bring the
analyzer and CFE into better alignment with each other and should
eliminate a class of subtle bugs. This long term goal is tracked in
https://github.com/dart-lang/sdk/issues/52189.
Fixes#52183.
Fixes#52241.
Bug: https://github.com/dart-lang/sdk/issues/52183, https://github.com/dart-lang/sdk/issues/52241.
Change-Id: I2449ce34c54325603bc2730d1660a7cfc7d79aec
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/298840
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This behaviour should only happen when a post-feature library implements
a pre-feature library declaration that has a final/base core library class as a super declaration.
Bug: https://github.com/dart-lang/sdk/issues/52115
Change-Id: If42129ba3ba7e337cc6ffc21604c6d0f2976344c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/301503
Reviewed-by: Nate Bosch <nbosch@google.com>
Commit-Queue: Kallen Tu <kallentu@google.com>
Reviewed-by: Johnni Winther <johnniwinther@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>
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>
In DDC the types [num], [int], [double], [String], and [bool] used in
bounds caused an "optimization" in type tests that was incorrect when
passing a subtype as the type argument.
Issue: https://github.com/dart-lang/sdk/issues/52243
Change-Id: I0ae8d21c907cd923d62dbae257085545840a8cdf
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/301140
Reviewed-by: Bob Nystrom <rnystrom@google.com>
