Add warning for returning without initializing out parameter (#2807)

* Add warning for returning without initializing out parameter

* Add unused prelude function to squash uninitialized out variable warnings

7 files changed, 97 insertions, 11 deletions

diff --git a/source/slang/core.meta.slang b/source/slang/core.meta.slang
index 849e4c642..5265d6cb6 100644
--- a/source/slang/core.meta.slang
+++ b/source/slang/core.meta.slang
@@ -2649,6 +2649,15 @@ __generic<T, U>
 __intrinsic_op($(kIROp_Reinterpret))
 T reinterpret(U value);
 
+// Use an otherwise unused value
+//
+// This can be used to silence the warning about returning before initializing
+// an out paramter.
+__generic<T>
+[__readNone]
+[ForceInline]
+void unused(inout T){}
+
 // Specialized function
 
 /// Given a string returns an integer hash of that string.
diff --git a/source/slang/slang-diagnostic-defs.h b/source/slang/slang-diagnostic-defs.h
index 130102e44..ec8131824 100644
--- a/source/slang/slang-diagnostic-defs.h
+++ b/source/slang/slang-diagnostic-defs.h
@@ -595,7 +595,9 @@ DIAGNOSTIC(40020, Error, cannotUnrollLoop, "loop does not terminate within the l
 DIAGNOSTIC(41000, Warning, unreachableCode, "unreachable code detected")
 
 DIAGNOSTIC(41010, Warning, missingReturn, "control flow may reach end of non-'void' function")
-DIAGNOSTIC(41015, Error, usingUninitializedValue, "use of uninitialized value.")
+DIAGNOSTIC(41015, Error, usingUninitializedValue, "use of uninitialized value '$0'")
+DIAGNOSTIC(41016, Warning, returningWithUninitializedOut, "returning without initializing out parameter '$0'")
+DIAGNOSTIC(41017, Warning, returningWithPartiallyUninitializedOut, "returning without fully initializing out parameter '$0'")
 
 DIAGNOSTIC(41011, Error, typeDoesNotFitAnyValueSize, "type '$0' does not fit in the size required by its conforming interface.")
 DIAGNOSTIC(41012, Note, typeAndLimit, "sizeof($0) is $1, limit is $2")
diff --git a/source/slang/slang-ir-use-uninitialized-out-param.cpp b/source/slang/slang-ir-use-uninitialized-out-param.cpp
index 9818cec53..64f5f8bd3 100644
--- a/source/slang/slang-ir-use-uninitialized-out-param.cpp
+++ b/source/slang/slang-ir-use-uninitialized-out-param.cpp
@@ -75,27 +75,33 @@ namespace Slang
                 }
             }
             // Check all address loads.
-            List<IRLoad*> loads;
+            List<IRInst*> loadsAndReturns;
             for (auto addr : addresses)
             {
                 for (auto use = addr->firstUse; use; use = use->nextUse)
                 {
                     if (auto load = as<IRLoad>(use->getUser()))
-                        loads.add(load);
+                        loadsAndReturns.add(load);
                 }
             }
+            for(const auto& b : func->getBlocks())
+            {
+                auto t = b->getTerminator();
+                if (t->m_op == kIROp_Return)
+                    loadsAndReturns.add(t);
+            }
 
             for (auto store : stores)
             {
                 // Remove insts from `loads` that is reachable from the store.
-                for (Index i = 0; i < loads.getCount();)
+                for (Index i = 0; i < loadsAndReturns.getCount();)
                 {
-                    auto load = loads[i];
-                    if (!canAddressesPotentiallyAlias(func, store.address, loads[i]->getPtr()))
+                    auto load = as<IRLoad>(loadsAndReturns[i]);
+                    if (load && !canAddressesPotentiallyAlias(func, store.address, load->getPtr()))
                         continue;
-                    if (reachability.isInstReachable(store.storeInst, load))
+                    if (reachability.isInstReachable(store.storeInst, loadsAndReturns[i]))
                     {
-                        loads.fastRemoveAt(i);
+                        loadsAndReturns.fastRemoveAt(i);
                     }
                     else
                     {
@@ -104,9 +110,14 @@ namespace Slang
                 }
             }
             // If there are any loads left, it means they are using uninitialized out params.
-            for (auto load : loads)
+            for (auto load : loadsAndReturns)
             {
-                sink->diagnose(load, Diagnostics::usingUninitializedValue);
+                sink->diagnose(
+                    load,
+                    load->m_op == kIROp_Return
+                        ? Diagnostics::returningWithUninitializedOut
+                        : Diagnostics::usingUninitializedValue,
+                    param);
             }
         }
     }
