[Clang][AST] Fix printing for atomic_test_and_set and atomic_clear

[devajithvs]

#121943 rewrote __atomic_test_and_set and __atomic_clear to be lowered through AtomicExpr

StmtPrinter::VisitAtomicExpr still treated them like other atomic builtins with a Val1 operand. This led to incorrect pretty-printing when dumping the AST.

Skip Val1 for these two builtins like atomic loads.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Devajith (devajithvs)

Changes

#121943 rewrote __atomic_test_and_set and __atomic_clear to be lowered through AtomicExpr

StmtPrinter::VisitAtomicExpr still treated them like other atomic builtins with a Val1 operand. This led to incorrect pretty-printing when dumping the AST.

Skip Val1 for these two builtins like atomic loads.

---

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

2 Files Affected:

• (modified) clang/lib/AST/StmtPrinter.cpp (+3-1)
• (modified) clang/test/SemaCXX/ast-print.cpp (+11)

diff --git a/clang/lib/AST/StmtPrinter.cpp b/clang/lib/AST/StmtPrinter.cpp
index 0030300521128..8f348f6e5bc73 100644
--- a/clang/lib/AST/StmtPrinter.cpp
+++ b/clang/lib/AST/StmtPrinter.cpp
@@ -2028,7 +2028,9 @@ void StmtPrinter::VisitAtomicExpr(AtomicExpr *Node) {
       Node->getOp() != AtomicExpr::AO__atomic_load_n &&
       Node->getOp() != AtomicExpr::AO__scoped_atomic_load_n &&
       Node->getOp() != AtomicExpr::AO__opencl_atomic_load &&
-      Node->getOp() != AtomicExpr::AO__hip_atomic_load) {
+      Node->getOp() != AtomicExpr::AO__hip_atomic_load &&
+      Node->getOp() != AtomicExpr::AO__atomic_test_and_set &&
+      Node->getOp() != AtomicExpr::AO__atomic_clear) {
     OS << ", ";
     PrintExpr(Node->getVal1());
   }
diff --git a/clang/test/SemaCXX/ast-print.cpp b/clang/test/SemaCXX/ast-print.cpp
index 2cb1ec440b6bb..2dc32c20b7296 100644
--- a/clang/test/SemaCXX/ast-print.cpp
+++ b/clang/test/SemaCXX/ast-print.cpp
@@ -176,6 +176,17 @@ float test15() {
   return __builtin_asinf(1.0F);
 }
 
+// CHECK: void test16() {
+// CHECK:   char *ptr;
+// CHECK:   __atomic_test_and_set(ptr, 0);
+// CHECK:   __atomic_clear(ptr, 0);
+// CHECK: }
+void test16() {
+  char *ptr;
+  __atomic_test_and_set(ptr, 0);
+  __atomic_clear(ptr, 0);
+}
+
 namespace PR18776 {
 struct A {
   operator void *();

[devajithvs]

We currently print:

$ echo 'char *a; void b() { __atomic_test_and_set(a, 0);}' | clang -cc1 -ast-print
char *a;
void b() {
    __atomic_test_and_set(a, <null expr>, 0);
}

This was probably missed in #121943

Discovered when debugging issues upgrading ROOT/Cling to LLVM20: root-project/root#19242

CC: @ostannard, @shafik, @jyknight

[cor3ntin]

This change needs a release note.
Please add an entry to clang/docs/ReleaseNotes.rst in the section the most adapted to the change, and referencing any Github issue this change fixes. Thanks!

[cor3ntin]

Do we have tests for the other atomic load builtins? It might be worth checking them all

[devajithvs]

Added test cases for all operations in: https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html

[cor3ntin]

Are there other places we do a similar check? Maybe we need that to me a member function of AtomicExpr

[devajithvs]

Yes, we do a similar check here, but it is part of a bigger switch case.

llvm-project/clang/lib/CodeGen/CGAtomic.cpp

Lines 904 to 911 in fba55c8

	case AtomicExpr::AO__atomic_load_n:
	case AtomicExpr::AO__scoped_atomic_load_n:
	case AtomicExpr::AO__c11_atomic_load:
	case AtomicExpr::AO__opencl_atomic_load:
	case AtomicExpr::AO__hip_atomic_load:
	case AtomicExpr::AO__atomic_test_and_set:
	case AtomicExpr::AO__atomic_clear:
	break;

[cor3ntin]

LGTM

[devajithvs]

Thanks for reviewing @cor3ntin. Would you mind merging this? (I don't have commit access yet)