dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.23k stars 1.58k forks source link

dart2js: Canonicalize static methods #35932

Open rakudrama opened 5 years ago

rakudrama commented 5 years ago

Generators like Angular Dart sometimes generate static methods that are the same as static methods in independently generated. It would be advantageous to have the compiler reuse the identical methods rather than have multiple copies of the method.

What does it mean for the methods to be 'identical'? Do we mean methods that have the same Kernel IR nodes, or methods that compile to the same JavaScript? Consider methods A1 (calls B1) and A2 (calls B2). These are not the same, but if we determine that B1 and B2 are he same, now A1 and A2 could become 'the same'.

How do we report stack traces? If a canonicalized method is in a stack trace, it is not clear which (of potentially hundreds) of methods the stack trace frame refers to. If the method is called directly, the call site will tell us which method the canonicalized method is acting as, but it will still be confusing as we won't have exact source information for all the merged methods. Things are more difficult if the method is called via a tear-off, since we have only an indirect call which doesn't narrow it down at all!

Issue #26932 suggests sharing closures with no free variables. This has similar problems, but potentially greater impact because there are many similar small closures in a large program.

eernstg commented 5 years ago

With respect to the language specification, the situation for static methods may share some elements with the situation for function literals (cf. https://github.com/dart-lang/sdk/issues/26932#issuecomment-463191130).

However, even though the specification does not explicitly require function objects obtained by a tear-off on distinct declarations to be non-identical (we have the converse: two constant expressions that denote the same static method / library function must evaluate to the same function object), it seems quite natural to assume that this would be the case. The issues around stack traces (and toString() for that matter) also seem to imply that it's a questionable idea to collapse them.

Wouldn't it be a cleaner approach to tell the Angular folks that it would be really nice if they could generate code that isn't so redundant? Would there be anything in the type system that prevents this (like: here are 200 functions with the same generated code, but they're treated differently during type analysis, so they could never be declared as a single declaration)?