Fix scope-based lookup when more than one function is found.

[dawn]

Summary

When multiple functions are found by name, lldb removes duplicate entries of functions with the same type, so the first function in the symbol context list is chosen, even if it isn't in scope. This patch uses the declaration context of the execution context to select the function which is in scope.

This fixes cases like the following:

int func();
namespace ns {
    int func();
    void here() {
        // Run to BP here and eval 'p func()';
        // lldb used to find ::func(), now finds ns::func().
    }
}

[dawn]

Updated patch to removed change related to Pascal language - it should be part of a separate patch.

[clayborg]

So one of two things needs to happen here:

• ClangASTContext::DeclContextCountDeclLevels() becomes a function that is on ClangASTContext only and the opaque arguments get changed into "clang::DeclContext *" args, then remove all DeclContextCountDeclLevels functions from TypeSystem.h and GoASTContext.h.
• Change ClangASTContext::DeclContextCountDeclLevels() over to use CompilerDeclContext objects for both "void *opaque_decl_ctx" and "void *opaque_find_decl_ctx" and add any functions you need to do CompilerDeclContext that is needed to complete the functionality of this function without downcasting to clang objects.

So either make it generic, or clang specific.

I would vote for the first method since this functionality is very clang and C++ specific. It doesn't seem like a nice general purpose function any other language would use. If you do go the second route, you really need to abstract the heck out of this and add any needed methods to CompilerDeclContext.

source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp

1443

"compiler_decl_context" should probably be named "frame_decl_ctx" for clarity

1467–1468

Why are we pulling out class methods here?

1485

There is no need for this lamba, inline the code at the one and only place that it is used.

1505–1509

Seems like you actually want this to be something like:

fdi.m_func_decl_lvl = func_decl_context.Depth(compiler_decl_context);

This would be a function on CompilerDeclContext that would calculate the depth of a decl context if the first argument (compiler_decl_context) is contained within the object (func_decl_context). This would be a useful API to add to CompilerDeclContext. It can return an integer and return "-1" if the compiler_decl_context ins't contained within func_decl_context, or zero or greater if it is.

Then you would need to add more functions the CompilerDeclContext to get "fdi.m_function_name" and "fdi.m_copied_function_type".

1517

wouldn't this be better as std::multimap<uint32_t, FuncDeclInfo> where the uint32_t is the depth? Then you can change your foo loop at 1531 to just grab the first entry in the map and iterate as long as the depth is the same...

1524

inline the initFuncDeclInfo lambda here without needing a lambda

[dawn]

Thanks Greg! To address your main point:

So either make it generic, or clang specific.

DeclContextCountDeclLevels's interface follows DeclContextFindDeclByName (authored by Paul Herman). If DeclContextCountDeclLevels is to change, then DeclContextFindDeclByName should also change for consistency.

My take on this is that the code in ClangExpressionDeclMap::FindExternalVisibleDecls which calls these scope-based lookup routines should be moved out of ClangExpressionDeclMap.cpp into CompilerExpressionDeclMap.cpp, so the interfaces to DeclContextFindDeclByName/DeclContextCountDeclLevels should remain generic while the implementations are compiler specific. That's way more than I'd like to take on as part of this patch however.

Paul, any feedback from you on this?

[tberghammer]

Paul is not working on LLDB anymore so I added Siva who took over most of his work.

[clayborg]

So one thing I don't want to propagate here is the "go parse everything inside a decl context" if we can avoid it. I believe the first CompilerDeclContext patch did this, but I don't remember the exact details so I could be wrong. But going forward I would like to see more of "find a struct named 'X' in CompilerDeclContext 'Y'" queries, instead of "parse everything in CompilerDeclContext 'Y' and then I would look for 'X'". Or if we know what we are looking for (function decl contexts and lexical block decl contexts in this case), then I would prefer to have "go parse all function and block decl contexts in CompilerDeclContext 'Y'".

[tberghammer]

I am not sure if doing all of the parsing lazily is the good approach because of speed considerations.

The problem is that if we do everything lazily then in one side we are parsing only the information what we need but on the other side it is very difficult (or impossible) to make it multi threaded. In case of a high end machine I think doing the parsing in 8-16 core (or even more) can be faster then the lazy approach if we have to parse a lot of information anyway.

