davide added inline comments.
================ Comment at: lldb/source/Symbol/Type.cpp:378 m_byte_size_has_value = true; + return m_byte_size; } ---------------- shafik wrote: > labath wrote: > > davide wrote: > > > shafik wrote: > > > > Wouldn't it be better to turn `m_byte_size` into an `Optional`? As this > > > > fix shows this having this additional state we carry around is error > > > > prone. > > > This is a much larger project, probably worth considering. > > > With that in mind, it wouldn't have prevented this bug from happening, as > > > this falls through and returns an `Optional<T>` anyway. > > > Yay fuzz testing (that found this), I would say. > > IIRC, this was done this way to reduce sizeof(Type). Note that the external > > interface is Optional<uint64_t> and the only place that knows (should > > know?) about this encoding is this function. > It would have prevented this bug as you just return `m_byte_size` no matter > what. If you think it's an improvement, please consider submitting a review, I would be eager to look at it! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86436/new/ https://reviews.llvm.org/D86436 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits