Warnings for uninitialized fields in constructors (#4680)

* Detect uninitialized fields in constructors

* Reachability check for early returns

* Specialized warnings for synthesized default initializers

* Handling quirks with constructors

* Addressing review comments

* Ignore synthesized constructors if they are not used

10 files changed, 278 insertions, 5 deletions

diff --git a/source/slang/slang-ast-decl.h b/source/slang/slang-ast-decl.h
index 8f12a0f61..af03ffbcb 100644
--- a/source/slang/slang-ast-decl.h
+++ b/source/slang/slang-ast-decl.h
@@ -371,6 +371,10 @@ class FunctionDeclBase : public CallableDecl
 class ConstructorDecl : public FunctionDeclBase
 {
     SLANG_AST_CLASS(ConstructorDecl)
+   
+    // Indicates whether the declaration was synthesized by
+    // slang and not actually provided by the user
+    bool isSynthesized = false;
 };
 
 // A subscript operation used to index instances of a type
diff --git a/source/slang/slang-check-decl.cpp b/source/slang/slang-check-decl.cpp
index 99aecd37c..59d8ba8b3 100644
--- a/source/slang/slang-check-decl.cpp
+++ b/source/slang/slang-check-decl.cpp
@@ -1813,6 +1813,7 @@ namespace Slang
         body->closingSourceLoc = ctor->closingSourceLoc;
         ctor->body = body;
         body->body = m_astBuilder->create<SeqStmt>();
+        ctor->isSynthesized = true;
         decl->addMember(ctor);
         return ctor;
     }
diff --git a/source/slang/slang-diagnostic-defs.h b/source/slang/slang-diagnostic-defs.h
index be15daae9..78e37821d 100644
--- a/source/slang/slang-diagnostic-defs.h
+++ b/source/slang/slang-diagnostic-defs.h
@@ -744,6 +744,8 @@ DIAGNOSTIC(41016, Warning, usingUninitializedVariable, "use of uninitialized var
 DIAGNOSTIC(41017, Warning, usingUninitializedGlobalVariable, "use of uninitialized global variable '$0'")
 DIAGNOSTIC(41018, Warning, returningWithUninitializedOut, "returning without initializing out parameter '$0'")
 DIAGNOSTIC(41019, Warning, returningWithPartiallyUninitializedOut, "returning without fully initializing out parameter '$0'")
+DIAGNOSTIC(41020, Warning, constructorUninitializedField, "exiting constructor without initializing field '$0'")
+DIAGNOSTIC(41021, Warning, fieldNotDefaultInitialized, "default initializer for '$0' will not initialize field '$1'")
 
 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-inst-defs.h b/source/slang/slang-ir-inst-defs.h
index 987486eae..2b5f35736 100644
--- a/source/slang/slang-ir-inst-defs.h
+++ b/source/slang/slang-ir-inst-defs.h
@@ -924,6 +924,7 @@ INST_RANGE(BindingQuery, GetRegisterIndex, GetRegisterSpace)
     INST_RANGE(StageAccessDecoration, StageReadAccessDecoration, StageWriteAccessDecoration)
 
     INST(SemanticDecoration, semantic, 2, 0)
+    INST(ConstructorDecoration, constructor, 1, 0)
     INST(PackOffsetDecoration, packoffset, 2, 0)
 
         // Reflection metadata for a shader parameter that provides the original type name.
diff --git a/source/slang/slang-ir-insts.h b/source/slang/slang-ir-insts.h
index f0fd38061..a5493d6ea 100644
--- a/source/slang/slang-ir-insts.h
+++ b/source/slang/slang-ir-insts.h
@@ -1358,6 +1358,13 @@ struct IRSemanticDecoration : public IRDecoration
     int getSemanticIndex() { return int(getIntVal(getSemanticIndexOperand())); }
 };
 
+struct IRConstructorDecorartion : IRDecoration
+{
+    IR_LEAF_ISA(ConstructorDecoration)
+
+    bool getSynthesizedStatus() { return cast<IRBoolLit>(getOperand(0))->getValue(); }
+};
+
 struct IRPackOffsetDecoration : IRDecoration
 {
     enum
@@ -4524,6 +4531,11 @@ public:
         return as<IRSemanticDecoration>(addDecoration(value, kIROp_SemanticDecoration, getStringValue(text), getIntValue(getIntType(), index)));
     }
 
+    void addConstructorDecoration(IRInst* value, bool synthesizedConstructor)
+    {
+        addDecoration(value, kIROp_ConstructorDecoration, getBoolValue(synthesizedConstructor));
+    }
+
     void addRequireSPIRVDescriptorIndexingExtensionDecoration(IRInst* value)
     {
         addDecoration(value, kIROp_RequireSPIRVDescriptorIndexingExtensionDecoration);
diff --git a/source/slang/slang-ir-use-uninitialized-values.cpp b/source/slang/slang-ir-use-uninitialized-values.cpp
index 1be43591a..e92eb7d7a 100644
--- a/source/slang/slang-ir-use-uninitialized-values.cpp
+++ b/source/slang/slang-ir-use-uninitialized-values.cpp
@@ -348,8 +348,140 @@ namespace Slang
         return loads;
     }
 
