Skip to content

Fix compilation with libcxx 16#144

Merged
markos merged 1 commit intoVectorCamp:developfrom
rschu1ze:rs/fix-libcxx16
Mar 29, 2023
Merged

Fix compilation with libcxx 16#144
markos merged 1 commit intoVectorCamp:developfrom
rschu1ze:rs/fix-libcxx16

Conversation

@rschu1ze
Copy link
Copy Markdown

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

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
@markos markos added this to the 5.4.10 milestone Mar 29, 2023
@markos markos merged commit d054911 into VectorCamp:develop Mar 29, 2023
@markos
Copy link
Copy Markdown

markos commented Mar 29, 2023

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

@rschu1ze rschu1ze deleted the rs/fix-libcxx16 branch March 29, 2023 10:11
azat added a commit to azat/ClickHouse that referenced this pull request May 23, 2023
markos added a commit that referenced this pull request Oct 24, 2025
markos added a commit that referenced this pull request Oct 24, 2025
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