mboehme added a comment. Just some top-level comments to begin with.
In D153469#4439770 <https://reviews.llvm.org/D153469#4439770>, @sammccall wrote: > Here we start to see benefits: Value becomes less reliant on inheritance, > flow conditions are no longer Values that can uselessly bear properties, > atomic variables are numbered in order of creation, we'll soon be able to > have distinct BoolValues with the same formula (but different properties). I like where this is going. Specifically, I like that we will soon no longer be relying in `BoolValue` object identity and will be able to have different `BoolValue`s with the same formula. I'm working on a similar change for `StructValue`, so I'm well aware of how problematic it is to share the same `Value` but then modify properties on it -- it seems like a potential source of really puzzling bugs. > I think I could separate the Value/Arena changes from the ones to use > Atom/Formula in DAContext if you think it's useful (the drawback is some risk > of ending up in merge hell...) I don't think that's necessary -- this patch is large in terms of numbers of lines changed, but when you eliminate the mechanical changes, there's not too much going on. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153469/new/ https://reviews.llvm.org/D153469 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
