hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

This LGTM; thanks!

In D131541#3721383 <https://reviews.llvm.org/D131541#3721383>, @royjacobson 
wrote:

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

Maybe adding code to `TemplateDeclInstantiator::VisitFriendDecl` will do the 
trick.



================
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();
+};
----------------
royjacobson wrote:
> 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?
Okay; I guess "invalid destructor names were incorrectly accepted on template 
classes" uses "on" and not "in" (and can be read to match the current scope of 
the fix).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131541

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to