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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to