First step towards having patch files for generic libraries.

First step towards having patch files for generic libraries.

As proof of concept, it patches a non-functional htmlEscape from web.dart.

Still has lots of restrictions:
- Only loads a normal library (no syntax extension to mark patches as
separate from additions). Assumes every element is a patch.
- Only handles top-level functions (probably won't handle getters/setters
or constructors), and not classes.
- Doesn't check that the patched function is marked external (no syntax for
that in the spec yet, doesn't even require it to be abstract).
- Doesn't check that the signature of the patch function matches the original
function.
- Probably won't work for 'coreimpl', as loaded from 'js_helper', since it's not
loaded using the 'dart:' scheme.

Committed: https://code.google.com/p/dart/source/detail?r=9246

[Johnni Winther, 2012-06-29 09:34:43]

https://chromiumcodereview.appspot.com/10689036/diff/1/lib/compiler/implement...
File lib/compiler/implementation/elements/elements.dart (right):

https://chromiumcodereview.appspot.com/10689036/diff/1/lib/compiler/implement...
lib/compiler/implementation/elements/elements.dart:207: String toString() {
Thank you!

https://chromiumcodereview.appspot.com/10689036/diff/1/lib/compiler/implement...
lib/compiler/implementation/elements/elements.dart:637: if (cachedNode !== null)
throw "Patch After Parsing";
Is that the appropriate way to signal internal errors? What about the reportX
functions in Compiler?

https://chromiumcodereview.appspot.com/10689036/diff/1/lib/compiler/implement...
lib/compiler/implementation/elements/elements.dart:675: Node
parseNode(DiagnosticListener listener) {
This is also overridden in ConstructorBodyElement (besides
PartialFunctionElement which you handle).

https://chromiumcodereview.appspot.com/10689036/diff/1/lib/compiler/implement...
lib/compiler/implementation/elements/elements.dart:751: ClassElement patch =
null;
This is currently unused, right?

https://chromiumcodereview.appspot.com/10689036/diff/1/lib/compiler/implement...
File lib/compiler/implementation/lib/web.dart (right):

https://chromiumcodereview.appspot.com/10689036/diff/1/lib/compiler/implement...
lib/compiler/implementation/lib/web.dart:4: 
What does the new content have to do with the old (generated) content? Did we
use the old content?

https://chromiumcodereview.appspot.com/10689036/diff/1/lib/compiler/implement...
File lib/compiler/implementation/lib/web.dartp (right):

https://chromiumcodereview.appspot.com/10689036/diff/1/lib/compiler/implement...
lib/compiler/implementation/lib/web.dartp:4: 
What does the new content have to do with the old content? Did we use the old
content?

[Lasse Reichstein Nielsen, 2012-06-29 12:01:35]

https://chromiumcodereview.appspot.com/10689036/diff/1/lib/compiler/implement...
File lib/compiler/implementation/elements/elements.dart (right):

https://chromiumcodereview.appspot.com/10689036/diff/1/lib/compiler/implement...
lib/compiler/implementation/elements/elements.dart:637: if (cachedNode !== null)
throw "Patch After Parsing";
It isn't really. The problem is that the element doesn't know the compiler, so
it can't report the error. This is just fallback error in case the caller hasn't
checked it already, which it should. And it doesn't, so I'll make it.

https://chromiumcodereview.appspot.com/10689036/diff/1/lib/compiler/implement...
lib/compiler/implementation/elements/elements.dart:675: Node
parseNode(DiagnosticListener listener) {
True. I don't try to handle constructors yet (or anything inside classes at all,
or getters anywhere), but I'll make sure to remember it.

https://chromiumcodereview.appspot.com/10689036/diff/1/lib/compiler/implement...
lib/compiler/implementation/elements/elements.dart:751: ClassElement patch =
null;
Yes. At some point there will be a patch class element here, and when we
parseNode the class, we will also apply the patch. That way it will still be
lazily parsed, both this class and the patch class.

https://chromiumcodereview.appspot.com/10689036/diff/1/lib/compiler/implement...
File lib/compiler/implementation/lib/web.dart (right):

https://chromiumcodereview.appspot.com/10689036/diff/1/lib/compiler/implement...
lib/compiler/implementation/lib/web.dart:4: 
Nothing. I think it's a case of git being too clever by half, and thinking this
is a modified copy of
lib/dom/templates/dom/native/dart_implementation.darttemplate

There is no prior version.

https://chromiumcodereview.appspot.com/10689036/diff/1/lib/compiler/implement...
File lib/compiler/implementation/lib/web.dartp (right):

https://chromiumcodereview.appspot.com/10689036/diff/1/lib/compiler/implement...
lib/compiler/implementation/lib/web.dartp:4: 
Again, there is no prior content, just git being a bit too fancy for it's own
good.

[Johnni Winther, 2012-06-29 12:09:04]

lgtm

[floitsch, 2012-06-29 12:11:51]

LGTM.
Any thoughts on testing that the patching is done?

https://chromiumcodereview.appspot.com/10689036/diff/1/tests/compiler/dart2js...
File tests/compiler/dart2js/mock_compiler.dart (right):

https://chromiumcodereview.appspot.com/10689036/diff/1/tests/compiler/dart2js...
tests/compiler/dart2js/mock_compiler.dart:159: // The mock library don't need
any patches.
doesn't

[Lasse Reichstein Nielsen, 2012-06-29 12:19:35]

> Any thoughts on testing that the patching is done?

For now I've just run a manual test. The web-library is just a test case, it
doesn't need patching. When we get to the real libraries, and start making
important methods do nothing, simply running correctly will be proof of
patching.

https://chromiumcodereview.appspot.com/10689036/diff/1/tests/compiler/dart2js...
File tests/compiler/dart2js/mock_compiler.dart (right):

https://chromiumcodereview.appspot.com/10689036/diff/1/tests/compiler/dart2js...
tests/compiler/dart2js/mock_compiler.dart:159: // The mock library don't need
any patches.
On 2012/06/29 12:11:51, floitsch wrote:
> doesn't

Done.