[PATCH] D127284: [WIP] [clang-repl] Support statements on global scope in incremental mode.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: clang/lib/Parse/ParseDecl.cpp:5247-5248
+  // ObjC
+  case tok::at:
+    return getLangOpts().ObjC;
+
----------------
In Objective-C, both declarations and expressions can start with `@`. In general we'd need to look at the next token to see what we have. But I'm not sure if there are any ObjC decl specifiers that use `@`-prefixed keywords.


================
Comment at: clang/lib/Parse/ParseDecl.cpp:5259-5260
 
+  case tok::kw_asm:
+    return true;
+
----------------
`asm` isn't a decl-specifier, so I think it should probably be handled at a different level.


================
Comment at: clang/lib/Parse/ParseDecl.cpp:5514-5521
+  // FIXME: this returns true for NS::f();
   // A right parenthesis, or ellipsis followed by a right parenthesis signals
   // that we have a constructor.
   if (Tok.is(tok::r_paren) ||
       (Tok.is(tok::ellipsis) && NextToken().is(tok::r_paren))) {
     TPA.Revert();
     return true;
----------------
I think we'll need to work out whether we're disambiguating versus a statement here. I think the key observation is that either:

1) We know we're parsing a declaration, because we're inside a class body or parsing after `template` or `template<...>` or similar.
2) A constructor declaration must be a definition, so if we don't see a `{`, `:`, or `try` after the declarator then it's an expression, not a declaration.

I'd suggest passing in a flag to say whether we know we're parsing a declaration, which is true unless we're disambiguating against a top-level statement; in that case, don't return here and instead keep going until we hit one of the relevant cases below.


================
Comment at: clang/lib/Parse/ParseTentative.cpp:77
+  case tok::identifier:
+    if (getLangOpts().CPlusPlus) {
+      // FIXME: This is quite broken as it considers f() from void f(){}; f();
----------------
Please only do this extra check when disambiguating against a top-level statement. I don't think we can do perfectly-correct disambiguation here, and it seems important that we don't regress our behavior or performance for the normal and very common case of parsing a function body where a statement starts with an identifier.


================
Comment at: clang/lib/Parse/ParseTentative.cpp:1428-1429
+  case tok::at:
+    if (getLangOpts().ObjC)
+      return TPResult::True;
+    LLVM_FALLTHROUGH;
----------------
As before, this should depend on what token follows the `@`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D127284