[PATCH] D24131: Add NewAddressDescription, which can describe any type of address.

Filipe Cabecinhas via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 9 07:25:06 PDT 2016


My problem is the extra copies by doing it this way (in a VS2013
supported way), but since you seem to feel strongly about it, I'll
change it to be more like your patch.

Will refresh soon. Thank you,

 Filipe

On Thu, Sep 8, 2016 at 9:42 PM, Vitaly Buka <vitalybuka at google.com> wrote:
> I am not quite understand. Existing code work with VS2013. Could please show
> some examples of the code which is needed, work with your version and do not
> work with mine.
> BTW. Also copying should not affect performance as operations on
> AddressDescription should be quite rare.
>
> On Thu, Sep 8, 2016 at 11:59 AM Filipe Cabecinhas
> <filcab+llvm.phabricator at gmail.com> wrote:
>>
>> If I could stop caring about VS2013, that would be good enough. My problem
>> is that having it this way will force us to copy the data whenever we want
>> to use it from a structure that need to be put in a union (because of
>> VS2013), since you can only put a *Data in there, not the full
>> AddressDescription.
>>
>> Thank you,
>>  Filipe
>>
>>
>> On Thursday, 8 September 2016, Vitaly Buka <vitalybuka at google.com> wrote:
>>>
>>> vitalybuka requested changes to this revision.
>>> This revision now requires changes to proceed.
>>>
>>> ================
>>> Comment at: lib/asan/asan_descriptions.h:185
>>> @@ +184,3 @@
>>> +  // We should be able to do without this struct after we deprecate
>>> VS2013.
>>> +  AddressKind kind;
>>> +  union {
>>> ----------------
>>> I like this way:  https://reviews.llvm.org/D24358
>>> WDYT?
>>>
>>>
>>> https://reviews.llvm.org/D24131
>>>
>>>
>>>
>


More information about the llvm-commits mailing list