[PATCH] D129573: [Clang] add a diagnostic note 'while loop outside functions' at global scope
YingChi Long via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 18 18:00:10 PDT 2022
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
More information about the cfe-commits
mailing list