Skip to content

Conversation

@kparzysz-quic
Copy link
Contributor

…ensor

ApplyHistoryBestNode declares an Array of Tensor. There are type traits used in Array that require that the element type is complete at the time of the declaration. With only a forward declaration compilation fails (clang 14.0.3, libc++).

…ensor

ApplyHistoryBestNode declares an Array of Tensor. There are type traits
used in Array that require that the element type is complete at the time
of the declaration. With only a forward declaration compilation fails
(clang 14.0.3, libc++).
@kparzysz-quic kparzysz-quic requested a review from junrushao June 15, 2022 14:58
Copy link
Member

@junrushao junrushao left a comment

Choose a reason for hiding this comment

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

Thanks for spotting this!! I will definitely pay more attention here

@junrushao junrushao merged commit a5df283 into apache:main Jun 15, 2022
@junrushao
Copy link
Member

Lesson learned here for me is that never do forward declaration if we can simply include a header :-)

@kparzysz-quic
Copy link
Contributor Author

It's actually a good practice to use forward declarations when the header is not needed. The problem is that in some cases the types must me complete, in this case it was because there was std::enable_if used in the declaration of Array. The trait checked in there required the checked type to be complete.
What I find surprising that it works with the compilers we use in CI. I'm not sure of this is because of newer clang, or because I'm using libc++.

@kparzysz-quic kparzysz-quic deleted the te-tensor-type-traits branch June 15, 2022 17:48
@junrushao
Copy link
Member

@kparzysz-quic Yeah I just verified with clang 14:

>>> clang++-14 --version
Ubuntu clang version 14.0.5-++20220603124341+2f0a69c32a4c-1~exp1~20220603124352.149
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

but yet cannot reproduce the compilation issue...so i guess probably it's something with libc++

@kparzysz-quic
Copy link
Contributor Author

Try setting -DCMAKE_CXX_FLAGS='-stdlib=libc++'.

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.

2 participants