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

[Pekka Paalanen]

On Wed, 26 Jul 2017 15:49:57 +0800
Jonas Ådahl <jadahl at gmail.com> 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 <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.

> > > @@ -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
-------------- 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/20170726/4accff2c/attachment.sig>