diff --git a/tests/autodiff/bsdf/bsdf-auto-rev.slang b/tests/autodiff/bsdf/bsdf-auto-rev.slang
index 7fae5e993..cd31f2097 100644
--- a/tests/autodiff/bsdf/bsdf-auto-rev.slang
+++ b/tests/autodiff/bsdf/bsdf-auto-rev.slang
@@ -39,6 +39,7 @@ bool bsdfGGXSample(in ShadingData sd, in Auto_Bwd_BSDFParameters params, out Aut
 
     if (wiLocal.z < 1e-6)
     {
+        unused(result);
         return false;
     }
 
diff --git a/tests/compute/ray-tracing-inline.slang b/tests/compute/ray-tracing-inline.slang
index 7699cf9eb..a8ef9683e 100644
--- a/tests/compute/ray-tracing-inline.slang
+++ b/tests/compute/ray-tracing-inline.slang
@@ -33,6 +33,7 @@ bool traceRayClosestHit(
         //primitiveIndex = q.CommittedPrimitiveIndex();
         return true;
     }
+    unused(t);
     return false;
 }
 
diff --git a/tests/diagnostics/uninitialized-out.slang b/tests/diagnostics/uninitialized-out.slang
index c285970da..d0d87449f 100644
--- a/tests/diagnostics/uninitialized-out.slang
+++ b/tests/diagnostics/uninitialized-out.slang
@@ -2,6 +2,56 @@
 
 float foo(out float3 v)
 {
+    // This should error as we haven't set v before we read from it
     float r = v.x + 1.0;
+    // This should warn as we haven't set v before we return
     return r;
 }
+
+// This should warn as we return without x being initialized
+float bar(out float x)
+{
+    return 0;
+}
+
+// This should also warn pointing at the implicit return
+void baz(out float x) {}
+
+void twoReturns(bool b, out float y)
+{
+    if(b)
+    {
+        // Should warn
+        return;
+    }
+    y = 0;
+    // Shouldn't warn
+    return;
+}
+
+void twoOkReturns(bool b, out float y)
+{
+    if(b)
+    {
+        // Shouldn't warn
+        unused(y);
+        return;
+    }
+    y = 0;
+    // Shouldn't warn
+    return;
+}
+
+// TODO: This should warn that n is potentially uninitialized
+int ok(bool b, out int n)
+{
+    if(b)
+        n = 0;
+    return n;
+}
+
+// TODO: This should warn that arr isn't fully initialized
+void partial(out float arr[2])
+{
+    arr[0] = 1;
+}
diff --git a/tests/diagnostics/uninitialized-out.slang.expected b/tests/diagnostics/uninitialized-out.slang.expected
index feb8c494a..5846c12df 100644
--- a/tests/diagnostics/uninitialized-out.slang.expected
+++ b/tests/diagnostics/uninitialized-out.slang.expected
@@ -1,8 +1,20 @@
 result code = -1
 standard error = {
-tests/diagnostics/uninitialized-out.slang(5): error 41015: use of uninitialized value.
+tests/diagnostics/uninitialized-out.slang(6): error 41015: use of uninitialized value 'v'
     float r = v.x + 1.0;
                 ^
+tests/diagnostics/uninitialized-out.slang(8): warning 41016: returning without initializing out parameter 'v'
+    return r;
+    ^~~~~~
+tests/diagnostics/uninitialized-out.slang(14): warning 41016: returning without initializing out parameter 'x'
+    return 0;
+    ^~~~~~
+tests/diagnostics/uninitialized-out.slang(18): warning 41016: returning without initializing out parameter 'x'
+void baz(out float x) {}
+                       ^
+tests/diagnostics/uninitialized-out.slang(25): warning 41016: returning without initializing out parameter 'y'
+        return;
+        ^~~~~~
 }
 standard output = {
 }