Quuxplusone added inline comments.
================
Comment at: clang/lib/Parse/ParseTemplate.cpp:172
+
+ // TODO. This can produce wrong detection in case of a later class
+ // declaration. Example:
----------------
I don't know the purpose of this code, but this //seems// like a super
important TODO.
The comment "look ahead until `{`" is also scary because
```
template<int I, int J> struct A {};
template<int J> struct A<int{}, J> {};
```
is also a partial specialization. Why do we need to know `isSpecialization`,
who's //supposed// to compute it, and why does their answer need to be fixed-up
right here?
And is this specific piece of the patch perhaps severable into a separate
review? Again, I don't know this code, but... It seems like you've got one part
of the patch — "add a `SuppressAccessGuard` around the call to
`ParseDeclarator`" — which is either a 100% correct or 100% incorrect
implementation of P0692R1. And then you've got this other piece, a parser hack,
which looks much more heuristic and incomplete in nature, and looks orthogonal
to P0692R1.
Btw, I'm not an approver on this (and shouldn't be); if you want better review
you might want to ping someone who's touched this code lately (according to
`git blame`).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92024/new/
https://reviews.llvm.org/D92024
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