+    static bool isInstStoredInto(ReachabilityContext& reachability, IRInst* reference, IRInst* inst)
+    {
+        List<IRInst*> stores;
+        List<IRInst*> loads;
+
+        for (auto alias : getAliasableInstructions(inst))
+        {
+            for (auto use = alias->firstUse; use; use = use->nextUse)
+            {
+                IRInst* user = use->getUser();
+                collectLoadStore(stores, loads, user, alias);
+            }
+        }
+
+        for (auto store : stores)
+        {
+            if (reachability.isInstReachable(store, reference))
+                return true;
+        }
+
+        return false;
+    }
+
+    static IRInst* traceInstOrigin(IRInst* inst)
+    {
+        if (auto load = as<IRLoad>(inst))
+            return traceInstOrigin(load->getPtr());
+
+        return inst;
+    }
+
+    static bool isReturnedValue(IRInst* inst)
+    {
+        for (auto use = inst->firstUse; use; use = use->nextUse)
+        {
+            IRInst* user = use->getUser();
+            if (as<IRReturn>(user))
+                return true;
+        }
+        return false;
+    }
+
+    static List<IRStructField*> checkFieldsFromExit(ReachabilityContext& reachability, IRReturn* ret, IRStructType* type)
+    {
+        IRInst* origin = traceInstOrigin(ret->getVal());
+
+        // We don't want to warn on delegated construction
+        if (!isUninitializedValue(origin))
+            return {};
+
+        // Now we can look for all references to fields
+        HashSet<IRStructKey*> usedKeys;
+        for (auto use = origin->firstUse; use; use = use->nextUse)
+        {
+            IRInst* user = use->getUser();
+            
+            auto fieldAddress = as<IRFieldAddress>(user);
+            if (!fieldAddress || !isInstStoredInto(reachability, ret, user))
+                continue;
+
+            IRInst* field = fieldAddress->getField();
+            usedKeys.add(as<IRStructKey>(field));
+        }
+
+        List<IRStructField*> uninitializedFields;
+
+        auto fields = type->getFields();
+        for (auto field : fields)
+        {
+            if (canIgnoreType(field->getFieldType(), nullptr))
+                continue;
+
+            if (!usedKeys.contains(field->getKey()))
+                uninitializedFields.add(field);
+        }
+        
+        return uninitializedFields;
+    }
+
+    static void checkConstructor(IRFunc* func, ReachabilityContext& reachability, DiagnosticSink* sink)
+    {
+        auto constructor = func->findDecoration<IRConstructorDecorartion>();
+        if (!constructor)
+            return;
+
+        IRStructType* stype = as<IRStructType>(func->getResultType());
+        if (!stype)
+            return;
+
+        // Don't bother giving warnings if its not being used
+        bool synthesized = constructor->getSynthesizedStatus();
+        if (synthesized && !func->firstUse)
+            return;
+        
+        auto printWarnings = [&](const List<IRStructField*>& fields, IRReturn* ret)
+        {
+            for (auto field : fields)
+            {
+                if (synthesized)
+                {
+                    sink->diagnose(field->getKey(),
+                        Diagnostics::fieldNotDefaultInitialized,
+                        stype,
+                        field->getKey());
+                }
+                else
+                {
+                    sink->diagnose(ret,
+                        Diagnostics::constructorUninitializedField,
+                        field->getKey());
+                }
+            }
+
+        };
+
+        // Work backwards, get exit points and find sources
+        for (auto block : func->getBlocks())
+        {
+            for (auto inst = block->getFirstInst(); inst; inst = inst->next)
+            {
+                auto ret = as<IRReturn>(inst);
+                if (!ret)
+                    continue;
+
+                auto fields = checkFieldsFromExit(reachability, ret, stype);
+                printWarnings(fields, ret);
+            }
+        }
+    }
+
     static void checkUninitializedValues(IRFunc* func, DiagnosticSink* sink)
     {
+        // Differentiable functions will generate undefined values
+        // strictly so that they can be set in a differentiable way
         if (isDifferentiableFunc(func))
             return;
 
@@ -359,6 +491,9 @@ namespace Slang
 
         ReachabilityContext reachability(func);
 
+        // Used for a further analysis and to skip usual return checks
+        auto constructor = func->findDecoration <IRConstructorDecorartion> ();
+
         // Check out parameters
         for (auto param : firstBlock->getParams())
         {
@@ -384,6 +519,10 @@ namespace Slang
                 if (!isUninitializedValue(inst))
                     continue;
 
+                // This will be looked into later
+                if (constructor && isReturnedValue(inst))
+                    continue;
+
                 IRType* type = inst->getFullType();
                 if (canIgnoreType(type, nullptr))
                     continue;
@@ -397,6 +536,9 @@ namespace Slang
                 }
             }
         }