I don't have any measurements but looking into the direction we are heading now we will need to parse more and more information to improve the expression evaluation (e.g. to support limit debug info) and to get it a parse a lot of thing upfront might be a better approach. In terms of the performance I expect it to be a small to medium difference (not sure about memory) but will speed up the debugging experience later. For me it is more annoying when I have to wait to evaluate an expression then to wait for the debugger to start up.

[clayborg]

That does make sense. Lets ignore the partial parsing until we find a performance problem that needs to be fixed.

By I really do want to see the API on CompilerDeclContext and CompilerDecl get a lot more member functions that can do useful things now that we have this infrastructure.

[clayborg]

In D15312#304652, @dawn wrote:

Thanks Greg! To address your main point:

So either make it generic, or clang specific.

DeclContextCountDeclLevels's interface follows DeclContextFindDeclByName (authored by Paul Herman). If DeclContextCountDeclLevels is to change, then DeclContextFindDeclByName should also change for consistency.

Indeed it should be. It should return a vector of CompilerDecl objects.

My take on this is that the code in ClangExpressionDeclMap::FindExternalVisibleDecls which calls these scope-based lookup routines should be moved out of ClangExpressionDeclMap.cpp into CompilerExpressionDeclMap.cpp, so the interfaces to DeclContextFindDeclByName/DeclContextCountDeclLevels should remain generic while the implementations are compiler specific. That's way more than I'd like to take on as part of this patch however.

I would still like to see your new ClangASTContext::DeclContextCountDeclLevels() broken up into one or more functions that are more useful on CompilerDeclContext. In my previous comment I mentioned a new CompilerDeclContext that could be used to get the depth:

int32_t CompilerDeclContext::GetDepth (const CompilerDeclContext &decl_ctx);

This would get the depth of one CompilerDeclContext within another CompilerDeclContext, it would return -1 if "decl_ctx" doesn't fall within the object, and zero or above it is is contained.

Not sure how to break out the other thing with the name and type, but thing about what would make a good API on CompilerDeclContext and what questions you might ask of it. The DeclContextCountDeclLevels seems to be too specific.

Paul, any feedback from you on this?

We should be using our CompilerDecl and CompilerDeclContext objects. No one should be using opaque pointers outside the API unless you are making the objects or changing the contained decl/decl context.

[clayborg]

BTW: I fixed DeclContextFindDeclByName to return a vector of CompilerDecl objects:

% svn commit
Sending include/lldb/Symbol/ClangASTContext.h
Sending include/lldb/Symbol/GoASTContext.h
Sending include/lldb/Symbol/TypeSystem.h
Sending source/Symbol/ClangASTContext.cpp
Sending source/Symbol/CompilerDeclContext.cpp
Sending source/Symbol/TypeSystem.cpp
Transmitting file data ......
Committed revision 255038.

[dawn]

Greg: But going forward I would like to see more of "find a struct named 'X'

in CompilerDeclContext 'Y'" queries, instead of ...

I think lldb should have both, a "search for anything named foo in my scope context" (for when a user does "p foo"), and a "search for a thingy (functions/types/variables/etc.) named foo in my scope context" (for when a user does "expr --type function -- foo").

tberghammer: I am not sure if doing all of the parsing lazily is the good approach because of speed considerations. ... looking into the direction we are heading now we will need to parse more and more information to improve the expression evaluation

Agreed.

Greg:

int32_t CompilerDeclContext::GetDepth (const CompilerDeclContext &decl_ctx);

This would get the depth of one CompilerDeclContext within another CompilerDeclContext, it would return -1 if "decl_ctx" doesn't fall within the object, and zero or above it is is contained.

The problem with this is that it won't work for using declarations which require information about the thing we're looking up before it can determine the proper scope levels.

BTW: I fixed DeclContextFindDeclByName to return a vector of CompilerDecl objects:

Cool, but it needs to accept an optional type to deal with function overloading. I can add that and see if I can come up with a new patch that uses the new DeclContextFindDeclByName. (In my copious spare time - heh). But...

One major performance benefit of my original implementation is that the function's name and type *only* need to be tested in the case of a using declaration, so in general it's much faster, because we only need to look for the function's parent's lookup scope. So, are you sure you want to go down this path?

