Add support for using expressions as enum values.

Add support for using expressions as enum values.

BUG=320090
R=darin@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=244233

[darin (slow to review), 2014-01-09 23:43:31]

I'll take this code review while Dave is out. Plus, I wrote mojo_parser.py ;-)

[darin (slow to review), 2014-01-10 00:36:34]

Is this CL missing the part of the code where expressions get evaluated?

https://codereview.chromium.org/130443003/diff/80001/mojo/public/bindings/par...
File mojo/public/bindings/parse/mojo_parser.py (right):

https://codereview.chromium.org/130443003/diff/80001/mojo/public/bindings/par...
mojo/public/bindings/parse/mojo_parser.py:196: # PLY lets us specify precedence
of operators, but since we don't actually
Perhaps we should evaluate these expressions here?

See for example: http://www.dabeaz.com/ply/example.html

Is there a reason to prefer evaluating expressions downstream in the code
generator?

[darin (slow to review), 2014-01-10 00:43:22]

I just patched this in. I was going to try running mojo_parser.py manually, but
python complains about the "from .mojo_lexer import Lexer" line.

$ ./mojo_parser.py ../sample/sample_service.mojom
Traceback (most recent call last):
  File "./mojo_parser.py", line 28, in <module>
    from .mojo_lexer import Lexer
ValueError: Attempted relative import in non-package

[Matt Perry, 2014-01-10 00:44:51]

Oh, I checked the git log and Dave was the first author listed. Thanks for
taking the review :)

On 2014/01/10 00:36:34, darin wrote:
> Is this CL missing the part of the code where expressions get evaluated?
> 
>
https://codereview.chromium.org/130443003/diff/80001/mojo/public/bindings/par...
> File mojo/public/bindings/parse/mojo_parser.py (right):
> 
>
https://codereview.chromium.org/130443003/diff/80001/mojo/public/bindings/par...
> mojo/public/bindings/parse/mojo_parser.py:196: # PLY lets us specify
precedence
> of operators, but since we don't actually
> Perhaps we should evaluate these expressions here?
> 
> See for example: http://www.dabeaz.com/ply/example.html
> 
> Is there a reason to prefer evaluating expressions downstream in the code
> generator?

I'm just letting the expressions be evaluated by the compiler. I was thinking
that's better because:
(1) the generated code contains the expression, which is clearer to anyone
inspecting it.
(2) if the expression contains constants, it's simpler to let the compiler
handle it rather than keeping track of what each constant is.

The downside is that some errors are missed by the code generator and only
caught by the compiler (like type mismatches, or referencing an invalid
constant). But the upside is we don't have to rebuild a full compiler in mojom.
WDYT?

[Matt Perry, 2014-01-10 00:46:20]

On 2014/01/10 00:43:22, darin wrote:
> I just patched this in. I was going to try running mojo_parser.py manually,
but
> python complains about the "from .mojo_lexer import Lexer" line.
> 
> $ ./mojo_parser.py ../sample/sample_service.mojom
> Traceback (most recent call last):
>   File "./mojo_parser.py", line 28, in <module>
>     from .mojo_lexer import Lexer
> ValueError: Attempted relative import in non-package

Oh, I've never tried running the parser directly. I've only run
mojom_bindings_generator.py. I'll try to figure out the right way to import
here.

[darin (slow to review), 2014-01-10 00:51:28]

This fixes the issue:

diff --git a/mojo/public/bindings/parse/mojo_parser.py
b/mojo/public/bindings/parse/mojo_parser.py
index 80d0a12..9acaee4 100755
--- a/mojo/public/bindings/parse/mojo_parser.py
+++ b/mojo/public/bindings/parse/mojo_parser.py
@@ -25,7 +25,7 @@ except ImportError:
   from ply import lex
   from ply import yacc

-from .mojo_lexer import Lexer
+from mojo_lexer import Lexer


 def ListFromConcat(*items):

[darin (slow to review), 2014-01-10 00:54:35]

On 2014/01/10 00:44:51, Matt Perry wrote:
...
> The downside is that some errors are missed by the code generator and only
> caught by the compiler (like type mismatches, or referencing an invalid
> constant). But the upside is we don't have to rebuild a full compiler in
mojom.
> WDYT?

Oh, I see. That seems reasonable.

When we go to implement support for constants, especially object, array and
string constants, we'll likely need to know something about the expressions
in the code generator.

LGTM for now w/ the one patch I suggested.

[Matt Perry, 2014-01-10 01:19:05]

On 2014/01/10 00:54:35, darin wrote:
> On 2014/01/10 00:44:51, Matt Perry wrote:
> ...
> > The downside is that some errors are missed by the code generator and only
> > caught by the compiler (like type mismatches, or referencing an invalid
> > constant). But the upside is we don't have to rebuild a full compiler in
> mojom.
> > WDYT?
> 
> Oh, I see. That seems reasonable.
> 
> When we go to implement support for constants, especially object, array and
> string constants, we'll likely need to know something about the expressions
> in the code generator.

True. I was thinking we could treat object and array constants specially (we'd
need to know how to tease apart the components and do the assignments in C++),
but that we could still be ignorant of the subtypes (e.g. array of int and array
of float are treated the same). But we'll burn that bridge when we come to it.

[commit-bot: I haz the power, 2014-01-10 01:24:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mpcomplete@chromium.org/130443003/180001

[commit-bot: I haz the power, 2014-01-10 01:46:40]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[Matt Perry, 2014-01-10 20:47:10]

Committed patchset #6 manually as r244233 (presubmit successful).