Fix compilation with libcxx 16#144
Merged
markos merged 1 commit intoVectorCamp:developfrom Mar 29, 2023
rschu1ze:rs/fix-libcxx16
Merged
Fix compilation with libcxx 16#144markos merged 1 commit intoVectorCamp:developfrom rschu1ze:rs/fix-libcxx16
markos merged 1 commit intoVectorCamp:developfrom
rschu1ze:rs/fix-libcxx16
Conversation
After upgrading our (ClickHouse's) libcxx from 15 to 16, the compiler
started to complain about usage of an incomplete type "RoseInstruction"
in this (header) function:
void RoseProgram::replace(Iter it, std::unique_ptr<RoseInstruction> ri) {
...
The reason is that libcxx 16 is the first version which implements C++23
constexpr std::unique_ptr (P2273R3, see (*)). RoseProgram::replace()
happens to be be const-evaluatable and the compiler tries to run
std::unique_ptr's ctor + dtor. This fails because at this point
RoseInstruction isn't defined yet.
There are two ways of fixing this:
1. Include rose_build_instruction.h (which contains RoseInstruction)
into rose_build_program.h. Disadvantage: The new include will
propagate transitively into all callers.
2. Move the function implementation into the source file which sees
RoseInstruction's definition already. Disadvantage: Template
instantiation is no longer automatic, instead there must be either a)
explicit template instantiation (e.g. in rose_build_program.cpp) or
b) all callers which instantiate the function must live in the same
source file and do the instantiations by themselves. Fortunately, the
latter is the case here, but potential future code outside
rose_build_program.cpp will require ugly explicit instantiation.
(*) https://en.cppreference.com/w/cpp/23
|
Though we don't yet test or support libcxx 16 your PR passed our CI on all platforms/compilers so it's good enough to merge. Thank you for your contribution, this will be included in the next version 5.4.10 |
azat
added a commit
to azat/ClickHouse
that referenced
this pull request
May 23, 2023
Everything that is requried already merged: - VectorCamp/vectorscan#144 - VectorCamp/vectorscan#149 - VectorCamp/vectorscan#148 Signed-off-by: Azat Khuzhin <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
After upgrading our (ClickHouse's) libcxx from 15 to 16, the compiler started to complain about usage of an incomplete type "RoseInstruction" in this (header) function:
The reason is that libcxx 16 is the first version which implements C++23 constexpr std::unique_ptr (P2273R3, see (*)). RoseProgram::replace() happens to be be const-evaluatable and the compiler tries to run std::unique_ptr's ctor + dtor. This fails because at this point RoseInstruction isn't defined yet.
There are two ways of fixing this:
(*) https://en.cppreference.com/w/cpp/23