Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Types lose visibility when member template friends exist #13763

Closed Quuxplusone closed 9 years ago

Quuxplusone commented 12 years ago
Bugzilla Link PR13699
Status RESOLVED FIXED
Importance P normal
Reported by Francisco Lopes (francisco@oblita.com)
Reported on 2012-08-25 14:44:18 -0700
Last modified on 2015-01-14 20:37:42 -0800
Version trunk
Hardware All All
CC dgregor@apple.com, francisco@oblita.com, richard-llvm@metafoo.co.uk, rjmccall@apple.com
Fixed by commit(s)
Attachments complete-template-friends-defined.patch (2901 bytes, text/plain)
complete-template-friends-defined.patch (2823 bytes, text/plain)
Blocks
Blocked by
See also
I'm working with libclang in VIM's clang_complete.
I'm having the following unexpected behavior when using iterators. Initially it
was showing up with the new C++ range-for but in the end I've reduced it to
iterator usage.

#include <vector>

int main()
{
    // std::vector type is visible here

    std::vector<int>::iterator i;

    // after declaring an iterator like above, std::vector
    // is not visible for completion anymore.

    return 0;
}
Quuxplusone commented 12 years ago

Also, I'm using libc++.

Quuxplusone commented 12 years ago
It seems this behavior comes from usage of the __wrap_iter template.

basic_string also declares its iterator with it like this:
typedef __wrap_iter<pointer> iterator;

And it suffers from the same behavior. After declaring:

std::basic_string<char>::iterator c;

std::basic_string ceases to be accessible from completion.

This __wrap_iter template exists in the libc++, which I'm using.
Quuxplusone commented 12 years ago
Digging the __wrap_iter I"ve reduced it to a problem of weird behavior.
If I put a header file, like dummy.h for example, inside /usr/local/lib/c++/v1,
which is a default search location for standard headers files by the Clang
compiler, and it's where libc++ lives at my system, with the following content:

namespace std
{
    struct __X{};
}

std::__X is not given for completion of std::

If I change the contents of dummy.h to

namespace std
{
    struct X{};
}

std::X is given for completion of std::

Now, if I put this dummy.h file with the first __X in a directory different
from libc++ headers location, I've tested putting it besides my test.cpp file,
then __X shows up for completion of std:: !
Quuxplusone commented 12 years ago
Anything with one or two underscores from <iterator>, and maybe other headers
from libc++ **location** is not given for completion.
Quuxplusone commented 12 years ago
Ok, the problem of no visibility of underscored types at libc++ location is not
the real cause for losing visibility for types after using its iterator types.

I've reduced the problem to template friendship usage:

namespace std
{
    template <class I> struct __W
    {
        template<class E> friend class V;
    };

    template <class T> struct V
    {
    };
}

int main()
{
    // There's visibility for std::V here

    std::__W<int> w;

    // There's NO visibility for std::V here
    std::

    return 0;
}

__wrap_iter and vector/basic_string have a pattern resembling the above, and
that's why after using a std::vector<int>::iterator, the std::vector template
is not given as a completion option after std::
Quuxplusone commented 11 years ago

This issue is also affecting boost headers, see the case for YouCompleteMe VIM plugin:

https://github.com/Valloric/YouCompleteMe/issues/114#issuecomment-16358809

Quuxplusone commented 11 years ago
The following resembles the problematic scenario for boost::shared_ptr:

namespace boost
{
    template<typename T> struct shared_ptr
    {
        template<typename U> friend struct shared_ptr;
    };
}

int main()
{
    // visibility here
    boost::shared_ptr<int> p;
    // no more visibility here
    boost::
}
Quuxplusone commented 10 years ago

Added discussion on http://clang-developers.42468.n3.nabble.com/Help-me-out-with-this-bug-td4038191.html

Quuxplusone commented 10 years ago
Since no progress has been made I have setup a debug version of clang,
right now I'm digging the sources and have inferred that

clang::Decl::setObjectOfFriendDecl   DeclBase.h:887   called at
clang::TemplateDeclInstantiator::VisitClassTemplateDecl
SemaTemplateInstantiateDecl.cpp:941

  Inst->setObjectOfFriendDecl();
  // TODO: do we want to track the instantiation progeny of this
  // friend target decl?

later affects

ResultBuilder::isInterestingDecl   SemaCodeComplete.cpp:496

  // Friend declarations and declarations introduced due to friends are never
  // added as results.
  if (IDNS & (Decl::IDNS_OrdinaryFriend | Decl::IDNS_TagFriend))
    return false;

Given this gist https://gist.github.com/oblitum/9302588, executing

  clang-hacking file.cpp 11 12

provides completion for the boost::shared_ptr class template but

  clang-hacking file.cpp 12 12

will not. This happens because IDNS of the class template is changed because of
the parsing of the first declaration coming at line 11, such parsing ends up
instantiating the friend class template, and changing its IDNS. After this first
change, no more completions for such type happens.

