Menu

#565 TWindow::SetMenu is not virtual

8
pending
1
2023-11-19
2023-10-21
No

TWindow::SetMenu is not declared as a virtual function. Consequently, TFrameWindow::SetMenu hides rather than overrides TWindow::SetMenu. This means that any call to SetMenu through a pointer to the base class TWindow will call TWindow::SetMenu, rather than SetMenu in TFrameWindow or another subclass thereof, despite SetMenu being declared virtual in TFrameWindow. This subtlety may be surprising and lead to bugs.

I cannot come up with a good rationale for this API design, and in general I would think that declaring a new virtual function with the same name as a non-virtual function in a base class is bad API design. So, I propose we change TWindow::SetMenu to be declared as a virtual function and thus be reliably overridable.

However, note that this is a breaking change. Code may rely on the existing behaviour, although I doubt this would be very common. More likely, existing code calling SetMenu through a pointer to the TWindow base class would want to call SetMenu in the actual subclass, and if so will have a bug, which will be fixed by the proposed change.

Related

Commit: [r6733]
Wiki: OWLNext_Roadmap_and_Prereleases

Discussion

  • Vidar Hasfjord

    Vidar Hasfjord - 2023-11-19
    • status: open --> pending
     
  • Vidar Hasfjord

    Vidar Hasfjord - 2023-11-19

    TWindow::SetMenu was made virtual in [r6730], and this change has been merged into Owlet [r6733].

    Conflicting OwlExt::TMenuButton::SetMenu was replaced by SetPopupMenu.

     

    Related

    Commit: [r6730]
    Commit: [r6733]


    Last edit: Vidar Hasfjord 2023-11-19

Log in to post a comment.