[PATCH] D138037: [analyzer] Remove unjustified assertion from EQClass::simplify

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a subscriber: vsavchenko.
steakhal added a comment.

In D138037#3941632 <https://reviews.llvm.org/D138037#3941632>, @xazax.hun wrote:

> I did not spend too much time thinking about this yet, but this sounds scary. I wonder if we should target the underlying problem instead, i.e., making sure we never have dead symbols as representatives for eq. classes. What do you think?

Yes, indeed scary. The answer is a bit complicated. I think a similar concern came up here, and Valeriy  summed up pretty well.

In D106136#2883707 <https://reviews.llvm.org/D106136#2883707>, @vsavchenko wrote:

> Representative symbol gives its equivalence class an ID.  We use this ID for everything, for comparing and for mapping.  Since we live in a persistent world, we can't just change this ID at some point, it will basically mean that we want to replace a class with another class.  So, all maps where this class participated (constraints, class, members, disequality) should remove the old class and add the new class.  This is a huge burden.  You need to carefully do all this, and bloat existing data structures.
>
> Trivial class is an optimization for the vast majority of symbols that never get into any classes.  We still need to associate constraints with those.  This is where implicit Symbol to Class conversion comes in handy.
>
> Now let's imagine that something else is merged into it.  We are obliged to officially map the symbol to its class, and the class to its members.  When this happens, it goes into the data structure FOREVER.  And when in the future, with only one member, instead of keeping something that we already have from other versions of the data structure, you decide to remove the item from the class map, you actually use more memory when it was there!  This is how persistent data structures work, different versions of the same data structure share data.  And I'm not even talking here about the fact that you now need to replace the class with one ID with the class with another ID in every relationship.
>
> You can probably measure memory footprint in your example and see it for yourself.

However, we will never know for sure if these allegations are true unless we implement and measure it, but I'm not volunteering.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138037