[clayborg]

In D15312#305392, @dawn wrote:

Greg: But going forward I would like to see more of "find a struct named 'X'

in CompilerDeclContext 'Y'" queries, instead of ...

I think lldb should have both, a "search for anything named foo in my scope context" (for when a user does "p foo"), and a "search for a thingy (functions/types/variables/etc.) named foo in my scope context" (for when a user does "expr --type function -- foo").

That is fine.

tberghammer: I am not sure if doing all of the parsing lazily is the good approach because of speed considerations. ... looking into the direction we are heading now we will need to parse more and more information to improve the expression evaluation

Agreed.

Greg:

int32_t CompilerDeclContext::GetDepth (const CompilerDeclContext &decl_ctx);

This would get the depth of one CompilerDeclContext within another CompilerDeclContext, it would return -1 if "decl_ctx" doesn't fall within the object, and zero or above it is is contained.

The problem with this is that it won't work for using declarations which require information about the thing we're looking up before it can determine the proper scope levels.

BTW: I fixed DeclContextFindDeclByName to return a vector of CompilerDecl objects:

Cool, but it needs to accept an optional type to deal with function overloading. I can add that and see if I can come up with a new patch that uses the new DeclContextFindDeclByName. (In my copious spare time - heh). But...

No rush for sure...

One major performance benefit of my original implementation is that the function's name and type *only* need to be tested in the case of a using declaration, so in general it's much faster, because we only need to look for the function's parent's lookup scope. So, are you sure you want to go down this path?

If you want to make this patch general then yes, but I wouldn't recommend it.

The functionality that you need is very specific to clang and your original implementation it just fine, I would say to just make a clang specific function that does exactly what you need and is only on ClangASTContext. We have discovered by our conversations above that the stuff you want to do doesn't map well onto CompilerDeclContext or CompilerDecl. Our needs are just too specific. So I vote to remove all code from TypeSystem.h and GoASTContext.h, change the version in ClangASTContext.h to be:

uint32_t
CountDeclLevels (clang::DeclContext *decl_ctx,
                             clang::DeclContext *child_decl_ctx,
                             ConstString *find_name = nullptr,
                             CompilerType *find_type = nullptr) override;

And just do what you need to do as efficiently as you need to. When you are in ClangExpressionDeclMap.cpp you know that your type system is a ClangASTContext already, so you should be able to just use your existing AST.

My whole aversion to the previous solution was you were putting very specify clang stuff into an API (TypeSystem) that is designed to be language agnostic.

[dawn]

Hi Greg, I'm working on a new revision to change the patch as you suggest (thanks for your review - you had some great suggestions!).

Sorry, if I'm missing something obviously here, but there's still some things I don't understand:

1. What exactly was clang specific about the DeclContextCountDeclLevels API?
2. How is it anymore clang specific than DeclContextFindDeclByName?

Anyway, I didn't want to drag this out any longer so am making the code clang specific. We can always generalize it in the future (I want it for Delphi).

source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp

1467–1468

To save time - they're thrown out by the loop that actually adds the functions which follows this, so no point in spending time on them.

1505–1509

As mentioned in prior discussions, this API is basically what you are suggesting. The name and type are not returned - they are optional inputs into the function to allow for languages which support using declarations (because using declarations need to check type and/or name). For example, in Delphi where using declarations are not supported, these would default to nullptr.

1517

That won't work because we can have functions at more than one level that we'll want to keep in our set. Using std::multimap<type, FuncDeclInfo> might help to clean things up a little however - will give it a try.

[clayborg]

In D15312#306201, @dawn wrote:

Hi Greg, I'm working on a new revision to change the patch as you suggest (thanks for your review - you had some great suggestions!).

Sorry, if I'm missing something obviously here, but there's still some things I don't understand:

1. What exactly was clang specific about the DeclContextCountDeclLevels API?

CountDeclLevels doesn't really tell me what the function does. For abstract APIs I would expect an API like:

int CompilerDeclContext::GetDeclDepth (const CompilerDeclContext &child_decl_ctx);

