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

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.


================
Comment at: clang/lib/Parse/ParseDecl.cpp:5555-5568
+  if (SS.isNotEmpty() && SS.getScopeRep()) {
+    NestedNameSpecifier *NNS = SS.getScopeRep();
+    if (NNS->getAsNamespace() || NNS->getAsNamespaceAlias()) {
+      TPA.Revert();
+      return false;
+    }
+    if (CtorII) {
----------------
rsmith wrote:
> I don't think any of this should be necessary. Per the function comment, it's a precondition of this function that we're looking at either `ClassName` or `ClassNameScope::ClassName` and we shouldn't be re-checking that.
> 
> I also think we should not be returning `true` here if we see `ClassName::ClassName` without looking at the rest of the tokens. That'll cause diagnostic quality regressions like the one you're seeing. We should continue to the rest of this function to see if we have `(Type` or `()` next.
Indeed. I've reverted that and I did not need to have changes in this routine anymore. Thanks!!


================
Comment at: clang/lib/Parse/ParseTentative.cpp:61
+      if (getLangOpts().CPlusPlus) {
+        if (isConstructorDeclarator(/*Unqualified=*/false,
+                                    /*DeductionGuide=*/false,
----------------
rsmith wrote:
> v.g.vassilev wrote:
> > aaron.ballman wrote:
> > > Do you need special handling for:
> > > ```
> > > struct S {
> > >   operator int();
> > > };
> > > 
> > > S::operator int() { return 0; }
> > > ```
> > We seem to need that. Thanks. I've added a FIXME.
> I think you should check `isCurrentClassName` before calling this, like the other callers of this function do. My understanding is that the intent is for `isConstructorDeclarator` to only check the (potential) function declarator that appears after the name, not to check the name itself.
Ah, okay, that was the missing bit.


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

https://reviews.llvm.org/D127284