The rti-need of an instantiation is not the same as the rti-need of the
instantiated generic function. The generic function might not use its
type arguments but the instantiation still needs the type arguments for
equality.
dart2js implements generic function instantiation by creating instances
of a helper class that is a subclass of `Closure`. The helper class needs
its type parameters since they are used in `==` and `toString()` methods.
The fix for #47054 is to add the dependency of the helper class type
parameters on the instantiation type arguments.
Change-Id: I9ecb18e1b61a8ad6549f35b572476b1ed7ebb88d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/212037
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Mayank Patke <fishythefish@google.com>
Commit-Queue: Stephen Adams <sra@google.com>
Treating createSentinel as returning a value of type Never caused issues
whenever its result flowed into a union, since the union would just
simplify to its other component. This meant that if a late field started
out uninitialized and was later initialized to a constant, we would see
it as only ever having the constant value. This caused isSentinel to
always return false even if the field hadn't been initialized yet.
Instead, we support a late sentinel value in the abstract value domain
and use this value as the return type of createSentinel during
inference. Additionally, inference treats _lateReadCheck as a simple
narrowing to exclude the late sentinel value.
Change-Id: I086bfd77576930e3ca2a4cfc9bd63476b6636685
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/210646
Reviewed-by: Stephen Adams <sra@google.com>
`int` variables can attain NaN values because web int arithmetic
implemented by JavaScript numbers (doubles) is not closed under many
operations. It is possible get NaN using only addition:
int a = 1, b = -1;
while (a + a != a) { a += a; b += b; }
int nan = a + b;
On the VM, a, b and nan are all zero.
On the web, a, b and nan are Infinity, -Infinity and NaN, respectively.
Since NaN can leak into int arithmetic, is it helpful if bounds checks
catch NaN indexes. NaN compares false in any comparison, so a test
of the form
if (index < 0 || index >= a.length) throw ioore(a, index);
fails to detect a NaN value of `index`.
This is fixed by negating the comparisons, and applying De Morgan's law:
if (!(index >= 0 && index < a.length)) throw ioore(a, index);
These changes have been applied to JSArray.[], JSArray.[]= and String.[]
For dart2js the change is a little more involved. Primitive indexing is
lowered to code with a HBoundsCheck check instruction. The code generated
for the instruction now uses, e.g. `!(i>=0)` instead of `i<0`.
This leads to a small code size regression.
There is no regression at -O4 since bounds checks are omitted at -O4.
At -O3 (where the regression is largest) the regression is
0.01% for cm
0.06% for flutter gallery -- array-heavy diff and layout
0.21% for Meteor -- array-heavy code
0.30% for Box2DOctane -- array-heavy code
I believe the regression can be largely alleviated by determining if
NaN is impossible at the index check, and if so, reverting to the smaller
code pattern. The analysis could be global, incorporating NaN into the
global abstract value domain, or a much simpler a local dataflow
analysis. Many indexes are loop driven and cannot reach infinity because
they are incremented by a small bump and eventually (even without a loop
guard) the index would stop growing when the increment falls below the
rounding error in O(2^53) iterations.
Change-Id: I23ab1eb779f1d0c9c6655e13d69f65d453db9284
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/210321
Commit-Queue: Stephen Adams <sra@google.com>
Reviewed-by: Mayank Patke <fishythefish@google.com>
These annotations have all been replaced with @pragma('dart2js:xxx')
versions:
@ForceInline, @NoInline, @NoThrows, @NoSideEffects, @AssumeDynamic.
Change-Id: Ia4730670c6864ccbe0fa4120108c3c16ab887c23
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/208863
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Stephen Adams <sra@google.com>
Tests static extension methods on JavaScriptObject work as
expected, as well as making sure it works with package:js. Once
extension types are added, these tests need to be changed to use
those instead.
Change-Id: I9504c8a3a8e466c680b221e259d93c1df5474218
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/199601
Commit-Queue: Srujan Gaddam <srujzs@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
This CL deletes BaseDartTypeVisitor and makes all of the visit* methods
in DartTypeVisitor abstract. This forces subclasses of DartTypeVisitor
to provide definitions for all of these methods rather than relying on
some default.
The old code led to a series of bugs with the same root cause: when a
new kind of DartType was added (and a corresponding visit* method added
to DartTypeVisitor), not all concrete implementations of the visitor
were updated to handle the new DartType. This didn't produce static
errors because DartTypeVisitor provided default no-op implementations
for visit* methods. In some cases, this was the desired behavior anyway,
but in practice, any time a new DartType is added, we want our tools to
yell at us until we've validated that it's properly handled everywhere
(even if the proper handling turns out to be "do nothing").
This CL also updates Namer.getTypeRepresentationForConstant to use a
visitor pattern.
Fixes: https://github.com/dart-lang/sdk/issues/46589
Change-Id: I451b592ae1ce4afff40de913535798a62e17b8b6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/206943
Reviewed-by: Joshua Litt <joshualitt@google.com>
Commit-Queue: Mayank Patke <fishythefish@google.com>
Previously, when checking S <: T, we only checked if S and T had the
same interface name once. However, it's possible that redirections
eventually cause them to become equal. For example, if S is an interop
type and T is JavaScriptObject, after following the redirection from S
to JavaScriptObject, we end up checking JavaScriptObject <:
JavaScriptObject. Therefore, we need to recheck after following each
redirection.
Change-Id: Ie3eb9530627a0e48a5ea704fd4078ae238a65c78
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/204744
Commit-Queue: Mayank Patke <fishythefish@google.com>
Reviewed-by: Stephen Adams <sra@google.com>
`operator==` is special.
In the specification, `operator==` methods cannot be called with `null`,
so do not _need_ a null-check for `--null-assertions`.
However, in the dart2js implementation, `operator==` methods are
augmented to handle a `null` argument rather than check the argument
for `null` at every call site. So it is necessary to be _ensure_ there
is no code added for the `--null-assertions` check.
Change-Id: I859b99d680f6d0fad698e1b59eafa1ba0b1749b2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/203842
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Mayank Patke <fishythefish@google.com>
Commit-Queue: Stephen Adams <sra@google.com>
There was a rare mismatch between arguments and parameters when
inlining a constructor body.
The full arguments at the call site of a constructor body (always in a
generative constructor factory) and the parameters at inlining would
not agree on needing a 'box' for the constructor's closed-over mutated
variables (parameters and locals) when the box was needed only for
locals closed over in an assertion.
The fix is to make the condition for accepting a box match the test
for generating the box in the caller.
Bug: 45943
Change-Id: I3b056cb710ffac72bec6943809e04472a838e5bf
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/198741
Reviewed-by: Mayank Patke <fishythefish@google.com>
Commit-Queue: Stephen Adams <sra@google.com>
Safari has a bug that makes it a syntax error for a function name
to overlap with names of parameters in functions with default
parameter values.
This CL changes DDC to generates code to circumvent this issue
when emitting top-level methods. Fortunately, the Safari bug
doesn't trigger with ES6 methods, so we don't need to do anything
for Dart instance methods.
The only other occurrence of named functions in DDC are
`function*` generators that we emit. Those take no arguments,
so we don't need any additional renaming there.
Fixes#43520
Change-Id: I2e4588701a294a8f3c5b47956826ada4ed973e6c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/198200
Reviewed-by: Nicholas Shahan <nshahan@google.com>
Commit-Queue: Sigmund Cherem <sigmund@google.com>
JSDouble represents doubles that are not integers. It is usually wrong
to use JSDouble, so use JSNumber instead where we mean 'all double
values'.
This fixes#44818 by not mistakenly pretending that division cannot
have an integral result.
Will follow up with a CL to rename JSDouble.
Fixed: 44818
Change-Id: Ic324df2ee1f2c7434bc0b064c0dbd7b6800ad93b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/183360
Commit-Queue: Stephen Adams <sra@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
This CL avoids creating locals using KernelToLocalsMap when computing
closure data for the J-model. Instead the closure data uses
ir.VariableDeclaration, JTypeVariable, and internal locals (ThisLocal
and BoxLocal) internally, and the accessor function require passes
a KernelToLocalsMap to convert the internal model to a Locals upon use.
The converted Locals are cached internal for performance only.
This prepares for splitting the GlobalLocalsMap and KernelToLocalsMap
from the JClosedWorld.
Previously landed in https://dart-review.googlesource.com/c/sdk/+/180820
but reverted.
Change-Id: Ic6a2ab63de7cb31c89dd50ca2af3f205a792fd74
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/183000
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Stephen Adams