kou commented on code in PR #33847:
URL: https://github.com/apache/arrow/pull/33847#discussion_r1086237224
##########
cpp/src/arrow/type.h:
##########
@@ -288,31 +288,46 @@ std::shared_ptr<DataType> GetPhysicalType(const
std::shared_ptr<DataType>& type)
class ARROW_EXPORT FixedWidthType : public DataType {
public:
using DataType::DataType;
+ // This is only for preventing defining this class in each
+ // translation unit to avoid one-definition-rule violation.
+ ~FixedWidthType() override;
Review Comment:
To be honest, I don't know much about this but...
I tried to create a small example to reproduce this case but couldn't create
it... So I explain what I observed:
If we don't have this, `.o` of a C++ code that has
`std::make_shared<arrow::XXXType>` such as `std::make_shared<arrow::ListType>`
has the following `nm` result:
```console
$ nm --demangle
src/arrow/CMakeFiles/arrow_objlib.dir/compute/kernels/scalar_cast_numeric.cc.o
| grep 'vtable for arrow::FloatingPointType'
00000000 W vtable for arrow::FloatingPointType
```
`W` is a global weak symbol.
https://linux.die.net/man/1/nm
> If lowercase, the symbol is local; if uppercase, the symbol is global
(external).
> "W"
> "w"
>
> The symbol is a weak symbol that has not been specifically tagged as a
weak object symbol.
If we have this, the symbol becomes a undefined symbol:
```console
$ nm --demangle
src/arrow/CMakeFiles/arrow_objlib.dir/compute/kernels/scalar_cast_numeric.cc.o
| grep 'vtable for arrow::FloatingPointType'
U vtable for arrow::FloatingPointType
```
https://linux.die.net/man/1/nm
> "U"
>
> The symbol is undefined.
If there are multiple `W vtable for arrow::FloatingPointType` in our `.o`.
linking `libarrow.so` with `-flto` is failed:
```text
[650/1111] Linking CXX shared library debug/libarrow.so.1100.0.0
FAILED: debug/libarrow.so.1100.0.0
: && /bin/c++ -fPIC -Wno-noexcept-type -flto -fdiagnostics-color=always
-Wall -Wno-conversion -Wno-sign-conversion -Wunused-result
-fno-semantic-interposition -msse4.2 -g -Werror -O0 -ggdb -shared
-Wl,-soname,libarrow.so.1100 -o debug/libarrow.so.1100.0.0 ...(**/*.o)... && :
cpp/src/arrow/type.h:313: error: virtual table of type ‘struct
FloatingPointType’ violates one definition rule [-Werror=odr]
313 | class ARROW_EXPORT FloatingPointType : public NumberType {
|
cpp/src/arrow/type.h:313:20: note: the conflicting type defined in another
translation unit
313 | class ARROW_EXPORT FloatingPointType : public NumberType {
| ^
<built-in>: note: virtual method ‘__cxa_pure_virtual’
...
```
It seems that `W` and `-flto` is a bad combination. If we change `W` to `U`
by defining a not-inlined deconstructor, we don't get the ODR error.
Do you notice anything from this observation?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]