[PATCH 3/5] scanner: introduce --object-type option

[Pekka Paalanen]

On Fri, 28 Jul 2017 13:13:48 +0100
Emil Velikov <emil.l.velikov at gmail.com> wrote:

> On 27 July 2017 at 14:25, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> > On Wed, 26 Jul 2017 14:56:19 +0100
> > Emil Velikov <emil.l.velikov at gmail.com> wrote:
> >  
> >> From: Emil Velikov <emil.velikov at collabora.com>
> >>
> >> The option is used to indicate how the code will be used - would it be a
> >> part of shared or static one.
> >>
> >> In the former case one needs to export the specific symbols, although
> >> normally people want to statically build the protocol code into their
> >> project.
> >>
> >> If the option is missing a warning is emitted, to point people and do
> >> the right thing.
> >>
> >> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
> >> ---
> >>  src/scanner.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
> >>  1 file changed, 56 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/src/scanner.c b/src/scanner.c
> >> index c345ed6..cc45b74 100644
> >> --- a/src/scanner.c
> >> +++ b/src/scanner.c
> >> @@ -57,6 +57,11 @@ enum side {
> >>       SERVER,
> >>  };
> >>
> >> +enum object_type {
> >> +     SHARED,
> >> +     STATIC,
> >> +};  
> >
> > Hi,
> >
> > I could go with this as well, as long as we have some solution to the
> > original problem of conflicting interface names that Jonas is trying to
> > solve.
> >  
> i think the original patch from Jonas, addressed that issue nicely.
> Let's keep that in the respective thread.

Hi Emil,

very good, let's continue that there.


...

> >>  static void
> >> -emit_code(struct protocol *protocol)
> >> +emit_code(struct protocol *protocol, enum object_type obj_type)
> >>  {
> >> +     const char *symbol_visibility;
> >>       struct interface *i, *next;
> >>       struct wl_array types;
> >>       char **p, *prev;
> >> @@ -1728,6 +1740,19 @@ emit_code(struct protocol *protocol)
> >>              "#include <stdint.h>\n"
> >>              "#include \"wayland-util.h\"\n\n");
> >>
> >> +     /* When building a shared library symbols must be exported, otherwise
> >> +      * we want to have the symbols hidden. */
> >> +     if (obj_type == STATIC) {
> >> +             symbol_visibility = "WL_PRIVATE";
> >> +             printf("#if defined(__GNUC__) && __GNUC__ >= 4\n"
> >> +                    "#define WL_PRIVATE __attribute__ ((visibility(\"hidden\")))\n"
> >> +                    "#else\n"
> >> +                    "#define WL_PRIVATE\n"
> >> +                    "#endif\n\n");
> >> +     } else {
> >> +             symbol_visibility = "WL_EXPORT";
> >> +     }  
> >
> > I wonder... would it be any better to just replace 'WL_EXPORT' and
> > 'extern' with tokens:
> >
> > - LOCAL_INTERFACE_DECL for declarations of symbols that will also be
> >   defined in the generated code file
> >
> > - EXTERN_INTERFACE_DECL for declarations of symbols that the generated
> >   files will need but the code file will not define, that is,
> >   interfaces defined on other XML files.
> >
> > - LOCAL_INTERFACE_DEF for symbol definitions in the generated code file.
> >
> > The exported configuration would then be:
> > LOCAL_INTERFACE_DECL=extern
> > EXTERN_INTERFACE_DECL=extern
> > LOCAL_INTERFACE_DEF=WL_EXPORT
> >
> > That would be far too flexible and no-one would use it right, right?
> >  
> I did not introduce separate tokens, since those are (and should be)
> used _only_ in the .c file.
> Personally then do not seem necessary, If you prefer we can add them though.

Ah, no, that was just a wild idea of something completely different. I
meant that the user project would be setting those macros before using
scanner-generated files, and if unset, the scanner-emitted code would
default to the legacy behaviour. That way there would be no visibility
modes in scanner itself. If it's not obviously better, then nevermind.
It certainly has a lot more room to go wrong than your proposal.


...

> > The patch looks pretty much correct to me, if we choose to go this way.
> >  
> Glad to hear.
> 
> I'll let me know once you guys are settled in on the approach, and
> I'll respin the series with all the comments addressed.

Cool, let's see if we can get the name conflict issue solved, and then
I'll try to remember to ping you.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20170818/bfc2a04a/attachment.sig>