[Lldb-commits] [lldb] [lldb] Refactor TypeQuery::ContextMatches, take 2 (PR #101333)
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Mon Aug 5 05:14:52 PDT 2024
https://github.com/labath updated https://github.com/llvm/llvm-project/pull/101333
>From e87b2b24cd673584aabd00eaf6ad8fc4c0c52c98 Mon Sep 17 00:00:00 2001
From: Pavel Labath <pavel at labath.sk>
Date: Tue, 16 Jul 2024 14:18:27 +0000
Subject: [PATCH 1/3] [lldb] Refactor TypeQuery::ContextMatches, take 2
This is an alternative, much simpler implementation of #99305. In this
version I replace the AnyModule wildcard match with a special TypeQuery
flag which achieves (mostly) the same thing.
It is a preparatory step for teaching ContextMatches about anonymous
namespaces. It started out as a way to remove the assumption that the
pattern and target contexts must be of the same length -- that's will
not be correct with anonymous namespaces, and probably isn't even
correct right now for AnyModule matches.
---
lldb/include/lldb/Symbol/Type.h | 13 +--
lldb/include/lldb/lldb-private-enumerations.h | 2 -
lldb/source/Symbol/Type.cpp | 74 ++++++--------
.../DWARF/clang-gmodules-type-lookup.c | 5 +-
.../SymbolFile/DWARF/x86/compilercontext.ll | 7 +-
lldb/tools/lldb-test/lldb-test.cpp | 18 +++-
lldb/unittests/Symbol/TestType.cpp | 97 ++++++++++++-------
7 files changed, 119 insertions(+), 97 deletions(-)
diff --git a/lldb/include/lldb/Symbol/Type.h b/lldb/include/lldb/Symbol/Type.h
index c6f30cde81867..a35f9974fa39a 100644
--- a/lldb/include/lldb/Symbol/Type.h
+++ b/lldb/include/lldb/Symbol/Type.h
@@ -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
@@ -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)
@@ -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.
diff --git a/lldb/include/lldb/lldb-private-enumerations.h b/lldb/include/lldb/lldb-private-enumerations.h
index 9d18316dcea25..c24a3538f58da 100644
--- a/lldb/include/lldb/lldb-private-enumerations.h
+++ b/lldb/include/lldb/lldb-private-enumerations.h
@@ -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.
diff --git a/lldb/source/Symbol/Type.cpp b/lldb/source/Symbol/Type.cpp
index e76574795733f..eb321407e3734 100644
--- a/lldb/source/Symbol/Type.cpp
+++ b/lldb/source/Symbol/Type.cpp
@@ -6,7 +6,9 @@
//
//===----------------------------------------------------------------------===//
+#include <algorithm>
#include <cstdio>
+#include <iterator>
#include <optional>
#include "lldb/Core/Module.h"
@@ -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"
@@ -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;
@@ -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();
+ 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 &&
+ 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 {
@@ -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;
diff --git a/lldb/test/Shell/SymbolFile/DWARF/clang-gmodules-type-lookup.c b/lldb/test/Shell/SymbolFile/DWARF/clang-gmodules-type-lookup.c
index a8da692e57b1f..4e5f8905adecf 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/clang-gmodules-type-lookup.c
+++ b/lldb/test/Shell/SymbolFile/DWARF/clang-gmodules-type-lookup.c
@@ -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;
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/compilercontext.ll b/lldb/test/Shell/SymbolFile/DWARF/x86/compilercontext.ll
index 8fa45e955abf1..f64b363b1a002 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/compilercontext.ll
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/compilercontext.ll
@@ -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" \
diff --git a/lldb/tools/lldb-test/lldb-test.cpp b/lldb/tools/lldb-test/lldb-test.cpp
index da3822393dad1..535422a6d827b 100644
--- a/lldb/tools/lldb-test/lldb-test.cpp
+++ b/lldb/tools/lldb-test/lldb-test.cpp
@@ -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));
@@ -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()) {
@@ -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);
@@ -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);
@@ -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)
diff --git a/lldb/unittests/Symbol/TestType.cpp b/lldb/unittests/Symbol/TestType.cpp
index 79201d6ba2e59..438f141ff3c1c 100644
--- a/lldb/unittests/Symbol/TestType.cpp
+++ b/lldb/unittests/Symbol/TestType.cpp
@@ -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"),
@@ -47,40 +50,64 @@ 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")})));
}
>From d2f3bfe17d06053a7f35e869d34166d67e0d92a1 Mon Sep 17 00:00:00 2001
From: Pavel Labath <pavel at labath.sk>
Date: Wed, 31 Jul 2024 16:04:53 +0200
Subject: [PATCH 2/3] reformat
---
lldb/unittests/Symbol/TestType.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lldb/unittests/Symbol/TestType.cpp b/lldb/unittests/Symbol/TestType.cpp
index 438f141ff3c1c..e4b56b9ff02f7 100644
--- a/lldb/unittests/Symbol/TestType.cpp
+++ b/lldb/unittests/Symbol/TestType.cpp
@@ -94,7 +94,8 @@ TEST(Type, CompilerContextPattern) {
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")})));
+ 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")}));
>From 527bddcac4a16233675bce7df2328991f413523d Mon Sep 17 00:00:00 2001
From: Pavel Labath <pavel at labath.sk>
Date: Mon, 5 Aug 2024 14:14:28 +0200
Subject: [PATCH 3/3] add SetIgnoreModules
---
lldb/include/lldb/Symbol/Type.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/lldb/include/lldb/Symbol/Type.h b/lldb/include/lldb/Symbol/Type.h
index a35f9974fa39a..420307e0dbcf0 100644
--- a/lldb/include/lldb/Symbol/Type.h
+++ b/lldb/include/lldb/Symbol/Type.h
@@ -264,6 +264,7 @@ class TypeQuery {
bool GetExactMatch() const { return (m_options & e_exact_match) != 0; }
bool GetIgnoreModules() const { return (m_options & e_ignore_modules) != 0; }
+ void SetIgnoreModules() { m_options &= ~e_ignore_modules; }
/// 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
More information about the lldb-commits
mailing list