Rakete1111 added inline comments.
================
Comment at: clang/lib/Parse/ParseDecl.cpp:4321
+ isCXXDeclarationSpecifier(ITC_Never, TPResult::True) !=
+ TPResult::True) ||
+ (!getLangOpts().CPlusPlus && !isDeclarationSpecifier(ITC_Yes))) {
----------------
rsmith wrote:
> It seems like a wording oversight that we don't assume `typename` in an
> //enum-base//. Probably would be good to raise this on the core reflector.
Do you know why this isn't allowed in `operator` ids?
================
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:
> 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.
================
Comment at: clang/lib/Sema/SemaTemplate.cpp:3377-3379
// FIXME: This is not quite correct recovery as we don't transform SS
// into the corresponding dependent form (and we don't diagnose missing
// 'template' keywords within SS as a result).
----------------
rsmith wrote:
> This FIXME is concerning. Is this a problem with this patch? (Is the FIXME
> wrong now?)
Yes you're right, I believe it not longer applies due to the change in
ParseDecl.cpp in ParseDeclarationSpecifiers.
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
