[PATCH] D92024: [clang] Implement P0692R1 from C++20 (access checking on specializations and instantiations)

[Arthur O'Dwyer via Phabricator via cfe-commits]

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