Previously, mixin deduplication transformation expected that isAnonymousMixin
classes could occur only in a superclass position. After this change,
deduplication also handles (canonicalizes) isAnonymousMixin classes in
implementedTypes, as CFE uses isAnonymousMixin classes in implementedTypes
to represent superclass constraints for mixin declarations.
No other uses of isAnonymousMixin classes supported.
Fixes https://github.com/dart-lang/sdk/issues/34704
Change-Id: I8558ce53c4fb1e5a06f839fa5ec8022958d9dafd
Reviewed-on: https://dart-review.googlesource.com/c/78383
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
Auto-Submit: Alexander Markov <alexmarkov@google.com>
The code in class_hierarchy.dart that looped over override pairs
included implementations inherited from its superclass as potential
overriders, presumably in a broken attempt to include some interface
checks into the mix.
With this problem fixed, the special case in kernel_class_builder.dart
that excluded overriders not declared in the current class (i.e.
declared in a mixin) could be removed without incurring false
positives.
These override checks performed on members from a mixin produce an
extra context message indicating the class where the two members meet,
giving rise to the override relation (similarly to interface checks).
Since the same member can now override more than one member (or even
the same member more than once), the diagnostics message duplication
check has been extended to also include the context in the comparison.
Fixes https://github.com/dart-lang/sdk/issues/34235
Fixes https://github.com/dart-lang/sdk/issues/34285
Change-Id: I990e5719ae1749fd1aad1ad478aaa6e173cb170b
Reviewed-on: https://dart-review.googlesource.com/c/76900
Reviewed-by: Daniel Hillerström <hillerstrom@google.com>
When a member in the interface of a non-abstract class is found to
have an implementation with the correct name which does not override
the representative declaration of the member in the interface (and thus
has not been checked against that member for override validity), call
the override check to check interface validity.
Override relations thus checked produce an extra context message
explaining that the override relation must hold because both members
are inherited by a non-abstract class.
Fixes https://github.com/dart-lang/sdk/issues/32014
Change-Id: I955f057e35fa30f33c19da37c9cea1262042431c
Reviewed-on: https://dart-review.googlesource.com/c/74642
Reviewed-by: Dmitry Stefantsov <dmitryas@google.com>
This is a reland of 7419667d67
I reverted this CL in
https://dart-review.googlesource.com/c/sdk/+/77520 because I thought
it had broken a bunch of bots. It turns out that the bots weren't
broken; they were just erroneously reporting a breakage because some
change-detection code had noticed that there were new failing tests,
and there is not yet a way to tell the change-detection code that the
failures are expected.
Original change's description:
> Test that the result of instantiate-to-bounds is appropriately checked for super-boundedness.
>
> When I implemented super-boundedness for the analyzer in
> b1a54154f8, very little test status
> changed, indicating we didn't have much test coverage for verifying
> that the result of instantiate-to-bounds was appropriately checked for
> super-boundedness. This CL fills in the coverage gap.
>
> Change-Id: I8f6fbb3aa4cae1e62861b5eddb113ee9b4bb085c
> Reviewed-on: https://dart-review.googlesource.com/77010
> Commit-Queue: Paul Berry <paulberry@google.com>
> Auto-Submit: Paul Berry <paulberry@google.com>
> Reviewed-by: Erik Ernst <eernst@google.com>
Change-Id: Ic84321e928aaf4c6533e6cf5b9d521a1da77dba7
Reviewed-on: https://dart-review.googlesource.com/c/77660
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
This reverts commit 7419667d67.
Reason for revert: Broke a lot of bots. Will investigate tomorrow.
Original change's description:
> Test that the result of instantiate-to-bounds is appropriately checked for super-boundedness.
>
> When I implemented super-boundedness for the analyzer in
> b1a54154f8, very little test status
> changed, indicating we didn't have much test coverage for verifying
> that the result of instantiate-to-bounds was appropriately checked for
> super-boundedness. This CL fills in the coverage gap.
>
> Change-Id: I8f6fbb3aa4cae1e62861b5eddb113ee9b4bb085c
> Reviewed-on: https://dart-review.googlesource.com/77010
> Commit-Queue: Paul Berry <paulberry@google.com>
> Auto-Submit: Paul Berry <paulberry@google.com>
> Reviewed-by: Erik Ernst <eernst@google.com>
TBR=paulberry@google.com,lrn@google.com,leafp@google.com,eernst@google.com
Change-Id: I536c9721a1d79b98b2e3c2a6f7650b134b19f561
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/77520
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
When I implemented super-boundedness for the analyzer in
b1a54154f8, very little test status
changed, indicating we didn't have much test coverage for verifying
that the result of instantiate-to-bounds was appropriately checked for
super-boundedness. This CL fills in the coverage gap.
Change-Id: I8f6fbb3aa4cae1e62861b5eddb113ee9b4bb085c
Reviewed-on: https://dart-review.googlesource.com/77010
Commit-Queue: Paul Berry <paulberry@google.com>
Auto-Submit: Paul Berry <paulberry@google.com>
Reviewed-by: Erik Ernst <eernst@google.com>
I'm trying to fix an analyzer bug where typedef type parameters
weren't checked properly, and it appears that we have no language_2
test cases for it.
Change-Id: I216a337eeaae6ac2008c0c26139797e00099ffcb
Reviewed-on: https://dart-review.googlesource.com/76707
Reviewed-by: Lasse R.H. Nielsen <lrn@google.com>
Reviewed-by: Erik Ernst <eernst@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Allow super calls to occur in mixin declarations if they target a
method from any of the superclass constraint interfaces.
Instead of compiling the Dart mixin declaration
mixin M on S0, S1 {...}
to Kernel:
abstract class _M&S0&S1 = S0 with S1;
abstract class M extends _M&S0&S1 { ... }
we compile it to Kernel:
abstract class _M&S0&S1 implements S0, S1 {}
abstract class M extends _M&S0&S1 { ... }
because the former is not symmetrical with respect to S0 and S1. It
will prefer a method from the 'mixin' S1 over one from S0 which can
give a compile-time error if the method from S0 is more general.
Modify mixin inference to support the new compilation of mixin
declarations. It still has to support old-style VM super mixins until
support for those is removed from the VM.
Change-Id: Ib945aa11cc19c457b07bc802beae10d1663ff6b7
Reviewed-on: https://dart-review.googlesource.com/76141
Reviewed-by: Jenny Messerly <jmesserly@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Dmitry Stefantsov <dmitryas@google.com>
Commit-Queue: Kevin Millikin <kmillikin@google.com>
When I first began working on the feature I mistakenly thought that
these two test cases would be free from compile-time errors, so it
seemed like it would be worth adding a test case to make sure others
don't have the same misunderstanding.
Change-Id: I322991c818f9d136180de443090fa4e15f6f0e27
Reviewed-on: https://dart-review.googlesource.com/76680
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Lasse R.H. Nielsen <lrn@google.com>
This change expands typedefs into their underlying function type in the
compiler. Typedefs no longer exist at runtime in DDC, so they now have
similar equality and identity behavior as other function types.
The compiler used to preserve typedefs so they could have a better
toString. But over time that support has been almost entirely lost;
the caching for function/interface types eliminates typedefs, and
DDC+Kernel does not appear to get any TypedefTypes from the common
front end. So in practice typedef types were almost never present at
runtime. Because of this, we can remove the remaining support with very
little user visible effect. This also brings DDC's canonicalization
roughly in line with dart2js for compile-time typedefs, so frameworks
like Angular will be able to continue their use of `identical` for fast
type comparisons.
This CL also fixes DDC's type caching for generic function types
(typeHashCode was incorrect for them), and has some refactoring to
improve names/comments and make the SDK runtime code more clear.
Change-Id: I4e34c0c8f737a8535ba474a4683967ff41bcd3c8
Reviewed-on: https://dart-review.googlesource.com/74664
Commit-Queue: Jenny Messerly <jmesserly@google.com>
Reviewed-by: Alan Knight <alanknight@google.com>
This required adding an implementation of super-bounded types to the
analyzer.
We really could use some more tests to verify that we disallow
super-bounded types in all the appropriate locations, but allow it in
other cases (it seems that there are no language_2 tests to verify
this). To avoid churn, I'll wait until I have confirmation that my
reasoning is correct in #34583 before submitting test cases.
Fixes#34532.
Fixes#34560.
Change-Id: I3c5def60bcac0d31b56bead31cb1aab445f18e96
Reviewed-on: https://dart-review.googlesource.com/76280
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
If a field is initialized with null (either explicitly or implicitly),
field store can be omitted in bytecode. In such case, bytecode should
still convey the information about this initialization to VM for
field guards to work correctly.
Change-Id: I1fd45b858c3c521b97fa5dbffe0e15b1ea75d92f
Reviewed-on: https://dart-review.googlesource.com/76060
Reviewed-by: Zach Anderson <zra@google.com>
Reviewed-by: Régis Crelier <regis@google.com>
I will soon start landing fixes to address some (but not necessarily
all) of the test cases in this test, and I want to make sure that my
fixes produce corresponding status file changes, to secure against
possible future regressions.
Change-Id: Iaec11b85a20d670c0702220cadbbdfc51c026d1f
Reviewed-on: https://dart-review.googlesource.com/75624
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Leaf Petersen <leafp@google.com>
Also modify async_backwards_compatibility_1_test so that fixing these
bugs won't cause it to start failing.
Change-Id: I7bb3b9587a0aeea8662df1923034bcb409a29522
Reviewed-on: https://dart-review.googlesource.com/75280
Reviewed-by: Lasse R.H. Nielsen <lrn@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
The bleeding edge console revealed that status files for
dartk-checked-linux-release-x64 configuration needed to be updated.
Change-Id: I7015427f55436947aebd019212765c5f3937f846
Reviewed-on: https://dart-review.googlesource.com/75440
Reviewed-by: Dmitry Stefantsov <dmitryas@google.com>
Commit-Queue: Daniel Hillerström <hillerstrom@google.com>
There are several crashes on kernel-precomp bots related to the new
tests. As front-end does not issue a compile-time error,
AOT transformations such as constant evaluation are not skipped and
may crash on incorrect hierarchy.
Stack trace:
Change-Id: Ieaf789535eb4c59a3cc4f2f3437b2b41b9e506f4
Reviewed-on: https://dart-review.googlesource.com/75401
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Leaf Petersen <leafp@google.com>
Fixes#34167. This implements the Dart 2 mixin proposal
(https://goo.gl/KEKQyv) for DDC. When the mixin is applied, a class
is created for the application that extends the correct superclass
and has all of the instance members, so `super` works correctly.
This also fixes a few minor issues in Analyzer's (mostly complete)
implementation:
- InterfaceType.isObject now returns false for Dart 2 mixins.
- Least upper bound calculation recognizes mixins are not Object.
- Interface of the mixin now implements its superclass constraints.
- Mixin superclass constraints are checked against the superclass and
all previously applied mixins (if any); this keeps it working with
the subtype fix above, and also prevents a not-yet-applied mixin
from satisfying the constraint
The language_2/mixin_declaration tests were updated with a few minor
fixes now that we can run Analyzer/dartdevc to test them.
This change implements super mixins for DDC's Kernel backend (DDK)
too. This will be enabled once Kernel adds a flag to recognize which
Class nodes are mixins (vs normal classes).
Change-Id: Ib3c4fcb12de9988345e52d92931196828d8227c3
Reviewed-on: https://dart-review.googlesource.com/74965
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Vijay Menon <vsm@google.com>
The point is that #32782 requests a fix such that
`typedef F1 = void Function(int)` and `typedef void F2(int)`
satisfy that `F1 == F2` evaluates to true. This is marked
'p3-low' for the common front end but 'p1-high' for DDC.
It seems likely to me that this could create the situation where
code is developed using DDC, is working, and then fails upon
deployment using the vm. Also, there is a single case where
`dart2js` fails in the associated `type_alias_equality_test.dart`,
so deployment on the web would also fail upon deployment, though
only in some of the cases.
However, with some input from Aske I concluded that the situation
might have arisen because there _is_ no work to do for this in the
common front end, because it will be handled by the backend (which
also explains why `dart2js` has it almost right).
This CL is just introducing a tiny change: It changes the issue
indicated for all VM related failures in said test to point to
#32783, which is presumably the right issue for backend work.
Apart from that, I've added you, the reviewers, in order to make
sure that the relevant people get this heads up. We may then decide
to land this CL, change the priority on #31359, do nothing, or
whatever turns out to be the right response. ;-)
Change-Id: I92672547d7fe795e877604c0da1e0e4579e4e04a
Reviewed-on: https://dart-review.googlesource.com/74403
Reviewed-by: Jenny Messerly <jmesserly@google.com>
We had implemented the rule for invalid returns in asynchronous
non-generator functions with return type T and a return expression
with static type S:
is an error if flatten(T) is void and flatten(S) is not void,
dynamic, or Null
and the actual rule should be:
is an error if T is void and flatten(S) is not void, dynamic, or
Null
which requires T to be exactly void and so signals fewer errors.
Change-Id: I296bb8a607d6e5ad862017674091eafe0f6016da
Reviewed-on: https://dart-review.googlesource.com/73740
Commit-Queue: Kevin Millikin <kmillikin@google.com>
Reviewed-by: Leaf Petersen <leafp@google.com>
Reviewed-by: Dmitry Stefantsov <dmitryas@google.com>
The valid return tests were misnamed and hence not run. Also remove
duplicate tests from one file, and add status to the informal proposal.
Change-Id: I20179e81a0281302056f601ee3b7a51e2aec180e
Reviewed-on: https://dart-review.googlesource.com/73562
Commit-Queue: Leaf Petersen <leafp@google.com>
Reviewed-by: Kevin Millikin <kmillikin@google.com>
This CL adds support for _dynamic_ calls (which are routed through
dynamic invocation forwarders) to bytecode pipeline.
This allows to skip argument type checks for non-covariant and
non-generic-convarint-impl parameters in bytecode when calling instance
members through known interface targets. In case of dynamic call, dynamic
invocation forwarders perform type checks which are otherwise skipped.
This CL also corrects argument type checks in forwarding stubs.
Forwarding stubs are synthetic methods, introduced by front-end to ensure
type safety if some of the argument type checks are skipped.
The specifics of forwarding stubs is that they should use parameter types
of a *target* of a stub for the argument type checks.
This change unifies how argument type checks are performed in bytecode
and in the VM in JIT mode. This allows to mix bytecode methods with
methods JIT-compiled from kernel AST in more cases.
Change-Id: I3a2720a488c8bbb91de1b189eb36ee8c51684a78
Reviewed-on: https://dart-review.googlesource.com/72543
Reviewed-by: Samir Jindel <sjindel@google.com>
Reviewed-by: Régis Crelier <regis@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Implement the feature spec for checking returns against inferred
function return types:
https://github.com/dart-lang/sdk/blob/master/docs/language/informal/invalid_returns.md
with the change that an invalid return without a subexpression is
a warning, not an error. This is because it is a warning in the
analyzer and it would be a breaking change to turn it into an
error.
The check for valid returns is moved to exactly where we handle
returns, rather than in ensureAssignable which is used as a
helper in a lot of places. A bug in type inference was fixed: we
would use `void` for the type of return without a subexpression
instead of `null`. To accommodate that we would use a
non-standard subtyping relation for returns. This could lead to
us inferring a return type of `void` in cases where we should
not.
Change-Id: Iee9ece9c722f47efa305f49490d3022d0bbb9f44
Reviewed-on: https://dart-review.googlesource.com/72403
Commit-Queue: Kevin Millikin <kmillikin@google.com>
Reviewed-by: Aske Simon Christensen <askesc@google.com>
The below code is not okay, even if we add a noSuchMethod to C.
```
class B {
foo(int x) => x;
}
class C extends B {
foo(int x, {int y});
}
```
Issue #32660.
Change-Id: I1ace4443c3732fdd5faefa50e0d2cb68023e97c4
Reviewed-on: https://dart-review.googlesource.com/72106
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Peter von der Ahé <ahe@google.com>
E.g. `new Map<String>` is now reported as an error.
Note that this does not add a check for whether correct types are given,
so e.g.
```
class Foo<X extends num> {}
main() {
new Foo<String>();
}
```
is not caught by this fix.
Fixes#32972, #32281.
Change-Id: I57513b5c13865a828890775c8e14f7433d00bdbe
Reviewed-on: https://dart-review.googlesource.com/72382
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Peter von der Ahé <ahe@google.com>
Dmitry Stefantsov