Proposal: Generic component export macro

[Ken Rockot]

This is a bit out of left field, but it's an idea I was kicking around the other day. I've since built a quick prototype which works to my satisfaction.

I find the need to create a new foo_export.h header for every component to be tedious and occasionally error-prone. There are also something like 180 such headers in the tree now, all doing basically the same thing for different components. We're even generating such headers now for individual mojom component targets. Blah.

We can introduce a shared COMPONENT(name) macro in //base which expands to the appropriate export or import annotation based on whether or not IS_${name}_IMPL expands to 1.

So instead of:

  // your own hand-crafted boilerplate header

  #include "foo/foo_export.h"

  class FOO_EXPORT Foo {};

  // GN

  component("foo") {

    // ...

    defines = [ "FOO_IMPL=1" ]

  }

you can just write:

  // shared header, yay

  #include "base/component.h"

  

  class COMPONENT(FOO) Foo {};

  // GN

  component("foo") {

    // ...

    defines = [ "IS_FOO_IMPL=1" ]

  }

Modulo churn concerns, does this seem like a good idea to anyone else?

[Yuzhu Shen]

It seems great to me to reduce boilerplate code.

One thing that may be worth discussing: In the example, "FOO" doesn't need to be defined anywhere, right? If we have a typo, say, "COMPONENT(FO0)". I imagine it probably will fail in some interesting way that is not very easy to figure out.

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CA%2BapAgE-7XExJeivULBWhaehSCm%2BDshx3ag6q0NpJs5epC0D%3DA%40mail.gmail.com.

[Daniel Cheng]

On Fri, Jan 19, 2018 at 11:18 AM Yuzhu Shen <yzs...@chromium.org> wrote:

It seems great to me to reduce boilerplate code.

One thing that may be worth discussing: In the example, "FOO" doesn't need to be defined anywhere, right? If we have a typo, say, "COMPONENT(FO0)". I imagine it probably will fail in some interesting way that is not very easy to figure out.

Hopefully, people aren't adding these /too/ often?

In general, this sort of things seems reasonable enough to add. We already have some other deep macro magic in //base already to support template export between components…

Daniel

 

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CABxY52AhKjD0wfPChfqX%3D%3Dr4PxKiHFgd%3DCmVj-fHfxJX9yRB6w%40mail.gmail.com.

[Yuzhu Shen]

On Fri, Jan 19, 2018 at 11:24 AM, Daniel Cheng <dch...@chromium.org> wrote:

On Fri, Jan 19, 2018 at 11:18 AM Yuzhu Shen <yzs...@chromium.org> wrote:

It seems great to me to reduce boilerplate code.

One thing that may be worth discussing: In the example, "FOO" doesn't need to be defined anywhere, right? If we have a typo, say, "COMPONENT(FO0)". I imagine it probably will fail in some interesting way that is not very easy to figure out.

Hopefully, people aren't adding these /too/ often?

COMPONENT(FOO) will need to be repeated for every exported definition. So it won't be surprising if there are some typos, I guess?

[Daniel Cheng]

Oh I misread the proposal. I thought that this would be a helper to expand out the boilerplate in the *_export.h file, and that everything else would still use FOO_EXPORT. Would something like that be viable?

Daniel

[Ken Rockot]

On Fri, Jan 19, 2018 at 11:36 AM, Thomas Sepez <tse...@google.com> wrote:

Maybe you could teach GN's component() fuction to automatically define IS_FOO_IMPL for component("foo") even ... 

I thought about that too, but I think that should be addressed separately and this thing would be nice to have even without that.

On Fri, Jan 19, 2018 at 11:31 AM, Yuzhu Shen <yzs...@chromium.org> wrote:

On Fri, Jan 19, 2018 at 11:24 AM, Daniel Cheng <dch...@chromium.org> wrote:

On Fri, Jan 19, 2018 at 11:18 AM Yuzhu Shen <yzs...@chromium.org> wrote:

It seems great to me to reduce boilerplate code.

One thing that may be worth discussing: In the example, "FOO" doesn't need to be defined anywhere, right? If we have a typo, say, "COMPONENT(FO0)". I imagine it probably will fail in some interesting way that is not very easy to figure out.

Hopefully, people aren't adding these /too/ often?

COMPONENT(FOO) will need to be repeated for every exported definition. So it won't be surprising if there are some typos, I guess?

Typos will lead to symbols not getting exported where expected, which will either lead to linker errors, or nothing (if nobody actually needs the export). Seems acceptable to me.

 

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CABxY52B9879OTGmhXzitaXz1qFAWrPm5Yt%3DN%3Dsm%3DY_AxpNzwtg%40mail.gmail.com.

[Ken Rockot]

On Fri, Jan 19, 2018 at 11:34 AM, Daniel Cheng <dch...@chromium.org> wrote:

Oh I misread the proposal. I thought that this would be a helper to expand out the boilerplate in the *_export.h file, and that everything else would still use FOO_EXPORT. Would something like that be viable?

Maybe, but I guess I don't see why we wouldn't just take it one step further and obviate the need for separate export headers altogether.

[Scott Graham]

I think Daniel was suggesting something like

#define CURRENT_COMPONENT FOO

#include "base/component_definer.h"

which would then ## CURRENT_COMPONENT to build a FOO_EXPORT symbol. That way we could avoid the header boilerplate, but maintain current usage patterns.

I feel like having "EXPORT" in the name better describes what it's doing, so I'd prefer that colour of shed to COMPONENT(FOO), but it doesn't matter too much.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CA%2BapAgETkNNLQM4-BQcS2mNXRE2E1m8feo4T_58RWp2zMA1Qng%40mail.gmail.com.

[Ken Rockot]

I don't feel strongly about the naming. I think COMPONENT(FOO) is actually better than COMPONENT_EXPORT(FOO) objectively (it expands to either export OR import...), but maybe because of the long history of using just FOO_EXPORT, it makes sense to keep EXPORT in the name.

I would strongly prefer we use a single parameterized macro though in lieu of  #define+#include "my_header.h"+#undef .  Is there some advantage to the latter that I'm not seeing?

[Mahfuzur Rahman]

Already unsubscribe
By Mahfuzur Rahman

[Ken Rockot]

FYI this support landed in r531136, and the documentation has been updated accordingly.

On Fri, Jan 19, 2018 at 9:58 AM, Ken Rockot <roc...@chromium.org> wrote: