workarounds for clang-cl

[xiaoqiangwang]

This is related to epics-base/epics-base#391 to add clang-cl compiler support to EPICS base.

• PVScalarValue and PVValueArray are template classes. They are specialized with different data types. The dllexport/dllimport attribute is applied to the template class.
• clang-cl has a msvc incompatible issue Clang prohibits initialization of static members of specialized __declspec(dllexport) class templates llvm/llvm-project#54814. These modifications are applied as workarounds.
• MinGW gcc 8 (from Strawberry Perl) fails to build. The lowest MinGW version I have access to is MinGW 10

I put this draft here and wish to hear whether clang-cl support is worth the rather ugly workaround.

[AppVeyorBot]

✅ Build pvDataCPP 1.0.31 completed (commit 949f930bb9 by @xiaoqiangwang)

[mdavidsaver]

I put this draft here and wish to hear whether clang-cl support is worth the rather ugly workaround.

This is indeed an ugly change. I'm not sure it is standards defined behavior. It is certainly not common practice. Although "common practice" would likely be a set of explicit specializations, which would be even more verbose.

cf. Why can’t I separate the definition of my templates class from its declaration and put it inside a .cpp file?.

[mdavidsaver]

I've encountered a linker error while trying to build this change locally. This is on Linux with GCC 10.2.1. For my locally builds I set -fvisibility=hidden -fvisibility-inlines-hidden, which I now notice that none of the CI builds do. This matches my understanding that marking a class with __attribute__ ((visibility("default"))) is not identical to marking all of on the members individually. Marking the class is necessary to make certain other symbols visible (typeinfo and I think some symbols involved in exception unwinding).

/usr/bin/g++ -o testOverrunBitSet  -L/home/mdavidsaver/work/epics/pv/pvData/lib/linux-x86_64 -L/home/mdavidsaver/work/epics/base-git/lib/linux-x86_64 -Wl,-rpath-link,/home/mdavidsaver/work/epics/pv/pvData/lib/linux-x86_64 -Wl,-rpath,\$ORIGIN/../../lib/linux-x86_64 -Wl,-rpath-link,/home/mdavidsaver/work/epics/base-git/lib/linux-x86_64 -Wl,-rpath,\$ORIGIN/../../../../base-git/lib/linux-x86_64          -rdynamic -m64         testOverrunBitSet.o    -lpvData -lCom  -lz 
/usr/bin/ld: testOverrunBitSet.o: warning: relocation against `_ZTIN5epics6pvData8PVStringE' in read-only section `.text'
/usr/bin/ld: testOverrunBitSet.o: in function `std::shared_ptr<epics::pvData::PVString> std::dynamic_pointer_cast<epics::pvData::PVString, epics::pvData::PVField>(std::shared_ptr<epics::pvData::PVField> const&)':
/usr/include/c++/10/bits/shared_ptr.h:602: undefined reference to `typeinfo for epics::pvData::PVString'
/usr/bin/ld: warning: creating DT_TEXTREL in a PIE

There is a possibility that doing something like explicitly instantiating the destructor while have the effect of making the typeinfo visible, but this is a guess on my part.

Overall, I expect that some alternate solution/workaround will need to be found.

[mdavidsaver]

From my reading of the clang issue, the trigger is PVScalarValue::typeCode.

pvDataCPP/src/pv/pvData.h

Line 386 in c16f19c

static const ScalarType typeCode;

and the associated explicit specializations.

pvDataCPP/src/factory/PVDataCreateFactory.cpp

Lines 34 to 35 in c16f19c

	template<> const ScalarType PVBoolean::typeCode = pvBoolean;
	template<> const ScalarType PVByte::typeCode = pvByte;

An alternative pattern/trick to include constant values in a template instance, which might work better, would be to replace the static member typeCode with an enum. Again, specialized for each value type. eg. ScalarTypeID

pvDataCPP/src/pv/pvIntrospect.h

