r300443 - 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.

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 17 16:38:53 PDT 2017


On 17 April 2017 at 14:41, Vassil Vassilev <v.g.vassilev at gmail.com> wrote:

> + 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...
>

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.


> 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 at lists.llvm.org> <cfe-commits at 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 listcfe-commits at lists.llvm.orghttp://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
>
> _______________________________________________
> cfe-commits mailing listcfe-commits at lists.llvm.orghttp://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170417/f79ee847/attachment-0001.html>


More information about the cfe-commits mailing list