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

[Kristina Bessonova via Phabricator via cfe-commits]

krisb added inline comments.


================
Comment at: clang/lib/Parse/ParseDecl.cpp:3080
+    // For instance this marked as unavailable:
+    //    class __attribute((unavailable)) UnavailableClass;`
+    auto RemoveAccessCheckingDiagnostics = [&TemplateInfo, this]() {
----------------
aorlov wrote:
> krisb wrote:
> > Basically, if `__attribute((unavailable))` should trigger the error for any use of an unavailable class, we have it already broken.
> > For example, for this code clang-12 doesn't produce any diagnostics:
> > ```
> > class __attribute((unavailable)) X {
> >   template <typename T> class __attribute((unavailable)) Y {};
> > };
> > class __attribute((unavailable)) A { 
> >     class __attribute((unavailable)) C {}; 
> > };
> > template <> class X::Y<A::C> {};
> > ```
> > So, I don't see much sense in inventing something new to workaround only the cases that come with this patch. It's better to either fix it globally or leave it broken atm with the corresponding FIXME.
> Anyway, I tried to remove the access diagnostics only. Who knows which diagnostics may be needed. Or you strongly prefer using `SuppressAccessChecks` instead?
Basically, I'd prefer consistency. Having two different ways of doing (almost) the same things doesn't look good to me. But I'm not sure about `SuppressAccessChecks` as turns all kinds of diagnostics off. Did you consider a possibility to improve `SuppressAccessChecks` to disable only a particular (specified) type of diagnostics? 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92024/new/

https://reviews.llvm.org/D92024