This is a small tidy up that changes `failTestOnAnyErrorNotification` to be automatically set when using `expectErrorNotification` so there's slightly less noise in tests.
Change-Id: I44056a23aca872b8e96109be4d4ba4356b8af53d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/359221
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Goma is off by default, and RBE is only enabled where it is known to
work, so there is no need to disable RBE instead. Some of these builders
additionally disable RBE in their infra/config definitions and there is
no need to duplicate that information here.
Bug: b/296994239
Change-Id: I9f9733ba5189b53b899289a23d68126f7c6adcc5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/357610
Commit-Queue: Jonas Termansen <sortie@google.com>
Reviewed-by: William Hesse <whesse@google.com>
Previously, the FFI used unboxed integers as a native representation
for pointers in FFI code, as the compiler only handled very specific
uses of untagged pointers flowing between instructions. Since then,
this restriction has been removed for untagged pointers that do not
point to memory managed by the GC, like FFI pointers, so now they can
have a more precise representation.
By being precise about when untagged (untagged pointers to freshly
allocated Handles and the contents of Pointer data fields) and tagged
(TypedData objects constructed to hold the byte representation of
compound data) values are expected, we can remove the need to have
untagged pointers to GC-movable objects and/or having untagged
pointers escape as unboxed integers in the generated IL.
This CL also renames kUnboxedFfiIntPtr -> kUnboxedAddress and limits
its uses specifically to where the unboxed integer represents the
numeric representation of an untagged pointer.
This CL changes CCall to take Representations for the arguments and
return value instead of what looks like an arbitrary
NativeCallingConvention. However, the serializer and deserializers for
CCall, used in IL tests, originally assumed that the argument and return
representations were kUnboxedFfiIntPtr, so providing an arbitrary
NativeCallingConvention which didn't match that assumption would cause
failures during IL tests. That assumption came from the fact that
the only creator of CCall instructions was in kernel_to_il.cc, and there
that was the case.
Now CCall builds the native calling convention during construction
and deserialization from the argument and return representations and
stores both the representations and built native calling convention
internally. In the future, if we want to create CCall instructions with
more arbitrary native calling conventions, then we'll need to handle
serialization/deserialization of arbitrary native calling conventions,
and also add consistency checks that the provided representations
appropriately match the native calling convention.
TEST=ffi vm/dart/regress_306327173_il vm/dart/address_local_pointer_il
Issue: https://github.com/dart-lang/sdk/issues/54710
Cq-Include-Trybots: luci.dart.try:vm-aot-android-release-arm64c-try,vm-aot-android-release-arm_x64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-debug-x64c-try,vm-aot-mac-release-arm64-try,vm-aot-mac-release-x64-try,vm-aot-obfuscate-linux-release-x64-try,vm-aot-optimization-level-linux-release-x64-try,vm-aot-win-debug-arm64-try,vm-appjit-linux-debug-x64-try,vm-asan-linux-release-x64-try,vm-checked-mac-release-arm64-try,vm-eager-optimization-linux-release-ia32-try,vm-eager-optimization-linux-release-x64-try,vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64c-try,vm-ffi-qemu-linux-release-arm-try,vm-ffi-qemu-linux-release-riscv64-try,vm-fuchsia-release-x64-try,vm-linux-debug-ia32-try,vm-linux-debug-x64-try,vm-linux-debug-x64c-try,vm-mac-debug-arm64-try,vm-mac-debug-x64-try,vm-msan-linux-release-x64-try,vm-reload-linux-debug-x64-try,vm-reload-rollback-linux-debug-x64-try,vm-ubsan-linux-release-x64-try,vm-win-debug-arm64-try,vm-win-debug-x64-try,vm-win-release-ia32-try
Change-Id: I34effe8fbdc80288b703e0152d5ba67ce2343400
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/353101
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
This keeps the array registers unchanged across the instruction,
which can save instructions if there are unused registers to allocate
for the temporaries and the source or destination registers are live
after the copy (e.g., if the destination array is then used in a
LoadIndexed instruction post-copy).
On IA32, we still use writable registers due to register pressure.
TEST=ci
Change-Id: Ia955f8db39e4aba0f01b9d06ea50992a7375a492
Cq-Include-Trybots: luci.dart.try:vm-aot-android-release-arm64c-try,vm-aot-android-release-arm_x64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-debug-x64c-try,vm-aot-mac-release-arm64-try,vm-aot-mac-release-x64-try,vm-aot-obfuscate-linux-release-x64-try,vm-aot-optimization-level-linux-release-x64-try,vm-aot-win-debug-arm64-try,vm-appjit-linux-debug-x64-try,vm-asan-linux-release-x64-try,vm-checked-mac-release-arm64-try,vm-eager-optimization-linux-release-ia32-try,vm-eager-optimization-linux-release-x64-try,vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64c-try,vm-ffi-qemu-linux-release-arm-try,vm-ffi-qemu-linux-release-riscv64-try,vm-fuchsia-release-x64-try,vm-linux-debug-ia32-try,vm-linux-debug-x64-try,vm-linux-debug-x64c-try,vm-mac-debug-arm64-try,vm-mac-debug-x64-try,vm-msan-linux-release-x64-try,vm-reload-linux-debug-x64-try,vm-reload-rollback-linux-debug-x64-try,vm-ubsan-linux-release-x64-try,vm-win-debug-arm64-try,vm-win-debug-x64-try,vm-win-release-ia32-try
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/357901
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
After https://dart-review.googlesource.com/c/sdk/+/330600, there were
more chances for the optimizing compiler to introduce or move
GC-triggering instructions like allocations or boxings between the
retrieval of an untagged pointer to GC-moveable memory and its use.
To limit the chance of this happening, this CL removes the explicit
loading of the untagged payload address when building the initial
flow graph in most cases when the array is not known to be an external
array (an external string, an external typed data object, or an FFI
Pointer).
The remaining case is during view allocation, which extracts the
payload address of the base typed data object underlying the view
(which may be GC-movable) to calculate the payload address that should
be stored in the data field of the view object. See
https://github.com/dart-lang/sdk/issues/54884.
During canonicalization of LoadIndexed, StoreIndexed, and MemoryCopy
instructions, if the cid of an array input is an external array
(external string, external typed data object, or Pointer), then a
LoadField instruction that extracts the untagged payload address
is inserted before the instruction and the corresponding input is
rebound to that LoadField instruction.
Once all compiler passes that involve code motion have been performed,
a new pass looks for LoadIndexed, StoreIndexed, or MemoryCopy where
the cid stored in the instruction for the array is a typed data cid.
In these cases, if the array is not an internal typed data object,
then the payload address is extracted. Waiting until this point ensures
that no GC-triggering instructions are inserted between the extraction
of the payload address and the use. (Internal typed data objects are
left as-is because the payload address is inside the object itself
and doesn't require indirection through the data field of the object).
This CL also replaces code conditional on the array cid with code
that is instead conditional on the array element representation in
cases where it makes sense to do so, since this is a less brittle
check than checking the array cid (e.g., checking for kUnboxedInt8
to load, store, or copy an signed byte from an array instead of
listing all possible array cids that store signed bytes).
This CL also fixes an issue with the ARM64 assembler where calling
LoadFromOffset with an Address that has a non-Offset type would
silently generate bad code instead of triggering the ASSERT in
PrepareLargeOffset.
TEST=vm/dart/typed_list_index_checkbound_il_test
Issue: https://github.com/dart-lang/sdk/issues/54710
Cq-Include-Trybots: luci.dart.try:vm-aot-android-release-arm64c-try,vm-aot-android-release-arm_x64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-debug-x64c-try,vm-aot-mac-release-arm64-try,vm-aot-mac-release-x64-try,vm-aot-obfuscate-linux-release-x64-try,vm-aot-optimization-level-linux-release-x64-try,vm-aot-win-debug-arm64-try,vm-appjit-linux-debug-x64-try,vm-asan-linux-release-x64-try,vm-checked-mac-release-arm64-try,vm-eager-optimization-linux-release-ia32-try,vm-eager-optimization-linux-release-x64-try,vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64c-try,vm-ffi-qemu-linux-release-arm-try,vm-ffi-qemu-linux-release-riscv64-try,vm-fuchsia-release-x64-try,vm-linux-debug-ia32-try,vm-linux-debug-x64-try,vm-linux-debug-x64c-try,vm-mac-debug-arm64-try,vm-mac-debug-x64-try,vm-msan-linux-release-x64-try,vm-reload-linux-debug-x64-try,vm-reload-rollback-linux-debug-x64-try,vm-ubsan-linux-release-x64-try,vm-win-debug-arm64-try,vm-win-debug-x64-try,vm-win-release-ia32-try
Change-Id: I25b5f314943e9254d3d28986d720a5d47f12feeb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/352363
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
E.g. the below command should update both strong, weak, etc and
incremental, parser etc suites with any test containing ffi:
`out/ReleaseX64/dart pkg/front_end/tool/update_expectations.dart *ffi*`
In the future we should, I think, update the tool to be more like
`unit_test_suites.dart` and get rid of the option for package:testing
to generate a program and then run that.
Change-Id: I19edfe2198cbe86326396dc2c8a8f871c599588d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/359061
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
To avoid accounting overhead during each bump pointer allocation, we add the size of the whole bump area when starting to use that area and
subtract the remaining size when switching to a new area.
TEST=ci
Change-Id: If0c429db47c27ca51aac5c055268e85f70bad9ac
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/359160
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
This change reworks the client resume permissions API to make it easier
for clients to deal with user provided `--pause-isolates-on-start` and
`--pause-isolates-on-exit` flags.
`requireUserPermissionToResume` should be called by the tool that
launches the Dart process to indicate whether or not the user provided
`--pause-isolates-on-{start,exit}`. The default behavior is to assume
that a tool set these flags for its own use (e.g., resetting breakpoints
after a hot restart in Flutter), where isolates will resume immediately
after each client that has indicated interest in that pause event has
invoked `readyToResume`.
If a user provided one of the previously mentioned flags, isolates will
not immediately resume after each relevant client has invoked
`readyToResume`. Instead, a call to `resume()` must be made to indicate
the user has triggered the resume request instead of tooling. If the
user permissions to resume are changed while the isolate is paused and
all relevant clients have invoked `readyToResume`, the isolate will
automatically resume if the user no longer requires us to wait for a
user resume.
`resume()` now also acts as a "force resume", bypassing any required
permissions set by tooling.
This behavior change is breaking, so the DDS protocol version is being
bumped to 2.0.
`package:dds_service_extensions` has also been updated to include the
following DDS RPCs:
- `requireUserPermissionToResume`
- `readyToResume`
Change-Id: Id5f0806b3c56507d39eb00b6305b8896bab13ae7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/357420
Reviewed-by: Elliott Brooks <elliottbrooks@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
This adds support for printing the DTD connection information to stdout
when --print-dtd-uri is passed.
This change also fixes an issue where DDS would fail to spawn an isolate
with the DTD snapshot when DDS was running in AOT mode. This means the
SDK must be shipped with both AppJIT and AOT DTD snapshots, at least
until dartdev is moved to run from AOT.
Fixes https://github.com/dart-lang/sdk/issues/55034
TEST=run_test.dart
Change-Id: I788ef9bfe76297a8d594992a2aac440ed9e2ecac
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/358541
Reviewed-by: Siva Annamalai <asiva@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Kenzie Davisson <kenzieschmoll@google.com>
This is mostly a very tidy extraction of `getTypeSource`. This method
relied on a handful of helper functions, all of which were _only_
present in order to help `getTypeSource`. This method also does not
rely on `_buffer` or `eol`. And it is only used by one piece of code,
`extract_method`. Almost a slam dunk!
However, there appeared some strange caveats that I must document
here:
CorrectionUtils also featured two mutable fields, `targetClassElement`
and `targetExecutableElement`. Theoretically, these fields are
supposed to be set before calling `getTypeSource`, to help understand
whether a TypeParameter will be visible when extracting a method, or
something like that. But it appears to be totally broken:
* The singular caller of `getTypeSource`, `ExtractMethodRefactoring`,
_never sets those fields_.
* Two other pieces of code, the AddTypeAnnotation fix, and the
CreateField, CreateFunction, and CreateOverrides fixes, all set the
fields, but _never call getTypeSource_.
Since this all looked like just very stale / rotten code, I removed
everything which looks to be dead: `targetClassElement` and
`targetExecutableElement`, and lo and behold: no tests fail! I've kept
those things deleted, and added a TODO for TypeParameters.
Lastly, since I was trying to figure out ExtractMethodRefactoring, I
tidied it up:
* Make it final
* Rename `ERROR_EXITS` to `errorExits`
* Privatize `searchEngine`, `resolveResult`, `selectionOffset`,
`selectionLength`, `librariesToImport`, and `variableType`.
* Both `selectionRange` and `utils` could be final as they were set
in the constructor body, so move that to initializers, make final,
and make private.
Change-Id: Ia3fb948e5424971eb4250e711c179707552ad8aa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/358663
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@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.
(See also https://dart-review.googlesource.com/c/sdk/+/295662, which
made the corresponding change to the `analysis_server` and `analyzer`
packages, and https://dart-review.googlesource.com/c/sdk/+/325121,
which made the corresponding change to the `linter` package).
Change-Id: Ifbd2f6bed468408b1906b25ab94f745eed0b3e55
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/358980
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Add `package:front_end/src/api_prototype/macros.dart` for utilities
related to compiling macros. Export `isMacroLibraryUri`.
Use `isMacroLibraryUri` instead of hard-coding in a few places.
Add TODO about need to handle macro deps to places that are currently
listing sources as deps.
R=jakemac@google.com, johnniwinther@google.com
Change-Id: I5d2473b810afd42f487178f0aad995804da945c4
Tested: Manually tested with Flutter; e2e tests will be added under tests/macro_build.
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/358700
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Morgan :) <davidmorgan@google.com>
Reviewed-by: Jake Macdonald <jakemac@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Auto-Submit: Morgan :) <davidmorgan@google.com>
The released layout is different from the "building Flutter to work on
it" layout, this should work for both. Of course, an end to end test
is needed--work in progress.
R=jakemac@google.com, johnniwinther@google.com
Change-Id: Ida664b6da62d8fd3b66b877f0441e771269d4fe5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/358443
Commit-Queue: Morgan :) <davidmorgan@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Jake Macdonald <jakemac@google.com>
Auto-Submit: Morgan :) <davidmorgan@google.com>
Avoid polymorphic character access by turning _ChunkedJsonParser
into a mixin instead of the base class.
TEST=ci
Change-Id: Id2080724e07d16e96734a80629c8bd8906dc590b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/358445
Reviewed-by: Daco Harkes <dacoharkes@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
Such invocations were simply skipped before, remaining not unaliased
in the output Kernel code, which prevented evaluating them as
constants.
Closes https://github.com/dart-lang/sdk/issues/55152
Change-Id: I3a54f7c806a0c4fbd8158691be113bd2deae04f4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/357621
Commit-Queue: Chloe Stefantsova <cstefantsova@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Codegen block order keeps loop blocks together and reflecting that in
live ranges allows to produce better register allocation decisions.
Consider for example the loop:
v = def();
while (cond) {
if (smth) { // (*)
use(v);
return true;
}
}
If block (*) is interspersed with loop blocks the register allocator
might decide to allocate it to the register in some of the loop blocks.
If the same block is "sunk" away from loop blocks - the register
allocator can clearly see that `v` does not necessarily have to live on
the register in the whole loop.
In JIT codegen block order is not topologically sorted and as such
is unsuitable for our linear scan.
TEST=ci
Cq-Include-Trybots: luci.dart.try:vm-aot-linux-debug-x64-try,vm-aot-mac-release-arm64-try,vm-aot-linux-release-x64-try
Change-Id: I0726815db998b559267949e157cd2158f5dd55f7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/358448
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Try to keep loop blocks together, previously AOT compiler would produce
bad block order for loops like these:
while (true) {
if (smth) {
// (*)
return;
}
}
And put `(*)` blocks in between loop header and other loop blocks.
TEST=ci
Cq-Include-Trybots: luci.dart.try:vm-aot-linux-debug-x64-try,vm-aot-mac-release-arm64-try,vm-aot-linux-release-x64-try
Change-Id: I52304810b61ff9288fe9df648b21c0258299d61e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/358447
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
This:
* Keeps the '...' syntax so bash-history doesn't become useless
* Adds support for '*' in selecting, e.g.
`pkg/front_end/test/fasta/strong_suite.dart strong/*ffi*` or simply
`pkg/front_end/test/fasta/strong_suite.dart *ffi*`
Change-Id: I357d1972e21268b1fb017b9ee92259ce6d42f1ed
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/358900
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
* Delete unused stuff
* rename 'path' to 'root'
* accept 'includeEndsWith' as a plain text string so we often can avoid
using regexp
* accept 'subRoots' to filter to directories faster and more precisly
than when using regexps in 'pattern'
* make 'list' async instead of async* (no more yield stuff which we
promptly turn into a list when actually using it)
Note that some changes in testing.json is not 100% semantic-preserving,
e.g. the "parser_all" suite previously had a pattern "/tests/.*\\.dart$"
which was probably meant to include all dart files in the "tests" folder
in the root, but in fact included all dart files in a "tests" folder
anywhere. The updated version does not.
Change-Id: Idd014274f86bf6214dee0753a7738ec80bc6a49e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/358442
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
This adds a custom command similar to `dart/textDocument/super` but for augmentations.
This won't show up anywhere on its own, but it will be added to the command palette in Dart-Code based on the server capabilities (and in the future, maybe CodeLens - although that will require some additional work in the server first since it will be the source of CodeLens even if they trigger client-side actions).
See https://github.com/dart-lang/sdk/issues/54742
Change-Id: Ib3a3fd08702e14b8bd251991035ef0b8d06ad996
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/358452
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Make FlowGraph constructor compute whether we are planning to reorder
blocks before code generation or not. This makes the state of the
graph at the end of the compilation more clear.
This is just a refactoring without any functional changes.
TEST=ci
Change-Id: Ieefb02237cc1ebd69d5d2b217bdc8ebbfdbf15c7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/358446
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
This adds a custom command similar to `dart/textDocument/super` but for augmentation targets ("augmented").
This won't show up anywhere on its own, but it will be added to the command palette in Dart-Code based on the server capabilities (and in the future, maybe CodeLens - although that will require some additional work in the server first since it will be the source of CodeLens even if they trigger client-side actions).
See https://github.com/dart-lang/sdk/issues/54742
Change-Id: I11ca115c61ade40bf51478f58b39b7caedac19c9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/358451
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
If the stream has been closed then re-enter with the appropriate error code.
Bug: #55017
Change-Id: Iadf5d039698a48d402a409b5b42ed268885439d1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/355040
Commit-Queue: Nate Biggs <natebiggs@google.com>
Reviewed-by: Mayank Patke <fishythefish@google.com>
This change removes support for external strings from the VM along with
Dart_NewExternalLatin1String, Dart_NewExternalUTF16String and
Dart_IsExternalString Dart C API functions.
External strings are not used by the VM nor any known embedder, but
Dart VM was paying the maintenance and performance price for
the external string implementation classes.
TEST=ci
Change-Id: I094cd2d2b7ec0840e9f09e1ca9e5a7acd4e78c28
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/358760
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Now that null contexts are impossible during analyzer resolution
(https://dart-review.googlesource.com/c/sdk/+/357522), it's possible
to make a lot of variables, fields and parameters that represent
contexts non-nullable.
This change propagates non-nullability of contexts through the
analyzer to the extent that can be done reasonably quickly and
safely. In particular, it makes the following changes:
- It changes the following helper methods so that they no longer
accept a `null` context:
- TypeSystemImpl.acceptsFunctionType
- TypeSystemImpl.refineNumericInvocationContext
- TypeSystemImpl.setupGenericTypeInference
- TypeSystemImpl._refineNumericInvocationContextNullSafe
- InvocationInferenceHelper.inferTearOff
- InvocationInferenceHelper.resolveMethodInvocation
- ElementResolver.visitMethodInvocation
- ErrorDetectionHelpers.getImplicitCallMethod
- ResolverVisitor.insertGenericFunctionInstantiation
- ResolverVisitor._insertImplicitCallReference
- ResolverVisitor._resolveRewrittenFunctionExpressionInvocation
- StaticTypeAnalyzer.visitConditionalExpression
- StaticTypeAnalyzer.visitIntegerLiteral
- AstResolver.resolveExpression
- It changes the following classes so that their `resolve` methods no
longer accept a `null` context (nor do the methods that those
`resolve` methods defer to):
- AssignmentExpressionResolver
- BinaryExpressionResolver
- ConstructorReferenceResolver
- FunctionExpressionInvocationResolver
- FunctionExpressionResolver
- InstanceCreationExpressionResolver
- MethodInvocationResolver
- PostfixExpressionResolver
- PrefixExpressionResolver
- PrefixedIdentifierResolver
- RecordLiteralResolver
- SimpleIdentifierResolver
- TypedLiteralResolver
- It changes all the classes in the `InvocationInferrer` class
hierarchy so that they no longer accept a `null` context.
- It changes the following helper methods so that they no longer
return a `null` context:
- AssignmentExpressionResolver._computeRhsContext
- InvocationInferrer._computeContextForArgument
- MethodInvocationInferrer._computeContextForArgument
- YieldStatementResolver._computeContextType
- It changes PropertyElementResolverResult.indexContextType so that it
no longer can be `null`.
- It ensures that the analyzer always passes a non-null context to the
_fe_analyzer_shared method `TypeAnalyzer.analyzeExpression`. This
will pave the way for a follow-up CL that makes that method stop
accepting `null`.
In many cases, to accomplish this, it was necessary to make explicit
use of the unknown type schema `_`
(UnknownInferredType.instance). Although adding these explicit uses
makes the code a bit more verbose, it's not really fundamentally more
complex than it was before; previously, the code used `null` to
represent `_` in these cases (sometimes implicitly).
Change-Id: I1479e7c59969c59ea67abef30daa6a516b15ed97
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/358662
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
As noted by Lasse, we're not really consistent and the singular is more "correct".
See: 71456592d0 (r140033857)
Change-Id: I55afeab8c42d9f20999e263682f71d54a2302ea5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/358682
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Phil Quitslund <pquitslund@google.com>
The analyzer's mechanism for passing contexts down the stack while
recursively resolving expressions is to add an optional `contextType`
parameter to some of the `visit` methods in `ResolverVisitor`. This
parameter is only included in `visit` methods that would actually use
it (e.g. `visitDoubleLiteral` doesn't have it, since the analysis of a
double literal doesn't depend on the context).
When the resolver needs to analyze a subexpression, if it needs to
supply a context, it uses `ExpressionImpl.resolveExpression` to
dispatch to the appropriate `visit` method; this passes the context
along to the optional `contextType` parameter. If, on the other hand,
it **doesn't** need to supply a context, it uses the standard
AstVisitor mechanism, which dispatches to the `visit` method without
supplying a context type, so the `contextType` parameter takes on its
default value.
Previous to this CL, the default value of each `contextType` parameter
was `null`; this had the unintended consequence of making a
distinction between a `null` context and a context of `_`
(`UnknownInferredType.instance`). The front end, by contrast, has a
visitor paradigm that allows passing a required argument through to
the `visit` method, so its context parameters are all non-nullable,
and it makes no such distinction.
Prior to addressing language issue 3648
(https://github.com/dart-lang/language/issues/3648), the difference
was only observable for `await` expressions. Now that that issue has
been addressed, there is no user-visible difference between `_` and
the null context. But it's easy to imagine a difference accidentally
sneaking in during future development.
To avoid future bugs, this change makes `_` the default value for each
`contextType` parameter. To do this, it was necessary to change
`UnknownInferredType.instance` from a final variable to a const.
To the best of my knowledge, this change should have no user-visible
effect.
Bug: https://github.com/dart-lang/language/issues/3648
Change-Id: Id74a513366831239df2d56ceac57c2dbe5c5084e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/357522
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
In the front end, type inference of an expression always takes place
with respect to a type schema (the "context"). In the analyzer, type
inference of an expression sometimes takes place with respect to a
context, but sometimes takes place with respect to no context at all;
the latter circumstance arises when the analyzer uses its standard
AstVisitor mechanism to call one of the visit methods in the
ResolverVisitor class, and so the visit method's contextType argument
takes on the value null. Because of this I am calling this situation a
"null context".
In all the circumstances where the analyzer infers an expression using
a null context, the front end infers the same expression using a
context of _. Furthermore, prior to this change, all but one of the
analyzer's visit methods treated a null context the same as they
treated a context of _. The one exception was visitAwaitExpression: in
this method, if the context was the null context, then the analyzer
analyzed the await expression's subexpression using a context of _;
otherwise, it analyzed it using a context of FutureOr<_>. Whereas the
front end, lacking any notion of a "null context", analyzes the await
expression's subexpression using a context of FutureOr<_> in the same
circumstances.
This change brings the analyzer behavior into line with the front end.
Fixes https://github.com/dart-lang/language/issues/3648.
Bug: https://github.com/dart-lang/language/issues/3648
Change-Id: Ifd77988010d4387ce48eaa20dff4356beec03753
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/357521
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Paul Berry <paulberry@google.com>
Change-Id: Icc4b89f1b25dd633279d87e9d51f85372d962c7b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/358450
Commit-Queue: Erik Ernst <eernst@google.com>
Reviewed-by: William Hesse <whesse@google.com>
