[C] Silence unreachable -Watomic-access diagnostics

[AaronBallman]

Accessing the member of a structure or union which is _Atomic-qualified is undefined behavior in C. We currently diagnose that with a warning that defaults to an error. In turn, this means we reject a valid program if the access it not reachable because of the error. e.g.,

if (0)
SomeAtomicStruct.Member = 12; // Was diagnosed

This silences the diagnostic if the member access is not reachable.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Aaron Ballman (AaronBallman)

Changes

Accessing the member of a structure or union which is _Atomic-qualified is undefined behavior in C. We currently diagnose that with a warning that defaults to an error. In turn, this means we reject a valid program if the access it not reachable because of the error. e.g.,

if (0)
SomeAtomicStruct.Member = 12; // Was diagnosed

This silences the diagnostic if the member access is not reachable.

---

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

3 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+13)
• (modified) clang/lib/Sema/SemaExprMember.cpp (+1-1)
• (modified) clang/test/Sema/atomic-expr.c (+17)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 31c517338c21f..f927c438c5d80 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -533,6 +533,19 @@ Improvements to Clang's diagnostics
 - A new off-by-default warning ``-Wms-bitfield-padding`` has been added to alert to cases where bit-field
   packing may differ under the MS struct ABI (#GH117428).
 
+- ``-Watomic-access`` no longer fires on unreachable code. e.g.,
+
+  ..code-block:: c
+
+    _Atomic struct S { int a; } s;
+    void func(void) {
+      if (0)
+        s.a = 12; // Previously diagnosed with -Watomic-access, now silenced
+      s.a = 12; // Still diagnosed with -Watomic-access
+      return;
+      s.a = 12; // Previously diagnosed, now silenced
+    }
+
 
 Improvements to Clang's time-trace
 ----------------------------------
diff --git a/clang/lib/Sema/SemaExprMember.cpp b/clang/lib/Sema/SemaExprMember.cpp
index 053414ff7a1a7..39c162c3b835d 100644
--- a/clang/lib/Sema/SemaExprMember.cpp
+++ b/clang/lib/Sema/SemaExprMember.cpp
@@ -1385,7 +1385,7 @@ static ExprResult LookupMemberExpr(Sema &S, LookupResult &R,
   // lvalue. Because this is inherently unsafe as an atomic operation, the
   // warning defaults to an error.
   if (const auto *ATy = BaseType->getAs<AtomicType>()) {
-    S.DiagRuntimeBehavior(OpLoc, nullptr,
+    S.DiagRuntimeBehavior(OpLoc, BaseExpr.get(),
                           S.PDiag(diag::warn_atomic_member_access));
     BaseType = ATy->getValueType().getUnqualifiedType();
     BaseExpr = ImplicitCastExpr::Create(
diff --git a/clang/test/Sema/atomic-expr.c b/clang/test/Sema/atomic-expr.c
index 7e5219dd3f14a..96571e3e68c87 100644
--- a/clang/test/Sema/atomic-expr.c
+++ b/clang/test/Sema/atomic-expr.c
@@ -114,6 +114,23 @@ void func_16(void) {
   (void)sizeof(xp->val);
   (void)sizeof(y.ival);
   (void)sizeof(yp->ival);
+
+  // Also, do not diagnose in unreachable code paths.
+  {
+    if (0) {
+      x.val = 12;
+      xp->val = 12;
+      (void)y.ival;
+      (void)yp->ival;
+    }
+
+    return;
+
+    x.val = 12;
+    xp->val = 12;
+    (void)y.ival;
+    (void)yp->ival;
+  }
 }
 
 // Ensure that we correctly implement assignment constraints from C2x 6.5.16.1.

[erichkeane]

Obviously going to still be somewhat imperfect, but seems fine to me.