[PATCH setup] Fix -Woverloaded-virtual warnings about Window::Create()

[PATCH setup] Fix -Woverloaded-virtual warnings about Window::Create()

[Jon Turney]

In gcc 13, -Wall turns on -Woverloaded-virtual
---

Notes:
    I think despite being marked virtual, these methods aren't actually
    callable on any derived object because they aren't declared for those
    sublasses.
    
    It seems that all calls to these in methods in derived objects use the
    method name qualified by an explcit classname.
    
    So we can simply fix this warning by removing the 'virtual' specifier.
    
    But someone double-checking my reasoning here is probably a good idea.

 Makefile.am |  2 +-
 proppage.h  | 10 +++++-----
 window.h    |  7 +++----
 3 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index b459d16..22ad30c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -22,7 +22,7 @@ SUBDIRS := @subdirs@ tests
 BASECXXFLAGS = -Werror -Wall -Wpointer-arith -Wcomments \
 	       -Wcast-align -Wwrite-strings -fno-builtin-sscanf \
 	       -Wno-attributes
-AM_CXXFLAGS = $(BASECXXFLAGS) -std=gnu++11 ${$(*F)_CXXFLAGS}
+AM_CXXFLAGS = $(BASECXXFLAGS) -Woverloaded-virtual -std=gnu++11 ${$(*F)_CXXFLAGS}
 AM_CFLAGS = $(BASECXXFLAGS) -Wmissing-declarations -Winline \
 	    -Wstrict-prototypes -Wmissing-prototypes
 AM_YFLAGS = -d
diff --git a/proppage.h b/proppage.h
index 64f822b..9db1a90 100644
--- a/proppage.h
+++ b/proppage.h
@@ -115,11 +115,11 @@ public:
     IsLast = false;
   };
 
