rsmith 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) {
----------------
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.
================
Comment at: clang/lib/Parse/ParseTentative.cpp:61
+ if (getLangOpts().CPlusPlus) {
+ if (isConstructorDeclarator(/*Unqualified=*/false,
+ /*DeductionGuide=*/false,
----------------
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.
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
