[PATCH] D68656: Add ExceptionStream to llvm::Object::minidump

[Joseph Tremoulet via Phabricator via llvm-commits]

JosephTremoulet added a comment.

In D68656#1701658 <https://reviews.llvm.org/D68656#1701658>, @labath wrote:

> Thanks for taking your time to do this. I have one question: It looks like you're not using the exception code enum in the follow-up patch. I think that's completely reasonable given that the enum values are overloaded and system-dependent. But given this fact, and the fact that I am not convinced the enum values are completely right (e.g. the linux signal numbers depend also on the architecture -- though this may not manifest itself on the architectures that breakpad supports right now), what would you say to just dropping that enumeration?


Yeah, I went back and forth myself on whether to include those.  The thing is that I need to replace the definition of `MinidumpException::DumpRequested` with something in these types.  When I did some research, I found that the breakpad constant in question is actually Linux-specific, and that there are similar constants defined for Mac and Windows.  So I went the route of pulling in the big list of constants for each OS group.  It's easy to miss, but in D68658 <https://reviews.llvm.org/D68658> I actually do reference `Linux_DumpRequested` in code and in a comment make a reference `Mac_Simulated` and `Windows_Simulated` though not by name.

Perhaps a better approach would be to define just those three, with a comment about how the field gets used more generally and where external constants come from?  I'll push an update that goes that route, please let me know what you think.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68656/new/

https://reviews.llvm.org/D68656