<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 17 April 2017 at 14:41, Vassil Vassilev <span dir="ltr"><<a href="mailto:v.g.vassilev@gmail.com" target="_blank">v.g.vassilev@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
  
    
  
  <div bgcolor="#FFFFFF" text="#000000">
    <div class="m_2112800568306230035moz-cite-prefix">+ Richard<br>
      <br>
      Thanks for the example. We've seen similar issue with inlines
      (without the reverted patch). My guess this patch exposed it.<br>
      <br>
      It is not clear to me why we think the declaration is a definition
      there...</div></div></blockquote><div><br></div><div>Well, the declaration is a definition. It's definitely odd that we have a note pointing to an inline definition and claiming it's non-inline; I'd guess the source location and the "not inline" information are taken from two different declarations that are implicitly expected to be consistent. But I don't know why redeclaration lookup in the second module would ever find a non-inline declaration.</div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000"><div class="m_2112800568306230035moz-cite-prefix"><div><div class="h5">
      On 17/04/17 23:10, Benjamin Kramer via cfe-commits wrote:<br>
    </div></div></div><div><div class="h5">
    <blockquote type="cite">
      <pre>This broke our internal build of libc++ with modules. Reduced test
case attached, courtesy of Richard Smith!

With your patch it doesn't compiler anymore:
While building module 'x':
In file included from <module-includes>:2:
In file included from ./c.h:1:
./a.h:3:32: error: inline declaration of 'f' follows non-inline definition
template<typename> inline void f() {}
                               ^
./a.h:3:32: note: previous definition is here
template<typename> inline void f() {}


I reverted this change in r300497.

On Mon, Apr 17, 2017 at 10:51 AM, Yaron Keren via cfe-commits
<a class="m_2112800568306230035moz-txt-link-rfc2396E" href="mailto:cfe-commits@lists.llvm.org" target="_blank"><cfe-commits@lists.llvm.org></a> wrote:
</pre>
      <blockquote type="cite">
        <pre>Author: yrnkrn
Date: Mon Apr 17 03:51:20 2017
New Revision: 300443

URL: <a class="m_2112800568306230035moz-txt-link-freetext" href="http://llvm.org/viewvc/llvm-project?rev=300443&view=rev" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=300443&view=rev</a>
Log:
Address <a class="m_2112800568306230035moz-txt-link-freetext" href="http://bugs.llvm.org/pr30994" target="_blank">http://bugs.llvm.org/pr30994</a> so that a non-friend can properly replace a friend, and a visible friend can properly replace an invisible friend but not vice verse, and definitions are not replaced. This fixes the two FIXME in SemaTemplate/friend-template.<wbr>cpp.

The code implements Richard Smith suggestion in comment 3 of the PR.

reviewer: Vassil Vassilev

Differential Revision: <a class="m_2112800568306230035moz-txt-link-freetext" href="https://reviews.llvm.org/D31540" target="_blank">https://reviews.llvm.org/<wbr>D31540</a>


Modified:
    cfe/trunk/include/clang/AST/<wbr>DeclBase.h
    cfe/trunk/lib/AST/Decl.cpp
    cfe/trunk/lib/AST/DeclBase.cpp
    cfe/trunk/test/SemaTemplate/<wbr>friend-template.cpp

Modified: cfe/trunk/include/clang/AST/<wbr>DeclBase.h
URL: <a class="m_2112800568306230035moz-txt-link-freetext" href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=300443&r1=300442&r2=300443&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/include/<wbr>clang/AST/DeclBase.h?rev=<wbr>300443&r1=300442&r2=300443&<wbr>view=diff</a>
==============================<wbr>==============================<wbr>==================
--- cfe/trunk/include/clang/AST/<wbr>DeclBase.h (original)
+++ cfe/trunk/include/clang/AST/<wbr>DeclBase.h Mon Apr 17 03:51:20 2017
@@ -417,6 +417,8 @@ public:
     return const_cast<Decl*>(this)-><wbr>getTranslationUnitDecl();
   }

+  bool isThisDeclarationADefinition() const;
+
   bool isInAnonymousNamespace() const;

   bool isInStdNamespace() const;

Modified: cfe/trunk/lib/AST/Decl.cpp
URL: <a class="m_2112800568306230035moz-txt-link-freetext" href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=300443&r1=300442&r2=300443&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/AST/<wbr>Decl.cpp?rev=300443&r1=300442&<wbr>r2=300443&view=diff</a>
==============================<wbr>==============================<wbr>==================
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Mon Apr 17 03:51:20 2017
@@ -1536,6 +1536,10 @@ bool NamedDecl::<wbr>declarationReplaces(Name
   if (isa<ObjCMethodDecl>(this))
     return false;

+  if (getFriendObjectKind() > OldD->getFriendObjectKind() &&
+      !isThisDeclarationADefinition(<wbr>))
+    return false;
+
   // For parameters, pick the newer one. This is either an error or (in
   // Objective-C) permitted as an extension.
   if (isa<ParmVarDecl>(this))

Modified: cfe/trunk/lib/AST/DeclBase.cpp
URL: <a class="m_2112800568306230035moz-txt-link-freetext" href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=300443&r1=300442&r2=300443&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/AST/<wbr>DeclBase.cpp?rev=300443&r1=<wbr>300442&r2=300443&view=diff</a>
==============================<wbr>==============================<wbr>==================
--- cfe/trunk/lib/AST/DeclBase.cpp (original)
+++ cfe/trunk/lib/AST/DeclBase.cpp Mon Apr 17 03:51:20 2017
@@ -861,6 +861,21 @@ const FunctionType *Decl::getFunctionTyp
   return Ty->getAs<FunctionType>();
 }