That makes sense. It seems like you combined "find this decl instance within a decl context" with "find a decl by name in the CompilerDeclContext and optionally get the type". This is what I didn't like about API you added. It was too specialized and didn't make for good API on the CompilerDeclContext class. The class name "CountDeclLevels" didn't really say what this function would do for me. What levels? Is level 10 better than level 1? Why would I supply a name to this function if I am trying to find a specific decl (from opaque_find_decl_ctx)? Why does this function not work if I supply a type to fill in (CompilerType *find_type = <valid_ptr>) but not a name (ConstString *find_name == nullptr)? I am still unclear as to exactly what this function does even after reading it many times.

2. How is it anymore clang specific than DeclContextFindDeclByName?

This is generic. In any CompilerDeclContext, you can find one or more decls by name. That makes sense to me:

CompilerDeclContext decl_ctx = ...;
std::vector<CompilerDecl> decls = decl_ctx.FindDeclByName("hello");

That makes sense in any language.

Anyway, I didn't want to drag this out any longer so am making the code clang specific. We can always generalize it in the future (I want it for Delphi).

You sure can, you will need to make sure the APIs make sense though. I am still unclear as to what the name and type are doing in DeclContextCountDeclLevels. I don't see how we would ever have a decl (in opaque_find_decl_ctx) that isn't unique where the same decl could have different names and different types?

[clayborg]

Above I meant to say I don't understand what the "else if (find_name)" does. I understand the rest of it. So in the "else if (find_name)" we are looking for any decl, regardless of wether it matches "opaque_find_decl_ctx" as long as the name and type are correct? Again if I understand it correctly, these should be two different searches. One for find the depth of this within this decl context, and the other being find all decls by name within an decl context (which we already have), and then check to see if those decls match in kind and type.

[dawn]

It seems like you combined "find this decl instance within a decl context" with "find a decl by name in the CompilerDeclContext and optionally get the type".

It it exactly "find this decl instance within a decl context".

I am still unclear as to what the name and type are doing in DeclContextCountDeclLevels. I don't see how we would ever have a decl (in opaque_find_decl_ctx) that isn't unique where the same decl could have different names and different types?

The optional name is required by languages (like C++) to handle using declarations like:

void poo();
namespace ns {
    void foo();
    void goo();
}
void bar() {
    using ns::foo;
    // Here, 'foo' is needed so that we can match it
    // with the using declaration.
    //
    // We want the API to return 0 for 'foo',
    // -1 for 'goo', and 1 for 'poo'.
}

The optional type might be required by languages which have a using declaration-like concept where a type can be specified, like:

void foo(int, int);
namespace ns {
    void foo();
    void foo(int);
}
void bar() {
    using_like ns::foo(int);
    // Here, 'foo' and its type are both needed so that
    // we can match it with the using_like declaration.
    //
    // We want the API to return 0 for { 'foo', 'void(int)' },
    // -1 for { 'foo', 'void()' },
    // and 1 for { 'foo', 'void(int, int)' },
}

The name and type are optional (default to 0) for languages which don't have these concepts.

[dawn]

This version of the patch makes the API specific to clang.

[dawn]

This version of the patch is the API "generic" version of the patch. Please choose to review which ever is preferred, and I will submit another patch based on that version if needed.

BTW Greg, thanks for the suggestion of multimap - it definitely simplified things! And if you're wondering why we want to keep a set of functions for each type in FindExternalVisibleDecls, it's because overload resolution hasn't chosen a function yet, so we must keep all possible candidates in our set.

[clayborg]

I like the clang specific patch here better. Mainly because if you add a function like this to TypeSystem.h:

virtual uint32_t
DeclContextCountDeclLevels (void *opaque_decl_ctx,
                            void *opaque_find_decl_ctx,
                            ConstString *find_name = nullptr,
                            CompilerType *find_type = nullptr) = 0;

It means that you will have a function in the CompilerDeclContext named "CountDeclLevels(...)". The "DeclContext" prefix is the convention in TypeSystem.h that says "I will add a corresponding function to CompilerDeclContext that will call through to this function.

I still like the clang specific version better because I still think it doesn't make a sensible API for CompilerDeclContext.

Greg

[dawn]

Updated Clang-specific version of patch.

[labath]

Hi,

