[PATCH] D87673: [clangd] Don't use zlib when it's unavailable.

David Blaikie via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 23 12:45:50 PDT 2020


dblaikie added a comment.

In D87673#2290926 <https://reviews.llvm.org/D87673#2290926>, @sammccall wrote:

> In D87673#2290838 <https://reviews.llvm.org/D87673#2290838>, @dblaikie wrote:
>
>> In D87673#2275940 <https://reviews.llvm.org/D87673#2275940>, @sammccall wrote:
>>
>>> Thanks, this seems better than crashing.
>>> The practical result isn't wonderful: the two are going to fight over index files to some extent. But this can happen with different clangd versions (that use different index format versions) anyway. So this is really just graceful recovery from a bad situation.
>>>
>>> In D87673#2273347 <https://reviews.llvm.org/D87673#2273347>, @ArcsinX wrote:
>>>
>>>> Unsure about test for this
>>>
>>> Agree, testing this seems hard and not that useful.
>>
>> FWIW here's how to test codepaths only reachable when zlib is not available:
>>
>> ./llvm/test/tools/llvm-dwp/X86/nocompress.test:
>>
>>   REQUIRES: !zlib
>
> Yep, that requires a checked-in binary file, which in the case of that test is in a standard stable format, which this data is not, so we'd also need tools to regenerate it. And we don't have APIs to generate the file without compression if zlib is available (because we never want to do that), so we'd need to add those or require anyone touching the file format to build a no-zlib tree to regenerate the file.
> None of this is impossible, but this is pretty unlikely to catch a bug, and it's very unlikely to be a high-impact bug..

Ah, fair enough. Might be nice to have a flag to request the uncompressed behavior even when compression's available - for testing purposes and such, but given the current implementation, yeah, don't see a tidy way to test it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87673/new/

https://reviews.llvm.org/D87673



More information about the cfe-commits mailing list