[PATCH] D129573: [Clang] add a diagnostic note 'while loop outside functions' at global scope

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.


================
Comment at: clang/test/Parser/while-loop-outside-function.c:8
+
+void some_fn();
+
----------------
inclyc wrote:
> mizvekov wrote:
> > Can you add a few more test cases showing how error recovery is performing here?
> > 
> > Have we parsed this function declaration at all, or were we skipping until the next ';'?
> > 
> > What happens if there are (multiple) statements in the loop's block?
> > 
> > How do we handle a `do ... while()` loop instead?
> > 
> > Does it make a difference if the loop contains a block or a (possibly empty) single statement?
> For this occasion, 
> 
> ```
> // RUN: %clang_cc1 -fsyntax-only -verify %s
> 
> while(true) {}; // expected-error {{while loop outside of function}}
> 
> // without semicolon
> while(true) {} // expected-error {{while loop outside of function}}
> 
> 
> while(true) {
>     // some statements
>     int some_var = 3;
>     some_var += 2;
> }
> 
> while(true) 
> {
>     // some statements
>     int some_var = 3;
>     some_var += 2;
> }
> 
> do {
>     int some_var = 1;
>     some_var += 3;
> } 
> while(true);
> 
> void someFunction() {
>     while(true) {};
> }
> 
> class SomeClass {
> public:
>     while(true) {}
>     void some_fn() {
>         while(true) {}
>     }
> };
> 
> ```
> 
> ```
> ./clang/test/Parser/while-loop-outside-function.cpp:3:1: error: while loop outside of function
> while(true) {}; // expected-error {{while loop outside of function}}
> ^
> ./clang/test/Parser/while-loop-outside-function.cpp:6:1: error: while loop outside of function
> while(true) {} // expected-error {{while loop outside of function}}
> ^
> ./clang/test/Parser/while-loop-outside-function.cpp:9:1: error: while loop outside of function
> while(true) {
> ^
> ./clang/test/Parser/while-loop-outside-function.cpp:15:1: error: while loop outside of function
> while(true) 
> ^
> ./clang/test/Parser/while-loop-outside-function.cpp:22:1: error: expected unqualified-id
> do {
> ^
> ./clang/test/Parser/while-loop-outside-function.cpp:26:1: error: while loop outside of function
> while(true);
> ^
> ./clang/test/Parser/while-loop-outside-function.cpp:34:5: error: expected member name or ';' after declaration specifiers
>     while(true) {}
>     ^
> 7 errors generated.
> ```
> 
> The parser generates the newly added error for every "top-level" declarator. For `do ... while(...)` loops it generates two errors. I think ideally only one error should be reported, only generating an error at the latter while token, without generating a "noise" at the preceding `do`. And, this patch doesn't seem to produce good results where classes, structs, etc. are also not part of the function body.
> 
> At first, when I solved this issue, I thought he only needed to report the appearance of the top-level while loop, without considering do-while, in a class, or in a structure. So the current code is only in Parser, this error is reported when the `while` token is encountered parsing declarators.
I see, thanks for the explanations.

I think this patch is a very simple change that brings some improvement, and apparently it does not regress anything or cause any unfortunate error recovery.

You don't really need to improve these other cases in the same patch, I am just checking that this was all considered :-)

But please do add these test cases to the patch!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129573/new/

https://reviews.llvm.org/D129573