[Lldb-commits] [lldb] r255439 - Fix scope-based lookup when more than one function is found.
Dawn Perchik via lldb-commits
lldb-commits at lists.llvm.org
Sat Dec 12 11:31:41 PST 2015
Author: dperchik
Date: Sat Dec 12 13:31:41 2015
New Revision: 255439
URL: http://llvm.org/viewvc/llvm-project?rev=255439&view=rev
Log:
Fix scope-based lookup when more than one function is found.
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().
}
}
Reviewed by: clayborg
Subscribers: lldb-commits
Differential Revision: http://reviews.llvm.org/D15312
Added:
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/ns.cpp
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/ns.h
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/ns2.cpp
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/ns3.cpp
Modified:
lldb/trunk/include/lldb/Symbol/ClangASTContext.h
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/Makefile
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/main.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
lldb/trunk/source/Symbol/ClangASTContext.cpp
Modified: lldb/trunk/include/lldb/Symbol/ClangASTContext.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/ClangASTContext.h?rev=255439&r1=255438&r2=255439&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Symbol/ClangASTContext.h (original)
+++ lldb/trunk/include/lldb/Symbol/ClangASTContext.h Sat Dec 12 13:31:41 2015
@@ -944,6 +944,15 @@ public:
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
//----------------------------------------------------------------------
Modified: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/Makefile
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/Makefile?rev=255439&r1=255438&r2=255439&view=diff
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/Makefile (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/Makefile Sat Dec 12 13:31:41 2015
@@ -1,5 +1,5 @@
LEVEL = ../../../make
-CXX_SOURCES := main.cpp
+CXX_SOURCES := main.cpp ns.cpp ns2.cpp ns3.cpp
include $(LEVEL)/Makefile.rules
Modified: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/main.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/main.cpp?rev=255439&r1=255438&r2=255439&view=diff
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/main.cpp (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/main.cpp Sat Dec 12 13:31:41 2015
@@ -8,6 +8,7 @@
//===----------------------------------------------------------------------===//
#include <cstdarg>
+#include "ns.h"
namespace {
typedef unsigned int my_uint_t;
@@ -80,7 +81,6 @@ namespace ns2 {
int value = 200;
}
-#include <stdio.h>
void test_namespace_scopes() {
do {
using namespace ns1;
@@ -113,5 +113,12 @@ int Foo::myfunc(int a)
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);
}
Added: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/ns.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/ns.cpp?rev=255439&view=auto
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/ns.cpp (added)
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/ns.cpp Sat Dec 12 13:31:41 2015
@@ -0,0 +1,32 @@
+//===-- 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
+}
Added: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/ns.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/ns.h?rev=255439&view=auto
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/ns.h (added)
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/ns.h Sat Dec 12 13:31:41 2015
@@ -0,0 +1,36 @@
+//===-- 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();
+ }
+}
Added: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/ns2.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/ns2.cpp?rev=255439&view=auto
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/ns2.cpp (added)
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/ns2.cpp Sat Dec 12 13:31:41 2015
@@ -0,0 +1,65 @@
+//===-- 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
+}
Added: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/ns3.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/ns3.cpp?rev=255439&view=auto
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/ns3.cpp (added)
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/namespace/ns3.cpp Sat Dec 12 13:31:41 2015
@@ -0,0 +1,27 @@
+//===-- 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
+}
Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp?rev=255439&r1=255438&r2=255439&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp (original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp Sat Dec 12 13:31:41 2015
@@ -1424,6 +1424,128 @@ ClangExpressionDeclMap::FindExternalVisi
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;
Modified: lldb/trunk/source/Symbol/ClangASTContext.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/ClangASTContext.cpp?rev=255439&r1=255438&r2=255439&view=diff
==============================================================================
--- lldb/trunk/source/Symbol/ClangASTContext.cpp (original)
+++ lldb/trunk/source/Symbol/ClangASTContext.cpp Sat Dec 12 13:31:41 2015
@@ -9682,6 +9682,123 @@ ClangASTContext::DeclContextFindDeclByNa
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)
{
More information about the lldb-commits
mailing list