[PATCH] D156277: [Parser][ObjC] Fix parser crash on nested top-level block with better recovery path

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: clang/docs/ReleaseNotes.rst:125
+  out early in case next top-level block occurs as if the previous one is
+  properly finished.
+  `Issue 64065 <https://github.com/llvm/llvm-project/issues/64065>`_
----------------
"Fixed a crash when parsing top-level ObjC blocks that aren't properly terminated.
Clang should now also recover better when an `@end` is missing between blocks."


================
Comment at: clang/lib/Parse/ParseObjc.cpp:725
+    case tok::objc_protocol:
+      goto MISSING_END;
+    default:
----------------
This use of `goto` is a little too spaghetti-ish for me — we're breaking out of a loop and then branching into one case of an `if`/`else` chain.  There are two reasonable ways to do this with more structure.  The first is that we can make sure that we emit the "missing @end" diagnostic before we break out; to avoid code repetition, you can extract that into a helper function.  The second is that we could set a flag to tell the code after the loop that we need to emit that diagnostic instead of trying to recreate the case analysis that we did in the loop.

If you make a little helper function like `isTopLevelObjCKeyword` then you can just `break` out instead of having the awkwardness of being inside a `switch`.


================
Comment at: clang/lib/Parse/ParseObjc.cpp:737
       break;
     } else if (DirectiveKind == tok::objc_not_keyword) {
       Diag(Tok, diag::err_objc_unknown_at);
----------------
I would suggest pulling this up to right after (or even before) the `isTopLevelObjCKeyword` check.  Then after the check you know that you're dealing with a keyword that you actually want to handle, and you  can consume both the `@` and the keyword token in one place.


================
Comment at: clang/lib/Parse/ParseObjc.cpp:827
   // EOF.  In the former case, eat the @end.  In the later case, emit an error.
   if (Tok.is(tok::code_completion)) {
     cutOffParsing();
----------------
I don't think this case is actually reachable — we handle code-completion within the loop and then return.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156277