inclyc added inline comments.
================
Comment at: clang/test/Parser/while-loop-outside-function.c:8
+
+void some_fn();
+
----------------
mizvekov wrote:
> 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!
Do you think we need to change something more to accurately report `while` /
`do-while` / `for` loops, and only report errors outside of a reasonable
control flow?
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