-  virtual bool Create (int TemplateID);
-  virtual bool Create (DLGPROC dlgproc, int TemplateID);
-  virtual bool Create (DLGPROC dlgproc,
-		       BOOL (*cmdproc) (HWND h, int id, HWND hwndctl,
-					UINT code), int TemplateID);
+  bool Create (int TemplateID);
+  bool Create (DLGPROC dlgproc, int TemplateID);
+  bool Create (DLGPROC dlgproc,
+               BOOL (*cmdproc) (HWND h, int id, HWND hwndctl,
+                                UINT code), int TemplateID);
 
   virtual void OnInit ()
   {
diff --git a/window.h b/window.h
index 1dfb2a9..dcc81c1 100644
--- a/window.h
+++ b/window.h
@@ -82,10 +82,9 @@ public:
   Window ();
   virtual ~ Window ();
 
-  virtual bool Create (Window * Parent = NULL,
-		       DWORD Style =
-		       WS_OVERLAPPEDWINDOW | WS_VISIBLE | WS_CLIPCHILDREN);
-  
+  bool Create (Window * Parent = NULL,
+               DWORD Style = WS_OVERLAPPEDWINDOW | WS_VISIBLE | WS_CLIPCHILDREN);
+
   static void SetAppInstance (HINSTANCE h)
   {
     // This only has to be called once in the entire app, before
-- 
2.43.0

[PATCH] In gcc 13, -Wall turns on -Woverloaded-virtual

[Corinna Vinschen]

Also fix ambiguous method declaration by dropping a default parameter.
---
Hi Jon,

I'm not sure removing virtual from all Create methods really fits the
bill in all cases, are you?

I had a go at fixing this while keeping the virtuality of the methods
intact.  While at it it also occured to me that there's a tricky problem
for the compiler to differentiate the method

  Create ();

from the method

  Create (Window * = NULL, DWORD = 42);

I fixed this ambiguity by making the Window parameter a non-default
parameter.

What do you think?


Corinna

 desktop.h    | 2 +-
 main.cc      | 2 +-
 proppage.h   | 1 +
 propsheet.cc | 2 +-
 propsheet.h  | 2 +-
 window.h     | 7 ++++++-
 6 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/desktop.h b/desktop.h
index d4ce72def757..87b5ccee5302 100644
--- a/desktop.h
+++ b/desktop.h
@@ -33,7 +33,7 @@ public:
   {
   };

-  bool Create ();
+  virtual bool Create ();

   virtual void OnInit ();
   virtual void OnActivate ();
diff --git a/main.cc b/main.cc
index b570c6cb18ec..9035260972b3 100644
--- a/main.cc
+++ b/main.cc
@@ -213,7 +213,7 @@ main_display ()
   MainWindow.AddPage (&Desktop);

   // Create the PropSheet main window
-  MainWindow.Create ();
+  MainWindow.Create (NULL);

   // Uninitalize COM
   if (sl)
diff --git a/proppage.h b/proppage.h
index 64f822b515be..5301a97a8221 100644
--- a/proppage.h
+++ b/proppage.h
@@ -115,6 +115,7 @@ public:
     IsLast = false;
   };

+  virtual bool Create () { return false; };
   virtual bool Create (int TemplateID);
   virtual bool Create (DLGPROC dlgproc, int TemplateID);
   virtual bool Create (DLGPROC dlgproc,
diff --git a/propsheet.cc b/propsheet.cc
index faf6583f0803..3563a388cd48 100644
--- a/propsheet.cc
+++ b/propsheet.cc
@@ -341,7 +341,7 @@ PropSheetProc (HWND hwndDlg, UINT uMsg, LPARAM lParam)
 }

 bool
-PropSheet::Create (const Window * Parent, DWORD Style)
+PropSheet::Create (Window * Parent, DWORD Style)
 {
   PROPSHEETHEADERW p;

diff --git a/propsheet.h b/propsheet.h
index b900e790c32f..870905cf9777 100644
--- a/propsheet.h
+++ b/propsheet.h
@@ -47,7 +47,7 @@ public:
   void SetHWNDFromPage (HWND h);
   void AdjustPageSize (HWND page);

-  virtual bool Create (const Window * Parent = NULL,
+  virtual bool Create (Window *Parent,
 		       DWORD Style =
 		       WS_OVERLAPPEDWINDOW | WS_VISIBLE | WS_CLIPCHILDREN);

diff --git a/window.h b/window.h
index 1dfb2a9f7514..bd3080e758fc 100644
--- a/window.h
+++ b/window.h
@@ -82,7 +82,12 @@ public:
   Window ();
   virtual ~ Window ();

-  virtual bool Create (Window * Parent = NULL,
+  virtual bool Create () { return false; };
+  virtual bool Create (int) { return false; };
+  virtual bool Create (DLGPROC, int) { return false; };
+  virtual bool Create (DLGPROC, BOOL (*) (HWND, int, HWND, UINT), int)
+  { return false; };
+  virtual bool Create (Window *Parent,
 		       DWORD Style =
 		       WS_OVERLAPPEDWINDOW | WS_VISIBLE | WS_CLIPCHILDREN);

--
2.43.0

Re: [PATCH] In gcc 13, -Wall turns on -Woverloaded-virtual

[Jon Turney]

On 07/02/2024 10:46, Corinna Vinschen via Cygwin-apps wrote:
> Also fix ambiguous method declaration by dropping a default parameter.
> ---
> Hi Jon,
> 
> I'm not sure removing virtual from all Create methods really fits the
> bill in all cases, are you?
> 
> I had a go at fixing this while keeping the virtuality of the methods
> intact.  While at it it also occured to me that there's a tricky problem
> for the compiler to differentiate the method

This seems like overkill.

 From auditing the code, as far as I can tell, we only ever instantiate 
PropPage-derived objects for each of the setup wizard pages, and one 
PropSheet object to contain them.

There are no instantiations of the Window class itself.

PropPage::Create() doesn't even call Window::Create(), so that method 
can actually be totally removed without problem...

Re: [PATCH] In gcc 13, -Wall turns on -Woverloaded-virtual

[Corinna Vinschen]

On Feb  8 16:07, Jon Turney via Cygwin-apps wrote:
> On 07/02/2024 10:46, Corinna Vinschen via Cygwin-apps wrote:
> > Also fix ambiguous method declaration by dropping a default parameter.
> > ---
> > Hi Jon,
> > 
> > I'm not sure removing virtual from all Create methods really fits the
> > bill in all cases, are you?
> > 
> > I had a go at fixing this while keeping the virtuality of the methods
> > intact.  While at it it also occured to me that there's a tricky problem
> > for the compiler to differentiate the method
> 
> This seems like overkill.
> 
> From auditing the code, as far as I can tell, we only ever instantiate
> PropPage-derived objects for each of the setup wizard pages, and one
> PropSheet object to contain them.
> 
> There are no instantiations of the Window class itself.
> 
> PropPage::Create() doesn't even call Window::Create(), so that method can
> actually be totally removed without problem...

I just thought it might be a good idea to keep virtuality, but your
assessment shows that was unnecessary.  No worries from my side.

Corinna