Revert MSVC compatibility feature that breaks conformant code

[frederic-tingaud-sonarsource]

Summary

The fix at https://reviews.llvm.org/D124613 mimics MSVC behavior with friend declarations, but it does break some C++ conformant code that was built with -fms-compatibility. This fix reverts it to avoid causing more problems.

[thakis]

This can be closed, it landed elsewhere.

clang/lib/Sema/SemaDecl.cpp

if (D.getDeclSpec().isModulePrivateSpecified()) {
       } else {
         NewFD->setModulePrivate();
         if (FunctionTemplate)
           FunctionTemplate->setModulePrivate();
       }
     }
 
     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(PerformFriendInjection);
+        FunctionTemplate->setObjectOfFriendDecl();
         FunctionTemplate->setAccess(AS_public);
       }
-      NewFD->setObjectOfFriendDecl(PerformFriendInjection);
+      NewFD->setObjectOfFriendDecl();
       NewFD->setAccess(AS_public);
     }
 
     // If a function is defined as defaulted or deleted, mark it as such now.
     // We'll do the relevant checks on defaulted / deleted functions later.
     switch (D.getFunctionDefinitionKind()) {
     case FunctionDefinitionKind::Declaration:
     case FunctionDefinitionKind::Definition:

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

-// 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@+3 {{no member named 'funtemp' in namespace 'ns'}}
-    // modern-error@+2 {{expected '(' for function-style cast or type construction}}
-    // modern-error@+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() {
-}

clang/unittests/AST/ASTImporterTest.cpp

 TEST_P(ImportFriendFunctions, Lookup) {
   auto FunctionPattern = functionDecl(hasName("f"));
   auto ClassPattern = cxxRecordDecl(hasName("X"));
 
   TranslationUnitDecl *FromTU =
       getTuDecl("struct X { friend void f(); };", Lang_CXX03, "input0.cc");
   auto *FromD = FirstDeclMatcher<FunctionDecl>().match(FromTU, FunctionPattern);
   ASSERT_TRUE(FromD->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
-  // Before CXX20, MSVC treats friend function declarations as function
-  // declarations
-  ASSERT_EQ(FromTU->getLangOpts().MSVCCompat,
-            FromD->isInIdentifierNamespace(Decl::IDNS_Ordinary));
+  ASSERT_FALSE(FromD->isInIdentifierNamespace(Decl::IDNS_Ordinary));
   {
     auto FromName = FromD->getDeclName();
     auto *Class = FirstDeclMatcher<CXXRecordDecl>().match(FromTU, ClassPattern);
     auto LookupRes = Class->noload_lookup(FromName);
     ASSERT_TRUE(LookupRes.empty());
     LookupRes = FromTU->noload_lookup(FromName);
     ASSERT_TRUE(LookupRes.isSingleResult());
   }
TranslationUnitDecl *FromTU =
                 // returns the found `NamedDecl` only if the set IDNS is matched
                 "void f();",
                 Lang_CXX03, "input0.cc");
   auto *FromFriend =
       FirstDeclMatcher<FunctionDecl>().match(FromTU, FunctionPattern);
   auto *FromNormal =
       LastDeclMatcher<FunctionDecl>().match(FromTU, FunctionPattern);
   ASSERT_TRUE(FromFriend->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
-  // Before CXX20, MSVC treats friend function declarations as function
-  // declarations
-  ASSERT_EQ(FromTU->getLangOpts().MSVCCompat,
-            FromFriend->isInIdentifierNamespace(Decl::IDNS_Ordinary));
+  ASSERT_FALSE(FromFriend->isInIdentifierNamespace(Decl::IDNS_Ordinary));
   ASSERT_FALSE(FromNormal->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
   ASSERT_TRUE(FromNormal->isInIdentifierNamespace(Decl::IDNS_Ordinary));
 
   auto FromName = FromFriend->getDeclName();
   auto *FromClass =
       FirstDeclMatcher<CXXRecordDecl>().match(FromTU, ClassPattern);
   auto LookupRes = FromClass->noload_lookup(FromName);
   ASSERT_TRUE(LookupRes.empty());
TranslationUnitDecl *FromFriendTU =
       getTuDecl("class X { friend void f(); };", Lang_CXX03, "input1.cc");
   auto *FromFriendF =
       FirstDeclMatcher<FunctionDecl>().match(FromFriendTU, Pattern);
   auto FromNormalName = FromNormalF->getDeclName();
   auto FromFriendName = FromFriendF->getDeclName();
 
   ASSERT_TRUE(FromNormalF->isInIdentifierNamespace(Decl::IDNS_Ordinary));
   ASSERT_FALSE(FromNormalF->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
-  // Before CXX20, MSVC treats friend function declarations as function
-  // declarations
-  ASSERT_EQ(FromFriendTU->getLangOpts().MSVCCompat,
-            FromFriendF->isInIdentifierNamespace(Decl::IDNS_Ordinary));
+  ASSERT_FALSE(FromFriendF->isInIdentifierNamespace(Decl::IDNS_Ordinary));
   ASSERT_TRUE(FromFriendF->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend));
   auto LookupRes = FromNormalTU->noload_lookup(FromNormalName);
   ASSERT_TRUE(LookupRes.isSingleResult());
   LookupRes = FromFriendTU->noload_lookup(FromFriendName);
   ASSERT_TRUE(LookupRes.isSingleResult());
 
   auto *ToNormalF = cast<FunctionDecl>(Import(FromNormalF, Lang_CXX03));
   TranslationUnitDecl *ToTU = ToAST->getASTContext().getTranslationUnitDecl();