Add CMake option DXC_CODEGEN_EXCEPTIONS_TRAP #6764
Conversation
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
I really don't like this change. It's an unfortunate hack around a problem rather than fixing it. That said, the problem isn't easy to fix and I can't really come up with a better way to address it that isn't insanely massive.
I wish we could just make exceptions an optional configuration, but that would be a huge change and even making our tests pass with exceptions replaced by fatal errors is a huge effort.
Exceptions are evil.
This code all makes me sad.
I refuse to say it "looks good" to me.
I will say it is an acceptable (although unfortunate) compromise.
One small change requested to use llvm::errs() instead of fprintf.
|
+1 I wish the exception-catching had not been inserted. After posting I realized this patch has a big downside: we don't get a stack trace. Which means bug report deduplication will get too agressive and we'll likely miss things. I converted to draft for now. |
dneto0
changed the title
When enabled, any hlsl::Exception thrown during code generation and optimization will cause the process to trap. When used in Chrome, the trap will trigger a meaningful crash report.
When enabled, any hlsl::Exception thrown during code generation and optimization will cause the process to trap.
edit: I've changed the implementation to generate a trap, instead of std::abort