Skip to content

[lldb] Refactor TypeQuery::ContextMatches, take 2 #101333

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 5, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions lldb/include/lldb/Symbol/Type.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,6 @@ struct CompilerContext {
llvm::raw_ostream &operator<<(llvm::raw_ostream &os,
const CompilerContext &rhs);

/// Match \p context_chain against \p pattern, which may contain "Any"
/// kinds. The \p context_chain should *not* contain any "Any" kinds.
bool contextMatches(llvm::ArrayRef<CompilerContext> context_chain,
llvm::ArrayRef<CompilerContext> pattern);

FLAGS_ENUM(TypeQueryOptions){
e_none = 0u,
/// If set, TypeQuery::m_context contains an exact context that must match
Expand All @@ -79,10 +74,13 @@ FLAGS_ENUM(TypeQueryOptions){
/// If set, TypeQuery::m_context is a clang module compiler context. If not
/// set TypeQuery::m_context is normal type lookup context.
e_module_search = (1u << 1),
/// If set, the query will ignore all Module entries in the type context,
/// even for exact matches.
e_ignore_modules = (1u << 2),
/// When true, the find types call should stop the query as soon as a single
/// matching type is found. When false, the type query should find all
/// matching types.
e_find_one = (1u << 2),
e_find_one = (1u << 3),
};
LLDB_MARK_AS_BITMASK_ENUM(TypeQueryOptions)

Expand Down Expand Up @@ -264,6 +262,9 @@ class TypeQuery {
bool LanguageMatches(lldb::LanguageType language) const;

bool GetExactMatch() const { return (m_options & e_exact_match) != 0; }

bool GetIgnoreModules() const { return (m_options & e_ignore_modules) != 0; }

/// The \a m_context can be used in two ways: normal types searching with
/// the context containing a stanadard declaration context for a type, or
/// with the context being more complete for exact matches in clang modules.
Expand Down
2 changes: 0 additions & 2 deletions lldb/include/lldb/lldb-private-enumerations.h
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,6 @@ enum class CompilerContextKind : uint16_t {
Builtin = 1 << 10,

Any = 1 << 15,
/// Match 0..n nested modules.
AnyModule = Any | Module,
/// Match any type.
AnyType = Any | ClassOrStruct | Union | Enum | Typedef | Builtin,
/// Math any declaration context.
Expand Down
74 changes: 31 additions & 43 deletions lldb/source/Symbol/Type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
//
//===----------------------------------------------------------------------===//

#include <algorithm>
#include <cstdio>
#include <iterator>
#include <optional>

#include "lldb/Core/Module.h"
Expand All @@ -30,6 +32,7 @@
#include "lldb/Target/Process.h"
#include "lldb/Target/Target.h"
#include "lldb/lldb-enumerations.h"
#include "lldb/lldb-private-enumerations.h"

#include "llvm/ADT/StringRef.h"

Expand All @@ -43,35 +46,6 @@ llvm::raw_ostream &lldb_private::operator<<(llvm::raw_ostream &os,
return os << lldb_stream.GetString();
}

bool lldb_private::contextMatches(llvm::ArrayRef<CompilerContext> context_chain,
llvm::ArrayRef<CompilerContext> pattern) {
auto ctx = context_chain.begin();
auto ctx_end = context_chain.end();
for (const CompilerContext &pat : pattern) {
// Early exit if the pattern is too long.
if (ctx == ctx_end)
return false;
if (*ctx != pat) {
// Skip any number of module matches.
if (pat.kind == CompilerContextKind::AnyModule) {
// Greedily match 0..n modules.
ctx = std::find_if(ctx, ctx_end, [](const CompilerContext &ctx) {
return ctx.kind != CompilerContextKind::Module;
});
continue;
}
// See if there is a kind mismatch; they should have 1 bit in common.
if (((uint16_t)ctx->kind & (uint16_t)pat.kind) == 0)
return false;
// The name is ignored for AnyModule, but not for AnyType.
if (pat.kind != CompilerContextKind::AnyModule && ctx->name != pat.name)
return false;
}
++ctx;
}
return true;
}

static CompilerContextKind ConvertTypeClass(lldb::TypeClass type_class) {
if (type_class == eTypeClassAny)
return CompilerContextKind::AnyType;
Expand Down Expand Up @@ -153,19 +127,36 @@ void TypeQuery::SetLanguages(LanguageSet languages) {

bool TypeQuery::ContextMatches(
llvm::ArrayRef<CompilerContext> context_chain) const {
if (GetExactMatch() || context_chain.size() == m_context.size())
return ::contextMatches(context_chain, m_context);
auto ctx = context_chain.rbegin(), ctx_end = context_chain.rend();
for (auto pat = m_context.rbegin(), pat_end = m_context.rend();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this loop be just a std::search?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the modules out of the way, I guess it could be, but then I'd have to rewrite it again to this form for anonymous namespaces.

pat != pat_end;) {

if (ctx == ctx_end)
return false; // Pattern too long.

// See if there is a kind mismatch; they should have 1 bit in common.
if ((ctx->kind & pat->kind) == CompilerContextKind())
return false;

if (ctx->name != pat->name)
return false;

++ctx;
++pat;
}

// Skip over any remaining module entries if we were asked to do that.
while (GetIgnoreModules() && ctx != ctx_end &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the m_options won't change, we could lift this condition like so:

Suggested change
while (GetIgnoreModules() && ctx != ctx_end &&
if (GetIgnoreModules())
while (ctx != ctx_end &&

But feel free to ignore

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I've thought about that when writing this, but I figured that we shouldn't make the compiler's job too easy :)

I also tried a version with std::find, but that ended up even longer.

ctx->kind == CompilerContextKind::Module)
++ctx;

// We don't have an exact match, we need to bottom m_context.size() items to
// match for a successful lookup.
if (context_chain.size() < m_context.size())
return false; // Not enough items in context_chain to allow for a match.
// At this point, we have exhausted the pattern and we have a partial match at
// least. If that's all we're looking for, we're done.
if (!GetExactMatch())
return true;

size_t compare_count = context_chain.size() - m_context.size();
return ::contextMatches(
llvm::ArrayRef<CompilerContext>(context_chain.data() + compare_count,
m_context.size()),
m_context);
// We have an exact match if we've exhausted the target context as well.
return ctx == ctx_end;
}

bool TypeQuery::LanguageMatches(lldb::LanguageType language) const {
Expand Down Expand Up @@ -223,9 +214,6 @@ void CompilerContext::Dump(Stream &s) const {
case CompilerContextKind::Typedef:
s << "Typedef";
break;
case CompilerContextKind::AnyModule:
s << "AnyModule";
break;
case CompilerContextKind::AnyType:
s << "AnyType";
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
// RUN: %clangxx_host -g -gmodules -fmodules -std=c99 -x c-header %S/Inputs/pch.h -g -c -o %t.pch
// RUN: %clangxx_host -g -gmodules -fmodules -std=c99 -x c -include-pch %t.pch %s -c -o %t.o
// RUN: %clangxx_host %t.o -o %t.exe
// RUN: lldb-test symbols -dump-clang-ast -find type --language=C99 \
// RUN: -compiler-context 'AnyModule:*,ClassOrStruct:TypeFromPCH' %t.exe | FileCheck %s
// RUN: lldb-test symbols -dump-clang-ast -find type --find-in-any-module \
// RUN: --language=C99 -compiler-context 'ClassOrStruct:TypeFromPCH' \
// RUN: %t.exe | FileCheck %s

anchor_t anchor;

Expand Down
7 changes: 2 additions & 5 deletions lldb/test/Shell/SymbolFile/DWARF/x86/compilercontext.ll
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,8 @@
; RUN: lldb-test symbols %t.o -find=type --language=C99 \
; RUN: -compiler-context="Module:CModule,Module:SubModule,ClassOrStruct:FromSubmodule" \
; RUN: | FileCheck %s
; RUN: lldb-test symbols %t.o -find=type --language=C99 \
; RUN: -compiler-context="Module:CModule,AnyModule:*,ClassOrStruct:FromSubmodule" \
; RUN: | FileCheck %s
; RUN: lldb-test symbols %t.o -find=type --language=C99 \
; RUN: -compiler-context="AnyModule:*,ClassOrStruct:FromSubmodule" \
; RUN: lldb-test symbols %t.o -find=type -find-in-any-module --language=C99 \
; RUN: -compiler-context="ClassOrStruct:FromSubmodule" \
; RUN: | FileCheck %s
; RUN: lldb-test symbols %t.o -find=type --language=C99 \
; RUN: -compiler-context="Module:CModule,Module:SubModule,AnyType:FromSubmodule" \
Expand Down
18 changes: 14 additions & 4 deletions lldb/tools/lldb-test/lldb-test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,12 @@ static cl::opt<std::string> CompilerContext(
cl::desc("Specify a compiler context as \"kind:name,...\"."),
cl::value_desc("context"), cl::sub(SymbolsSubcommand));

static cl::opt<bool> FindInAnyModule(
"find-in-any-module",
cl::desc("If true, the type will be searched for in all modules. Otherwise "
"the modules must be provided in -compiler-context"),
cl::sub(SymbolsSubcommand));

static cl::opt<std::string>
Language("language", cl::desc("Specify a language type, like C99."),
cl::value_desc("language"), cl::sub(SymbolsSubcommand));
Expand Down Expand Up @@ -312,7 +318,6 @@ llvm::SmallVector<CompilerContext, 4> parseCompilerContext() {
.Case("Variable", CompilerContextKind::Variable)
.Case("Enum", CompilerContextKind::Enum)
.Case("Typedef", CompilerContextKind::Typedef)
.Case("AnyModule", CompilerContextKind::AnyModule)
.Case("AnyType", CompilerContextKind::AnyType)
.Default(CompilerContextKind::Invalid);
if (value.empty()) {
Expand Down Expand Up @@ -581,11 +586,13 @@ Error opts::symbols::findTypes(lldb_private::Module &Module) {
if (!ContextOr)
return ContextOr.takeError();

TypeQueryOptions Opts = TypeQueryOptions::e_module_search;
if (FindInAnyModule)
Opts |= TypeQueryOptions::e_ignore_modules;
TypeResults results;
if (!Name.empty()) {
if (ContextOr->IsValid()) {
TypeQuery query(*ContextOr, ConstString(Name),
TypeQueryOptions::e_module_search);
TypeQuery query(*ContextOr, ConstString(Name), Opts);
if (!Language.empty())
query.AddLanguage(Language::GetLanguageTypeFromString(Language));
Symfile.FindTypes(query, results);
Expand All @@ -596,7 +603,7 @@ Error opts::symbols::findTypes(lldb_private::Module &Module) {
Symfile.FindTypes(query, results);
}
} else {
TypeQuery query(parseCompilerContext(), TypeQueryOptions::e_module_search);
TypeQuery query(parseCompilerContext(), Opts);
if (!Language.empty())
query.AddLanguage(Language::GetLanguageTypeFromString(Language));
Symfile.FindTypes(query, results);
Expand Down Expand Up @@ -816,6 +823,9 @@ Expected<Error (*)(lldb_private::Module &)> opts::symbols::getAction() {
if (Regex && Name.empty())
return make_string_error("-regex used without a -name");

if (FindInAnyModule && (Find != FindType::Type))
return make_string_error("-find-in-any-module only works with -find=type");

switch (Find) {
case FindType::None:
if (!Context.empty() || !Name.empty() || !File.empty() || Line != 0)
Expand Down
98 changes: 63 additions & 35 deletions lldb/unittests/Symbol/TestType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,16 @@
//
//===----------------------------------------------------------------------===//

#include "gmock/gmock.h"
#include "gtest/gtest.h"

#include "lldb/Symbol/Type.h"
#include "lldb/lldb-enumerations.h"
#include "lldb/lldb-private-enumerations.h"

using namespace lldb;
using namespace lldb_private;
using testing::Not;

TEST(Type, GetTypeScopeAndBasename) {
EXPECT_EQ(Type::GetTypeScopeAndBasename("int"),
Expand Down Expand Up @@ -47,40 +50,65 @@ TEST(Type, GetTypeScopeAndBasename) {
EXPECT_EQ(Type::GetTypeScopeAndBasename("foo<::bar"), std::nullopt);
}

namespace {
MATCHER_P(Matches, pattern, "") {
TypeQuery query(pattern, TypeQueryOptions::e_none);
return query.ContextMatches(arg);
}
MATCHER_P(MatchesIgnoringModules, pattern, "") {
TypeQuery query(pattern, TypeQueryOptions::e_ignore_modules);
return query.ContextMatches(arg);
}
} // namespace

TEST(Type, CompilerContextPattern) {
std::vector<CompilerContext> mmc = {
{CompilerContextKind::Module, ConstString("A")},
{CompilerContextKind::Module, ConstString("B")},
{CompilerContextKind::ClassOrStruct, ConstString("S")}};
std::vector<CompilerContext> mc = {
{CompilerContextKind::Module, ConstString("A")},
{CompilerContextKind::ClassOrStruct, ConstString("S")}};
std::vector<CompilerContext> mac = {
{CompilerContextKind::Module, ConstString("A")},
{CompilerContextKind::AnyModule, ConstString("*")},
{CompilerContextKind::ClassOrStruct, ConstString("S")}};
EXPECT_TRUE(contextMatches(mmc, mac));
EXPECT_TRUE(contextMatches(mc, mac));
EXPECT_FALSE(contextMatches(mac, mc));
std::vector<CompilerContext> mmmc = {
{CompilerContextKind::Module, ConstString("A")},
{CompilerContextKind::Module, ConstString("B")},
{CompilerContextKind::Module, ConstString("C")},
{CompilerContextKind::ClassOrStruct, ConstString("S")}};
EXPECT_TRUE(contextMatches(mmmc, mac));
std::vector<CompilerContext> mme = {
{CompilerContextKind::Module, ConstString("A")},
{CompilerContextKind::Module, ConstString("B")},
{CompilerContextKind::Enum, ConstString("S")}};
std::vector<CompilerContext> mma = {
{CompilerContextKind::Module, ConstString("A")},
{CompilerContextKind::Module, ConstString("B")},
{CompilerContextKind::AnyType, ConstString("S")}};
EXPECT_TRUE(contextMatches(mme, mma));
EXPECT_TRUE(contextMatches(mmc, mma));
std::vector<CompilerContext> mme2 = {
{CompilerContextKind::Module, ConstString("A")},
{CompilerContextKind::Module, ConstString("B")},
{CompilerContextKind::Enum, ConstString("S2")}};
EXPECT_FALSE(contextMatches(mme2, mma));
auto make_module = [](llvm::StringRef name) {
return CompilerContext(CompilerContextKind::Module, ConstString(name));
};
auto make_class = [](llvm::StringRef name) {
return CompilerContext(CompilerContextKind::ClassOrStruct,
ConstString(name));
};
auto make_any_type = [](llvm::StringRef name) {
return CompilerContext(CompilerContextKind::AnyType, ConstString(name));
};
auto make_enum = [](llvm::StringRef name) {
return CompilerContext(CompilerContextKind::Enum, ConstString(name));
};
auto make_namespace = [](llvm::StringRef name) {
return CompilerContext(CompilerContextKind::Namespace, ConstString(name));
};

EXPECT_THAT(
(std::vector{make_module("A"), make_module("B"), make_class("C")}),
Matches(
std::vector{make_module("A"), make_module("B"), make_class("C")}));
EXPECT_THAT(
(std::vector{make_module("A"), make_module("B"), make_class("C")}),
Not(Matches(std::vector{make_class("C")})));
EXPECT_THAT(
(std::vector{make_module("A"), make_module("B"), make_class("C")}),
MatchesIgnoringModules(std::vector{make_class("C")}));
EXPECT_THAT(
(std::vector{make_module("A"), make_module("B"), make_class("C")}),
MatchesIgnoringModules(std::vector{make_module("B"), make_class("C")}));
EXPECT_THAT(
(std::vector{make_module("A"), make_module("B"), make_class("C")}),
Not(MatchesIgnoringModules(
std::vector{make_module("A"), make_class("C")})));
EXPECT_THAT((std::vector{make_module("A"), make_module("B"), make_enum("C")}),
Matches(std::vector{make_module("A"), make_module("B"),
make_any_type("C")}));
EXPECT_THAT(
(std::vector{make_module("A"), make_module("B"), make_class("C")}),
Matches(
std::vector{make_module("A"), make_module("B"), make_any_type("C")}));
EXPECT_THAT(
(std::vector{make_module("A"), make_module("B"), make_enum("C2")}),
Not(Matches(std::vector{make_module("A"), make_module("B"),
make_any_type("C")})));
EXPECT_THAT((std::vector{make_class("C")}),
Matches(std::vector{make_class("C")}));
EXPECT_THAT((std::vector{make_namespace("NS"), make_class("C")}),
Not(Matches(std::vector{make_any_type("C")})));
}
Loading