Replace /* unhandled */ in source emit with a real error (shader-slang#1313)

For a long time the various source-to-source back-ends have been emitted the text `/* unhandled */` when they encounter an IR instruction opcode that didn't have any emit logic implemented. This choice had two apparent benefits:

* In most common cases, emitting `/* unhandled */` in place of an expression would lead to downstream compilation failure, so most catastrophic cases seemed to work as desired (e.g., if we emit `int i = /* unhandled */;` we get a downstream parse error and we know something is wrong.

* In a few cases, if a dead-but-harmless instruction slips through (e.g., a type formed in the body of a specialized generic function), we would emit `/* unhandled */;`, which is a valid empty statement.

It is already clear from the above write-up that the benefits of the policy aren't really that compelling, and where it has recently turned out to be a big liability is that there are actually plenty of cases where emitting `/* unhandled */` instead of a sub-expression won't cause downstream compilation failure, and will instead silently compute incorrect results:

* Emitting `/* unhandled */ + b` instead of `a + b`

* Emitting `/* unhandled */(a)` instead of `f(a)`, or even `/* unhandled */(a, b)` instead of `f(a,b)`

* Emitting `f(/*unhandled*/)` instead of `f(a)` in cases where `f` is a built-in with both zero-argument and one-argument overloads

The right fix here is simple: where we would have emitted `/* unhandled */` to the output we should instead diagnose an internal compiler error, thus leading to compilation failure.

This change appears to pass all our current tests, but it is possible that there are going to be complicated cases in user code that were relying on the previous lax behavior. I know from experience that we sometimes see `/* unhandled */` in output for generics, and while we have eliminated many of those cases I don't have confidence we've dealt with them all.

When this change lands we should make sure that the first release that incorporates it is marked as potentially breaking for clients, and we should make sure to test the changes in the context of the client codebases before those codebases integrate the new release.

Author: Tim Foley

source/slang/slang-emit-c-like.cpp

@@ -1883,6 +1883,11 @@ void CLikeSourceEmitter::emitInstExpr(IRInst* inst, const EmitOpInfo& inOuterPre
     defaultEmitInstExpr(inst, inOuterPrec);
 }
 
+void CLikeSourceEmitter::diagnoseUnhandledInst(IRInst* inst)
+{
+    getSink()->diagnose(inst, Diagnostics::unimplemented, "unexpected IR opcode during code emit");
+}
+
 void CLikeSourceEmitter::defaultEmitInstExpr(IRInst* inst, const EmitOpInfo& inOuterPrec)
 {
     EmitOpInfo outerPrec = inOuterPrec;
@@ -2195,7 +2200,7 @@ void CLikeSourceEmitter::defaultEmitInstExpr(IRInst* inst, const EmitOpInfo& inO
         break;
 
     default:
-        m_writer->emit("/* unhandled */");
+        diagnoseUnhandledInst(inst);
         break;
     }
     maybeCloseParens(needClose);

source/slang/slang-emit-c-like.h

@@ -196,6 +196,7 @@ class CLikeSourceEmitter: public RefObject
 
     void emitInstExpr(IRInst* inst, EmitOpInfo const& inOuterPrec);
     void defaultEmitInstExpr(IRInst* inst, EmitOpInfo const& inOuterPrec);
+    void diagnoseUnhandledInst(IRInst* inst);
 
     BaseType extractBaseType(IRType* inType);
 

source/slang/slang-emit-glsl.cpp

@@ -1128,7 +1128,7 @@ bool GLSLSourceEmitter::tryEmitInstExprImpl(IRInst* inst, const EmitOpInfo& inOu
             switch (toType)
             {
                 default:
-                    m_writer->emit("/* unhandled */");
+                    diagnoseUnhandledInst(inst);
                     break;
 
                 case BaseType::UInt:

source/slang/slang-emit-hlsl.cpp

@@ -439,7 +439,7 @@ bool HLSLSourceEmitter::tryEmitInstExprImpl(IRInst* inst, const EmitOpInfo& inOu
             switch (toType)
             {
                 default:
-                    m_writer->emit("/* unhandled */");
+                    diagnoseUnhandledInst(inst);
                     break;
                 case BaseType::UInt:
                     break;