ayzhao marked an inline comment as done.
ayzhao added inline comments.
================
Comment at: clang/lib/Parse/ParseDecl.cpp:5100-5101
bool IsConstructor = false;
- if (isDeclarationSpecifier())
+ if (isDeclarationSpecifier(ITC_Never))
IsConstructor = true;
else if (Tok.is(tok::identifier) ||
----------------
rsmith wrote:
> Rakete1111 wrote:
> > rsmith wrote:
> > > Oh, this could be a problem.
> > >
> > > If this *is* a constructor declaration, then this is implicit typename
> > > context: this is either a "//parameter-declaration// in a
> > > //member-declaration//" ([temp.res]/5.2.3) or a
> > > "//parameter-declaration// in a //declarator// of a function or function
> > > template declaration whose //declarator-id// is qualified". But if it's
> > > *not* a constructor declaration, then this is either the
> > > //declarator-id// of a declaration or the //nested-name-specifier// of a
> > > pointer-to-member declarator:
> > >
> > > ```
> > > template<typename T>
> > > struct C {
> > > C(T::type); // implicit typename context
> > > friend C (T::fn)(); // not implicit typename context, declarator-id of
> > > friend declaration
> > > C(T::type::*x)[3]; // not implicit typename context, pointer-to-member
> > > type
> > > };
> > > ```
> > >
> > > I think we need to use `ITC_Yes` here, in order to correctly disambiguate
> > > the first example above. Please add tests for the other two cases to make
> > > sure this doesn't break them -- but I'm worried this **will** break the
> > > second case, because it will incorrectly annotate `T::fn` as a type.
> > Yeah it does the break the second. Would just checking whether a `friend`
> > is there be good enough? clang doesn't seem to actively propose to add a
> > friend if there's one missing, so if we add a `IsFriend` parameter and then
> > check if it is set than always return `false`, it should work. Is that
> > acceptable? It would break the nice error message you get if you write
> > `friend C(int);`, but if we restrict it when `isDeclarationSpecifier`
> > return false (with `Never`) would that be better (that's what I'm doing
> > right now)?
> >
> > Another solution would be to tentatively parse the whole thing, but that
> > can be pretty expensive I believe.
> This seems a little fragile against future grammar changes, but I think the
> `IsFriend` check is correct -- I *think* the only way we can see a
> //qualified-id// here in a valid non-constructor, non-nested-name-specifier
> case is in a friend declaration.
I _think_ the first comment in this chain can be marked as done given that the
test cases are now in `p5.cpp` and Clang compiles them without errors with the
help of the `IsFriend` check.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D53847/new/
https://reviews.llvm.org/D53847
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
