Only track scalar `out` params (#5078)

The UninitializedValues analysis doesn't correctly handle arrays or pointer objects, so we should not enable out parameter analysis for those types. Additionally, because the HLSL ASTs incorrectly represent structure out parameters as non-reference types, the analysis will generate incorrect diagnostics for uses of structs in call expressions. There isn't really an easy solution for that. This change takes a conservative approach to addressing those issues by disabling the analysis for values that we can't correctly identify.

This change also disables the "suggest initialization" fix-it for out parameters. Default initialization for out parameters doesn't work, and would be confusing.

tools/clang/lib/Analysis/UninitializedValues.cpp

@@ -35,8 +35,15 @@ using namespace clang;
 
 static bool isTrackedVar(const VarDecl *vd, const DeclContext *dc) {
   // HLSL Change Begin - Treat `out` parameters as uninitialized values.
-  if (vd->hasAttr<HLSLOutAttr>() && !vd->hasAttr<HLSLInAttr>())
-    return true;
+  if (vd->hasAttr<HLSLOutAttr>() && !vd->hasAttr<HLSLInAttr>()) {
+    QualType ty = vd->getType().getNonReferenceType();
+    // FIXME: HLSL doesn't model parameter passing of structs correctly in the
+    // AST. Struct types are passed by value in the AST regardless of whether
+    // they are out or inout, which results in LValueToRValue casts in the AST
+    // which look like uses (because they are). For now this analysis can
+    // generate too many false positives for structs.
+    return ty->isScalarType();
+  }
   // HLSL Change End - Treat `out` parameters as uninitialized values.
   if (vd->isLocalVarDecl() && !vd->hasGlobalStorage() &&
       !vd->isExceptionVariable() && !vd->isInitCapture() &&
@@ -87,6 +94,8 @@ void DeclToIndex::computeMap(const DeclContext &dc) {
     if (isTrackedVar(vd, &dc))
       map[vd] = count++;
     // HLSL Change Begin - Treat `out` parameters as uninitialized values.
+    else
+      continue;
     // Keep HLSL parameters in a separate index.
     if (vd->hasAttr<HLSLOutAttr>() && !vd->hasAttr<HLSLInAttr>())
       hlslOutParams.push_back(vd);

tools/clang/lib/Sema/AnalysisBasedWarnings.cpp

@@ -612,6 +612,11 @@ static bool SuggestInitializationFixit(Sema &S, const VarDecl *VD) {
     return true;
   }
 
+  // HLSL Change Start - Don't issue fixit for out parameters
+  if (VD->hasAttr<HLSLOutAttr>())
+    return false;
+  // HLSL Change End
+
   // Don't issue a fixit if there is already an initializer.
   if (VD->getInit())
     return false;

tools/clang/test/HLSL/more-operators.hlsl

@@ -42,7 +42,7 @@ float  i11_to_float(int1x1 v) { return v; }
 void into_out_i(out int i) { i = g_i11; }
 void into_out_i3(out int3 i3) { i3 = int3(1, 2, 3); } // expected-note {{candidate function}} expected-note {{passing argument to parameter 'i3' here}} fxc-pass {{}}
 void into_out_f(out float i) { i = g_i11; }
-void into_out_f3_s(out f3_s i) { } // expected-warning{{parameter 'i' is uninitialized when used here}} expected-note{{initialize the variable 'i' to silence this warning}}
+void into_out_f3_s(out f3_s i) { }
 void into_out_ss(out SamplerState ss) { ss = g_SamplerState; }
 
 float4 plain(float4 param4 /* : FOO */) /*: FOO */{

tools/clang/test/HLSL/out-param-diagnostics.hlsl

@@ -147,13 +147,8 @@ struct SomeObj {
   int Float;
 };
 
-void UnusedObjectOut(out SomeObj V) {} // expected-warning {{parameter 'V' is uninitialized when used here}} expected-note {{initialize the variable 'V' to silence this warning}}
-
-// We don't have per-field analysis, so this will count as an initialization if
-// any field is initiailzed.
-void SomethingObjectOut(out SomeObj V) {
-  V.Integer = 1;
-}
+// No errors are generated for struct types :(
+void UnusedObjectOut(out SomeObj V) {}
 
 // This test case is copied from tools/clang/test/HLSL/functions.hlsl to verify
 // that the analysis does produce a diagnostic for this case. Because
@@ -174,3 +169,19 @@ void MaybeUsedMaybeUnused([maybe_unused] out int Val, int Cnt) { // expected-not
   if (Cnt % 2) // expected-warning{{parameter 'Val' is used uninitialized whenever 'if' condition is fals}} expected-note{{remove the 'if' if its condition is always true}}
     Val = 1;
 } // expected-note{{uninitialized use occurs here}}
+
+// Neither of these will warn because we don't support element-based tracking.
+void UnusedSizedArray(out uint u[2]) { }
+void UnusedUnsizedArray(out uint u[]) { }
+
+// Warnings for struct types are not supported yet.
+struct S { uint a; uint b; };
+
+void Initializes(out S a) {
+  a.a = 1;
+  a.b = 1;
+}
+
+void InitializesIndirectly(out S a) {
+  Initializes(a);
+}