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