[llvm] [ADT] Make Zippy more iterator-like for lifetime safety (PR #112441)

[David Blaikie via llvm-commits]

dwblaikie wrote:

(sorry, missed some details in the original description that could've avoided the extra roundtrip/questions - thanks for asking!)

> What's the impact on compilation times? If I'm reading this right, this will increase the size of Zippy to the size of the value type? For example, if I zip two vectors of `std::string`, the `value_type` will be something like `tuple<string&, string&>` ==> 16B?

That's correct.

> I think concat works around it without an extra member?

concat doesn't synthesize a new value - its value are the values of the underlying iterators/ranges - so it doesn't have this problem/need a workaround.

I've tried to address/ensure others address this issue on iterators/range adapters in ADT, but I've certainly not been the most consistent.

mapped_iterator would be an example of another range/iterator with this problem, if the mapping function returns by value (if it returns by reference, it's fine - the lifetime of the object is managed elsewhere) - doesn't look like we do anything to ensure the mapping function returns by reference...  - though at least when it does return by reference you get the good behavior, etc. And if it returns by value - it won't compose well/you'll have lifetime issues on composition.

`make_second_range` & similar seem to workaround the possibility that the underlying range might return a pair by value - returning by value if the underlying range returns by value... 

> https://github.com/llvm/llvm-project/blob/df0551298868b164197a4e54e9444120dc96ff53/llvm/include/llvm/ADT/STLExtras.h#L1068-L1089
> 
> > identifier/encountered a lifetime issue when using concat+zip, zip would return by value, concat would take references to that value and use them in its result after they had expired.
> 
> Could you share a repro? Last time I checked, `concat` also forwarded ranges to its storage, so I'm not exactly sure what the issue is.

Not so much about the storage of the ranges itself, but of the values produced by the iterators.

I'll see about an example. I /think/ this is sufficient: https://godbolt.org/z/aK8drvGvY - (godbolt can't link to the LLVM libraries, it only has headers, so I can't create a runnable example) - I expect the place where this was encountered as an asan failure was where LLVM was installed as a system library, in which case the permissive error would've been silenced & the bug would've compiled without any compiler diagnostics.



https://github.com/llvm/llvm-project/pull/112441