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

7 files changed, 165 insertions, 0 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