[clang] ad47114 - In MSVC compatibility mode, friend function declarations behave as function declarations

[Marco Antognini via cfe-commits]

Author: Fred Tingaud
Date: 2022-05-03T11:31:50+02:00
New Revision: ad47114ad8500c78046161d492ac13a8e3e610eb

URL: https://github.com/llvm/llvm-project/commit/ad47114ad8500c78046161d492ac13a8e3e610eb
DIFF: https://github.com/llvm/llvm-project/commit/ad47114ad8500c78046161d492ac13a8e3e610eb.diff

LOG: In MSVC compatibility mode, friend function declarations behave as function declarations

Before C++20, MSVC treated any friend function declaration as a function declaration, so the following code would compile despite funGlob being declared after its first call:

```
class Glob {
public:
  friend void funGlob();

  void test() {
    funGlob();
  }
};

void funGlob() {}
```
This proposed patch mimics the MSVC behavior when in MSVC compatibility mode

Reviewed By: rnk

Differential Revision: https://reviews.llvm.org/D124613

Added: 
    clang/test/SemaCXX/ms-friend-function-decl.cpp

Modified: 
    clang/lib/Sema/SemaDecl.cpp
    clang/unittests/AST/ASTImporterTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 5890bbc7d574b..f2b87c6d2e37e 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -9632,11 +9632,15 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
     }
 
     if (isFriend) {
+      // In MSVC mode for older versions of the standard, friend function
+      // declarations behave as declarations
+      bool PerformFriendInjection =
+          getLangOpts().MSVCCompat && !getLangOpts().CPlusPlus20;
       if (FunctionTemplate) {
-        FunctionTemplate->setObjectOfFriendDecl();
+        FunctionTemplate->setObjectOfFriendDecl(PerformFriendInjection);
         FunctionTemplate->setAccess(AS_public);
       }
-      NewFD->setObjectOfFriendDecl();
+      NewFD->setObjectOfFriendDecl(PerformFriendInjection);
       NewFD->setAccess(AS_public);
     }
 

diff  --git a/clang/test/SemaCXX/ms-friend-function-decl.cpp b/clang/test/SemaCXX/ms-friend-function-decl.cpp
new file mode 100644
index 0000000000000..d146305800738
--- /dev/null
+++ b/clang/test/SemaCXX/ms-friend-function-decl.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -std=c++03 -fms-compatibility -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++17 -fms-compatibility -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++20 -fms-compatibility -fsyntax-only -verify=modern %s
+#if __cplusplus < 202002L
+// expected-no-diagnostics
+#endif
+
+namespace ns {
+
+class C {
+public:
+  template <typename T>
+  friend void funtemp();
+
+  friend void fun();
+
+  void test() {
+    ::ns::fun(); // modern-error {{no member named 'fun' in namespace 'ns'}}
+
+    // modern-error at +3 {{no member named 'funtemp' in namespace 'ns'}}
+    // modern-error at +2 {{expected '(' for function-style cast or type construction}}
+    // modern-error at +1 {{expected expression}}
+    ::ns::funtemp<int>();
+  }
+};
+
+void fun() {
+}
+
+template <typename T>
+void funtemp() {}
+
+} // namespace ns
+
+class Glob {
+public:
+  friend void funGlob();
+
+  void test() {
+    funGlob(); // modern-error {{use of undeclared identifier 'funGlob'}}
+  }
+};
+
+void funGlob() {
+}

diff  --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp
index 856010cd4d036..2cda013a45edc 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -2658,7 +2658,10 @@ TEST_P(ImportFriendFunctions, Lookup) {
       getTuDecl("struct X { friend void f(); };", Lang_CXX03, "input0.cc");
   auto *FromD = FirstDeclMatcher<FunctionDecl>().match(FromTU, FunctionPattern);
   ASSERT_TRUE(FromD->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
-  ASSERT_FALSE(FromD->isInIdentifierNamespace(Decl::IDNS_Ordinary));
+  // Before CXX20, MSVC treats friend function declarations as function
+  // declarations
+  ASSERT_EQ(FromTU->getLangOpts().MSVCCompat,
+            FromD->isInIdentifierNamespace(Decl::IDNS_Ordinary));
   {
     auto FromName = FromD->getDeclName();
     auto *Class = FirstDeclMatcher<CXXRecordDecl>().match(FromTU, ClassPattern);
@@ -2702,7 +2705,10 @@ TEST_P(ImportFriendFunctions, LookupWithProtoAfter) {
   auto *FromNormal =
       LastDeclMatcher<FunctionDecl>().match(FromTU, FunctionPattern);
   ASSERT_TRUE(FromFriend->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
-  ASSERT_FALSE(FromFriend->isInIdentifierNamespace(Decl::IDNS_Ordinary));
+  // Before CXX20, MSVC treats friend function declarations as function
+  // declarations
+  ASSERT_EQ(FromTU->getLangOpts().MSVCCompat,
+            FromFriend->isInIdentifierNamespace(Decl::IDNS_Ordinary));
   ASSERT_FALSE(FromNormal->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
   ASSERT_TRUE(FromNormal->isInIdentifierNamespace(Decl::IDNS_Ordinary));
 
@@ -2793,7 +2799,10 @@ TEST_P(ImportFriendFunctions, ImportFriendChangesLookup) {
 
   ASSERT_TRUE(FromNormalF->isInIdentifierNamespace(Decl::IDNS_Ordinary));
   ASSERT_FALSE(FromNormalF->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
-  ASSERT_FALSE(FromFriendF->isInIdentifierNamespace(Decl::IDNS_Ordinary));
+  // Before CXX20, MSVC treats friend function declarations as function
+  // declarations
+  ASSERT_EQ(FromFriendTU->getLangOpts().MSVCCompat,
+            FromFriendF->isInIdentifierNamespace(Decl::IDNS_Ordinary));
   ASSERT_TRUE(FromFriendF->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
   auto LookupRes = FromNormalTU->noload_lookup(FromNormalName);
   ASSERT_TRUE(LookupRes.isSingleResult());