[clang] [clang][Sema] Fix false-positive -Wshadow for friend functions (PR #221249)

[ahmed mohamed kamel via cfe-commits]

================
@@ -8627,12 +8627,12 @@ void Sema::CheckShadow(NamedDecl *D, NamedDecl *ShadowedDecl,
   DeclContext *NewDC = D->getDeclContext();
 
   if (FieldDecl *FD = dyn_cast<FieldDecl>(ShadowedDecl)) {
-    if (const auto *MD =
-            dyn_cast<CXXMethodDecl>(getFunctionLevelDeclContext())) {
-      // Fields aren't shadowed in C++ static members or in member functions
-      // with an explicit object parameter.
+    DeclContext *FnDC = getFunctionLevelDeclContext();
+    if (const auto *MD = dyn_cast<CXXMethodDecl>(FnDC)) {
       if (MD->isStatic() || MD->isExplicitObjectMemberFunction())
         return;
+    } else if (isa<FunctionDecl>(FnDC)) {
+      return;
----------------
AhmedKamel10 wrote:

> There should be a comment to explain which case you intend to catch here

I added a comment explaining what that `else if ` catches.

> Even better would be if you can test explicitly for an inline-defined friend function here (but I don't know how to do that myself).

I went back and forth on this one. Currently doing something like `FnDC->getFriendObjectKind() != Decl::FOK_None` produces identical results, the reason is that the **only way** to get a plain `FunctionDecl` (not a `CXXMethodDecl`) via `getFunctionLevelDeclContext()` starting from something declared inside a class body is an inline-defined friend function. I kept `isa<FunctionDecl>` since it checks the real reason shadowing is impossible (no implicit this), rather than friend-ness specifically,  so it'd still be correct if some other non-member case ever got developed in the future. The downside is it's less obvious from the code alone that friends are what's being caught, which is why I added the comment. Happy to switch to the explicit check if you think it is better tho.

https://github.com/llvm/llvm-project/pull/221249