https://github.com/NagyDonat approved this pull request.

Looks good to me, I'm satisfied with the logic introduced by this commit.

For extent symbols, this "if the region is alive, keep its extent alive" seems 
to be the logically correct behavior: this preserves the extent while it's 
relevant and ensures that the extent is garbage-collected after that point 
(assuming that the region itself is garbage-collected properly).

-------

> I wish you could write up what do we currently have and options to fix this 
> for extents (and possibly for any other symbols for that matter because 
> extents are not alone in this issue).

@steakhal What do you mean by this?

The old discussion at 
https://discourse.llvm.org/t/keeping-the-extent-symbol-alive/76676/3 seems to 
be a fairly complete write-up of the current situation (which hadn't changed 
since then) and the options for fixing the lifetime issues _of extent symbols_.

I think that lifetime issues of _other symbols_ are mostly irrelevant for this 
PR: we shouldn't postpone this concrete and clear solution to wait for vague 
hypothetical improvements in partially related areas.

Also, I strongly suspect that even a highly advanced symbol lifetime system 
will need the concrete logic for extent symbols (which is added here), so this 
commit will be a good step toward the complete solution (even if we don't see 
the complete solution yet).

https://github.com/llvm/llvm-project/pull/163562
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to