Skip to content

Conversation

@dirk-zimoch
Copy link
Contributor

No description provided.


// allow to test deprecated functions without causing compiler warnings
#include <compilerSpecific.h>
#undef EPICS_DEPRECATED
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anjohnson Do we support including compilerSpecific.h directly?

An alternative might be:

#include <compilerDependencies.h>
#undef EPICS_DEPRECATED
#define EPICS_DEPRECATED

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our compilerDependencies.h immediately includes compilerSpecific.h so there must be a version of that file for every target compiler. Some of the those implementations might not define all of the macros, in which case compilerDependencies.h provides an empty macro for the missing definitions.

Here testByteBuffer.cpp doesn't include any other EPICS headers before this conditional, so the headers that do get included lower down that use our compiler macros must be including compilerDependencies.h anyway. As a result I think Dirk's version should be safe as given.

Future edits to his version that added an inclusion of compilerDependencies.h above these lines though would leave the compilerDependencies.h include-guard defined but EPICS_DEPRECATED undefined, so the later includes wouldn't define EPICS_DEPRECATED at all, and would break the build. The author of such changes should notice the break so it seems unlikely to cause problems, but this does look brittle and increases the coupling between the implementations of compilerDependencies.h and testByteBuffer.cpp.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to spend more effort on removing the use of deprecated functions and less on the implementation details of the workaround.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even with the alternative, future edits must make sure that the removal of EPICS_DEPRECATED happens before the header file containing the deprecated function is included.

@AppVeyorBot
Copy link

Build pvDataCPP 1.0.54 failed (commit 1509a2f8cf by @dirk-zimoch)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants