[clang][Parse] Diagnose useless null statements / empty init-statements
Summary:
clang has `-Wextra-semi` (D43162), which is not dictated by the currently selected standard.
While that is great, there is at least one more source of need-less semis - 'null statements'.
Sometimes, they are needed:
```
for(int x = 0; continueToDoWork(x); x++)
; // Ugly code, but the semi is needed here.
```
But sometimes they are just there for no reason:
```
switch(X) {
case 0:
return -2345;
case 5:
return 0;
default:
return 42;
}; // <- oops
;;;;;;;;;;; <- OOOOPS, still not diagnosed. Clearly this is junk.
```
Additionally:
```
if(; // <- empty init-statement
true)
;
switch (; // empty init-statement
x) {
...
}
for (; // <- empty init-statement
int y : S())
;
}
As usual, things may or may not go sideways in the presence of macros.
While evaluating this diag on my codebase of interest, it was unsurprisingly
discovered that Google Test macros are *very* prone to this.
And it seems many issues are deep within the GTest itself, not
in the snippets passed from the codebase that uses GTest.
So after some thought, i decided not do issue a diagnostic if the semi
is within *any* macro, be it either from the normal header, or system header.
Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=39111 | PR39111 ]]
Reviewers: rsmith, aaron.ballman, efriedma
Reviewed By: aaron.ballman
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D52695
llvm-svn: 347339
2018-11-21 02:59:05 +08:00
|
|
|
// RUN: %clang_cc1 -fsyntax-only -Wextra-semi-stmt -verify %s
|
|
|
|
// RUN: cp %s %t
|
|
|
|
// RUN: %clang_cc1 -x c++ -Wextra-semi-stmt -fixit %t
|
|
|
|
// RUN: %clang_cc1 -x c++ -Wextra-semi-stmt -Werror %t
|
|
|
|
|
|
|
|
#define GOODMACRO(varname) int varname
|
|
|
|
#define BETTERMACRO(varname) GOODMACRO(varname);
|
|
|
|
#define NULLMACRO(varname)
|
|
|
|
|
|
|
|
enum MyEnum {
|
|
|
|
E1,
|
|
|
|
E2
|
|
|
|
};
|
|
|
|
|
|
|
|
void test() {
|
|
|
|
; // expected-warning {{empty expression statement has no effect; remove unnecessary ';' to silence this warning}}
|
|
|
|
;
|
|
|
|
|
|
|
|
// This removal of extra semi also consumes all the comments.
|
2022-01-05 14:49:03 +08:00
|
|
|
// clang-format off
|
[clang][Parse] Diagnose useless null statements / empty init-statements
Summary:
clang has `-Wextra-semi` (D43162), which is not dictated by the currently selected standard.
While that is great, there is at least one more source of need-less semis - 'null statements'.
Sometimes, they are needed:
```
for(int x = 0; continueToDoWork(x); x++)
; // Ugly code, but the semi is needed here.
```
But sometimes they are just there for no reason:
```
switch(X) {
case 0:
return -2345;
case 5:
return 0;
default:
return 42;
}; // <- oops
;;;;;;;;;;; <- OOOOPS, still not diagnosed. Clearly this is junk.
```
Additionally:
```
if(; // <- empty init-statement
true)
;
switch (; // empty init-statement
x) {
...
}
for (; // <- empty init-statement
int y : S())
;
}
As usual, things may or may not go sideways in the presence of macros.
While evaluating this diag on my codebase of interest, it was unsurprisingly
discovered that Google Test macros are *very* prone to this.
And it seems many issues are deep within the GTest itself, not
in the snippets passed from the codebase that uses GTest.
So after some thought, i decided not do issue a diagnostic if the semi
is within *any* macro, be it either from the normal header, or system header.
Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=39111 | PR39111 ]]
Reviewers: rsmith, aaron.ballman, efriedma
Reviewed By: aaron.ballman
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D52695
llvm-svn: 347339
2018-11-21 02:59:05 +08:00
|
|
|
;;;
|
2022-01-05 14:49:03 +08:00
|
|
|
// clang-format on
|
[clang][Parse] Diagnose useless null statements / empty init-statements
Summary:
clang has `-Wextra-semi` (D43162), which is not dictated by the currently selected standard.
While that is great, there is at least one more source of need-less semis - 'null statements'.
Sometimes, they are needed:
```
for(int x = 0; continueToDoWork(x); x++)
; // Ugly code, but the semi is needed here.
```
But sometimes they are just there for no reason:
```
switch(X) {
case 0:
return -2345;
case 5:
return 0;
default:
return 42;
}; // <- oops
;;;;;;;;;;; <- OOOOPS, still not diagnosed. Clearly this is junk.
```
Additionally:
```
if(; // <- empty init-statement
true)
;
switch (; // empty init-statement
x) {
...
}
for (; // <- empty init-statement
int y : S())
;
}
As usual, things may or may not go sideways in the presence of macros.
While evaluating this diag on my codebase of interest, it was unsurprisingly
discovered that Google Test macros are *very* prone to this.
And it seems many issues are deep within the GTest itself, not
in the snippets passed from the codebase that uses GTest.
So after some thought, i decided not do issue a diagnostic if the semi
is within *any* macro, be it either from the normal header, or system header.
Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=39111 | PR39111 ]]
Reviewers: rsmith, aaron.ballman, efriedma
Reviewed By: aaron.ballman
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D52695
llvm-svn: 347339
2018-11-21 02:59:05 +08:00
|
|
|
|
2022-01-05 14:49:03 +08:00
|
|
|
// clang-format off
|
[clang][Parse] Diagnose useless null statements / empty init-statements
Summary:
clang has `-Wextra-semi` (D43162), which is not dictated by the currently selected standard.
While that is great, there is at least one more source of need-less semis - 'null statements'.
Sometimes, they are needed:
```
for(int x = 0; continueToDoWork(x); x++)
; // Ugly code, but the semi is needed here.
```
But sometimes they are just there for no reason:
```
switch(X) {
case 0:
return -2345;
case 5:
return 0;
default:
return 42;
}; // <- oops
;;;;;;;;;;; <- OOOOPS, still not diagnosed. Clearly this is junk.
```
Additionally:
```
if(; // <- empty init-statement
true)
;
switch (; // empty init-statement
x) {
...
}
for (; // <- empty init-statement
int y : S())
;
}
As usual, things may or may not go sideways in the presence of macros.
While evaluating this diag on my codebase of interest, it was unsurprisingly
discovered that Google Test macros are *very* prone to this.
And it seems many issues are deep within the GTest itself, not
in the snippets passed from the codebase that uses GTest.
So after some thought, i decided not do issue a diagnostic if the semi
is within *any* macro, be it either from the normal header, or system header.
Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=39111 | PR39111 ]]
Reviewers: rsmith, aaron.ballman, efriedma
Reviewed By: aaron.ballman
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D52695
llvm-svn: 347339
2018-11-21 02:59:05 +08:00
|
|
|
;NULLMACRO(ZZ);
|
2022-01-05 14:49:03 +08:00
|
|
|
// clang-format on
|
[clang][Parse] Diagnose useless null statements / empty init-statements
Summary:
clang has `-Wextra-semi` (D43162), which is not dictated by the currently selected standard.
While that is great, there is at least one more source of need-less semis - 'null statements'.
Sometimes, they are needed:
```
for(int x = 0; continueToDoWork(x); x++)
; // Ugly code, but the semi is needed here.
```
But sometimes they are just there for no reason:
```
switch(X) {
case 0:
return -2345;
case 5:
return 0;
default:
return 42;
}; // <- oops
;;;;;;;;;;; <- OOOOPS, still not diagnosed. Clearly this is junk.
```
Additionally:
```
if(; // <- empty init-statement
true)
;
switch (; // empty init-statement
x) {
...
}
for (; // <- empty init-statement
int y : S())
;
}
As usual, things may or may not go sideways in the presence of macros.
While evaluating this diag on my codebase of interest, it was unsurprisingly
discovered that Google Test macros are *very* prone to this.
And it seems many issues are deep within the GTest itself, not
in the snippets passed from the codebase that uses GTest.
So after some thought, i decided not do issue a diagnostic if the semi
is within *any* macro, be it either from the normal header, or system header.
Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=39111 | PR39111 ]]
Reviewers: rsmith, aaron.ballman, efriedma
Reviewed By: aaron.ballman
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D52695
llvm-svn: 347339
2018-11-21 02:59:05 +08:00
|
|
|
|
|
|
|
{}; // expected-warning {{empty expression statement has no effect; remove unnecessary ';' to silence this warning}}
|
|
|
|
|
|
|
|
{
|
|
|
|
; // expected-warning {{empty expression statement has no effect; remove unnecessary ';' to silence this warning}}
|
|
|
|
}
|
|
|
|
|
|
|
|
if (true) {
|
|
|
|
; // expected-warning {{empty expression statement has no effect; remove unnecessary ';' to silence this warning}}
|
|
|
|
}
|
|
|
|
|
|
|
|
GOODMACRO(v0); // OK
|
|
|
|
|
|
|
|
GOODMACRO(v1;) // OK
|
|
|
|
|
|
|
|
BETTERMACRO(v2) // OK
|
|
|
|
|
|
|
|
BETTERMACRO(v3;) // Extra ';', but within macro expansion, so ignored.
|
|
|
|
|
|
|
|
BETTERMACRO(v4); // expected-warning {{empty expression statement has no effect; remove unnecessary ';' to silence this warning}}
|
|
|
|
|
|
|
|
BETTERMACRO(v5;); // expected-warning {{empty expression statement has no effect; remove unnecessary ';' to silence this warning}}
|
|
|
|
|
|
|
|
NULLMACRO(v6) // OK
|
|
|
|
|
|
|
|
NULLMACRO(v7); // OK. This could be either GOODMACRO() or BETTERMACRO() situation, so we can't know we can drop it.
|
|
|
|
|
|
|
|
if (true)
|
|
|
|
; // OK
|
|
|
|
|
|
|
|
while (true)
|
|
|
|
; // OK
|
|
|
|
|
|
|
|
do
|
|
|
|
; // OK
|
|
|
|
while (true);
|
|
|
|
|
|
|
|
for (;;) // OK
|
|
|
|
; // OK
|
|
|
|
|
|
|
|
MyEnum my_enum;
|
|
|
|
switch (my_enum) {
|
|
|
|
case E1:
|
|
|
|
// stuff
|
|
|
|
break;
|
|
|
|
case E2:; // OK
|
|
|
|
}
|
|
|
|
|
|
|
|
for (;;) {
|
|
|
|
for (;;) {
|
|
|
|
goto contin_outer;
|
|
|
|
}
|
|
|
|
contin_outer:; // OK
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
;
|
|
|
|
|
|
|
|
namespace NS {};
|
|
|
|
|
|
|
|
void foo(int x) {
|
|
|
|
switch (x) {
|
|
|
|
case 0:
|
|
|
|
[[fallthrough]];
|
|
|
|
case 1:
|
|
|
|
return;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
[[]];
|