+ Richard

Thanks for the example. We've seen similar issue with inlines (without the reverted patch). My guess this patch exposed it.

It is not clear to me why we think the declaration is a definition there...
On 17/04/17 23:10, Benjamin Kramer via cfe-commits wrote:
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
<cfe-commits@lists.llvm.org> wrote:
Author: yrnkrn
Date: Mon Apr 17 03:51:20 2017
New Revision: 300443

URL: http://llvm.org/viewvc/llvm-project?rev=300443&view=rev
Log:
Address http://bugs.llvm.org/pr30994 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.cpp.

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

reviewer: Vassil Vassilev

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


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

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

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

    bool isInStdNamespace() const;

Modified: cfe/trunk/lib/AST/Decl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=300443&r1=300442&r2=300443&view=diff
==============================================================================
--- 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::declarationReplaces(Name
    if (isa<ObjCMethodDecl>(this))
      return false;

+  if (getFriendObjectKind() > OldD->getFriendObjectKind() &&
+      !isThisDeclarationADefinition())
+    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: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=300443&r1=300442&r2=300443&view=diff
==============================================================================
--- 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::isThisDeclarationADefinition() const {
+  if (auto *TD = dyn_cast<TagDecl>(this))
+    return TD->isThisDeclarationADefinition();
+  if (auto *FD = dyn_cast<FunctionDecl>(this))
+    return FD->isThisDeclarationADefinition();
+  if (auto *VD = dyn_cast<VarDecl>(this))
+    return VD->isThisDeclarationADefinition();
+  if (auto *CTD = dyn_cast<ClassTemplateDecl>(this))
+    return CTD->isThisDeclarationADefinition();
+  if (auto *FTD = dyn_cast<FunctionTemplateDecl>(this))
+    return FTD->isThisDeclarationADefinition();
+  if (auto *VTD = dyn_cast<VarTemplateDecl>(this))
+    return VTD->isThisDeclarationADefinition();
+  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/friend-template.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/friend-template.cpp?rev=300443&r1=300442&r2=300443&view=diff
==============================================================================
--- cfe/trunk/test/SemaTemplate/friend-template.cpp (original)
+++ cfe/trunk/test/SemaTemplate/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}}
+}


_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to