buffer: use compiler version rather than other platform ifdefs to determine library call to use to serialize doubles#29532
Conversation
…erialization mechanism Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
|
Hey Joshua, thanks for jumping on this so quick! This change does not fix the build in the case I was hitting (which is the default configuration in the I'm fairly ignorant about compilers/stdlibs, but is the GCC/clang differentiation important here, or is it just a requirement of the libraries having the correct function/template defined? As I mentioned in the other PR comment, this works with clang-14 and libstdc++-11 |
|
Thanks for the heads up. What exactly is the 'devcontainer'? Is that this: https://github.com/envoyproxy/envoy/blob/main/.devcontainer/README.md ? Are libstd++ versions supposed to match clang versions? What works best for me is to download the version of clang that matches what's used in Envoy CI, 14.06, which is found at https://github.com/llvm/llvm-project/releases/tag/llvmorg-14.0.6 . That includes a library (libclang.so.14.0.0). Why would you use a compiler of one version with a library from a different version? |
|
Yeah, that is the one. It seems pretty loosely supported, but it hasn't completely languished. I had been using vim with VERY spotty LSP for several months. After recently looking at #27680 I gave up and switched to vscode+devcontainer, which has been working pretty much flawlessly. That said though, the documentation in bazel/README.md instructs users to download llvm, run I really don't have any practical or ideological preference here, just following what documentation/issue resolution I've been able to find has led me to this sorta Frankenstein's monster of a toolchain setup that purports to be the best practice for Envoy dev. I'm happy to be instructed otherwise and help update the documentation! |
…of std::to_chars on a double Signed-off-by: Joshua Marantz <jmarantz@google.com>
Signed-off-by: Joshua Marantz <jmarantz@google.com>
|
Thanks for testing this @jbohanon -- can you see if this latest version compiles for you? |
jbohanon
left a comment
There was a problem hiding this comment.
Yes, that works for my combo
|
Thanks @jbohanon ! |
Commit Message: In #29500 I introduced a conditional-compilation for using the best available library API to serialize doubles. This is because clang library v14, the primary development platform, has a better API available than older compilers.
The conditional compilation structure I used focused on excluding GCC and Apple, and that passed on CI.
However, the ifdefing didn't quite work for people that develop with older version of clang, or hybrid library environments. I did not think people would be doing that, but they are :) So this changes the conditional compilation strategy to look specifically for clang library versions >= 14.
Additional Description: n/a
Risk Level: low
Testing: just CI
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a