+bool Decl::<wbr>isThisDeclarationADefinition() const {
+  if (auto *TD = dyn_cast<TagDecl>(this))
+    return TD-><wbr>isThisDeclarationADefinition()<wbr>;
+  if (auto *FD = dyn_cast<FunctionDecl>(this))
+    return FD-><wbr>isThisDeclarationADefinition()<wbr>;
+  if (auto *VD = dyn_cast<VarDecl>(this))
+    return VD-><wbr>isThisDeclarationADefinition()<wbr>;
+  if (auto *CTD = dyn_cast<ClassTemplateDecl>(<wbr>this))
+    return CTD-><wbr>isThisDeclarationADefinition()<wbr>;
+  if (auto *FTD = dyn_cast<FunctionTemplateDecl><wbr>(this))
+    return FTD-><wbr>isThisDeclarationADefinition()<wbr>;
+  if (auto *VTD = dyn_cast<VarTemplateDecl>(<wbr>this))
+    return VTD-><wbr>isThisDeclarationADefinition()<wbr>;
+  return false;
+}

 /// Starting at a given context (a Decl or DeclContext), look for a
 /// code context that is not a closure (a lambda, block, etc.).

Modified: cfe/trunk/test/SemaTemplate/<wbr>friend-template.cpp
URL: <a class="m_2112800568306230035moz-txt-link-freetext" href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/friend-template.cpp?rev=300443&r1=300442&r2=300443&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/test/<wbr>SemaTemplate/friend-template.<wbr>cpp?rev=300443&r1=300442&r2=<wbr>300443&view=diff</a>
==============================<wbr>==============================<wbr>==================
--- cfe/trunk/test/SemaTemplate/<wbr>friend-template.cpp (original)
+++ cfe/trunk/test/SemaTemplate/<wbr>friend-template.cpp Mon Apr 17 03:51:20 2017
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s
 // PR5057
 namespace test0 {
   namespace std {
@@ -68,17 +68,12 @@ namespace test3 {
   Foo<int> foo;

   template<typename T, T Value> struct X2a;
-
-  template<typename T, int Size> struct X2b;
+  template<typename T, int Size> struct X2b;    // expected-note {{previous non-type template parameter with type 'int' is here}}

   template<typename T>
   class X3 {
     template<typename U, U Value> friend struct X2a;
-
-    // FIXME: the redeclaration note ends up here because redeclaration
-    // lookup ends up finding the friend target from X3<int>.
-    template<typename U, T Value> friend struct X2b; // expected-error {{template non-type parameter has a different type 'long' in template redeclaration}} \
-      // expected-note {{previous non-type template parameter with type 'int' is here}}
+    template<typename U, T Value> friend struct X2b; // expected-error {{template non-type parameter has a different type 'long' in template redeclaration}}
   };

   X3<int> x3i; // okay
@@ -297,14 +292,11 @@ namespace PR12585 {
   int n = C::D<void*>().f();

   struct F {
-    template<int> struct G;
+    template<int> struct G; // expected-note {{previous}}
   };
   template<typename T> struct H {
-    // FIXME: As with cases above, the note here is on an unhelpful declaration,
-    // and should point to the declaration of G within F.
     template<T> friend struct F::G; // \
-      // expected-error {{different type 'char' in template redeclaration}} \
-      // expected-note {{previous}}
+      // expected-error {{different type 'char' in template redeclaration}}
   };
   H<int> h1; // ok
   H<char> h2; // expected-note {{instantiation}}
@@ -329,3 +321,11 @@ namespace rdar12350696 {
     foo(b); // expected-note {{in instantiation}}
   }
 }
+namespace PR30994 {
+  void f();
+  struct A {
+    [[deprecated]] friend void f() {} // \
+      expected-note {{has been explicitly marked deprecated here}}
+  };
+  void g() { f(); } // expected-warning {{is deprecated}}
+}


______________________________<wbr>_________________
cfe-commits mailing list
<a class="m_2112800568306230035moz-txt-link-abbreviated" href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>
<a class="m_2112800568306230035moz-txt-link-freetext" href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a>
</pre>
        <br>
        <fieldset class="m_2112800568306230035mimeAttachmentHeader"></fieldset>
        <br>
        <pre>______________________________<wbr>_________________
cfe-commits mailing list
<a class="m_2112800568306230035moz-txt-link-abbreviated" href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>
<a class="m_2112800568306230035moz-txt-link-freetext" href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a>
</pre>
      </blockquote>
    </blockquote>
    <p><br>
    </p>
  </div></div></div>

</blockquote></div><br></div></div>