[PATCH] D149009: [Sema]Select correct lexical context during template instantiate

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added inline comments.


================
Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:3598
+      FD->isDefined(FDFriend, true) &&
+      FDFriend->getFriendObjectKind() != Decl::FriendObjectKind::FOK_None) {
+    // if Function defined by inline friend, use inline fried as DeclContext
----------------
erichkeane wrote:
> HerrCai0907 wrote:
> > erichkeane wrote:
> > > HerrCai0907 wrote:
> > > > erichkeane wrote:
> > > > > So in what case would the currently-instantiated definition NOT also be a friend?  I would think this last condition should be able to be an assert instead.
> > > > Last condition cannot be an assert, define and declare in difference place is common case, what we need to identifier in here is inlined friend define.
> > > Can you be more clear here?  WHEN can a definition and declaration NOT have the same friend object kind?  THAT is probably incorrect a bug.
> > Sorry I cannot get the point. 
> > Here I have 3 conditions:
> > 1. FD->getFriendObjectKind() == Decl::FriendObjectKind::FOK_None
> > FD(declaration) is not friend object.
> > 2. FD->isDefined(FDFriend, true)
> > get FDFriend(definition) from FD(declaration).
> > 3. FDFriend->getFriendObjectKind() != Decl::FriendObjectKind::FOK_None)
> > FDFriend(definition) is friend object.
> > 
> > matching those 3 condition and then we can say FDFriend is a inline friend like
> > ```
> > template <class F1> int foo(F1 X); // FD
> > template <int A1> struct A {
> > template <class F1> friend int foo(F1 X) { return A1; } // FDFriend
> > };
> > ```
> Ok, my question is: WHEN is #3 "false" but #1 and #2 are "true"?  It should not be possible for #1 to be "true" but #3 to be "false", as far as I know.
It looks like `getFriendObjectKind()` returns a declaration specific value such that, for the example given, it returns `FOK_None` for the first declaration and `FOK_Declared` for the friend definition. (I haven't tested that, but that is what my brief reading of the code suggests).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149009