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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
