ymandel added a comment.

In D121158#3365494 <https://reviews.llvm.org/D121158#3365494>, @ymandel wrote:

> In D121158#3365415 <https://reviews.llvm.org/D121158#3365415>, @xazax.hun 
> wrote:
>
>>> When pre-initializing fields in the environment, the code assumed that all 
>>> fields of a struct would be initialized
>>
>> Was this assumption ever correct given that it was already skipping the 
>> initialization of recursive cases?
>
> Yeah, I had that thought of that and I'm pretty sure it wasn't.  But that 
> does provide a way to test this without creating a very large struct. Let me 
> see if I can add a test to this patch. Otherwise, I'll write a followup.

I added a test for the recursive case and tested (a variant of) it on the code 
before this patch to verify that it triggered a failure. Interestingly, it 
triggers a different failure mode in `getChild` -- the assertion that the field 
will be present. The guard on the recursive case prevents the field from being 
added at all, so you don't have a nullptr added to the map. But, you also don't 
have the field added, so the (old) assert would fail. The patch addresses this 
by allowing for lack of field presence.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121158

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

Reply via email to