llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Benedek Kaibas (benedekaibas) <details> <summary>Changes</summary> Until now, there was no consensus between using single spaces or double spaces after sentence endings in the emitted diagnostics by the Clang Static Analyzer (CSA). This led to inconsistently formatted diagnostics by the CSA. Currently there are the following cases where the CSA is not consistent with the Clang compiler: - 1: Using double spaces after sentence endings - 2: Using full sentences in diagnostics - 3: Using a capital letter at the start of the diagnostic This PR resolves (1) by removing the double spaces after sentence endings in `BugReporter` and converts those multiples sentences into one using `;`. #<!-- -->228777 resolves (1) in the rest within the CSA while also partially resolves (2) because for the affected lines it removes the full sentence diagnostics as well. To uplift downstream tests someone needs to just remove the double spaces in the test suite emitted by the `BugReporter`'s loop body notes and convert the multiple sentences into one using `;`. In a follow up PR I plan to fully resolve (2) as well. As of now there is not enough opinion gathered for (3) in the RFC, so it is out of scope until there is consensus on it. --- Full diff: https://github.com/llvm/llvm-project/pull/236804.diff 8 Files Affected: - (modified) clang/lib/StaticAnalyzer/Core/BugReporter.cpp (+2-2) - (modified) clang/test/Analysis/dangling-ptr-deref.cpp (+1-1) - (modified) clang/test/Analysis/diagnostics/track_subexpressions.cpp (+2-2) - (modified) clang/test/Analysis/loop-widening-notes.cpp (+4-4) - (modified) clang/test/Analysis/loop-widening.cpp (+2-2) - (modified) clang/test/Analysis/return-ptr-range.cpp (+2-2) - (modified) clang/test/Analysis/track-control-dependency-conditions.cpp (+3-3) - (modified) clang/test/Analysis/use-after-move.cpp (+12-12) ``````````diff diff --git a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp index 25d10df604bda1..a3d77c4971b3d4 100644 --- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp +++ b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -972,7 +972,7 @@ void PathDiagnosticBuilder::generateMinimalDiagForBlockEdge( C.getActivePath().push_front( std::make_shared<PathDiagnosticControlFlowPiece>( - Start, End, "Loop condition is false. Exiting loop")); + Start, End, "Loop condition is false; exiting loop")); } break; @@ -996,7 +996,7 @@ void PathDiagnosticBuilder::generateMinimalDiagForBlockEdge( C.getActivePath().push_front( std::make_shared<PathDiagnosticControlFlowPiece>( - Start, End, "Loop condition is true. Entering loop body")); + Start, End, "Loop condition is true; entering loop body")); } break; diff --git a/clang/test/Analysis/dangling-ptr-deref.cpp b/clang/test/Analysis/dangling-ptr-deref.cpp index 8572b6416f150f..f512425aeb5097 100644 --- a/clang/test/Analysis/dangling-ptr-deref.cpp +++ b/clang/test/Analysis/dangling-ptr-deref.cpp @@ -72,7 +72,7 @@ void test_case_six() { void test_case_seven() { int *ptr = nullptr; - // expected-note@+3 {{Loop condition is true. Entering loop body}} + // expected-note@+3 {{Loop condition is true; entering loop body}} // expected-note@+2 {{Assuming 'i' is >= 10}} // expected-note@+1 {{Loop condition is false. Execution continues on line}} for (int i = 0; i < 10; ++i) { // expected-note {{'i' initialized to 0}} diff --git a/clang/test/Analysis/diagnostics/track_subexpressions.cpp b/clang/test/Analysis/diagnostics/track_subexpressions.cpp index c61738ce7b136d..3b5d389ca154e3 100644 --- a/clang/test/Analysis/diagnostics/track_subexpressions.cpp +++ b/clang/test/Analysis/diagnostics/track_subexpressions.cpp @@ -37,7 +37,7 @@ void testArrayIndexTracking() { int arr[10]; for (int i = 0; i < 3; ++i) - // expected-note@-1 3{{Loop condition is true. Entering loop body}} + // expected-note@-1 3{{Loop condition is true; entering loop body}} // expected-note@-2 {{Loop condition is false. Execution continues on line 43}} arr[i] = 0; int x = getInt(); @@ -69,7 +69,7 @@ void testArrayIndexTracking() { int arr[2][10]; for (int i = 0; i < 3; ++i) - // expected-note@-1 3{{Loop condition is true. Entering loop body}} + // expected-note@-1 3{{Loop condition is true; entering loop body}} // expected-note@-2 {{Loop condition is false. Execution continues on line 75}} arr[1][i] = 0; int x = getInt(); diff --git a/clang/test/Analysis/loop-widening-notes.cpp b/clang/test/Analysis/loop-widening-notes.cpp index a3f030dfe98826..d16ac61e040d0c 100644 --- a/clang/test/Analysis/loop-widening-notes.cpp +++ b/clang/test/Analysis/loop-widening-notes.cpp @@ -7,7 +7,7 @@ int test_for_bug_25609() { if (p_a == 0) // expected-note {{Assuming 'p_a' is equal to null}} // expected-note@-1 {{Taking true branch}} bar(); - for (int i = 0; // expected-note {{Loop condition is true. Entering loop body}} + for (int i = 0; // expected-note {{Loop condition is true; entering loop body}} // expected-note@-1 {{Loop condition is false. Execution continues on line 16}} ++i, // expected-note {{Value assigned to 'p_a'}} i < flag_a; @@ -22,7 +22,7 @@ int flag_b; int while_analyzer_output() { flag_b = 100; int num = 10; - while (flag_b-- > 0) { // expected-note {{Loop condition is true. Entering loop body}} + while (flag_b-- > 0) { // expected-note {{Loop condition is true; entering loop body}} // expected-note@-1 {{Value assigned to 'num'}} // expected-note@-2 {{Loop condition is false. Execution continues on line 30}} num = flag_b; @@ -43,7 +43,7 @@ int flag_c; int do_while_analyzer_output() { int num = 10; do { // expected-note {{Loop condition is true. Execution continues on line 47}} - // expected-note@-1 {{Loop condition is false. Exiting loop}} + // expected-note@-1 {{Loop condition is false; exiting loop}} num--; } while (flag_c-- > 0); //expected-note {{Value assigned to 'num'}} int local = 0; @@ -57,7 +57,7 @@ int do_while_analyzer_output() { int flag_d; int test_for_loop() { int num = 10; - for (int i = 0; // expected-note {{Loop condition is true. Entering loop body}} + for (int i = 0; // expected-note {{Loop condition is true; entering loop body}} // expected-note@-1 {{Loop condition is false. Execution continues on line 67}} new int(10), // expected-note {{Value assigned to 'num'}} i < flag_d; diff --git a/clang/test/Analysis/loop-widening.cpp b/clang/test/Analysis/loop-widening.cpp index fbcb72dee160ae..6b56110292cc10 100644 --- a/clang/test/Analysis/loop-widening.cpp +++ b/clang/test/Analysis/loop-widening.cpp @@ -14,8 +14,8 @@ void fn1() { A a; A *b = &a; - for (;;) { // expected-note{{Loop condition is true. Entering loop body}} - // expected-note@-1{{Loop condition is true. Entering loop body}} + for (;;) { // expected-note{{Loop condition is true; entering loop body}} + // expected-note@-1{{Loop condition is true; entering loop body}} // expected-note@-2{{Value assigned to 'b'}} // no crash during bug report construction diff --git a/clang/test/Analysis/return-ptr-range.cpp b/clang/test/Analysis/return-ptr-range.cpp index 507720a47ea7d5..a60bced521057b 100644 --- a/clang/test/Analysis/return-ptr-range.cpp +++ b/clang/test/Analysis/return-ptr-range.cpp @@ -8,7 +8,7 @@ int arr[10]; // expected-note{{Original object declared here}} expected-note{{Or int *ptr; int *test_global_ptr() { - do { // expected-note{{Loop condition is false. Exiting loop}} + do { // expected-note{{Loop condition is false; exiting loop}} int x = conjure_index(); ptr = arr + x; // expected-note{{Value assigned to 'ptr'}} if (x != 20) // expected-note{{Assuming 'x' is equal to 20}} @@ -22,7 +22,7 @@ int *test_global_ptr() { int *test_local_ptr() { int *local_ptr; - do { // expected-note{{Loop condition is false. Exiting loop}} + do { // expected-note{{Loop condition is false; exiting loop}} int x = conjure_index(); local_ptr = arr + x; // expected-note{{Value assigned to 'local_ptr'}} if (x != 20) // expected-note{{Assuming 'x' is equal to 20}} diff --git a/clang/test/Analysis/track-control-dependency-conditions.cpp b/clang/test/Analysis/track-control-dependency-conditions.cpp index b762992b6a8f0b..75bde938f0c795 100644 --- a/clang/test/Analysis/track-control-dependency-conditions.cpp +++ b/clang/test/Analysis/track-control-dependency-conditions.cpp @@ -858,7 +858,7 @@ void f(int flag) { bar(); assert(cond1); // expected-note-re{{{{^}}Assuming 'cond1' is not equal to 0{{$}}}} // expected-note-re@-1{{{{^}}Taking false branch{{$}}}} - // expected-note-re@-2{{{{^}}Loop condition is false. Exiting loop{{$}}}} + // expected-note-re@-2{{{{^}}Loop condition is false; exiting loop{{$}}}} if (flag) // expected-note-re{{{{^}}Assuming 'flag' is not equal to 0{{$}}}} // expected-note-re@-1{{{{^}}Taking true branch{{$}}}} @@ -902,7 +902,7 @@ void f(int flag) { // expected-note-re@-2{{{{^}}Left side of '&&' is true{{$}}}} // expected-note-re@-3{{{{^}}Assuming the condition is false{{$}}}} // expected-note-re@-4{{{{^}}Taking false branch{{$}}}} - // expected-note-re@-5{{{{^}}Loop condition is false. Exiting loop{{$}}}} + // expected-note-re@-5{{{{^}}Loop condition is false; exiting loop{{$}}}} if (flag) // expected-note-re{{{{^}}Assuming 'flag' is not equal to 0{{$}}}} // expected-note-re@-1{{{{^}}Taking true branch{{$}}}} @@ -945,7 +945,7 @@ void f(int flag) { // expected-note-re@-1{{{{^}}Assuming 'cond1' is not equal to 0{{$}}}} // expected-note-re@-2{{{{^}}Left side of '||' is true{{$}}}} // expected-note-re@-3{{{{^}}Taking false branch{{$}}}} - // expected-note-re@-4{{{{^}}Loop condition is false. Exiting loop{{$}}}} + // expected-note-re@-4{{{{^}}Loop condition is false; exiting loop{{$}}}} if (flag) // expected-note-re{{{{^}}Assuming 'flag' is not equal to 0{{$}}}} // expected-note-re@-1{{{{^}}Taking true branch{{$}}}} diff --git a/clang/test/Analysis/use-after-move.cpp b/clang/test/Analysis/use-after-move.cpp index fa7893c9a21c8b..bfbc98a746c005 100644 --- a/clang/test/Analysis/use-after-move.cpp +++ b/clang/test/Analysis/use-after-move.cpp @@ -303,8 +303,8 @@ void loopTest() { } { A a; - for (int i = 0; i < 2; i++) { // peaceful-note {{Loop condition is true. Entering loop body}} - // peaceful-note@-1 {{Loop condition is true. Entering loop body}} + for (int i = 0; i < 2; i++) { // peaceful-note {{Loop condition is true; entering loop body}} + // peaceful-note@-1 {{Loop condition is true; entering loop body}} // peaceful-note@-2 {{Loop condition is false. Execution jumps to the end of the function}} rightRefCall(std::move(a)); // no-warning } @@ -317,8 +317,8 @@ void loopTest() { } { A a; - for (int i = 0; i < 2; i++) { // peaceful-note {{Loop condition is true. Entering loop body}} - // peaceful-note@-1 {{Loop condition is true. Entering loop body}} + for (int i = 0; i < 2; i++) { // peaceful-note {{Loop condition is true; entering loop body}} + // peaceful-note@-1 {{Loop condition is true; entering loop body}} // peaceful-note@-2 {{Loop condition is false. Execution jumps to the end of the function}} leftRefCall(a); // no-warning } @@ -331,8 +331,8 @@ void loopTest() { } { A a; - for (int i = 0; i < 2; i++) { // peaceful-note {{Loop condition is true. Entering loop body}} - // peaceful-note@-1 {{Loop condition is true. Entering loop body}} + for (int i = 0; i < 2; i++) { // peaceful-note {{Loop condition is true; entering loop body}} + // peaceful-note@-1 {{Loop condition is true; entering loop body}} // peaceful-note@-2 {{Loop condition is false. Execution jumps to the end of the function}} constCopyOrMoveCall(a); // no-warning } @@ -345,8 +345,8 @@ void loopTest() { } { A a; - for (int i = 0; i < 2; i++) { // peaceful-note {{Loop condition is true. Entering loop body}} - // peaceful-note@-1 {{Loop condition is true. Entering loop body}} + for (int i = 0; i < 2; i++) { // peaceful-note {{Loop condition is true; entering loop body}} + // peaceful-note@-1 {{Loop condition is true; entering loop body}} // peaceful-note@-2 {{Loop condition is false. Execution jumps to the end of the function}} moveInsideFunctionCall(a); // no-warning } @@ -359,16 +359,16 @@ void loopTest() { } { A a; - for (int i = 0; i < 2; i++) { // peaceful-note {{Loop condition is true. Entering loop body}} - // peaceful-note@-1 {{Loop condition is true. Entering loop body}} + for (int i = 0; i < 2; i++) { // peaceful-note {{Loop condition is true; entering loop body}} + // peaceful-note@-1 {{Loop condition is true; entering loop body}} // peaceful-note@-2 {{Loop condition is false. Execution jumps to the end of the function}} copyOrMoveCall(a); // no-warning } } { A a; - for (int i = 0; i < bignum(); i++) { // peaceful-note {{Loop condition is true. Entering loop body}} - // peaceful-note@-1 {{Loop condition is true. Entering loop body}} + for (int i = 0; i < bignum(); i++) { // peaceful-note {{Loop condition is true; entering loop body}} + // peaceful-note@-1 {{Loop condition is true; entering loop body}} constCopyOrMoveCall(std::move(a)); // peaceful-note {{Object 'a' is moved}} // peaceful-warning@-1 {{Moved-from object 'a' is moved}} // peaceful-note@-2 {{Moved-from object 'a' is moved}} `````````` </details> https://github.com/llvm/llvm-project/pull/236804 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