all of the new tests in TestNamespaceLookup were failing on linux, against all tested compilers (http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/9405), so I have XFAILed them. I don't know if this is something you're interested about, but I thought I'd point out that now all test in this file are marked as XFAIL on linux.

include/lldb/Symbol/ClangASTContext.h

     CompilerType
     GetTypeForFormatters (void* type) override;
     
+#define LLDB_INVALID_DECL_LEVEL            UINT32_MAX
+    // LLDB_INVALID_DECL_LEVEL is returned by CountDeclLevels if
+    // child_decl_ctx could not be found in decl_ctx.
+    uint32_t
+    CountDeclLevels (clang::DeclContext *frame_decl_ctx,
+                     clang::DeclContext *child_decl_ctx,
+                     ConstString *child_name = nullptr,
+                     CompilerType *child_type = nullptr);
+
     //----------------------------------------------------------------------
     // Modifying RecordType
     //----------------------------------------------------------------------

packages/Python/lldbsuite/test/lang/cpp/namespace/Makefile

 LEVEL = ../../../make
 
-CXX_SOURCES := main.cpp
+CXX_SOURCES := main.cpp ns.cpp ns2.cpp ns3.cpp
 
 include $(LEVEL)/Makefile.rules

packages/Python/lldbsuite/test/lang/cpp/namespace/main.cpp

 //===----------------------------------------------------------------------===//
 
 #include <cstdarg>
+#include "ns.h"
 
 namespace {
     typedef unsigned int my_uint_t;

     int value = 200;
 }
 
-#include <stdio.h>
 void test_namespace_scopes() {
     do {
         using namespace ns1;

 int
 main (int argc, char const *argv[])
 {
+    test_lookup_at_global_scope();
+    test_lookup_at_file_scope();
+    A::test_lookup_at_ns_scope();
+    A::B::test_lookup_at_nested_ns_scope();
+    A::B::test_lookup_at_nested_ns_scope_after_using();
+    test_lookup_before_using_directive();
+    test_lookup_after_using_directive();
     return Foo::myfunc(12);
 }

packages/Python/lldbsuite/test/lang/cpp/namespace/ns.h

+//===-- ns.h ------------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include <stdio.h>
+
+void test_lookup_at_global_scope();
+void test_lookup_at_file_scope();
+void test_lookup_before_using_directive();
+void test_lookup_after_using_directive();
+int func(int a);
+namespace A {
+    int foo();
+    int func(int a);
+    inline int func()
+    {
+        printf("A::func()\n");
+        return 3;
+    }
+    inline int func2()
+    {
+        printf("A::func2()\n");
+        return 3;
+    }
+    void test_lookup_at_ns_scope();
+    namespace B {
+        int func();
+        void test_lookup_at_nested_ns_scope();
+        void test_lookup_at_nested_ns_scope_after_using();
+    }
+}

packages/Python/lldbsuite/test/lang/cpp/namespace/ns.cpp

+//===-- ns.cpp ------------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ns.h"
+
+int foo()
+{
+    printf("global foo()\n");
+    return 42;
+}
+int func()
+{
+    printf("global func()\n");
+    return 1;
+}
+int func(int a)
+{
+    printf("global func(int)\n");
+    return a + 1;
+}
+void test_lookup_at_global_scope()
+{
+    // BP_global_scope
+    printf("at global scope: foo() = %d\n", foo()); // eval foo(), exp: 42
+    printf("at global scope: func() = %d\n", func()); // eval func(), exp: 1
+}

packages/Python/lldbsuite/test/lang/cpp/namespace/ns2.cpp

+//===-- ns2.cpp ------------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ns.h"
+
+static int func()
+{
+    printf("static m2.cpp func()\n");
+    return 2;
+}
+void test_lookup_at_file_scope()
+{
+    // BP_file_scope
+    printf("at file scope: func() = %d\n", func()); // eval func(), exp: 2
+    printf("at file scope: func(10) = %d\n", func(10)); // eval func(10), exp: 11
+}
+namespace A {
+    namespace B {
+        int func()
+        {
+            printf("A::B::func()\n");
+            return 4;
+        }
+        void test_lookup_at_nested_ns_scope()
+        {
+            // BP_nested_ns_scope
+            printf("at nested ns scope: func() = %d\n", func()); // eval func(), exp: 4
+
+            //printf("func(10) = %d\n", func(10)); // eval func(10), exp: 13
+            // NOTE: Under the rules of C++, this test would normally get an error
+            // because A::B::func() hides A::func(), but lldb intentionally
+            // disobeys these rules so that the intended overload can be found
+            // by only removing duplicates if they have the same type.
+        }
+        void test_lookup_at_nested_ns_scope_after_using()
+        {
+            // BP_nested_ns_scope_after_using
+            using A::func;
+            printf("at nested ns scope after using: func() = %d\n", func()); // eval func(), exp: 3
+        }
+    }
+}
+int A::foo()
+{
+    printf("A::foo()\n");
+    return 42;
+}
+int A::func(int a)
+{
+    printf("A::func(int)\n");
+    return a + 3;
+}
+void A::test_lookup_at_ns_scope()
+{
+    // BP_ns_scope
+    printf("at nested ns scope: func() = %d\n", func()); // eval func(), exp: 3
+    printf("at nested ns scope: func(10) = %d\n", func(10)); // eval func(10), exp: 13
+    printf("at nested ns scope: foo() = %d\n", foo()); // eval foo(), exp: 42
+}

packages/Python/lldbsuite/test/lang/cpp/namespace/ns3.cpp

+//===-- ns3.cpp ------------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ns.h"
+extern int func();
+
+// Note: the following function must be before the using.
+void test_lookup_before_using_directive()
+{
+    // BP_before_using_directive
+    printf("before using directive: func() = %d\n", func()); // eval func(), exp: 1
+}
+using namespace A;
+void test_lookup_after_using_directive()
+{
+    // BP_after_using_directive
+    //printf("func() = %d\n", func()); // eval func(), exp: error, amiguous
+    printf("after using directive: func2() = %d\n", func2()); // eval func2(), exp: 3
+    printf("after using directive: ::func() = %d\n", ::func()); // eval ::func(), exp: 1
+    printf("after using directive: B::func() = %d\n", B::func()); // eval B::func(), exp: 4
+}

source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp

                                                   sc_list);
             }
 
