Proposal: Auto-generating java enum constants from C headers

[Jonathan Dixon]

[ long email summary: how about we auto generate java enums from generic include files like net_error_list.h? ]

Howdy, 

In the android port, we have a bunch of places where we need to synchronize the values of integer constants / enums across java and C++ code.

To date, the convention has been to duplicate a list of explicitly valued constants, and have a cross-ref comment stating the two should be kept in sync

(trivial example: http://src.chromium.org/viewvc/chrome/trunk/src/base/android/java/src/org/chromium/base/PathService.java?revision=149842&view=markup

    // Must match the value of DIR_MODULE in base/base_paths.h!
    public static final int DIR_MODULE = 3;

This has obviously brittleness and is fairly unattractive, so we've played with alternatives like using bootstrapping the constant values at runtime (a-la swig), but these tend to be worse: much more boiler plate code, cause bloat by forcing the compiler to including all the string *names* of the constants in *both* binaries (java and C++) where before it may have been able to inline the numeric literal values, slows startup (not only wiring up the constants, but also registering the JNI methods needed in order to wire up the constants), and means one side or other has some not-really-constant constants (if they need initializing at runtime), and can still leave random runtime errors for what should be a compile time issue.

So, looking at the gargantuan list of codes in net_error_list.h gave me inspiration to auto-generate the java side constants from the same source file. Example patch to do this: http://crrev.com/10870050 example output: http://pastebin.com/CWMCGqqt

This is so naively simple it's untrue, but AFAICT it addresses all the previous drawbacks completely. (no RAM/ROM/runtime/startup/code maintenance costs. generated consts have no overhead at all if not used, assuming we proguard the jar).

It has a couple implications though:

- locks in the "DECLARE_CONST(name, value)" format of net_error_list.h, as we'll have some less conventional tools reading it

- likely promotes use of this multiple-include header style in more places in the code (and especially in android-specific code) as everyone will want to make their constants easily accessible in java too

(- deepens our use of SHOUTING_CASE constants rather than kCamelConstants, as this is the common denominator style that works nicely in java and C++. but really, we've got this already even with manually synchronized constants)

So, for those that read this far, wdyt? Is that a acceptable impact on rest of the project? alternatives?

Some answers to other FAQs

- we're not looking at swig etc at this time, as we have java <-> native invocation working very nicely using a simple java->C code generation tool

- the above tool can't handle constants so well, as in many of places the authoritative value already lives in the native side, and we don't particularly fancy foisting a java->C conversion step on non-java platforms just to get some constants

- the use of gcc preprocessor in my patch above is really for illustration only :) but, yeah, it works pretty well for now

- a more advanced tool could render the need for a checked-in net_error_java.template file redundant too. The java class name and package, and (optional) constant name prefix (ERR_...) would be easy to set in gyp params. (actually, we could do that with preprocessor define too, if we really wanted)

[Nico Weber]

The above sounded kind of scary, but the patch actually looks pretty
nice. I like it, for what it's worth.

> --
> Chromium Developers mailing list: chromi...@chromium.org
> View archives, change email options, or unsubscribe:
> http://groups.google.com/a/chromium.org/group/chromium-dev

[Ryan Sleevi]

On Thu, Aug 23, 2012 at 5:57 PM, Jonathan Dixon <jo...@google.com> wrote:

My only concern would be if this encouraged a proliferation of
auto-generated prefixes, since it makes it harder for 'simple' search
schemes to find the actual definition (ie: the general problem with
macros). This doesn't come up when using kNaming style, nor does it
typically come up when directly defining SHOUTING_CASE constants, but
becomes incredibly tempting once the use of function-macros have been
blessed.

Other than that, it looks like a rather reasonable and clever solution.

[Jonathan Dixon]

By the auto-generated pre-fixes, you mean like the way ERR_ is hidden in the macro?

Yep, I've been annoyed by the lack of grepability on those constants myself. I think we could remove the proliferation of that if we fixup net_error_list to spell out the prefix. Yeah.. I've not tested this fully yet, but doesn't seem too hard: https://chromiumcodereview.appspot.com/10879058/

[Satoru Takabayashi]

What about defining enums in a .proto file and generating code for both C++ and Java?

[Ami Fischman]

On Thu, Aug 23, 2012 at 5:57 PM, Jonathan Dixon <jo...@google.com> wrote:

[ long email summary: how about we auto generate java enums from generic include files like net_error_list.h? ]

SGTM.  Even better, generate actual java enums to get the additional benefits of enum-value-name-as-string as well as an all-legal-values accessor?

Plus, once we decide this is the way the project does constants, we can get a single technique for stringifying enum value names even in C++ (like net does, but for the rest of the codebase).

-a

[James Robinson]

On Thu, Aug 23, 2012 at 5:57 PM, Jonathan Dixon <jo...@google.com> wrote:

[ long email summary: how about we auto generate java enums from generic include files like net_error_list.h? ]

Howdy, 

In the android port, we have a bunch of places where we need to synchronize the values of integer constants / enums across java and C++ code.

To date, the convention has been to duplicate a list of explicitly valued constants, and have a cross-ref comment stating the two should be kept in sync

(trivial example: http://src.chromium.org/viewvc/chrome/trunk/src/base/android/java/src/org/chromium/base/PathService.java?revision=149842&view=markup

    // Must match the value of DIR_MODULE in base/base_paths.h!
    public static final int DIR_MODULE = 3;

This has obviously brittleness and is fairly unattractive, so we've played with alternatives like using bootstrapping the constant values at runtime (a-la swig), but these tend to be worse: much more boiler plate code, cause bloat by forcing the compiler to including all the string *names* of the constants in *both* binaries (java and C++) where before it may have been able to inline the numeric literal values, slows startup (not only wiring up the constants, but also registering the JNI methods needed in order to wire up the constants), and means one side or other has some not-really-constant constants (if they need initializing at runtime), and can still leave random runtime errors for what should be a compile time issue.

So, looking at the gargantuan list of codes in net_error_list.h gave me inspiration to auto-generate the java side constants from the same source file. Example patch to do this: http://crrev.com/10870050 example output: http://pastebin.com/CWMCGqqt

This is so naively simple it's untrue, but AFAICT it addresses all the previous drawbacks completely. (no RAM/ROM/runtime/startup/code maintenance costs. generated consts have no overhead at all if not used, assuming we proguard the jar).

It has a couple implications though:

- locks in the "DECLARE_CONST(name, value)" format of net_error_list.h, as we'll have some less conventional tools reading it

- likely promotes use of this multiple-include header style in more places in the code (and especially in android-specific code) as everyone will want to make their constants easily accessible in java too

(- deepens our use of SHOUTING_CASE constants rather than kCamelConstants, as this is the common denominator style that works nicely in java and C++. but really, we've got this already even with manually synchronized constants)

If you are going to autogenerate these anyway, could you have the script generate "kCamelCase" for C++ and SHOUTING_CASE for Java?

- James

 

So, for those that read this far, wdyt? Is that a acceptable impact on rest of the project? alternatives?

Some answers to other FAQs

- we're not looking at swig etc at this time, as we have java <-> native invocation working very nicely using a simple java->C code generation tool

- the above tool can't handle constants so well, as in many of places the authoritative value already lives in the native side, and we don't particularly fancy foisting a java->C conversion step on non-java platforms just to get some constants

- the use of gcc preprocessor in my patch above is really for illustration only :) but, yeah, it works pretty well for now

- a more advanced tool could render the need for a checked-in net_error_java.template file redundant too. The java class name and package, and (optional) constant name prefix (ERR_...) would be easy to set in gyp params. (actually, we could do that with preprocessor define too, if we really wanted)

[David Turner]

On Fri, Aug 24, 2012 at 5:06 AM, Ami Fischman <fisc...@chromium.org> wrote:

On Thu, Aug 23, 2012 at 5:57 PM, Jonathan Dixon <jo...@google.com> wrote:

[ long email summary: how about we auto generate java enums from generic include files like net_error_list.h? ]

SGTM.  Even better, generate actual java enums to get the additional benefits of enum-value-name-as-string as well as an all-legal-values accessor?

 

FWIW, the general consensus on the Android team is to avoid Java enums as much as possible. Their footprint and performance cost are way to high.

Personally, I'd prefer avoiding them in Chrome on Android too. 

Plus, once we decide this is the way the project does constants, we can get a single technique for stringifying enum value names even in C++ (like net does, but for the rest of the codebase).

What would be the benefit of this exactly? Just curious.

 

-a

[Ami Fischman]

Plus, once we decide this is the way the project does constants, we can get a single technique for stringifying enum value names even in C++ (like net does, but for the rest of the codebase).

What would be the benefit of this exactly? Just curious.

Avoid the need to write one-off functions to do the enum->string conversions, like the 3 in media_log.cc or more generally many of the codesearch hits for ^const\ char\*\ .*ToString\(.*\{.

-a

[Jonathan Dixon]

Thanks for all the feedback, looks like there's something here worth pushing ahead with then!

On 23 August 2012 19:52, Satoru Takabayashi <sat...@chromium.org> wrote:

What about defining enums in a .proto file and generating code for both C++ and Java?

Ah, interesting idea. We don't currently do any .proto handling on the Java side AFAIK, so would need a bit more bring up work. I'll take a look though. If it means we pull in a support library etc (as is the case on the native side) we'll probably want to stay clear.

Also, would base/ (for example) be happy having a dependency on proto compiler step?

On 23 August 2012 20:20, James Robinson <jam...@google.com> wrote:

It has a couple implications though:

- locks in the "DECLARE_CONST(name, value)" format of net_error_list.h, as we'll have some less conventional tools reading it

- likely promotes use of this multiple-include header style in more places in the code (and especially in android-specific code) as everyone will want to make their constants easily accessible in java too

(- deepens our use of SHOUTING_CASE constants rather than kCamelConstants, as this is the common denominator style that works nicely in java and C++. but really, we've got this already even with manually synchronized constants)

If you are going to autogenerate these anyway, could you have the script generate "kCamelCase" for C++ and SHOUTING_CASE for Java?

True, although it often proves convenient (for me anyway) to be able to grep for a given constant across both codebases.