[PATCH] D130516: [llvm] compression classes

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 2 12:35:38 PDT 2022


MaskRay added a comment.

In D130516#3694366 <https://reviews.llvm.org/D130516#3694366>, @dblaikie wrote:

> In D130516#3694151 <https://reviews.llvm.org/D130516#3694151>, @MaskRay wrote:
>
>> In D130516#3688236 <https://reviews.llvm.org/D130516#3688236>, @dblaikie wrote:
>>
>>> In D130516#3688123 <https://reviews.llvm.org/D130516#3688123>, @MaskRay wrote:
>>>
>>>> I'd like to make a few arguments for the current namespace+free function design, as opposed to the class+member function design as explored in this patch (but thanks for the exploration!).
>>>> Let's discuss several use cases.
>>>>
>>>> (a) if a use case just calls compress/uncompress. The class design has slightly more boilerplate as it needs to get the algorithm class, a new instance, or a singleton instance.
>>>> For each new use, the number of lines may not differ, but the involvement of a a static class member or an instance make the reader wonder whether the object will be reused or thrown away.
>>>> There is some slight cognitive burden.
>>>> The class design has a non-trivial one-shot cost to have a function returning the singleton instance.
>>>
>>> Though there must've been a condition that dominates this use somewhere - I'd suggest that condition could be where the algorithm is retrieved, and then passed to this code to use unconditionally.
>>>
>>> If the algorithm object is const and raw pointers/references are used, I think it makes it clear to the reader that there's no ownership here, and it's not stateful when compressing/decompressing.
>>
>> A pointer to a singleton compression class is isomorphic to an `enum class CompressionType` variable.
>
> I don't mean to suggest that either design is fundamentally more or less functional - I'm totally OK with/agree that both design directions allow the implementation of all the desired final/end-user-visible functionality.
>
> I'm trying to make a point about which, I think, achieves that goal in a "better" way - that's the space of design discussions, I think - what kinds of (developer, maintenance, etc) costs different designs incur.
>
> [...]
>
> I don't understand what you're saying here. Could you rephrase/expand a bit?
>
> Maybe it's easier if I either post a patch, or at least more explicitly flesh out what I'm picturing/proposing/suggesting:

I agree that it is easier if you post a patch so that we can have discussion on concrete code. I think we are now at the "a picture is worth a thousand words"  stage:)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130516



More information about the llvm-commits mailing list