https://github.com/zyn0217 created https://github.com/llvm/llvm-project/pull/210920
For binary expression `1, 2`, StmtPrinter used to print it as `1 , 2` which doesn't look very pretty. >From 3d1d056e6412e0ca00dca2c102fec0e50989de65 Mon Sep 17 00:00:00 2001 From: Younan Zhang <[email protected]> Date: Tue, 21 Jul 2026 17:32:32 +0800 Subject: [PATCH] [Clang] Don't print extra whitespace for comma expression in StmtPrinter For binary expression `1, 2`, StmtPrinter used to print it as `1 , 2` which doesn't look very pretty. --- clang/lib/AST/StmtPrinter.cpp | 8 +++ .../OpenMP/declare_reduction_ast_print.cpp | 4 +- clang/test/OpenMP/for_ast_print.cpp | 2 +- clang/test/OpenMP/parallel_ast_print.cpp | 4 +- clang/test/PCH/cxx-explicit-specifier.cpp | 2 +- .../instantiate-requires-clause.cpp | 6 +-- .../instantiate-requires-expr.cpp | 8 +-- clang/unittests/AST/StmtPrinterTest.cpp | 54 +++++++++---------- 8 files changed, 47 insertions(+), 41 deletions(-) diff --git a/clang/lib/AST/StmtPrinter.cpp b/clang/lib/AST/StmtPrinter.cpp index 9a97d52fd3020..79467f83bd8d7 100644 --- a/clang/lib/AST/StmtPrinter.cpp +++ b/clang/lib/AST/StmtPrinter.cpp @@ -161,6 +161,8 @@ namespace { void VisitCXXNamedCastExpr(CXXNamedCastExpr *Node); + void VisitBinComma(BinaryOperator *Node); + #define ABSTRACT_STMT(CLASS) #define STMT(CLASS, PARENT) \ void Visit##CLASS(CLASS *Node); @@ -1900,6 +1902,12 @@ void StmtPrinter::VisitImplicitCastExpr(ImplicitCastExpr *Node) { PrintExpr(Node->getSubExpr()); } +void StmtPrinter::VisitBinComma(BinaryOperator *Node) { + PrintExpr(Node->getLHS()); + OS << BinaryOperator::getOpcodeStr(Node->getOpcode()) << " "; + PrintExpr(Node->getRHS()); +} + void StmtPrinter::VisitBinaryOperator(BinaryOperator *Node) { PrintExpr(Node->getLHS()); OS << " " << BinaryOperator::getOpcodeStr(Node->getOpcode()) << " "; diff --git a/clang/test/OpenMP/declare_reduction_ast_print.cpp b/clang/test/OpenMP/declare_reduction_ast_print.cpp index dcb78ff1c63b8..24548e924f96e 100644 --- a/clang/test/OpenMP/declare_reduction_ast_print.cpp +++ b/clang/test/OpenMP/declare_reduction_ast_print.cpp @@ -38,11 +38,11 @@ class SSS { // CHECK: template <class T> class SSS { // CHECK: #pragma omp declare reduction (fun : T : omp_out += omp_in) initializer(omp_priv = omp_orig + 15) -// CHECK: #pragma omp declare reduction (fun1 : T : omp_out = 1 , omp_out = foo(omp_in)) initializer(omp_priv = omp_orig + 14) +// CHECK: #pragma omp declare reduction (fun1 : T : omp_out = 1, omp_out = foo(omp_in)) initializer(omp_priv = omp_orig + 14) // CHECK: }; // CHECK: template<> class SSS<int> { // CHECK: #pragma omp declare reduction (fun : int : omp_out += omp_in) initializer(omp_priv = omp_orig + 15) -// CHECK: #pragma omp declare reduction (fun1 : int : omp_out = 1 , omp_out = foo(omp_in)) initializer(omp_priv = omp_orig + 14) +// CHECK: #pragma omp declare reduction (fun1 : int : omp_out = 1, omp_out = foo(omp_in)) initializer(omp_priv = omp_orig + 14) // CHECK: }; SSS<int> d; diff --git a/clang/test/OpenMP/for_ast_print.cpp b/clang/test/OpenMP/for_ast_print.cpp index f793050067d85..6e8c960eb17f5 100644 --- a/clang/test/OpenMP/for_ast_print.cpp +++ b/clang/test/OpenMP/for_ast_print.cpp @@ -274,7 +274,7 @@ int main(int argc, char **argv) { // CHECK-NEXT: for (int i = 0; i < 10; ++i) // CHECK-NEXT: for (int j = 0; j < 10; ++j) // CHECK-NEXT: for (auto x : arr) - // CHECK-NEXT: foo() , (void)x; + // CHECK-NEXT: foo(), (void)x; char buf[9] = "01234567"; char *p, *q; #pragma omp parallel diff --git a/clang/test/OpenMP/parallel_ast_print.cpp b/clang/test/OpenMP/parallel_ast_print.cpp index 28dc611bf864d..8d9cf0ec81fe6 100644 --- a/clang/test/OpenMP/parallel_ast_print.cpp +++ b/clang/test/OpenMP/parallel_ast_print.cpp @@ -275,9 +275,9 @@ int mainVC(int argc, int *argv) { // OMP60-NEXT: int h[20]; // OMP60-NEXT: int j; // OMP60-NEXT: #pragma omp parallel shared(global) default(firstprivate:aggregate) -// OMP60-NEXT: bar(h[1]) , h[1] = global; +// OMP60-NEXT: bar(h[1]), h[1] = global; // OMP60-NEXT: #pragma omp parallel private(global2) default(private:scalar) -// OMP60-NEXT: bar(global2) , j = global2; +// OMP60-NEXT: bar(global2), j = global2; enum Enum { }; diff --git a/clang/test/PCH/cxx-explicit-specifier.cpp b/clang/test/PCH/cxx-explicit-specifier.cpp index c7cd2c5002405..6b922d17f4fd6 100644 --- a/clang/test/PCH/cxx-explicit-specifier.cpp +++ b/clang/test/PCH/cxx-explicit-specifier.cpp @@ -28,7 +28,7 @@ namespace inheriting_constructor { U<S, char> a = foo('0'); } -//CHECK: explicit(((void)char{} , true)) +//CHECK: explicit(((void)char{}, true)) #endif diff --git a/clang/test/SemaTemplate/instantiate-requires-clause.cpp b/clang/test/SemaTemplate/instantiate-requires-clause.cpp index 3f438d8885a20..602b43fd1e662 100644 --- a/clang/test/SemaTemplate/instantiate-requires-clause.cpp +++ b/clang/test/SemaTemplate/instantiate-requires-clause.cpp @@ -1,7 +1,7 @@ // RUN: %clang_cc1 -std=c++2a -x c++ %s -Wno-unused-value -verify template <typename... Args> requires ((sizeof(Args) == 1), ...) -// expected-note@-1 {{because '(sizeof(int) == 1) , (sizeof(char) == 1) , (sizeof(int) == 1)' evaluated to false}} +// expected-note@-1 {{because '(sizeof(int) == 1), (sizeof(char) == 1), (sizeof(int) == 1)' evaluated to false}} void f1(Args&&... args) { } // expected-note@-1 {{candidate template ignored: constraints not satisfied [with Args = <int, char, int>]}} @@ -13,7 +13,7 @@ using f13 = decltype(f1(1, 'b', 2)); template <typename... Args> void f2(Args&&... args) requires ((sizeof(args) == 1), ...) { } // expected-note@-1 {{candidate template ignored: constraints not satisfied [with Args = <int, char, int>]}} -// expected-note@-2 {{because '(sizeof (args) == 1) , (sizeof (args) == 1) , (sizeof (args) == 1)' evaluated to false}} +// expected-note@-2 {{because '(sizeof (args) == 1), (sizeof (args) == 1), (sizeof (args) == 1)' evaluated to false}} using f21 = decltype(f2('a')); using f22 = decltype(f2(1, 'b')); @@ -21,7 +21,7 @@ using f23 = decltype(f2(1, 'b', 2)); // expected-error@-1 {{no matching function for call to 'f2'}} template <typename... Args> requires ((sizeof(Args) == 1), ...) -// expected-note@-1 {{because '(sizeof(int) == 1) , (sizeof(char) == 1) , (sizeof(int) == 1)' evaluated to false}} +// expected-note@-1 {{because '(sizeof(int) == 1), (sizeof(char) == 1), (sizeof(int) == 1)' evaluated to false}} void f3(Args&&... args) requires ((sizeof(args) == 1), ...) { } // expected-note@-1 {{candidate template ignored: constraints not satisfied [with Args = <int, char, int>]}} diff --git a/clang/test/SemaTemplate/instantiate-requires-expr.cpp b/clang/test/SemaTemplate/instantiate-requires-expr.cpp index 688cb9884749d..a5a18acd069f3 100644 --- a/clang/test/SemaTemplate/instantiate-requires-expr.cpp +++ b/clang/test/SemaTemplate/instantiate-requires-expr.cpp @@ -84,7 +84,7 @@ namespace type_requirement { template<typename T> struct a { template<typename U> requires (requires { typename T::a::a; }, false) - // expected-note@-1{{because 'requires { <<error-type>>; } , false' evaluated to false}} + // expected-note@-1{{because 'requires { <<error-type>>; }, false' evaluated to false}} struct r {}; }; @@ -126,7 +126,7 @@ namespace expr_requirement { template<typename T> struct a { - template<typename U> requires (requires { sizeof(T::a); }, false) // expected-note{{because 'requires { <<error-expression>>; } , false' evaluated to false}} + template<typename U> requires (requires { sizeof(T::a); }, false) // expected-note{{because 'requires { <<error-expression>>; }, false' evaluated to false}} struct r {}; }; @@ -151,7 +151,7 @@ namespace expr_requirement { template<typename T> struct b { - template<typename U> requires (requires { { 0 } -> C1<typename T::a>; }, false) // expected-note{{because 'requires { { 0 } -> <<error-type>>; } , false' evaluated to false}} + template<typename U> requires (requires { { 0 } -> C1<typename T::a>; }, false) // expected-note{{because 'requires { { 0 } -> <<error-type>>; }, false' evaluated to false}} struct r {}; }; @@ -190,7 +190,7 @@ namespace nested_requirement { template<typename T> struct a { template<typename U> requires - (requires { requires sizeof(T::a) == 0; }, false) // expected-note{{because 'requires { requires <<error-expression>>; } , false' evaluated to false}} + (requires { requires sizeof(T::a) == 0; }, false) // expected-note{{because 'requires { requires <<error-expression>>; }, false' evaluated to false}} struct r {}; }; diff --git a/clang/unittests/AST/StmtPrinterTest.cpp b/clang/unittests/AST/StmtPrinterTest.cpp index 2b75253fa6698..23b8a4212c020 100644 --- a/clang/unittests/AST/StmtPrinterTest.cpp +++ b/clang/unittests/AST/StmtPrinterTest.cpp @@ -112,40 +112,38 @@ PrintedStmtObjCMatches(StringRef Code, const T &NodeMatch, TEST(StmtPrinter, TestIntegerLiteral) { ASSERT_TRUE(PrintedStmtCXXMatches(StdVer::CXX98, - "void A() {" - " 1, -1, 1U, 1u," - " 1L, 1l, -1L, 1UL, 1ul," - " 1LL, -1LL, 1ULL;" - "}", - FunctionBodyMatcher("A"), - "1 , -1 , 1U , 1U , " - "1L , 1L , -1L , 1UL , 1UL , " - "1LL , -1LL , 1ULL")); - // Should be: with semicolon + "void A() {" + " 1, -1, 1U, 1u," + " 1L, 1l, -1L, 1UL, 1ul," + " 1LL, -1LL, 1ULL;" + "}", + FunctionBodyMatcher("A"), + "1, -1, 1U, 1U, " + "1L, 1L, -1L, 1UL, 1UL, " + "1LL, -1LL, 1ULL")); + // Should be: with semicolon } TEST(StmtPrinter, TestMSIntegerLiteral) { - ASSERT_TRUE(PrintedStmtMSMatches( - "void A() {" - " 1i8, -1i8, 1ui8, " - " 1i16, -1i16, 1ui16, " - " 1i32, -1i32, 1ui32, " - " 1i64, -1i64, 1ui64;" - "}", - FunctionBodyMatcher("A"), - "1i8 , -1i8 , 1Ui8 , " - "1i16 , -1i16 , 1Ui16 , " - "1 , -1 , 1U , " - "1LL , -1LL , 1ULL")); - // Should be: with semicolon + ASSERT_TRUE(PrintedStmtMSMatches("void A() {" + " 1i8, -1i8, 1ui8, " + " 1i16, -1i16, 1ui16, " + " 1i32, -1i32, 1ui32, " + " 1i64, -1i64, 1ui64;" + "}", + FunctionBodyMatcher("A"), + "1i8, -1i8, 1Ui8, " + "1i16, -1i16, 1Ui16, " + "1, -1, 1U, " + "1LL, -1LL, 1ULL")); + // Should be: with semicolon } TEST(StmtPrinter, TestFloatingPointLiteral) { - ASSERT_TRUE(PrintedStmtCXXMatches(StdVer::CXX98, - "void A() { 1.0f, -1.0f, 1.0, -1.0, 1.0l, -1.0l; }", - FunctionBodyMatcher("A"), - "1.F , -1.F , 1. , -1. , 1.L , -1.L")); - // Should be: with semicolon + ASSERT_TRUE(PrintedStmtCXXMatches( + StdVer::CXX98, "void A() { 1.0f, -1.0f, 1.0, -1.0, 1.0l, -1.0l; }", + FunctionBodyMatcher("A"), "1.F, -1.F, 1., -1., 1.L, -1.L")); + // Should be: with semicolon } TEST(StmtPrinter, TestStringLiteralOperatorTemplate_Pack) { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
