sammccall marked 3 inline comments as done. sammccall added inline comments.
================ Comment at: clang/include/clang/Analysis/FlowSensitive/Arena.h:61 + /// Passing in the same formula will result in the same BoolValue. + /// FIXME: This canonicalization isn't sound e.g. if we create two BoolValues + /// with the same formula but attach different properties. ---------------- gribozavr2 wrote: > Per our offline conversation, do you still believe in this fixme? No, I'm no longer convinced "unsound" is true. However, I do think there's something to be fixed: if we really believe in it, we should be interning other types of values too (or have a clear idea about why bools are special). And I think I still lean towards removing it: it looks like a power-vs-complexity tradeoff in the Value concept, and I suspect the historical reason to intern values was to intern formulas, not because we needed the power. Reworded the fixme to say we should decide and be more consistent - text doesn't take a position on which way we should decide. 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