+            // If we found more than one function, see if we can use the
+            // frame's decl context to remove functions that are shadowed
+            // by other functions which match in type but are nearer in scope.
+            //
+            // AddOneFunction will not add a function whose type has already been
+            // added, so if there's another function in the list with a matching
+            // type, check to see if their decl context is a parent of the current
+            // frame's or was imported via a and using statement, and pick the
+            // best match according to lookup rules.
+            if (sc_list.GetSize() > 1)
+            {
+                // Collect some info about our frame's context.
+                StackFrame *frame = m_parser_vars->m_exe_ctx.GetFramePtr();
+                SymbolContext frame_sym_ctx;
+                if (frame != nullptr)
+                    frame_sym_ctx = frame->GetSymbolContext(lldb::eSymbolContextFunction|lldb::eSymbolContextBlock);
+                CompilerDeclContext frame_decl_context = frame_sym_ctx.block != nullptr ? frame_sym_ctx.block->GetDeclContext() : CompilerDeclContext();
+
+                // We can't do this without a compiler decl context for our frame.
+                if (frame_decl_context)
+                {
+                    clang::DeclContext *frame_decl_ctx = (clang::DeclContext *)frame_decl_context.GetOpaqueDeclContext();
+                    ClangASTContext *ast = llvm::dyn_cast_or_null<ClangASTContext>(frame_decl_context.GetTypeSystem());
+
+                    // Structure to hold the info needed when comparing function
+                    // declarations.
+                    struct FuncDeclInfo
+                    {
+                        ConstString m_name;
+                        CompilerType m_copied_type;
+                        uint32_t m_decl_lvl;
+                        SymbolContext m_sym_ctx;
+                    };
+
+                    // First, symplify things by looping through the symbol contexts
+                    // to remove unwanted functions and separate out the functions we
+                    // want to compare and prune into a separate list.
+                    // Cache the info needed about the function declarations in a
+                    // vector for efficiency.
+                    SymbolContextList sc_sym_list;
+                    uint32_t num_indices = sc_list.GetSize();
+                    std::vector<FuncDeclInfo> fdi_cache;
+                    fdi_cache.reserve(num_indices);
+                    for (uint32_t index = 0; index < num_indices; ++index)
+                    {
+                        FuncDeclInfo fdi;
+                        SymbolContext sym_ctx;
+                        sc_list.GetContextAtIndex(index, sym_ctx);
+
+                        // We don't know enough about symbols to compare them,
+                        // but we should keep them in the list.
+                        Function *function = sym_ctx.function;
+                        if (!function)
+                        {
+                            sc_sym_list.Append(sym_ctx);
+                            continue;
+                        }
+                        // Filter out functions without declaration contexts, as well as
+                        // class/instance methods, since they'll be skipped in the
+                        // code that follows anyway.
+                        CompilerDeclContext func_decl_context = function->GetDeclContext();
+                        if (!func_decl_context || func_decl_context.IsClassMethod(nullptr, nullptr, nullptr))
+                            continue;
+                        // We can only prune functions for which we can copy the type.
+                        CompilerType func_clang_type = function->GetType()->GetFullCompilerType();
+                        CompilerType copied_func_type = GuardedCopyType(func_clang_type);
+                        if (!copied_func_type)
+                        {
+                            sc_sym_list.Append(sym_ctx);
+                            continue;
+                        }
+
+                        fdi.m_sym_ctx = sym_ctx;
+                        fdi.m_name = function->GetName();
+                        fdi.m_copied_type = copied_func_type;
+                        fdi.m_decl_lvl = LLDB_INVALID_DECL_LEVEL;
+                        if (fdi.m_copied_type && func_decl_context)
+                        {
+                            // Call CountDeclLevels to get the number of parent scopes we
+                            // have to look through before we find the function declaration.
+                            // When comparing functions of the same type, the one with a
+                            // lower count will be closer to us in the lookup scope and
+                            // shadows the other.
+                            clang::DeclContext *func_decl_ctx = (clang::DeclContext *)func_decl_context.GetOpaqueDeclContext();
+                            fdi.m_decl_lvl = ast->CountDeclLevels(frame_decl_ctx,
+                                                                  func_decl_ctx,
+                                                                  &fdi.m_name,
+                                                                  &fdi.m_copied_type);
+                        }
+                        fdi_cache.emplace_back(fdi);
+                    }
+
+                    // Loop through the functions in our cache looking for matching types,
+                    // then compare their scope levels to see which is closer.
+                    std::multimap<CompilerType, const FuncDeclInfo*> matches;
+                    for (const FuncDeclInfo &fdi : fdi_cache)
+                    {
+                        const CompilerType t = fdi.m_copied_type;
+                        auto q = matches.find(t);
+                        if (q != matches.end())
+                        {
+                            if (q->second->m_decl_lvl > fdi.m_decl_lvl)
+                                // This function is closer; remove the old set.
+                                matches.erase(t);
+                            else if (q->second->m_decl_lvl < fdi.m_decl_lvl)
+                                // The functions in our set are closer - skip this one.
+                                continue;
+                        }
+                        matches.insert(std::make_pair(t, &fdi));
+                    }
+
+                    // Loop through our matches and add their symbol contexts to our list.
+                    SymbolContextList sc_func_list;
+                    for (const auto &q : matches)
+                        sc_func_list.Append(q.second->m_sym_ctx);
+
+                    // Rejoin the lists with the functions in front.
+                    sc_list = sc_func_list;
+                    sc_list.Append(sc_sym_list);
+                }
+            }
+
             if (sc_list.GetSize())
             {
                 Symbol *extern_symbol = NULL;

source/Symbol/ClangASTContext.cpp

     return found_decls;
 }
 
