labath added a comment. Thanks. I think this is great. @grimar, do you have any more comments?
================ Comment at: llvm/lib/ObjectYAML/MinidumpYAML.cpp:377-383 + // TODO: We could provide a reasonable default for ThreadContext by searching + // the Thread stream for a thread with the given ID and using its Context. + // That would require a couple changes: + // 1. We'd need to pass the whole dump around as a context argument. + // 2. We'd need to ensure that the Thread stream got processed before + // the Exception stream (or make Exception's ThreadContext required + // when the Exception stream is processed before the Thread stream). ---------------- JosephTremoulet wrote: > labath wrote: > > I've been thinking about this for a while now, and while that idea has some > > appeal, I am not sure if this would ever really be a "good" idea. > > Currently, this format allows you to easily create > > syntactically-valid-but-probably-nonsensical minidumps (multiple thread > > list streams, multiple threads with the same ID, etc..). All of that would > > be more difficult if we started depending on strict "correctness" of other > > parts of the minidump in order to compute something here. > > > > Even if I was doing this, I'd probably implement this differently -- make > > the context always optional, but then check for consistency higher up (the > > validate call of the entire minidump object or something). Anyway, maybe > > just delete this todo? :) > I've removed the TODO, but I reserve the right to think this would be a good > idea :). Speficially because I think you could still model in YAML anything > you could put in a minidump file, by explicitly providing the ThreadContext > even when it has a default. > > (This is in contrast to the other validation stuff I had in earlier > revisions, where I was just misunderstanding the point of yaml validation -- > so thanks for explaining it!) Fair enough. :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68657/new/ https://reviews.llvm.org/D68657 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
