MetaEngine::save() is re-implemented in DMetadata without to use virtual property.

[Maik Qualmann]

I think the code is completely correct. If we make save() virtual from 
MetaEngine, a save() call within the base class of MetaEngine would first call 
save() from DMetadata. That doesn't always have to be desired.

Maik

Am Freitag, 13. November 2020, 18:01:11 CET schrieb Gilles Caulier:
> Hi Marcel,
> 
> Yes the code is valid, but so far wrong in the concept of oriented
> objects programming.
> 
> I know the suggested override compiler method, and it's activated here.
> 
> Also all static analyzers must report this kind of overload method,
> but nothing appears...
> 
> Best
> 
> Gilles
> 
> Le ven. 13 nov. 2020 à 17:41, Marcel Mathis <maeseee at gmail.com> a écrit :
> > Hi Gilles
> > 
> > This is completely valid code if the virtual is missing. So I don't think
> > there is a compiler error or warning. See the attached example.
> > 
> > I would advise to use the compiler flag -Werror=suggest-override. So you
> > can guarantee that all overridden functions are at least virtual.
> > 
> > Hope to help you with this information.
> > 
> > Cheers
> > Marcel
> > 
> > On Fri, Nov 13, 2020 at 4:16 PM Gilles Caulier <caulier.gilles at gmail.com> 
wrote:
> >> A partial response can be found here :
> >> 
> >> https://stackoverflow.com/questions/1853896/is-it-possible-to-override-a-> >> non-virtual-method
> >> 
> >> This is typically possible but highly not recommended and not portable
> >> everywhere... as i understand...
> >> 
> >> Gilles
> >> 
> >> Le ven. 13 nov. 2020 à 16:11, Gilles Caulier
> >> 
> >> <caulier.gilles at gmail.com> a écrit :
> >> > And it's the same for MetaEngine::applyChanges() ....
> >> > 
> >> > Gilles
> >> > 
> >> > Le ven. 13 nov. 2020 à 16:10, Gilles Caulier
> >> > 
> >> > <caulier.gilles at gmail.com> a écrit :
> >> > > Hi, all,
> >> > > 
> >> > > I currently review all Clazy static analyzer reports, and i plan to
> >> > > merge DMetadata class to MetaEngine.
> >> > > 
> >> > > While reviewing the code I discovered that MetadaEngine::save() is
> >> > > implemented into DMetadata without being declared as virtual.
> >> > > 
> >> > > I'm surprised to not see a G++ warning about this code, which can
> >> > > introduce side effects... Look by yourself :
> >> > > 
> >> > > https://invent.kde.org/graphics/digikam/-/blob/master/core/libs/metad
> >> > > ataengine/engine/metaengine.h#L372
> >> > > 
> >> > > https://invent.kde.org/graphics/digikam/-/blob/master/core/libs/metad
> >> > > ataengine/dmetadata/dmetadata.h#L95
> >> > > 
> >> > > How is it technically possible ?
> >> > > 
> >> > > Best
> >> > > 
> >> > > Gilles Caulier