On 13699 there're two small patterns that bring this issue up. Also, as such
patterns are not very uncommon, it affects for-range loops over libc++
containers, boost classes and annoys some programmers.

This is all on revision 202241.

I'm still not a clang developer (still not), all I know comes from this
debugging session, so I think there can be more wise patches than mine now.
Quuxplusone commented 10 years ago

consider "On 13699 there're [...]" in the previous comment as referring to this issue itself.

Quuxplusone commented 10 years ago
This is the fix I propose:

Index: lib/Sema/SemaCodeComplete.cpp
===================================================================
--- lib/Sema/SemaCodeComplete.cpp   (revisão 202241)
+++ lib/Sema/SemaCodeComplete.cpp   (cópia de trabalho)
@@ -492,9 +492,10 @@
     return false;

   // Friend declarations and declarations introduced due to friends are never
-  // added as results.
+  // added as results when not previously declared.
   if (IDNS & (Decl::IDNS_OrdinaryFriend | Decl::IDNS_TagFriend))
-    return false;
+    if (!(IDNS & (Decl::IDNS_Ordinary | Decl::IDNS_Tag | Decl::IDNS_Type)))
+      return false;

   // Class template (partial) specializations are never added as results.
   if (isa<ClassTemplateSpecializationDecl>(ND) ||

Test would include code based on the two previous use cases mentioned in the
issue.

I'll try to provide a full patch later.
Quuxplusone commented 10 years ago
Hi,
attached is a patch that tries to fix libclang bug
13699.
Please review.

--
Francisco Lopes

Index: lib/Sema/SemaCodeComplete.cpp
===================================================================
--- lib/Sema/SemaCodeComplete.cpp   (revisão 202241)
+++ lib/Sema/SemaCodeComplete.cpp   (cópia de trabalho)
@@ -492,9 +492,10 @@
     return false;

   // Friend declarations and declarations introduced due to friends are never
-  // added as results.
+  // added as results when not previously declared.
   if (IDNS & (Decl::IDNS_OrdinaryFriend | Decl::IDNS_TagFriend))
-    return false;
+    if (!(IDNS & (Decl::IDNS_Ordinary | Decl::IDNS_Tag | Decl::IDNS_Type)))
+      return false;

   // Class template (partial) specializations are never added as results.
   if (isa<ClassTemplateSpecializationDecl>(ND) ||
Index: test/Index/complete-template-friends-defined.cpp
===================================================================
--- test/Index/complete-template-friends-defined.cpp    (revisão 0)
+++ test/Index/complete-template-friends-defined.cpp    (cópia de trabalho)
@@ -0,0 +1,33 @@
+// The run lines are below, because this test is line- and
+// column-number sensitive.
+
+namespace N {
+  template<typename T> struct A {
+    template<typename U> friend class B;
+  };
+
+  template<typename T> struct B { };
+}
+
+void foo() {
+  N::A<int> a1;
+  N::A<int> a2;
+}
+
+namespace M {
+  template<typename T> struct C {
+    template<typename U> friend struct C;
+  };
+}
+
+void bar() {
+  M::C<int> c1;
+  M::C<int> c2;
+}
+
+// RUN: c-index-test -code-completion-at=%s:14:6 %s | FileCheck -check-
prefix=CHECK-ACCESS-1 %s
+// CHECK-ACCESS-1: ClassTemplate:{TypedText A}{LeftAngle <}{Placeholder
typename T}{RightAngle >} (50)
+// CHECK-ACCESS-1: ClassTemplate:{TypedText B}{LeftAngle <}{Placeholder
typename U}{RightAngle >} (50)
+
+// RUN: c-index-test -code-completion-at=%s:25:6 %s | FileCheck -check-
prefix=CHECK-ACCESS-2 %s
+// CHECK-ACCESS-2: ClassTemplate:{TypedText C}{LeftAngle <}{Placeholder
typename U}{RightAngle >} (50)
Quuxplusone commented 10 years ago
Francisco Lopes Sat, 08 Mar 2014 09:06:25 -0800 (sent to cfe-
commits@cs.uiuc.edu)

Hello, situation now at revision 203332 seems even worst, for example,
it's not providing completion for boost::shared_ptr not even once, as was
happening before.

Also, this patch doesn't solve all the situation, I just realized the single
side effect I've found in this is that, the first completion shows template
parameters of the template declaration, which is right, but at the second
completion it starts to show template parameter names as set in the
template friend, not at the original declaration.

I'm not sure what's the way out of this, any suggestion is appreciated.

Regards,
Francisco Lopes.
Quuxplusone commented 10 years ago

Attached complete-template-friends-defined.patch (2901 bytes, text/plain): new patch

Quuxplusone commented 10 years ago

Attached complete-template-friends-defined.patch (2823 bytes, text/plain): patch 3 (reformatted)

Quuxplusone commented 9 years ago

Should be fixed as of r226083.

Quuxplusone commented 9 years ago

Thanks for looking into this and the much better patch =)