[llvm-dev] Unionizing a struct

[Craig Topper via llvm-dev]

I don't think PointerUnion works with std::unique_ptr. I think you'll need
to use PointerUnion<Record *, ForeachLoop *, Record::AssertionTuple *> and
call the correct delete from the RecordsEntry destructor based on which
type is active.

~Craig

On Thu, Apr 15, 2021 at 11:48 AM Paul C. Anagnostopoulos <
paul at windfall-software.com> wrote:

> C++ seems willing to compile a PointerUnion of three unique_ptr's. Does
> that make sense, or does it have to be unique_ptr * ?
>
> There is only one use of unique_ptr with PointerUnion in the system, and
> it uses unique_ptr *.
> On 4/15/2021 1:47 PM, Craig Topper wrote:
>
> Depending on the alignment requirement of the 3 types involved, you might
> be able to use PointerUnion to store the discrimination enum in the lower 2
> bits of the pointer.
>
> ~Craig
>
>
> On Thu, Apr 15, 2021 at 10:33 AM Paul C. Anagnostopoulos via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> In TableGen, there is the following struct. In any given instance, only
>> one of the pointers is filled in.
>>
>>   struct RecordsEntry {
>>     std::unique_ptr<Record> Rec;
>>     std::unique_ptr<ForeachLoop> Loop;
>>     std::unique_ptr<Record::AssertionTuple> Assertion;
>>
>>     void dump() const;
>>
>>     RecordsEntry() {}
>>     RecordsEntry(std::unique_ptr<Record> Rec) : Rec(std::move(Rec)) {}
>>     RecordsEntry(std::unique_ptr<ForeachLoop> Loop)
>>       : Loop(std::move(Loop)) {}
>>     RecordsEntry(std::unique_ptr<Record::AssertionTuple> Assertion)
>>       : Assertion(std::move(Assertion)) {}
>>   };
>>
>> It has been suggested that the three pointers be made a union. This
>> requires a discrimination enum. I believe that would reduce the struct from
>> 24 to 12 bytes, and I think it requires a custom destructor.
>>
>> What do people think about this? If I should do it, could someone help me
>> with the changes?
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210415/62aca151/attachment.html>