[clang] [clang] Fix suppressing diagnostics for uninitialized variables (PR #148336)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Igor Kudrin (igorkudrin)

<details>
<summary>Changes</summary>

When one kind of diagnostics is disabled, this should not preclude other diagnostics from displaying, even if they have lower priority. For example, this should print a warning about passing an uninitialized variable as a const reference:
```
> cat test.cpp
void foo(const int &);
int f(bool a) {
  int v;
  if (a) {
    foo(v);
    v = 5;
  }
  return v;
}
> clang test.cpp -fsyntax-only -Wuninitialized -Wno-sometimes-uninitialized
```

---
Full diff: https://github.com/llvm/llvm-project/pull/148336.diff


2 Files Affected:

- (modified) clang/lib/Sema/AnalysisBasedWarnings.cpp (+48-40) 
- (added) clang/test/SemaCXX/warn-no-sometimes-uninitialized.cpp (+12) 


``````````diff
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index ec8acbdff3b49..a4e5519f13d18 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -987,10 +987,11 @@ static void DiagUninitUse(Sema &S, const VarDecl *VD, const UninitUse &Use,
 }
 
 /// Diagnose uninitialized const reference usages.
-static void DiagnoseUninitializedConstRefUse(Sema &S, const VarDecl *VD,
+static bool DiagnoseUninitializedConstRefUse(Sema &S, const VarDecl *VD,
                                              const UninitUse &Use) {
   S.Diag(Use.getUser()->getBeginLoc(), diag::warn_uninit_const_reference)
       << VD->getDeclName() << Use.getUser()->getSourceRange();
+  return !S.getDiagnostics().isLastDiagnosticIgnored();
 }
 
 /// DiagnoseUninitializedUse -- Helper function for diagnosing uses of an
@@ -1022,7 +1023,7 @@ static bool DiagnoseUninitializedUse(Sema &S, const VarDecl *VD,
       if (CR.doesContainReference()) {
         S.Diag(DRE->getBeginLoc(), diag::warn_uninit_self_reference_in_init)
             << VD->getDeclName() << VD->getLocation() << DRE->getSourceRange();
-        return true;
+        return !S.getDiagnostics().isLastDiagnosticIgnored();
       }
     }
 
@@ -1045,7 +1046,7 @@ static bool DiagnoseUninitializedUse(Sema &S, const VarDecl *VD,
     S.Diag(VD->getBeginLoc(), diag::note_var_declared_here)
         << VD->getDeclName();
 
-  return true;
+  return !S.getDiagnostics().isLastDiagnosticIgnored();
 }
 
 namespace {
@@ -1559,43 +1560,7 @@ class UninitValsDiagReporter : public UninitVariablesHandler {
       UsesVec *vec = V.getPointer();
       bool hasSelfInit = V.getInt();
 
-      // Specially handle the case where we have uses of an uninitialized
-      // variable, but the root cause is an idiomatic self-init.  We want
-      // to report the diagnostic at the self-init since that is the root cause.
-      if (!vec->empty() && hasSelfInit && hasAlwaysUninitializedUse(vec))
-        DiagnoseUninitializedUse(S, vd,
-                                 UninitUse(vd->getInit()->IgnoreParenCasts(),
-                                           /* isAlwaysUninit */ true),
-                                 /* alwaysReportSelfInit */ true);
-      else {
-        // Sort the uses by their SourceLocations.  While not strictly
-        // guaranteed to produce them in line/column order, this will provide
-        // a stable ordering.
-        llvm::sort(*vec, [](const UninitUse &a, const UninitUse &b) {
-          // Move ConstRef uses to the back.
-          if (a.isConstRefUse() != b.isConstRefUse())
-            return b.isConstRefUse();
-          // Prefer a more confident report over a less confident one.
-          if (a.getKind() != b.getKind())
-            return a.getKind() > b.getKind();
-          return a.getUser()->getBeginLoc() < b.getUser()->getBeginLoc();
-        });
-
-        for (const auto &U : *vec) {
-          if (U.isConstRefUse()) {
-            DiagnoseUninitializedConstRefUse(S, vd, U);
-            break;
-          }
-
-          // If we have self-init, downgrade all uses to 'may be uninitialized'.
-          UninitUse Use = hasSelfInit ? UninitUse(U.getUser(), false) : U;
-
-          if (DiagnoseUninitializedUse(S, vd, Use))
-            // Skip further diagnostics for this variable. We try to warn only
-            // on the first point at which a variable is used uninitialized.
-            break;
-        }
-      }
+      diagnoseUnitializedVar(vd, hasSelfInit, vec);
 
       // Release the uses vector.
       delete vec;
@@ -1612,6 +1577,49 @@ class UninitValsDiagReporter : public UninitVariablesHandler {
              U.getKind() == UninitUse::AfterDecl;
     });
   }
+
+  // Print the diagnostic for the variable.  We try to warn only on the first
+  // point at which a variable is used uninitialized.  After the first
+  // diagnostic is printed, further diagnostics for this variable are skipped.
+  void diagnoseUnitializedVar(const VarDecl *vd, bool hasSelfInit,
+                              UsesVec *vec) {
+    // Specially handle the case where we have uses of an uninitialized
+    // variable, but the root cause is an idiomatic self-init.  We want
+    // to report the diagnostic at the self-init since that is the root cause.
+    if (hasSelfInit && hasAlwaysUninitializedUse(vec)) {
+      if (DiagnoseUninitializedUse(S, vd,
+                                   UninitUse(vd->getInit()->IgnoreParenCasts(),
+                                             /*isAlwaysUninit=*/true),
+                                   /*alwaysReportSelfInit=*/true))
+        return;
+    }
+
+    // Sort the uses by their SourceLocations.  While not strictly
+    // guaranteed to produce them in line/column order, this will provide
+    // a stable ordering.
+    llvm::sort(*vec, [](const UninitUse &a, const UninitUse &b) {
+      // Prefer the direct use of an uninitialized variable over its use via
+      // constant reference.
+      if (a.isConstRefUse() != b.isConstRefUse())
+        return b.isConstRefUse();
+      // Prefer a more confident report over a less confident one.
+      if (a.getKind() != b.getKind())
+        return a.getKind() > b.getKind();
+      return a.getUser()->getBeginLoc() < b.getUser()->getBeginLoc();
+    });
+
+    for (const auto &U : *vec) {
+      if (U.isConstRefUse()) {
+        if (DiagnoseUninitializedConstRefUse(S, vd, U))
+          return;
+      } else {
+        // If we have self-init, downgrade all uses to 'may be uninitialized'.
+        UninitUse Use = hasSelfInit ? UninitUse(U.getUser(), false) : U;
+        if (DiagnoseUninitializedUse(S, vd, Use))
+          return;
+      }
+    }
+  }
 };
 
 /// Inter-procedural data for the called-once checker.
diff --git a/clang/test/SemaCXX/warn-no-sometimes-uninitialized.cpp b/clang/test/SemaCXX/warn-no-sometimes-uninitialized.cpp
new file mode 100644
index 0000000000000..71e5faeec63fb
--- /dev/null
+++ b/clang/test/SemaCXX/warn-no-sometimes-uninitialized.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fsyntax-only -Wuninitialized -Wno-sometimes-uninitialized -verify %s
+
+void foo(const int &);
+
+int f(bool a) {
+  int v;
+  if (a) {
+    foo(v); // expected-warning {{variable 'v' is uninitialized when passed as a const reference argument here}}
+    v = 5;
+  }
+  return v;
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/148336