[PATCH wayland 2/2] scanner: Generate all SINCE_VERSION macros for everyone

Thu Aug 11 11:09:41 UTC 2016

On Wed, 10 Aug 2016 16:15:58 +0300
Pekka Paalanen <ppaalanen at gmail.com> wrote:

> On Wed, 10 Aug 2016 14:47:06 +0200
> Quentin Glidic <sardemff7+wayland at sardemff7.net> wrote:
> 
> > On 10/08/2016 14:36, Pekka Paalanen wrote:  
> > > On Tue, 9 Aug 2016 11:05:49 +0300
> > > Pekka Paalanen <ppaalanen at gmail.com> wrote:
> > >    
> > >> On Mon, 8 Aug 2016 15:59:37 +0200
> > >> Quentin Glidic <sardemff7+wayland at sardemff7.net> wrote:
> > >>    
> > >>> On 08/08/2016 15:45, Pekka Paalanen wrote:    
> > >>>> On Tue,  5 Jul 2016 20:41:50 +0200
> > >>>> Quentin Glidic <sardemff7+wayland at sardemff7.net> wrote:
> > >>>>    
> > >>>>> From: Quentin Glidic <sardemff7+git at sardemff7.net>
> > >>>>>
> > >>>>> Practical example: a client supporting version 2 of wl_output will wait
> > >>>>> for the wl_output.done event before starting wl_output-related
> > >>>>> operations. However, if the server only supports version 1, no event
> > >>>>> will ever come, and it must fallback to use the wl_output.geometry event
> > >>>>> alone.
> > >>>>> Without this macro, it cannot check for that in a nice way.
> > >>>>>
> > >>>>> Signed-off-by: Quentin Glidic <sardemff7+git at sardemff7.net>
> > >>>>> ---
> > >>>>>
> > >>>>> I do not have a real world use-case for the request macro on the server-side,
> > >>>>> but I guess you could do the same: wait the for a "commit" request if client is
> > >>>>> new enough, otherwise use some older request as commit.
> > >>>>>
> > >>>>> Actually I think there was something like that somewhere, now that I write that,
> > >>>>> but I do not remember where exactly.
> > >>>>>
> > >>>>>
> > >>>>>  src/scanner.c | 2 ++
> > >>>>>  1 file changed, 2 insertions(+)
> > >>>>>
> > >>>>> diff --git a/src/scanner.c b/src/scanner.c
> > >>>>> index 4708cae..a4c1984 100644
> > >>>>> --- a/src/scanner.c
> > >>>>> +++ b/src/scanner.c
> > >>>>> @@ -1544,10 +1544,12 @@ emit_header(struct protocol *protocol, enum side side)
> > >>>>>  			emit_structs(&i->request_list, i, side);
> > >>>>>  			emit_opcodes(&i->event_list, i);
> > >>>>>  			emit_opcode_versions(&i->event_list, i);
> > >>>>> +			emit_opcode_versions(&i->request_list, i);
> > >>>>>  			emit_event_wrappers(&i->event_list, i);
> > >>>>>  		} else {
> > >>>>>  			emit_structs(&i->event_list, i, side);
> > >>>>>  			emit_opcodes(&i->request_list, i);
> > >>>>> +			emit_opcode_versions(&i->event_list, i);
> > >>>>>  			emit_opcode_versions(&i->request_list, i);
> > >>>>>  			emit_stubs(&i->request_list, i);
> > >>>>>  		}    
> > >>>>
> > >>>> Hi,
> > >>>>
> > >>>> I have just one question about this. Users must be able to include both
> > >>>> server and client headers in the same compilation unit. Wouldn't this
> > >>>> cause the same thing to be #defined in two different headers? More
> > >>>> importantly, are we sure it won't cause problems?    
> > >>>
> > >>> At least with GCC, you can have twice the same #define (same name + same
> > >>> value) without issue. I do not know about the C standard take on that
> > >>> though.    
> > >>
> > >> Hi,
> > >>
> > >> I would need more proof/opinions than just a "it worked for me".
> > >>    
> > >>> If that would cause problems, it would be because a request and an event
> > >>> have the same name, and come from different versions. But if a user must
> > >>> be able to include both client and server headers, it is already an issue.    
> > >>
> > >> I think we can assume that client and server side should use the same
> > >> version of the XML files.
> > >>
> > >> How is it already an issue? Do we already #define something in both
> > >> headers?    
> > 
> > The issue shows up for this kind of protocol:
> > <request name="something" since="3" />
> > <event name="something" since="2" />
> > 
> > Currently, with this protocol, including both client and server headers 
> > will fail, because one #define will be 3 and the other 2.
> > This patch will break even when including one header.
> > 
> > I guess this kind of protocol is just considered bad design? :-)  
> 
> Indeed. Are there any known cases of this?
> 
> If not, let's go ahead and see if we can get away with it.
> 
> 
> Thanks,
> pq
> 
> > 
> >   
> > >> At least we already seem to have a test in the Wayland test suite that
> > >> includes both client and server protocol headers in the same
> > >> compilation unit.
> > >>    
> > >>> If you really want, I can make server-side define events + prefixed
> > >>> requests and the other way around.
> > >>>
> > >>> server.h:
> > >>> #define NAMESPACE_INTERFACE_SOMEEV_SINCE_VERSION 2
> > >>> #define NAMESPACE_INTERFACE_REQUEST_SOMEREQ_SINCE_VERSION 3
> > >>> client.h:
> > >>> #define NAMESPACE_INTERFACE_EVENT_SOMEEV_SINCE_VERSION 2
> > >>> #define NAMESPACE_INTERFACE_SOMEREQ_SINCE_VERSION 3    
> > >>
> > >> I'd like to avoid that naming hassle if possible. I just want to know
> > >> if it is possible.    
> > >
> > > Hi all,
> > >
> > > after talking to my colleagues and noting that no Microsoft
> > > compiler would ever be used to compile this code, my worries have
> > > cleared.
> > >
> > > GCC has this:
> > > https://gcc.gnu.org/onlinedocs/cpp/Undefining-and-Redefining-Macros.html
> > >
> > > I assume Clang would be similar.
> > >
> > > We can add duplicate #defines just fine. In the unlikely case that it
> > > will blow up something, we can fix the generator to emit
> > > #ifndef/#define/#endif instead of just #define.
> > >
> > > How's that for a contingency plan?    
> > 
> > Good enough for me.
> > 
> >   
> > > The patch is
> > > Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>    
> > 

This patch pushed:
   993df57..5636cb2  master -> master

Whoops, sorry Yong. I pushed before noticing your R-b. :-/

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