Implement DR329. We already did the right thing here in C++98 mode, but r104014

(which implemented the DR) was disabled in C++11.

llvm-svn: 200673

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -3586,9 +3586,6 @@ def err_definition_of_explicitly_defaulted_member : Error<
 def err_redefinition_extern_inline : Error<
   "redefinition of a 'extern inline' function %0 is not supported in "
   "%select{C99 mode|C++}1">;
-def warn_cxx98_compat_friend_redefinition : Warning<
-  "friend function %0 would be implicitly redefined in C++98">,
-  InGroup<CXX98Compat>, DefaultIgnore;
 
 def note_deleted_dtor_no_operator_delete : Note<
   "virtual destructor requires an unambiguous, accessible 'operator delete'">;

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

@@ -1225,7 +1225,6 @@ static QualType adjustFunctionTypeForInstantiation(ASTContext &Context,
 /// don't make it here.  This function serves two purposes:
 ///   1) instantiating function templates
 ///   2) substituting friend declarations
-/// FIXME: preserve function definitions in case #2
 Decl *TemplateDeclInstantiator::VisitFunctionDecl(FunctionDecl *D,
                                        TemplateParameterList *TemplateParams) {
   // Check whether there is already a function template specialization for
@@ -1435,35 +1434,22 @@ Decl *TemplateDeclInstantiator::VisitFunctionDecl(FunctionDecl *D,
     PrincipalDecl->setObjectOfFriendDecl();
     DC->makeDeclVisibleInContext(PrincipalDecl);
 
-    bool queuedInstantiation = false;
+    bool QueuedInstantiation = false;
 
-    // C++98 [temp.friend]p5: When a function is defined in a friend function
-    //   declaration in a class template, the function is defined at each
-    //   instantiation of the class template. The function is defined even if it
-    //   is never used.
-    // C++11 [temp.friend]p4: When a function is defined in a friend function
-    //   declaration in a class template, the function is instantiated when the
-    //   function is odr-used.
-    //
-    // If -Wc++98-compat is enabled, we go through the motions of checking for a
-    // redefinition, but don't instantiate the function.
-    if ((!SemaRef.getLangOpts().CPlusPlus11 ||
-         SemaRef.Diags.getDiagnosticLevel(
-             diag::warn_cxx98_compat_friend_redefinition,
-             Function->getLocation())
-           != DiagnosticsEngine::Ignored) &&
-        D->isThisDeclarationADefinition()) {
+    // C++11 [temp.friend]p4 (DR329):
+    //   When a function is defined in a friend function declaration in a class
+    //   template, the function is instantiated when the function is odr-used.
+    //   The same restrictions on multiple declarations and definitions that
+    //   apply to non-template function declarations and definitions also apply
+    //   to these implicit definitions.
+    if (D->isThisDeclarationADefinition()) {
       // Check for a function body.
       const FunctionDecl *Definition = 0;
       if (Function->isDefined(Definition) &&
           Definition->getTemplateSpecializationKind() == TSK_Undeclared) {
-        SemaRef.Diag(Function->getLocation(),
-                     SemaRef.getLangOpts().CPlusPlus11 ?
-                       diag::warn_cxx98_compat_friend_redefinition :
-                       diag::err_redefinition) << Function->getDeclName();
+        SemaRef.Diag(Function->getLocation(), diag::err_redefinition)
+            << Function->getDeclName();
         SemaRef.Diag(Definition->getLocation(), diag::note_previous_definition);
-        if (!SemaRef.getLangOpts().CPlusPlus11)
-          Function->setInvalidDecl();
       }
       // Check for redefinitions due to other instantiations of this or
       // a similar friend function.
@@ -1472,36 +1458,34 @@ Decl *TemplateDeclInstantiator::VisitFunctionDecl(FunctionDecl *D,
                 R != REnd; ++R) {
         if (*R == Function)
           continue;
-        switch (R->getFriendObjectKind()) {
-        case Decl::FOK_None:
-          if (!SemaRef.getLangOpts().CPlusPlus11 &&
-              !queuedInstantiation && R->isUsed(false)) {
-            if (MemberSpecializationInfo *MSInfo
-                = Function->getMemberSpecializationInfo()) {
-              if (MSInfo->getPointOfInstantiation().isInvalid()) {
-                SourceLocation Loc = R->getLocation(); // FIXME
-                MSInfo->setPointOfInstantiation(Loc);
-                SemaRef.PendingLocalImplicitInstantiations.push_back(
-                                                 std::make_pair(Function, Loc));
-                queuedInstantiation = true;
-              }
+
+        // If some prior declaration of this function has been used, we need
+        // to instantiate its definition.
+        if (!QueuedInstantiation && R->isUsed(false)) {
+          if (MemberSpecializationInfo *MSInfo =
+                  Function->getMemberSpecializationInfo()) {
+            if (MSInfo->getPointOfInstantiation().isInvalid()) {
+              SourceLocation Loc = R->getLocation(); // FIXME
+              MSInfo->setPointOfInstantiation(Loc);
+              SemaRef.PendingLocalImplicitInstantiations.push_back(
+                                               std::make_pair(Function, Loc));
+              QueuedInstantiation = true;
             }
           }
-          break;
-        default:
-          if (const FunctionDecl *RPattern
-              = R->getTemplateInstantiationPattern())
+        }
+
+        // If some prior declaration of this function was a friend with an
+        // uninstantiated definition, reject it.
+        if (R->getFriendObjectKind()) {
+          if (const FunctionDecl *RPattern =
+                  R->getTemplateInstantiationPattern()) {
             if (RPattern->isDefined(RPattern)) {
-              SemaRef.Diag(Function->getLocation(),
-                           SemaRef.getLangOpts().CPlusPlus11 ?
-                             diag::warn_cxx98_compat_friend_redefinition :
-                             diag::err_redefinition)
+              SemaRef.Diag(Function->getLocation(), diag::err_redefinition)
                 << Function->getDeclName();
               SemaRef.Diag(R->getLocation(), diag::note_previous_definition);
-              if (!SemaRef.getLangOpts().CPlusPlus11)
-                Function->setInvalidDecl();
               break;
             }
+          }
         }
       }
     }

clang/test/CXX/drs/dr0xx.cpp

@@ -464,23 +464,15 @@ namespace dr46 { // dr46: yes
   template template struct A<int>::B<int>; // expected-error {{expected unqualified-id}}
 }
 
-namespace dr47 { // dr47: no
+namespace dr47 { // dr47: sup 329
   template<typename T> struct A {
-    friend void f() { T t; }
+    friend void f() { T t; } // expected-error {{redefinition}} expected-note {{previous}}
   };
   A<int> a;
-  A<float> b;
-#if __cplusplus < 201103L
-  // expected-error@-5 {{redefinition}} expected-note@-5 {{previous}}
-  // expected-note@-3 {{instantiation of}}
-#else
+  A<float> b; // expected-note {{instantiation of}}
+
   void f();
-  // FIXME: We should produce some kind of error here. C++11 [temp.friend]p4
-  // says we instantiate 'f' when it's odr-used, but that doesn't imply that
-  // this is valid; we still have multiple definitions of 'f' even if we never
-  // instantiate any of them.
   void g() { f(); }
-#endif
 }
 
 namespace dr48 { // dr48: yes

clang/test/CXX/drs/dr3xx.cpp

@@ -316,25 +316,17 @@ namespace dr328 { // dr328: yes
   A *p = new A[0]; // expected-error {{incomplete}}
 }
 
-namespace dr329 { // dr329: no
-  // FIXME: The C++98 behavior here is right, the C++11-onwards behavior
-  // is wrong.
+namespace dr329 { // dr329: 3.5
   struct B {};
   template<typename T> struct A : B {
     friend void f(A a) { g(a); }
     friend void h(A a) { g(a); } // expected-error {{undeclared}}
-    friend void i(B b) {}
+    friend void i(B b) {} // expected-error {{redefinition}} expected-note {{previous}}
   };
   A<int> a;
-  A<char> b;
-#if __cplusplus < 201103L
-  // expected-error@-5 {{redefinition}} expected-note@-5 {{previous}}
-  // expected-note@-3 {{instantiation}}
-#endif
+  A<char> b; // expected-note {{instantiation}}
 
   void test() {
     h(a); // expected-note {{instantiation}}
-    i(a);
-    i(b);
   }
 }

clang/test/SemaCXX/cxx98-compat.cpp

@@ -225,13 +225,6 @@ template<typename T> typename T::ImPrivate SFINAEAccessControl(T t) { // expecte
 int SFINAEAccessControl(...) { return 0; }
 int CheckSFINAEAccessControl = SFINAEAccessControl(PrivateMember()); // expected-note {{while substituting deduced template arguments into function template 'SFINAEAccessControl' [with T = PrivateMember]}}
 
-template<typename T>
-struct FriendRedefinition {
-  friend void Friend() {} // expected-warning {{friend function 'Friend' would be implicitly redefined in C++98}} expected-note {{previous}}
-};
-FriendRedefinition<int> FriendRedef1;
-FriendRedefinition<char> FriendRedef2; // expected-note {{requested here}}
-
 namespace CopyCtorIssues {
   struct Private {
     Private();

clang/www/cxx_dr_status.html

@@ -321,7 +321,7 @@
     <td><a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_closed.html#47">47</a></td>
     <td>NAD</td>
     <td>Template friend issues</td>
-    <td class="none" align="center">No</td>
+    <td class="svn" align="center">Superseded by 329</td>
   </tr>
   <tr>
     <td><a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#48">48</a></td>
@@ -2015,7 +2015,7 @@ of class templates</td>
     <td><a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#329">329</a></td>
     <td>CD1</td>
     <td>Evaluation of friends of templates</td>
-    <td class="none" align="center">No</td>
+    <td class="svn" align="center">SVN</td>
   </tr>
   <tr class="open">
     <td><a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#330">330</a></td>