[clang] [clang] Extend -Wunused-but-set-variable to static globals (PR #178342)

[John Paul Jepko via cfe-commits]

================
@@ -1620,6 +1620,43 @@ void Sema::ActOnEndOfTranslationUnit() {
   if (Context.hasAnyFunctionEffects())
     performFunctionEffectAnalysis(Context.getTranslationUnitDecl());
 
+  // Diagnose unused-but-set static globals in a deterministic order.
+  // Not tracking shadowing info for static globals; there's nothing to shadow.
+  struct LocAndDiag {
+    SourceLocation Loc;
+    PartialDiagnostic PD;
+  };
+  SmallVector<LocAndDiag, 16> DeclDiags;
+  auto addDiag = [&DeclDiags](SourceLocation Loc, PartialDiagnostic PD) {
+    DeclDiags.push_back(LocAndDiag{Loc, std::move(PD)});
+  };
+
+  // For -Wunused-but-set-variable we only care about variables that were
+  // referenced by the TU end.
+  SmallVector<const VarDecl *, 16> DeclsToErase;
+  for (const auto &Ref : RefsMinusAssignments) {
+    const VarDecl *VD = Ref.first;
+    // Only diagnose static file vars defined in the main file to match
+    // -Wunused-variable behavior and avoid false positives from header vars.
----------------
jpjepko wrote:

> Oh I didn't make it clear. I meant modifying code, calling `ShouldRemoveFromUnused`.

I ran into some problems trying to make this change. If we replace the call to `isMainFileLoc` with `ShouldRemoveFromUnused`, we lose the main file check since `ShouldRemoveFromUnused` checks `isUsed` at the beginning; it will return early because it counts a variable getting set as a use:

https://github.com/llvm/llvm-project/blob/0da2aecb01412588c571038d7c887674c291a08f/clang/lib/Sema/Sema.cpp#L880-L882

There's a similar problem for `ShouldWarnIfUnusedFileScopedDecl`, which actually contains the call to `isMainFileLoc` but will again never reach because of the `isUsed` check that it also does.

Unless there's an alternative approach or something else I am missing here, I believe the current approach is sufficient and minimal.

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