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.

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

QPDF incompatible with LLVM 18's LibC++ due to removal of non-standard char_traits<unsigned char> #1024

Closed
zclifford opened this issue Aug 22, 2023 · 5 comments

Comments

@zclifford
Copy link
Contributor

zclifford commented Aug 22, 2023

Hello,

Once again my company is trying to compile with bleeding edge C++. And tasked me with resolving a QPDF compilation error that occurs with LLVM 18's LibC++ -- I don't think this has been released yet, much less to any distros, so standard tooling won't yet reproduce the issue.

This time it's char_traits. Previously LibC++ came with a non-standard implementation of char_traits which would happily work with types not listed in the standard. (Further links: Announcement, Bug, Commit)

We are deprecating std::char_traits with types other than char, wchar_t, char16_t, char32_t and char8_t, and that specialization will go away in LLVM 18.

(n.b. char8_t was added to the spec in C++20, and isn't part of C++17)

It seems like QPDF relies on this here:

...
In file included from third_party/qpdf/src/libqpdf/Pl_Buffer.cc:1:
In file included from third_party/qpdf/src/include/qpdf/Pl_Buffer.hh:32:
In file included from third_party/qpdf/src/include/qpdf/Pipeline.hh:42:
In file included from third_party/stl/cxx17/string:26:
third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/string:723:46: error: implicit instantiation of undefined template 'std::char_traits<unsigned char>'
  723 |     static_assert(( is_same<_CharT, typename traits_type::char_type>::value),
      |                                              ^
third_party/qpdf/src/include/qpdf/Pl_Buffer.hh:80:42: note: in instantiation of template class 'std::basic_string<unsigned char>' requested here
   80 |         std::basic_string<unsigned char> data;
      |                                          ^
third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/__fwd/string.h:23:29: note: template is declared here
   23 | struct _LIBCPP_TEMPLATE_VIS char_traits;
      |                             ^
...

Here unsigned char is none of the above types, and so fails to compile.

I'll have to take a closer look at how this is used to think of how to work around that, but I assume you'd be OK with me sending another PR once I understand it a bit better, to keep things standards compatible?

Probably it could either be a std::string or an std::vector<unsigned char>

@zclifford
Copy link
Contributor Author

zclifford commented Aug 22, 2023

My understanding is that either std::string (e.g. std::basic_string<char>) or std::vector<unsigned char> would be equally valid (based on https://stackoverflow.com/a/15172304 anyway). Though this code is probably just as easy to express as a vector than as a string, and that allows keeping the unsigned char type instead of casting. I'll try it out and send a PR.

Thanks.

@zclifford
Copy link
Contributor Author

zclifford commented Aug 22, 2023

Pull Request: #1025

@jberkenbilt
Copy link
Contributor

@zclifford Thanks for coming back and closing the ticket. I wasn't keeping it open just because it was a power of 2. :-)

@zclifford
Copy link
Contributor Author

It definitely felt like I got a lucky number there 😊. I assume that means that C++ has blessed me and I didn't mess up the pointer arithmetic in my pull request.

@zclifford
Copy link
Contributor Author

This was regressed in 11.9.0 by QPDF_Name::analyzeJSONEncoding in 4319874

No biggie, I expected this could happen (and now I can catch it real fast since my workplace switched to a compiler config that hits it :).

In file included from third_party/stl/cxx17/string_view:4:
third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/string_view:296:43: error: implicit instantiation of undefined template 'std::char_traits<unsigned char>'
  296 |   static_assert((is_same<_CharT, typename traits_type::char_type>::value),
      |                                           ^
third_party/qpdf/src/libqpdf/QPDF_Name.cc:60:43: note: in instantiation of template class 'std::basic_string_view<unsigned char>' requested here
   60 |     std::basic_string_view<unsigned char> view{
      |                                           ^
third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/__fwd/string.h:23:29: note: template is declared here
   23 | struct _LIBCPP_TEMPLATE_VIS char_traits;
      |                             ^
1 error generated.

I can send a follow up commit to re-fix it tomorrow, haven't looked too deeply yet (I realized there was a QPDF update at like 17:00).

But gah this issue is still annoying, clang is too picky :D

I'm not aware of a replacement for basic_string_view here (except maybe C++20 ranges, which aren't in C++17), so I guess the easiest fix is moving the cast down to the for loop and working with a normal std::string_view.

@zclifford zclifford reopened this Feb 28, 2024
zclifford added a commit to zclifford/qpdf-char-traits-2 that referenced this issue Feb 28, 2024
string_view<unsigned char> leads to char_traits<unsigned char> which is not standard C++ (background in qpdf#1024).

This triggers compilation failures with certain C++20 compiler configurations.

Here the goal was to iterate over a string without copying it, but treating the chars as unsigned. It seems like moving the char -> unsigned char cast to the for loop's range declaration should work.
zclifford added a commit to zclifford/qpdf-char-traits-2 that referenced this issue Feb 28, 2024
…d in qpdf#1024).

This triggers compilation failures with certain C++20 compiler configurations.

To avoid this I moved the cast to the loop's body.
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

No branches or pull requests

2 participants