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

Matheus Izvekov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 18 12:32:58 PDT 2022


mizvekov added a comment.

In D129573#3659404 <https://reviews.llvm.org/D129573#3659404>, @inclyc wrote:

> Now it only generates 1 error encountering token "while"

Yes thank you, that is what I meant!

But now we just gotta make sure this stays true for more complex test cases.



================
Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:542
+def err_while_loop_outside_function : Error<
+  "while loop outside of function">; 
 def err_brackets_go_after_unqualified_id : Error<
----------------
Looking at other diagnostics which contain the word `outside`, there are many different ways to spell this.

Other possibilities include:
* while loop outside function
* while loop outside of a function
* while loop outside of a function body

All of these alternates could be justified with some prior art here.
I have no strong opinions either way, this seems fine, just making a note.


================
Comment at: clang/test/Parser/while-loop-outside-function.c:3
+
+while(1) {}; // expected-error {{while loop outside of function}}
+
----------------
You could break this down further into more lines to test exactly where the error is placed. Id expect it to be placed at the `while` keyword.


================
Comment at: clang/test/Parser/while-loop-outside-function.c:8
+
+void some_fn();
+
----------------
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?


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

https://reviews.llvm.org/D129573



More information about the cfe-commits mailing list