ymandel added inline comments.
================ Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:1551-1552 + const Value *FooVal = Env.getValue(*FooLoc); + // TODO: Initialise values inside unions, then change below to + // ASSERT_TRUE. + ASSERT_FALSE(isa_and_nonnull<IntegerValue>(FooVal)); ---------------- merrymeerkat wrote: > ymandel wrote: > > Why push this off to another patch? > good point! I was pushing it because this is just a quick fix to avoid a > crash, and the current changes are sufficient for that SGTM. Please use FIXME instead of TODO, though. But, glad to see this is enough to avoid the crashing! That said, can you expand on where the actual crash was happening? I'm concerned that its possible that the crash site should be more robust and that this patch is hiding that weakness. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140696/new/ https://reviews.llvm.org/D140696 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