+
+        // Separate analysis for constructors
+        checkConstructor(func, reachability, sink);
     }
 
     static void checkUninitializedGlobals(IRGlobalVar* variable, DiagnosticSink* sink)
diff --git a/source/slang/slang-lower-to-ir.cpp b/source/slang/slang-lower-to-ir.cpp
index b86b7b177..d3770753c 100644
--- a/source/slang/slang-lower-to-ir.cpp
+++ b/source/slang/slang-lower-to-ir.cpp
@@ -9692,6 +9692,9 @@ struct DeclLoweringVisitor : DeclVisitor<DeclLoweringVisitor, LoweredValInfo>
                         subContext->irBuilder->emitStore(thisVar, allocatedObj);
                     }
                 }
+
+                // Used for diagnostics
+                getBuilder()->addConstructorDecoration(irFunc, constructorDecl->isSynthesized);
             }
 
             // We lower whatever statement was stored on the declaration
diff --git a/tests/compute/struct-default-init.slang b/tests/compute/struct-default-init.slang
index 50933ae84..dc0e0218a 100644
--- a/tests/compute/struct-default-init.slang
+++ b/tests/compute/struct-default-init.slang
@@ -3,9 +3,9 @@
 
 struct Test
 {
-	int a;
+	int a = 0;
 	int b = 1;
-	int c;
+	int c = 0;
 	int d = 1 + 1;
 }
 
@@ -37,4 +37,4 @@ void computeMain(uint3 dispatchThreadID : SV_DispatchThreadID)
 	int outVal = test(inVal);
 
 	outputBuffer[tid] = outVal;
-}
\ No newline at end of file
+}
diff --git a/tests/diagnostics/uninitialized-fields.slang b/tests/diagnostics/uninitialized-fields.slang
new file mode 100644
index 000000000..29e065d2b
--- /dev/null
+++ b/tests/diagnostics/uninitialized-fields.slang
@@ -0,0 +1,108 @@
+//TEST:SIMPLE(filecheck=CHK): -target spirv
+
+extern static const int flag;
+
+struct None
+{
+    int a;
+    int b;
+
+    __init()
+    {
+        a = 0;
+        b = 0;
+    }
+}
+
+struct Simple
+{
+    int value;
+    Simple *next;
+
+    __init(int v, Simple* ptr)
+    {
+        value = v;
+        //CHK-DAG: warning 41020: exiting constructor without initializing field 'next'
+    }
+}
+
+struct Conditional
+{
+    int field;
+    int another_field;
+
+    __init()
+    {
+        field = 0;
+        if (flag == 0)
+            //CHK-DAG: warning 41020: exiting constructor without initializing field 'another_field'
+            return;
+
+        another_field = 0;
+    }
+};
+
+struct Partial
+{
+    int inline = 0;
+    int body = 0;
+
+    __init(int x)
+    {
+        body = x;
+    }
+}
+
+// Warnings here should be a bit different
+struct Implicit
+{
+    //CHK-DAG: warning 41021: default initializer for 'Implicit' will not initialize field 'a'
+    int a;
+    int b = 1;
+    //CHK-DAG: warning 41021: default initializer for 'Implicit' will not initialize field 'c'
+    int c;
+    int d = 1 + 1;
+}
+
+int using_implicit(int x)
+{
+    Implicit impl;
+    impl.a = x;
+    return impl.c;
+}
+
+struct UnusedImplicit
+{
+    // Shouldn't warn here, since its never used
+    int a;
+    int b = 1;
+    int c;
+    int d = 1 + 1;
+}
+
+struct Pass
+{
+    int x = 0;
+    int y = 0;
+}
+
+struct Impl
+{
+    float x;
+
+    __init(float val)
+    {
+        x = val;
+    }
+
+    __init()
+    {
+        float val = 2.0;
+
+        // Shouldn't trigger a warning here
+        return Impl(val);
+    }
+}
+
+//CHK-NOT: warning 41020
+//CHK-NOT: warning 41021
diff --git a/tests/language-feature/struct-field-initializers/struct-field-initializer.slang b/tests/language-feature/struct-field-initializers/struct-field-initializer.slang
index 0a03644aa..7c4eeeef4 100644
--- a/tests/language-feature/struct-field-initializers/struct-field-initializer.slang
+++ b/tests/language-feature/struct-field-initializers/struct-field-initializer.slang
@@ -11,7 +11,7 @@ static int myThree = 1+2;
 
 struct DefaultStructNoInit
 {
-    int data0;
+    int data0 = 0;
     int data1 = myTwo;
     int data2 = 2;
 };
@@ -57,4 +57,4 @@ void computeMain(uint3 dispatchThreadID: SV_DispatchThreadID)
         && withInit2.data1 == 4
         && withInit2.data2 == 4
         ;
-}
\ No newline at end of file
+}