kicad-developers team mailing list archive

[Sean Silva <chisophugis@xxxxxxxxx>]

Hi all, I'm just wanting to make sure that some warnings that I'm seeing
are not intentional before I submit a patch to fix them.

Clang's -Woverloaded-virtual warning [*] fires many times when building,
primarily of the form:

In file included from
/home/sean/pg/others/kicad/kicad/eeschema/lib_polyline.cpp:35:
/home/sean/pg/others/kicad/kicad/include/wxstruct.h:202:10: warning:
      'EDA_BASE_FRAME::IsActive' hides overloaded virtual function
      [-Woverloaded-virtual]
    bool IsActive() const { return m_FrameIsActive; }
         ^
/usr/include/wx-2.8/wx/gtk/toplevel.h:69:18: note: hidden overloaded
virtual function
      'wxTopLevelWindowGTK::IsActive' declared here: different qualifiers
      (none vs const)
    virtual bool IsActive();

Marking this function `const` has unintended consequences for overload
resolution, and I think that the result is that this code has a bug.
Basically, the `const` makes the compiler treat it as though it were a
separate function for the purposes of overriding, which means that the
subclass's version won't get called through virtual calls. An example of
this is the following small program:

#include <iostream>

struct Base {
  virtual bool IsActive() {
    std::cout << "Base\n";
    return true;
  }
};

struct Derived : public Base {
  bool IsActive() const {
    std::cout << "Derived\n";
    return true;
  }
};

int main() {
  Base *p = new Derived();
  p->IsActive();
  return 0;
}

You might expect it to print "Derived", but it actually prints "Base". The
fix is simple: remove the `const`.

There are also some -Woverloaded-virtual warnings that fire on stuff like
like:

/home/sean/pg/others/kicad/kicad/include/plot_common.h:898:18: warning:
      'DXF_PLOTTER::Text' hides overloaded virtual function
[-Woverloaded-virtual]
    virtual void Text( const wxPoint&              aPos,
                 ^
/home/sean/pg/others/kicad/kicad/include/plot_common.h:247:18: note: hidden
      overloaded virtual function 'PLOTTER::Text' declared here: different
number of
      parameters (11 vs 10)
    virtual void Text( const wxPoint&              aPos,

The same issue arises here: DXF_PLOTTER::Text will not be called through a
virtual call on a PLOTTER, which I think is unintended.

If these warnings seem like unintentional mistakes, I'll go ahead and send
in a patch fixing them.

[*] I am an LLVM/Clang developer, so my Clang is bleeding edge; this
warning may not (yet) be in packaged Clang versions.

-- Sean Silva