[PATCH] D131541: [Sema] Fix friend destructor declarations after D130936

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added reviewers: erichkeane, hubert.reinterpretcast, shafik.
royjacobson added a comment.

Updated the patch and added more test coverage.

I still don't diagnose the dependent friend case, but I didn't see how to do it easily.



================
Comment at: clang/lib/Sema/SemaDecl.cpp:11518-11520
+      // FIXME: We still don't diagnose on this case
+      // template <class T>
+      // struct A { friend T::S::~V(); };
----------------
hubert.reinterpretcast wrote:
> There's nothing we can diagnose about this without an instantiation (because `S` could be an alias for a class having `V` as the injected-class-name).
> 
> It is, however, true that we don't diagnose this even with problematic instantiations:
> ```
> struct R { struct V; ~R(); };
> struct QQ { using S = R; };
> 
> template <class T>
> struct A { friend T::S::~V(); };
> 
> A<QQ> a;
> ```
Wow, thanks! That's some edge case I haven't thought about..



================
Comment at: clang/test/SemaCXX/member-class-11.cpp:36-40
+// FIXME: We should diagnose here.
+template <typename T>
+struct E {
+  friend T::S::~V();
+};
----------------
hubert.reinterpretcast wrote:
> Please replace this with the case where there is an instantiation. Also, the prior change to the release notes in https://reviews.llvm.org/D130936 should be adjusted to reflect the new scope of what is fixed.
Updated the test cases accordingly.

I don't think there's things to add in the release notes as this is just fixing breakage from my previous patch, not really diagnosing new cases?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131541