void added a comment.

In D121063#3364815 <https://reviews.llvm.org/D121063#3364815>, @erichkeane 
wrote:

> In D121063#3364810 <https://reviews.llvm.org/D121063#3364810>, @void wrote:
>
>> In D121063#3364780 <https://reviews.llvm.org/D121063#3364780>, @void wrote:
>>
>>> In D121063#3363852 <https://reviews.llvm.org/D121063#3363852>, @erichkeane 
>>> wrote:
>>>
>>>> I suspect this works because we never really treated this as a LL, just as 
>>>> a pair of iterators.  Two things:
>>>>
>>>> 1- Can you produce some situation where this is valuable to do?
>>>
>>> Yes. In the randstruct feature I'm working on, if this code isn't there it 
>>> goes into an infinite loop: https://reviews.llvm.org/D120857. It's possible 
>>> that the way it's constructing and using the list of Decls is somehow 
>>> wrong, but I wasn't able to identify how.
>>>
>>>> 2- Can you switch this over so that the NextInContextAndBits initializes 
>>>> to nullptr/0 so that this line isn't necessary?  I can't imagine we ever 
>>>> re-call this on a decl and have the answer be different.
>>>
>>> I'll give it a shot. I'm with @urnathan though that it should have already 
>>> been like that. :-) Probably just an oversight.
>>
>> After looking at the randstruct code again, it's possible that it's not 
>> doing the correct thing. (#include "MildShockMeme.h"!) The Decls already 
>> have a their pointers set, but then we shuffle them and all Hell breaks 
>> loose when we call that function because the end is pointing back to 
>> somewhere within the structure beginning. This patch is probably not a bad 
>> idea in general, but if you want me to I can fix it on my end.
>
> Ah!  Shuffling declarations in a chain is not likely to do 'good things', I'm 
> surprised that this is the first issue we've seen!.

Same :-)

> I presume that there needs to be a 'RebuildDeclChain' for the purposes of 
> 'RandStruct' that first nulls-out the next-in-context-and-bits.

Without this patch, then yes. It might actually be a better idea to have it in 
the DeclContext anyway just in case someone else wants to use it.

> Alternatively, perhaps "RandStruct" should be 'randomizing' on the first call 
> to this BuildDeclChain function.

I assume that `BuildDeclChain` is called once (and only once) per Record? Will 
randomizing when calling `BuildDeclChain` mess up the ABI somehow? Or is it 
safer to do it here because the ABI is decided afterwards?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121063

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to