Page MenuHomeWildfire Games

Add virtual base initialization for rP22587
ClosedPublic

Authored by elexis on Thu, Aug 1, 11:41 PM.

Details

Summary

gcc 9 / clang / VS2015 don't complain about absent virtual base initialization in rP22587, but Jenkins pointed it out.

Test Plan

Make Jenkins happy.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

elexis created this revision.Thu, Aug 1, 11:41 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/286/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/287/display/redirect

elexis added a comment.Fri, Aug 2, 1:49 AM

I could only obtain the correct Jenkins build output by clicking through Jenkins, not linked via Phabricator, https://jenkins.wildfiregames.com/job/docker-svn/122/console

../../../source/gui/IGUIButtonBehavior.cpp: In constructor 'IGUIButtonBehavior::IGUIButtonBehavior()':
../../../source/gui/IGUIButtonBehavior.cpp:25:59: error: no matching function for call to 'IGUIObject::IGUIObject()'
 IGUIButtonBehavior::IGUIButtonBehavior() : m_Pressed(false)
                                                           ^
In file included from ../../../source/gui/GUIutil.h:38:0,
                 from ../../../source/gui/GUI.h:46,
                 from ../../../source/pch/gui/precompiled.h:28:
../../../source/gui/IGUIObject.h:110:2: note: candidate: IGUIObject::IGUIObject(CGUI*)
  IGUIObject(CGUI* pGUI);
  ^~~~~~~~~~
../../../source/gui/IGUIObject.h:110:2: note:   candidate expects 1 argument, 0 provided
In file included from ../../../source/gui/GUIutil.h:38:0,
                 from ../../../source/gui/GUI.h:46,
                 from ../../../source/pch/gui/precompiled.h:28:
../../../source/gui/IGUIObject.h:97:7: note: candidate: IGUIObject::IGUIObject(const IGUIObject&)
 class IGUIObject
       ^~~~~~~~~~
../../../source/gui/IGUIObject.h:97:7: note:   candidate expects 1 argument, 0 provided

Compiled without this patch with

  • gcc 9
  • clang 8.0.1
  • VS2015

yet none of these complain about the committed code being broken, only Jenkins, and Jenkins doesn't perform tests anymore since the commit.
So it seems I have to figure this out alone.

IGUITextOwner, IGUIScrollBarOwner, IGUIButtonBehavior are the only occurrences of :*virtual, so those are the only virtually inherited classes in source/.

  • IGUIObject is inherited by CImage, CProgressBar, CSlider, IGUIButtonBehavior, CGUIDummyObject, IGUIScrollBarOwner, IGUITextOwner, CMiniMap
  • IGUITextOwner is inherited by CButton, CChart, CCheckBox, CList, CText, CTooltip
  • IGUIButtonBehavior is inherited by CButton, CCheckBox, CDropDown, CRadioButton.
  • IGUIScrollBarOwner is inherited by CInput, CList, COList, CText
  • CList is inherited by CDropDown, COList
  • CCheckBox is inherited by CRadioButton

I don't see any further classes derived from these three.

I noticed that CDropDown inherits CList, but it doesn't do so virtually. Same CRadioButton inheriting CCheckbox. If one tries to initialize that base, gcc complains:

../../../source/gui/CRadioButton.cpp: In constructor ‘CRadioButton::CRadioButton(CGUI*)’:
../../../source/gui/CRadioButton.cpp:25:39: error: type ‘IGUITextOwner’ is not a direct or virtual base of ‘CRadioButton’
   25 |  : CCheckBox(pGUI), IGUIObject(pGUI), IGUITextOwner(pGUI), IGUIButtonBehavior(pGUI)
      |                                       ^~~~~~~~~~~~~
../../../source/gui/CRadioButton.cpp:25:60: error: type ‘IGUIButtonBehavior’ is not a direct or virtual base of ‘CRadioButton’
   25 |  : CCheckBox(pGUI), IGUIObject(pGUI), IGUITextOwner(pGUI), IGUIButtonBehavior(pGUI)
      |                                                            ^~~~~~~~~~~~~~~~~~
../../../source/gui/CDropDown.cpp: In constructor ‘CDropDown::CDropDown(CGUI*)’:
../../../source/gui/CDropDown.cpp:30:17: error: type ‘IGUIScrollBarOwner’ is not a direct or virtual base of ‘CDropDown’
   30 |  : CList(pGUI), IGUIScrollBarOwner(pGUI), IGUITextOwner(pGUI), IGUIObject(pGUI),
      |                 ^~~~~~~~~~~~~~~~~~
../../../source/gui/CDropDown.cpp:30:43: error: type ‘IGUITextOwner’ is not a direct or virtual base of ‘CDropDown’
   30 |  : CList(pGUI), IGUIScrollBarOwner(pGUI), IGUITextOwner(pGUI), IGUIObject(pGUI),
      |                                           ^~~~~~~~~~~~~

So I leave these out for now. It's not the first time that the virtual inheritance is making problems. For example it requires using a dynamic_cast if one wants to obtain the derived class from the base class, which is 78 times slower than a static_cast if the pointer is already known, see D2136, and one of the proposed solutions was to delete the virtual inheritance. It's the only incidence of virtual inheritance in the codebase, which also explains why there are no dynamic_casts around currently.

elexis edited the summary of this revision. (Show Details)Fri, Aug 2, 1:53 AM
This revision was not accepted when it landed; it landed in state Needs Review.Fri, Aug 2, 1:55 AM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.