On Wed, 26 Jul 2017 15:49:57 +0800
Jonas Ådahl <[email protected]> wrote:

> On Tue, Jul 25, 2017 at 02:44:29PM +0300, Pekka Paalanen wrote:
> > On Tue, 25 Jul 2017 18:39:56 +0800
> > Jonas Ådahl <[email protected]> wrote:
> >   
> > > Add a --visibility flag that enables the user to tweak the visibility
> > > of the symbols generated by wayland-scanner. Three alternatives are
> > > exposed:
> > > 
> > > 'export': as has always been done up until now, export the symbols
> > > using WL_EXPORT, making making them exposed externally. This is the
> > > default in order to not break backward compatibility.
> > > 
> > > 'compiler-default': use whatever visibility the compiler defaults to.
> > > This is most likely the most visibility that protocol implementations
> > > or users actually wants, as it doesn't expose any unwanted
> > > implementation details.
> > > 
> > > 'static': each symbol will only be visible to the compilation unit it
> > > is included in. This means that a protocol implementations and users
> > > needs to include both the 'code' file and either the 'client-header' or
> > > 'server-header' (or both) in the source file implementing or using the
> > > protocol in question.
> > > 
> > > Using 'static' is a method to avoid situations where otherwise exposed
> > > symbols of different protocols would conflict, for example if they have
> > > the same interface name.
> > > 
> > > When no visibility is specified, 'export' is assumed, but a warning is
> > > printed to stderr, as it is unlikely that 'export' is what is actually
> > > desired.
> > > 
> > > Signed-off-by: Jonas Ådahl <[email protected]>
> > > ---
> > >  Makefile.am   |  10 ++---
> > >  src/scanner.c | 124 
> > > +++++++++++++++++++++++++++++++++++++++++++++++++++-------
> > >  2 files changed, 114 insertions(+), 20 deletions(-)  
> > 
> > Hi Jonas,
> > 
> > thanks for writing this patch. The commit message is well written.

> > > @@ -1556,6 +1575,18 @@ emit_header(struct protocol *protocol, enum side 
> > > side)
> > >   wl_array_release(&types);
> > >   printf("\n");
> > >  
> > > + switch (ctx->visibility) {
> > > + case VISIBILITY_EXPORT:
> > > +         symbol_decl_tag = "extern ";
> > > +         break;
> > > + case VISIBILITY_COMPILER_DEFAULT:
> > > +         symbol_decl_tag = "";
> > > +         break;
> > > + case VISIBILITY_STATIC:
> > > +         symbol_decl_tag = "static ";
> > > +         break;
> > > + }
> > > +
> > >   wl_list_for_each(i, &protocol->interface_list, link) {
> > >           printf("/**\n"
> > >                  " * @page page_iface_%s %s\n",
> > > @@ -1575,8 +1606,9 @@ emit_header(struct protocol *protocol, enum side 
> > > side)
> > >           if (i->description && i->description->text)
> > >                   format_text_to_comment(i->description->text, false);
> > >           printf(" */\n");
> > > -         printf("extern const struct wl_interface "
> > > -                "%s_interface;\n", i->name);
> > > +         printf("%sconst struct wl_interface "
> > > +                "%s_interface;\n",
> > > +                symbol_decl_tag, i->name);  
> > 
> > I believe the "extern" here is correct and required in all cases, so it
> > should be left untouched. It just tells the compiler that this is not
> > the definition of the global symbol, it is just a declaration.
> > 
> > If you drop "extern", this becomes a definition filled with zeroes.
> > 
> > What I do not understand is why do I not see any compiler warnings
> > about duplicate variable definitions. When I looked through the
> > generated files, we clearly have the same static variable defined
> > multiple times in the same compilation unit - in case of the .inc file
> > even in literally the same file.  
> 
> We need to make it 'static ' when using static visibility still,
> otherwise we get a compilation error:
> 
> error: static declaration of ‘tiny_intf_obj_interface’ follows non-static 
> declaration

Argh. Ok, so static it must be for the static visibility.

> For 'compiler-default' I suppose it makes more sense to still use
> 'extern', as 'compiler-default' vs 'export' only affects how the same
> "type" of variable is made visible externally.

Yes.


Thanks,
pq

Attachment: pgpQqrEVEO0YV.pgp
Description: OpenPGP digital signature

_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to