Fix Clang 15 warnings

[anjohnson]

The latest version of this PR only fixes compiler warnings from Clang-15. Earlier versions also changed the CI to use GHA and Appveyor, but those have been merged into master already.

The first commit fixes compiler warnings from Clang-15 about an internal AlarmRange enum.
The second commit changes an API that I suspect isn't actually being used by anyone (@sveseli please check that, see the description in the RELEASE_NOTES) which was overloading a virtual method improperly, leading to more warnings.

[anjohnson]

Hmm, what do I have to do to enable GHA (and Appveyor?) builds for this repo?

[mdavidsaver]

Hmm, what do I have to do to enable GHA (and Appveyor?) builds for this repo?

I believe that some workflow must be defined on whichever is the default branch before GHA will build for PRs.

[anjohnson]

Thanks, that idea also occurred to me overnight as well, I will rebase my commits and merge the GHA and Appveyor additions immediately.

Any ideas why the 3.15 build wasn't checking out and building pvData and pvAccess as stand-alone dependencies before building the main module in this run?

[ralphlange]

Probably because they are not defined as dependent modules.

For 7.0, this works as they are part of Base (check-out with submodules is default).
For 3.15, they need to be explicitly defined.

[anjohnson]

@ralphlange Don't these lines in the .ci-local/default.set file do that?

MODULES=PVDATA PVACCESS

PVDATA_REPONAME=pvDataCPP
PVDATA_REPOOWNER=epics-base

PVACCESS_REPONAME=pvAccessCPP
PVACCESS_REPOOWNER=epics-base

The pva2pva module's CI builds them correctly with 3.15, so I copied that file exactly. Is there something else I need to configure?

[mdavidsaver]

default.set != defaults.set.

[anjohnson]

defaults.set

Aargh, thanks that fixed 3.15 but broke the RTEMS cross-builds. I'll look at updating the GHA script.

[anjohnson]

Success, I will push the CI changes to master, then rebase this PR on top.

[mdavidsaver]

... fixes compiler warnings from Clang-15 ...

For this, and similar, I think it would be useful to document the precise warning(s) seen to help search engines direct people to this issue, and hopefully avoid duplicates. (I am trying to be more consistent about this myself...)

The first commit fixes compiler warnings from Clang-15 about an internal AlarmRange enum.

I am unsure of the original intent. Maybe an attempt at the C pattern typedef enum {} name; where typedef was omitted? Anyway, these bits are private, and the new version is clear.

The second commit changes an API that I suspect isn't actually being used by anyone ...

A common suspicion with these APIs. Still, this is an incompatible change to a public API, and I think should be communicated as such.

I had to look more than twice to (maybe) understand what is going on here. Having the compiler warning text would help. I think the compiler complaint is having two virtual methods with different argument lists (-Woverloaded-virtual).

Did you (or @sveseli) look at adding c++11 override annotations? This might satisfy the compiler about programmer intent without an API change.

fyi. adding override and final helped me make sense of the class hierarchies in pvDataCPP and pvAccessCPP. I would recommend the exercise by itself.

With ScalarAlarmSupport, explicitly overriding the two argument init() base class method alongside the local three argument might also clarify, and can silence the warning. example

Further, if you really want to prevent the two argument version from being called through ScalarAlarmSupport, then make that overload private. (probably excessive, but fyi.)

From the RELEASE_NOTES

... the base-class implementation always returns false as it should not be used ...

The canonical way to express this intent would be with a pure virtual method. Though again, it would mean an API change.

[sveseli]

I am unsure what the original intent was here, but if all classes that inherit from PVSupport were supposed to have init(pvValue, pvSupport), I do not see a problem with leaving this (and ControlSupport class) as it was, and only changing/renaming init() method on line 48 in ScalarAlarmSupport, which would presumably fix the warnings, and would be a smaller API change. I do not quite see a point of introducing a new init() method here that is not being used. Sorry if I am missing something...

[anjohnson]

I think Marty was suggesting that classes derived from PVSupport would provide their own init() method to set up the their added member data, as his class ScalarAlarmSupport (in src/pv/scalarAlarmSupport.{h,cpp}) does. His Doxygen comment said the method would be optional (line 41 above) implying that some might not need to, but I think that in most cases the derived class would need additional parameters anyway (as class ScalarAlarmSupport does), so it doesn't actually make sense for init() to be a virtual method with a fixed set of parameters anyway.

My change was written to try and keep what I thought his design intent was. If we don't care about that then I agree that these changes to the base class are unnecessary.

[sveseli]

The first commit fixes compiler warnings from Clang-15 about an internal AlarmRange enum. The second commit changes an API that I suspect isn't actually being used by anyone (@sveseli please check that, see the description in the RELEASE_NOTES) which was overloading a virtual method improperly, leading to more warnings.

I am not using PVSupport and its derived classes, and I am not aware of anyone else using them.

[anjohnson]

When building this module (without these changes) using

Apple clang version 15.0.0 (clang-1500.1.0.2.5)
Target: x86_64-apple-darwin22.6.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

I get these warnings:

c++              -DUNIX  -Ddarwin      -O3 -g   -Wall       -arch x86_64    -fno-common  -fPIC -I. -I../O.Common -I. -I. -I.. -I../../src/copy -I../../src/database -I../../src/pvAccess -I../../src/support -I../../src/special -I/Users/anj/Software/epics/base-7.0/include/compiler/clang -I/Users/anj/Software/epics/base-7.0/include/os/Darwin -I/Users/anj/Software/epics/base-7.0/include -I/Users/anj/Software/epics/base-7.0/include/compiler/clang -I/Users/anj/Software/epics/base-7.0/include/os/Darwin -I/Users/anj/Software/epics/base-7.0/include        -c ../../src/support/scalarAlarmSupport.cpp
In file included from ../../src/support/scalarAlarmSupport.cpp:22:
../pv/scalarAlarmSupport.h:48:18: warning: 'epics::pvDatabase::ScalarAlarmSupport::init' hides overloaded virtual function [-Woverloaded-virtual]
    virtual bool init(
                 ^
../pv/pvSupport.h:49:18: note: hidden overloaded virtual function 'epics::pvDatabase::PVSupport::init' declared here: different number of parameters (2 vs 3)
    virtual bool init(
                 ^
In file included from ../../src/support/scalarAlarmSupport.cpp:22:
../pv/scalarAlarmSupport.h:88:7: warning: private field 'AlarmRange' is not used [-Wunused-private-field]
    } AlarmRange;
      ^
2 warnings generated.