Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Compilation fails using Clang 17 toolchain #3654

Closed
ghost opened this issue Sep 24, 2023 · 6 comments
Closed

Compilation fails using Clang 17 toolchain #3654

ghost opened this issue Sep 24, 2023 · 6 comments

Comments

@ghost
Copy link

ghost commented Sep 24, 2023 •
edited by vitaut
Loading

MSYS2 just updated their clang and associated packages to 17.0.1. We are now seeing the following compilation error:

   17 | #  include <__std_stream>

     |            ^~~~~~~~~~~~~~

This seems to be related to the following change in libcxx: https://reviews.llvm.org/D147855
@lazka lazka mentioned this issue Sep 24, 2023
Closed
@vitaut
Copy link
Contributor

vitaut commented Sep 26, 2023

Removed the nonportable include in 2dd4fa8. Thanks for reporting.

@vitaut vitaut closed this as completed Sep 26, 2023
@lazka
Copy link
Contributor

lazka commented Sep 29, 2023

Note that there is still one include left here:

fmt/src/fmt.cc

Lines 66 to 68 in 79dbd3f

# elif defined(_LIBCPP_VERSION)
# include <__std_stream>
# endif
which makes the build with "cmake -DFMT_MODULE=ON" fail.

@vitaut
Copy link
Contributor

vitaut commented Sep 30, 2023

@lazka, could you submit a PR to remove it too?

lazka added a commit to lazka/fmt that referenced this issue Oct 1, 2023
@lazka
Remove leftover usage of the __std_stream header
0428c81 
2dd4fa8 removed all usage of __std_stream because
it is no logner available with clang v17. That missed one place
where the header was still used (only used when building with -DFMT_MODULE=ON).
Remove it there too.

See fmtlib#3654
lazka added a commit to lazka/fmt that referenced this issue Oct 1, 2023
@lazka
Remove leftover usage of the __std_stream header
20ce953 
2dd4fa8 removed all usage of __std_stream because
it is no longer available with clang v17. That commit missed one place
where the header was still used (only used when building with -DFMT_MODULE=ON).
Remove it there too.

See fmtlib#3654
@lazka lazka mentioned this issue Oct 1, 2023
Merged
@lazka
Copy link
Contributor

lazka commented Oct 1, 2023

sure, #3663

vitaut pushed a commit that referenced this issue Oct 1, 2023
@lazka
Remove leftover usage of the __std_stream header (#3663)
b3bf23f 
2dd4fa8 removed all usage of __std_stream because
it is no longer available with clang v17. That commit missed one place
where the header was still used (only used when building with -DFMT_MODULE=ON).
Remove it there too.

See #3654
vitaut referenced this issue Oct 23, 2023
@vitaut
Remove an ostream hack incompatible with libc++ 17
2dd4fa8
@Roman-Koshelev
Copy link
Contributor

@vitaut wouldn't it make more sense to leave this hack for libc++ < 17? (native_handle() should be at 18)

@vitaut
Copy link
Contributor

vitaut commented Oct 28, 2023 •
edited
Loading

I don't think it's worth spending any efforts on conditionally supporting this hack for iostreams which are a legacy API.

@Biswa96 Biswa96 mentioned this issue Oct 29, 2023
Closed
@MarkCallow MarkCallow mentioned this issue Oct 30, 2023
Closed
ckerr pushed a commit to transmission/fmt that referenced this issue Nov 7, 2023
@lazka @ckerr
Remove leftover usage of the __std_stream header (fmtlib#3663)
f32dba4 
2dd4fa8 removed all usage of __std_stream because
it is no longer available with clang v17. That commit missed one place
where the header was still used (only used when building with -DFMT_MODULE=ON).
Remove it there too.

See fmtlib#3654
@lazka lazka mentioned this issue Nov 25, 2023
Merged
@BrewTestBot BrewTestBot mentioned this issue Jan 1, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@vitaut @lazka @Roman-Koshelev