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