[Development] changing Q_GADGET

[Jędrzej Nowacki]

On Friday 28 of November 2014 13:55:44 Simon Hausmann wrote:
> On Friday 28. November 2014 12.41.47 Olivier Goffart wrote:
> > On Friday 28 November 2014 12:19:45 Simon Hausmann wrote:
> > > Hi,
> > > 
> > > Monsieur Goffart did awesome work in the dev branch on allowing
> > > structures
> > > with Q_GADGET to have properties and invokable methods. This brings the
> > > macro to a much wider audience and I'd like to use this opportunity to
> > > propose a slight change to it that may be controversial:
> > > 
> > > The macros Q_OBJECT and Q_GADGET both - towards the end of their
> > > definition
> > > - change the member access permission level to "private". It's always
> > > been
> > > that way.
> > > 
> > > I'd like to propose changing Q_GADGET to not change the access
> > > permission
> > > level, so that you can write
> > > 
> > > struct MyStructure
> > > {
> > > 
> > >     Q_PROPERTY(int x MEMBER x)
> > >     Q_GADGET
> > >     
> > >     int x;
> > > 
> > > }
> > > 
> > > 
> > > I feel that Q_GADGET has its primary use with structures and the default
> > > access level for those is public. I find it awkward that you currently
> > > have
> > > to write:
> > > 
> > > structure MyStructure
> > > {
> > > 
> > >     Q_PROPERTY(int x MEMBER X)
> > >     Q_GADGET
> > > 
> > > public:
> > >     int x;
> > > 
> > > }
> > > 
> > > 
> > > The proposed change would have two effects:
> > > 
> > > 1) It makes any existing code that _relies_ on Q_GADGET turning to
> > > private
> > > suddenly expose members in structures.
> > > 
> > > 2) On paper it breaks binary compatibility with MSVC, in the unlikely
> > > event
> > > that somebody
> > > 
> > >     a) produces a DLL and cares about binary compatibility
> > >     b) exposes bare structures
> > >     c) relies on Q_GADGET turning access permission levels to private
> > > 
> > > I feel that the effects are negligible compared to the benefit of a
> > > better
> > > API.
> > > 
> > > What do you think?
> > 
> > I have several times been bitten by the same problem when i used Q_OBJECT
> > in a struct.
> > But when I weight the pro and the cons, i'm not sure if it's a good idea.
> > 
> > Pros:
> >  + Save one line when declaring a Q_GADGET in a struct
> > 
> > Cons:
> >  - The change may change some member (data or function) from private to
> > 
> > public - Is inconsistent with Q_OBJECT (and both are usually used in
> > class)
> > - Something that is incorrectly set as private will result in compilation
> > errors that will be easily spotted and fixed. Something inadvertently left
> > public will stay unnoticed until it's too late to change it.
> > 
> > 
> > Notice that in our code we recommend to always use class and almost never
> > struct.  All the Q_GADGET today are class.
> 
> Hmm, that's a good point. I may be living in an over-simplified world where
> I use struct more often than others. (in my dream world class also defaults
> to public). The reality is probably that both macros are perhaps more
> likely to be used in the class context and that consistency between the
> macros might outweigh the inconsistency to struct.
> 
> 
> 
> Simon
> 
> > An alternative to save that line might also be to put the Q_GADGET macro
> > at
> > the end of the struct.

Hi,

  Personally, I think Q_OBJECT should not change access level and so Q_GADGET. 
It is not really about one additional line to type, it is about surprise 
effect, while reading code. So if we would design the Q_OBJECT macro again, 
would we change the access level again?

Pros: 
 -  it always keep the same access level (may be easier to keep BC on 
Windows?)

Cons:
 - surprise factor, as it doesn't behave as standard, property based, c++ code
 - one line more for structs (minor)

Now, we can not change Q_OBJECT, at least up to Qt6, then should we follow 
it's practice? Q_OBJECT is the only macro, we ship, that changes access level, 
so from these perspective it is an outsider, but I agree that Q_GADGET is it's 
a twin brother. So keeping them in sync is not bad either. 

> > - Something that is incorrectly set as private will result in compilation
> > errors that will be easily spotted and fixed. Something inadvertently left
> > public will stay unnoticed until it's too late to change it.
True, but in my opinion, it is not a problem. Private api should be protected 
by a convention not by a tool, because you can always workaround a tool. But 
that discussion is probably out of scope.

> > Notice that in our code we recommend to always use class and almost never
> > struct.  All the Q_GADGET today are class.
Do you know why (and where) we recommend that?


Cheers,
  Jędrek