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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
