[PATCH 1/2] scanner: Add --visibility flag for setting symbol visibility

[Pekka Paalanen]

On Tue, 25 Jul 2017 14:44:29 +0300
Pekka Paalanen <ppaalanen at gmail.com> wrote:

> On Tue, 25 Jul 2017 18:39:56 +0800
> Jonas Ådahl <jadahl at gmail.com> 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 <jadahl at gmail.com>
> > ---
> >  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.

> > @@ -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.

Bloody hell, it's what the C spec calls "a tentative definition". It's
valid, and it does what we intended, but I think it's still confusing
as hell.

Can we just keep the "extern" in the declarations we did not intend to
be (tentative) definitions?


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/20170725/6df90475/attachment.sig>