-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cleanup clang compiler warnings #63
base: master
Are you sure you want to change the base?
Conversation
The namespace changes resolve these messages: warning: non-type template argument referring to function 'whatever' with internal linkage is a C++11 extension [-Wc++11-extensions]
Destroyable is deprecated but, here we're just testing it.
❌ Build pva2pva 1.0.48 failed (commit 2f1548b337 by @anjohnson) |
❌ Build pva2pva 1.0.51 failed (commit 2f1548b337 by @anjohnson) |
It looks like the ci update that I did may need some Appveyor config changes; any advice on that would be welcome (I haven't checked the ci-scripts release notes yet). The vs2019 builds all fail immediately like this, but both vs2015 and vs2017 builds succeed:
|
By all means, that should be available on the GHA Windows runners. |
❌ Build pva2pva 1.0.49 failed (commit ef9cdb0951 by @anjohnson) |
❌ Build pva2pva 1.0.52 failed (commit ef9cdb0951 by @anjohnson) |
✅ Build pva2pva 1.0.53 completed (commit 8f91792199 by @anjohnson) |
✅ Build pva2pva 1.0.50 completed (commit 8f91792199 by @anjohnson) |
I think this PR is ready to go now, unless we also want to suppress the deprecation warning for the |
@@ -34,7 +34,7 @@ namespace pva = epics::pvAccess; | |||
|
|||
extern int p2pReadOnly; | |||
|
|||
namespace { | |||
namespace p2pgw { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this causing a warning?
Anonymous namespaces date to C++98. This is the c++ equivalent of static functions, also effecting special symbols for classes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strictly speaking symbols in an anonymous namespace were not specified to have internal linkage until c++11, but in practice clang, gcc, and msvc have done so for many years.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, they're supported but annoyingly warned about:
../gwmain.cpp:245:57: warning: non-type template argument referring to function 'iocsh_drop' with internal linkage is a C++11 extension [-Wc++11-extensions]
epics::iocshRegister<const char*, const char*, &iocsh_drop>("drop", "client", "channel");
^~~~~~~~~~
../gwmain.cpp:205:6: note: non-type template argument refers to function here
void iocsh_drop(const char *client, const char *channel)
^
This PR is about cleaning up compiler warnings, and on macOS with Clang-15 I'm getting several of the above from both gwmain.cpp and qsrv.cpp.
An alternative approach would be to turn off those c++11-extensions
warnings, preferably in the Base build configuration files. The following local PoC works for me, but it would have to be compiler-specific, maybe even dependent on the GCC & Clang versions:
diff --git a/p2pApp/Makefile b/p2pApp/Makefile
index 602518b..6a27152 100644
--- a/p2pApp/Makefile
+++ b/p2pApp/Makefile
@@ -6,6 +6,7 @@ include $(TOP)/configure/CONFIG
#=============================
USR_CPPFLAGS += -I$(TOP)/common
+USR_CXXFLAGS += -Wno-c++11-extensions
PROD_HOST = p2p
After these changes there's only one build warning left from pva2pva builds with Clang 15.0 on macOS, a deprecation warning for
epicsThreadExitMain()
which I deliberately left in. I removed the other deprecation warnings because they are in test files that apparently check the deprecated API.I had to update the .ci module to various targets to build, but I didn't get around to removing the CentOS-7 build so that still dies.