+// Look for child_decl_ctx's lookup scope in frame_decl_ctx and its parents,
+// and return the number of levels it took to find it, or LLDB_INVALID_DECL_LEVEL
+// if not found.  If the decl was imported via a using declaration, its name and/or
+// type, if set, will be used to check that the decl found in the scope is a match.
+//
+// The optional name is required by languages (like C++) to handle using declarations
+// like:
+//
+//     void poo();
+//     namespace ns {
+//         void foo();
+//         void goo();
+//     }
+//     void bar() {
+//         using ns::foo;
+//         // CountDeclLevels returns 0 for 'foo', 1 for 'poo', and
+//         // LLDB_INVALID_DECL_LEVEL for 'goo'.
+//     }
+//
+// The optional type is useful in the case that there's a specific overload
+// that we're looking for that might otherwise be shadowed, like:
+//
+//     void foo(int);
+//     namespace ns {
+//         void foo();
+//     }
+//     void bar() {
+//         using ns::foo;
+//         // CountDeclLevels returns 0 for { 'foo', void() },
+//         // 1 for { 'foo', void(int) }, and
+//         // LLDB_INVALID_DECL_LEVEL for { 'foo', void(int, int) }.
+//     }
+//
+// NOTE: Because file statics are at the TranslationUnit along with globals, a
+// function at file scope will return the same level as a function at global scope.
+// Ideally we'd like to treat the file scope as an additional scope just below the
+// global scope.  More work needs to be done to recognise that, if the decl we're
+// trying to look up is static, we should compare its source file with that of the
+// current scope and return a lower number for it.
+uint32_t
+ClangASTContext::CountDeclLevels (clang::DeclContext *frame_decl_ctx,
+                                  clang::DeclContext *child_decl_ctx,
+                                  ConstString *child_name,
+                                  CompilerType *child_type)
+{
+    if (frame_decl_ctx)
+    {
+        std::set<DeclContext *> searched;
+        std::multimap<DeclContext *, DeclContext *> search_queue;
+        SymbolFile *symbol_file = GetSymbolFile();
+
+        // Get the lookup scope for the decl we're trying to find.
+        clang::DeclContext *parent_decl_ctx = child_decl_ctx->getParent();
+
+        // Look for it in our scope's decl context and its parents.
+        uint32_t level = 0;
+        for (clang::DeclContext *decl_ctx = frame_decl_ctx; decl_ctx != nullptr; decl_ctx = decl_ctx->getParent())
+        {
+            if (!decl_ctx->isLookupContext())
+                continue;
+            if (decl_ctx == parent_decl_ctx)
+                // Found it!
+                return level;
+            search_queue.insert(std::make_pair(decl_ctx, decl_ctx));
+            for (auto it = search_queue.find(decl_ctx); it != search_queue.end(); it++)
+            {
+                if (searched.find(it->second) != searched.end())
+                    continue;
+                searched.insert(it->second);
+                symbol_file->ParseDeclsForContext(CompilerDeclContext(this, it->second));
+
+                for (clang::Decl *child : it->second->decls())
+                {
+                    if (clang::UsingDirectiveDecl *ud = llvm::dyn_cast<clang::UsingDirectiveDecl>(child))
+                    {
+                        clang::DeclContext *ns = ud->getNominatedNamespace();
+                        if (ns == parent_decl_ctx)
+                            // Found it!
+                            return level;
+                        clang::DeclContext *from = ud->getCommonAncestor();
+                        if (searched.find(ns) == searched.end())
+                            search_queue.insert(std::make_pair(from, ns));
+                    }
+                    else if (child_name)
+                    {
+                        if (clang::UsingDecl *ud = llvm::dyn_cast<clang::UsingDecl>(child))
+                        {
+                            for (clang::UsingShadowDecl *usd : ud->shadows())
+                            {
+                                clang::Decl *target = usd->getTargetDecl();
+                                clang::NamedDecl *nd = llvm::dyn_cast<clang::NamedDecl>(target);
+                                if (!nd)
+                                    continue;
+                                // Check names.
+                                IdentifierInfo *ii = nd->getIdentifier();
+                                if (ii == nullptr || !ii->getName().equals(child_name->AsCString(nullptr)))
+                                    continue;
+                                // Check types, if one was provided.
+                                if (child_type)
+                                {
+                                    CompilerType clang_type = ClangASTContext::GetTypeForDecl(nd);
+                                    if (!AreTypesSame(clang_type, *child_type, /*ignore_qualifiers=*/true))
+                                        continue;
+                                }
+                                // Found it!
+                                return level;
+                            }
+                        }
+                    }
+                }
+            }
+            ++level;
+        }
+    }
+    return LLDB_INVALID_DECL_LEVEL;
+}
+
 bool
 ClangASTContext::DeclContextIsStructUnionOrClass (void *opaque_decl_ctx)
 {