Lines 1434 to 1439 in c16f19c

	#define OP(ENUM, TYPE) \
	template<> struct ScalarTypeTraits<ENUM> {typedef TYPE type;}; \
	template<> struct ScalarTypeID<TYPE> { enum {value=ENUM}; }; \
	template<> struct ScalarTypeID<const TYPE> { enum {value=ENUM}; };
	OP(pvBoolean, boolean)

This is a bit uglier as it requires casting the "dummy" enum back into the actual type. eg.

pvDataCPP/src/misc/pv/sharedVector.h

Line 703 in c16f19c

,m_vtype((ScalarType)ScalarTypeID<FROM>::value)

However, an enum has no associated symbols, and so causes no additional dllimport/export issues.

[mdavidsaver]

Merge branch 'epics-base:master' into fix_clang_windows

When updating, please git rebase. Do not include merge commits in PRs for the module.

[mdavidsaver]

Also as a note, I'm still not certain that I understand why this change can work at all. My thinking is that the presence of the explicit specializations of typeCode have the side effect of emitting some/all symbols for these classes into PVDataCreateFactory.o. And if this is so, I'm not clear of how portable this behavior is.

pvDataCPP/src/factory/PVDataCreateFactory.cpp

Lines 34 to 35 in c16f19c

	template<> const ScalarType PVBoolean::typeCode = pvBoolean;
	template<> const ScalarType PVByte::typeCode = pvByte;

Reading https://en.cppreference.com/w/cpp/language/class_template I find this paragraph under "Explicit instantiation".

When an explicit instantiation names a class template specialization, it serves as an explicit instantiation of the same kind (declaration or definition) of each of its non-inherited non-template members that has not been previously explicitly specialized in the translation unit. If this explicit instantiation is a definition, it is also an explicit instantiation definition only for the members that have been defined at this point.

Which almost describes the situation, except for the non-template members exclusion.

[xiaoqiangwang]

As searching for a better workaround, I found clang-cl works if typeCode has the constexpr specifier. The following change makes use of constexpr if C++11 is detected. xiaoqiangwang@c09f6d8

The change assumes that user code will not specialize PVScalarValue and PVValueArray templates with other data types.

[mdavidsaver]

... makes use of constexpr if C++11 is detected ...

Which would then be required when clang-cl is used? I guess this seems workable for what is effectively a new compiler, with its own set of quirks.

Will you be updating your PR? (I'd suggest retaining your current set of changes as a local branch)

Also, how can clang-cl be distinguished from "classic" clang (clang-gcc?) or actual MSVC? Or for that matter, from some of the clang + mingw variants (which I've only read about, but not used).

eg. if we want an explicit error if this limitation is encountered, instead of failing with an obscure linker error. Would something like the following work?

#if defined(_MSC_VER) && defined(__clang__) && __cplusplus < 201103L
#  error clang-ci requires >= c++11
#endif

[xiaoqiangwang]

Will you be updating your PR? (I'd suggest retaining your current set of changes as a local branch)

Do I understand that to keep this PR, I would need to reset this branch (after backing up) and then apply the constexpr fixes.

Also, how can clang-cl be distinguished from "classic" clang (clang-gcc?) or actual MSVC? Or for that matter, from some of the clang + mingw variants (which I've only read about, but not used).

I have the same question in the epics-base/epics-base#391 (comment).

eg. if we want an explicit error if this limitation is encountered, instead of failing with an obscure linker error. Would something like the following work?

I checked that "llvm-rc.exe"(needed for libcom module) is not created until LLVM 7, which defaults to C++14. So the minimum requirement could be probably simply imposed on the whole support of windows clang.

Update: llvm-rc.exe cannot compile the Com.rc file until LLVM 9 13. So that would be the minimum required version.

[AppVeyorBot]

✅ Build pvDataCPP 1.0.34 completed (commit 71cbcf24a7 by @xiaoqiangwang)