|
|
Chromium Code Reviews|
Created:
5 years ago by ochafik Modified:
5 years ago CC:
dev-compiler+reviews_dartlang.org Base URL:
git@github.com:dart-lang/dev_compiler.git@master Target Ref:
refs/heads/master Visibility:
Public. |
DescriptionUse string interpolation in JS intrinsics to simplify large templates.
This helps define large runtime methods that refer to lots of helpers.
The following pattern:
f(a, b, c) => JS('', '''((a, b, c, someHelper) => {
return someHelper(a + b + c);
})(#, #, #)''', a, b, c, someHelper);
Becomes:
f(a, b, c) => JS('', ''(() => {
return $someHelper($a + $b + $c);
})()''');
BUG=
R=vsm@google.com
Committed: https://github.com/dart-lang/dev_compiler/commit/13e0c5c1ca587800eb6016ab0fd1ab135423bc52
Patch Set 1 #Patch Set 2 : #
Messages
Total messages: 12 (4 generated)
Description was changed from
==========
Use string interpolation in JS intrinsics to simplify large templates.
The following pattern:
f(a, b, c) => JS('', '''((a, b, c, someHelper) => {
return someHelper(a + b + c);
})(#, #, #)''', a, b, c, someHelper);
Becomes:
f(a, b, c) => JS('', ''(() => {
return $someHelper($a + $b + $c);
})()''');
This helps define large runtime methods that refer to lots of helpers.
BUG=
==========
to
==========
Use string interpolation in JS intrinsics to simplify large templates.
This helps define large runtime methods that refer to lots of helpers.
The following pattern:
f(a, b, c) => JS('', '''((a, b, c, someHelper) => {
return someHelper(a + b + c);
})(#, #, #)''', a, b, c, someHelper);
Becomes:
f(a, b, c) => JS('', ''(() => {
return $someHelper($a + $b + $c);
})()''');
BUG=
==========
ochafik@google.com changed reviewers: + jmesserly@google.com, vsm@google.com
nice! lgtm.
Description was changed from
==========
Use string interpolation in JS intrinsics to simplify large templates.
This helps define large runtime methods that refer to lots of helpers.
The following pattern:
f(a, b, c) => JS('', '''((a, b, c, someHelper) => {
return someHelper(a + b + c);
})(#, #, #)''', a, b, c, someHelper);
Becomes:
f(a, b, c) => JS('', ''(() => {
return $someHelper($a + $b + $c);
})()''');
BUG=
==========
to
==========
Use string interpolation in JS intrinsics to simplify large templates.
This helps define large runtime methods that refer to lots of helpers.
The following pattern:
f(a, b, c) => JS('', '''((a, b, c, someHelper) => {
return someHelper(a + b + c);
})(#, #, #)''', a, b, c, someHelper);
Becomes:
f(a, b, c) => JS('', ''(() => {
return $someHelper($a + $b + $c);
})()''');
BUG=
R=vsm@google.com
Committed:
https://github.com/dart-lang/dev_compiler/commit/13e0c5c1ca587800eb6016ab0fd1...
==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) manually as 13e0c5c1ca587800eb6016ab0fd1ab135423bc52 (presubmit successful).
Message was sent while issue was closed.
jmesserly@google.com changed reviewers: + sra@google.com
Message was sent while issue was closed.
Hi guys, I'm really confused by this change. It looks like string interpolation, but it's not, right? There are cases where an object value is interpolated. I *think* what you want is for Dart to have "tagged templates", like JS now does. But we don't have this feature in Dart. The design of "JS" builtin (which we got from dart2js) was very intentional. For one, it can save work by simplifying the parsing of JS snippets, if the same template string is used repeatedly. But it also allows the interpolation to be expressed using a clean model. The "JS" builtin works like an "eval" call, conceptually. That is no longer the case after this CL, because those aren't really string interpolations. JS now behaves more like a very special macro. Maybe that's okay, but I'm a little torn on whether it's a good idea. I get that we want an easy to read runtime code, but I'm not sure it's worth a fundamental change to how "JS" works...
Message was sent while issue was closed.
On 2015/12/15 22:08:38, John Messerly wrote: > Hi guys, > > I'm really confused by this change. It looks like string interpolation, but it's > not, right? There are cases where an object value is interpolated. > > I *think* what you want is for Dart to have "tagged templates", like JS now > does. But we don't have this feature in Dart. > > > The design of "JS" builtin (which we got from dart2js) was very intentional. For > one, it can save work by simplifying the parsing of JS snippets, if the same > template string is used repeatedly. But it also allows the interpolation to be > expressed using a clean model. The "JS" builtin works like an "eval" call, > conceptually. That is no longer the case after this CL, because those aren't > really string interpolations. JS now behaves more like a very special macro. > > Maybe that's okay, but I'm a little torn on whether it's a good idea. I get that > we want an easy to read runtime code, but I'm not sure it's worth a fundamental > change to how "JS" works... In dart2js the string must be a constant. The template library caches the parsed expression, so the total number of JS expressions over all code ever compiled (e.g. by a compilation server) must be bounded by a constant. It is not clear this approach maintains that. Further, you can't put `#` (or some other form of interpolated parameter) in a function body. In dart2js we now reject such templates. JavaScript capture rules are totally different to Dart, and at the whim of where the JS-generated-expression is placed in surrounding code. Write template code that is immune to these problems (i.e. will still work if inlined into a loop). JS is only used in internal libraries. I suggest keeping it compatible with dart2js. The extra 20 lines of implementation (with no unit tests) is not worth the mostly gratuitous difference.
Message was sent while issue was closed.
Thanks for the comments!
On 2015/12/15 22:34:47, sra1 wrote:
> On 2015/12/15 22:08:38, John Messerly wrote:
> > I'm really confused by this change. It looks like string interpolation, but
> > it's not, right? There are cases where an object value is interpolated.
Sorry for the lacking documentation. This is just syntax sugar to move the
values from the JS arguments to the string pattern (kind of the old debate
between C-style printf formatting vs. string interpolation).
JS intrinsics *cannot* have string interpolation (since their pattern is
evaluated
statically, during compilation), so there is no technical ambiguity here: it
works more on the same level as Scala's (or Lisp's) quasiquotes: it's an
"AST interpolation".
> > I *think* what you want is for Dart to have "tagged templates", like JS now
> > does. But we don't have this feature in Dart.
> >
> >
> > The design of "JS" builtin (which we got from dart2js) was very intentional.
> For
> > one, it can save work by simplifying the parsing of JS snippets, if the same
> > template string is used repeatedly. But it also allows the interpolation to
be
> > expressed using a clean model. The "JS" builtin works like an "eval" call,
> > conceptually.
JS would rather be comparable to eval if it were a string interpolation /
templating
mechanism, but it's really already a quasiquotes / AST splicing mechanism.
>> That is no longer the case after this CL, because those aren't
> > really string interpolations. JS now behaves more like a very special macro.
JS is already a very special macro, but I believe it retains the same semantic
as the original scheme. Might be better if there were a 'q' prefix in front of
the
string literal as in Scala (just as 'r' for raw strings, for instance), but I
think it would
be overkill (again, there can be no ambiguity since JS patterns are
statically-evaluated).
> > Maybe that's okay, but I'm a little torn on whether it's a good idea. I get
> that
> > we want an easy to read runtime code, but I'm not sure it's worth a
> fundamental
> > change to how "JS" works...
The change is purely syntactic: `JS('', '$a $b')` becomes `JS('', '# #', a, b)`,
that's it :-)
Stephen: here is some missing context btw: I'm trying to write DDC's runtime
files in Dart
instead of JavaScript (this will allow us to recompile it to target different
module systems, different
syntaxes - TypeScript, Closure, etc:
https://github.com/dart-lang/dev_compiler/issues/310).
Because of the semantic differences and bare-metal needs, we need JS intrinsics
here and there.
In fact, if we just added intrinsics where they're needed, they would be nearly
at
every corner. Instead, I'm writing top-level vars (typically, functions) wholly
written in multiline intrinsics. They typically need "clean" references to some
parameters (can be passed with the `JS('', '((a, b) => { ... })(#, #)', a, b)`
pattern as I did recently for `dart_utils.js` -> `utils.dart`), but for larger
pieces of code we need *many* references to many functions, classes,
top-levels (dealing with library import aliases, etc). Hash patterns become
completely impracticable, and the quasiquotes give things like the following:
https://codereview.chromium.org/1530563003/patch/1/5734977488551936
(every $x there gets transformed to a # + a pushed arg to the JS args)
Which works, runs all language tests, and gives back mostly the same JS with
proper
symbol resolution.
> In dart2js the string must be a constant.
> The template library caches the parsed expression, so the total number of JS
> expressions over all code ever compiled (e.g. by a compilation server) must be
> bounded by a constant.
> It is not clear this approach maintains that.
No change here: I actually convert the interpolated string to its #-filled
equivalent (with one extra arg per ${interpolatedAst}). The max number of
patterns ever instantiated is exactly the number of JS occurrences in the
Dart sources.
> Further, you can't put `#` (or some other form of interpolated parameter) in a
> function body.
> In dart2js we now reject such templates.
Not sure what limitation this is, but I didn't hit it when converting *all*
of DDC's runtime (bar dart_library.js) to such interpolated string in
https://codereview.chromium.org/1530563003 (see
input_sdk/private/{classes, errors, generators, operations, rtti, runtime,
types, utils}.dart)
> JavaScript capture rules are totally different to Dart, and at the whim of
where
> the JS-generated-expression is placed in surrounding code. Write template
code
> that is immune to these problems (i.e. will still work if inlined into a
loop).
(sorry for repetition) The templating system stays the same, it just accepts
interpolated inclusions (which still get resolved at compile time). The intent
is to be used internally by the compiler to create code at the fine border
between Dart and JS by compiler developers, just in line with the existing
design I believe.
> JS is only used in internal libraries.
> I suggest keeping it compatible with dart2js.
> The extra 20 lines of implementation (with no unit tests) is not worth the
> mostly gratuitous difference.
Sorry for the lack of unit tests, it's unclear how to unit test these (are there
some in the existing dart2js codebase? Happy to help setup some if not). There
is a
massive test in the form of https://codereview.chromium.org/1530563003, though,
so I don't anticipate any risk of unnoticed regression.
In any case I can assure you this is not gratuitous (I've tried many other
painful ways to write those runtime files in Dart :-S).
Cheers!
Message was sent while issue was closed.
On 2015/12/16 01:46:22, ochafik wrote:
> Thanks for the comments!
>
> On 2015/12/15 22:34:47, sra1 wrote:
> > On 2015/12/15 22:08:38, John Messerly wrote:
> > > I'm really confused by this change. It looks like string interpolation,
but
> > > it's not, right? There are cases where an object value is interpolated.
>
> Sorry for the lacking documentation. This is just syntax sugar to move the
> values from the JS arguments to the string pattern (kind of the old debate
> between C-style printf formatting vs. string interpolation).
>
> JS intrinsics *cannot* have string interpolation (since their pattern is
> evaluated
> statically, during compilation), so there is no technical ambiguity here: it
> works more on the same level as Scala's (or Lisp's) quasiquotes: it's an
> "AST interpolation".
>
> > > I *think* what you want is for Dart to have "tagged templates", like JS
now
> > > does. But we don't have this feature in Dart.
> > >
> > >
> > > The design of "JS" builtin (which we got from dart2js) was very
intentional.
> > For
> > > one, it can save work by simplifying the parsing of JS snippets, if the
same
> > > template string is used repeatedly. But it also allows the interpolation
to
> be
> > > expressed using a clean model. The "JS" builtin works like an "eval" call,
> > > conceptually.
>
> JS would rather be comparable to eval if it were a string interpolation /
> templating
> mechanism, but it's really already a quasiquotes / AST splicing mechanism.
>
> >> That is no longer the case after this CL, because those aren't
> > > really string interpolations. JS now behaves more like a very special
macro.
>
> JS is already a very special macro, but I believe it retains the same semantic
> as the original scheme. Might be better if there were a 'q' prefix in front of
> the
> string literal as in Scala (just as 'r' for raw strings, for instance), but I
> think it would
> be overkill (again, there can be no ambiguity since JS patterns are
> statically-evaluated).
>
> > > Maybe that's okay, but I'm a little torn on whether it's a good idea. I
get
> > that
> > > we want an easy to read runtime code, but I'm not sure it's worth a
> > fundamental
> > > change to how "JS" works...
>
> The change is purely syntactic: `JS('', '$a $b')` becomes `JS('', '# #', a,
b)`,
> that's it :-)
>
> Stephen: here is some missing context btw: I'm trying to write DDC's runtime
> files in Dart
> instead of JavaScript (this will allow us to recompile it to target different
> module systems, different
> syntaxes - TypeScript, Closure, etc:
> https://github.com/dart-lang/dev_compiler/issues/310).
>
> Because of the semantic differences and bare-metal needs, we need JS
intrinsics
> here and there.
> In fact, if we just added intrinsics where they're needed, they would be
nearly
> at
> every corner. Instead, I'm writing top-level vars (typically, functions)
wholly
> written in multiline intrinsics. They typically need "clean" references to
some
> parameters (can be passed with the `JS('', '((a, b) => { ... })(#, #)', a, b)`
> pattern as I did recently for `dart_utils.js` -> `utils.dart`), but for larger
> pieces of code we need *many* references to many functions, classes,
> top-levels (dealing with library import aliases, etc). Hash patterns become
> completely impracticable, and the quasiquotes give things like the following:
>
> https://codereview.chromium.org/1530563003/patch/1/5734977488551936
>
> (every $x there gets transformed to a # + a pushed arg to the JS args)
>
> Which works, runs all language tests, and gives back mostly the same JS with
> proper
> symbol resolution.
>
> > In dart2js the string must be a constant.
> > The template library caches the parsed expression, so the total number of JS
> > expressions over all code ever compiled (e.g. by a compilation server) must
be
> > bounded by a constant.
> > It is not clear this approach maintains that.
>
> No change here: I actually convert the interpolated string to its #-filled
> equivalent (with one extra arg per ${interpolatedAst}). The max number of
> patterns ever instantiated is exactly the number of JS occurrences in the
> Dart sources.
>
> > Further, you can't put `#` (or some other form of interpolated parameter) in
a
> > function body.
> > In dart2js we now reject such templates.
>
> Not sure what limitation this is, but I didn't hit it when converting *all*
> of DDC's runtime (bar dart_library.js) to such interpolated string in
> https://codereview.chromium.org/1530563003 (see
> input_sdk/private/{classes, errors, generators, operations, rtti, runtime,
> types, utils}.dart)
>
> > JavaScript capture rules are totally different to Dart, and at the whim of
> where
> > the JS-generated-expression is placed in surrounding code. Write template
> code
> > that is immune to these problems (i.e. will still work if inlined into a
> loop).
>
> (sorry for repetition) The templating system stays the same, it just accepts
> interpolated inclusions (which still get resolved at compile time). The intent
> is to be used internally by the compiler to create code at the fine border
> between Dart and JS by compiler developers, just in line with the existing
> design I believe.
>
> > JS is only used in internal libraries.
> > I suggest keeping it compatible with dart2js.
> > The extra 20 lines of implementation (with no unit tests) is not worth the
> > mostly gratuitous difference.
>
> Sorry for the lack of unit tests, it's unclear how to unit test these (are
there
> some in the existing dart2js codebase? Happy to help setup some if not). There
> is a
> massive test in the form of https://codereview.chromium.org/1530563003,
though,
> so I don't anticipate any risk of unnoticed regression.
>
> In any case I can assure you this is not gratuitous (I've tried many other
> painful ways to write those runtime files in Dart :-S).
>
> Cheers!
Your CL does two independent things:
1. Make JS(..., '... $a ...') be syntax for JS(..., '... # ...', a)
2. rewrite ((p)=>{... p ...})(e) (()=>{... e ...})()
#2 has a multitude of hazards.
JS is declared as a function, so we must keep the order of evaluation of
arguments.
Substituting expressions in a JS function body could change the order.
JavaScript variables may be captured and subsequently updated.
dart2js will reject function bodies with placeholders, DDC just does not enforce
that yet.
Given that you should not do #2, i see no advantage.
The extra 'tax' is restating the parameters in the JS function, a line of #'s in
the outer function application, and restating the parameters in the JS 'call.
You have to do this anyway to preserve function call semantics.
JS(..., '((p)=>{... p ...})($a)')
is little improvement over
JS(..., '((p)=>{... p ...})(#)', a)
Message was sent while issue was closed.
On 2015/12/16 02:34:45, sra1 wrote:
> Your CL does two independent things:
>
> 1. Make JS(..., '... $a ...') be syntax for JS(..., '... # ...', a)
Yes (and that's all it does, see below)
> 2. rewrite ((p)=>{... p ...})(e) (()=>{... e ...})()
Nope, sorry if the bogus issue description does this shortcut: this was done by
another CL (only for functions which body is a JS intrinsic), see discussion
with John in
https://codereview.chromium.org/1486473002/diff/60001/lib/src/codegen/js_code...
We should cut back on it now that there's quasiquotes, see below.
> #2 has a multitude of hazards.
>
> JS is declared as a function, so we must keep the order of evaluation of
> arguments.
> Substituting expressions in a JS function body could change the order.
Yup, but please note this is done only in the context of macro-expanding
"function body" JS calls, as an internal convention, which assumes the
simplified identifiers are stable / local (+ only simplifies the expression if
they are indeed identifiers). This allows defining top-level js functions that
look like js functions (and not like `function f() { return (() => { ... })();
}`).
We can & should definitely ditch that with the new quasiquotes / string
interpolation, and only optimize the no-arg case (which should always safe, but
I think we should only do that in JS intrinsics: `(() => { ... })()` -> `{ ...
}`).
> JavaScript variables may be captured and subsequently updated.
> dart2js will reject function bodies with placeholders, DDC just does not
enforce
> that yet.
I think these are two separate matters: the placeholder thing is a static matter
happening during compilation, while side-effects of JS captures (the good old
problem of macro expansion) will happen at runtime.
I'd be keen to see a concrete example to explain the rationale behind that
enforcement, as I can think of cases without function bodies that can still
cause the same kind of issues :-)
(again, Lisp and Scala have had quasiquotes for ages, they work great no matter
how big the code is, and it works massively well with DDC)
> Given that you should not do #2, i see no advantage.
> The extra 'tax' is restating the parameters in the JS function, a line of #'s
in
> the outer function application, and restating the parameters in the JS 'call.
> You have to do this anyway to preserve function call semantics.
>
> JS(..., '((p)=>{... p ...})($a)')
>
> is little improvement over
>
> JS(..., '((p)=>{... p ...})(#)', a)
What I'm suggesting is directly this:
JS(..., '(() => {... $a ...})()')
And to have it simplified, in the case of a top-level function, to:
JS(..., '... $a ...')
Note that there are 160 such JS calls, many of which pull many references. Take
isClassSubType in types.dart, for instance: with # templates, we would need a
definition like:
isClassSubType(t1, t2) => JS('', '((t1, t2, assert_, canonicalType,
getGenericArgs, getGenericClass, getImplements, getMixins, isClassSubType,
isSubtype, Object) => {
...t1...assert_...
})(#, #, #, #, #, #, #, #, #, #, #)''', t1, t2, assert_, canonicalType,
getGenericArgs, getGenericClass, getImplements, getMixins, isClassSubType,
isSubtype, Object);
Instead, with quasiquotes, it is:
isClassSubType(t1, t2) => JS('', '''(() => {
...$t1...$assert_...
})()''');
The # version is hard to write / maintain, you need to count how many # you
inserted, make sure there's no mismatch of args (neither Dart nor JavaScript
will help you much here), etc.
Both versions are *exactly* equivalent (they're strictly compiled to the same
code), and just work.
And this is not even an extreme case (see more in
https://codereview.chromium.org/1530563003/)...
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
