Skip to content

Commit

Permalink
Make Get(Canonical)SwiftType(opaque_type) private (NFC)
Browse files Browse the repository at this point in the history
to avoid accidentally importing types into a non-scratch
SwiftASTContext. All uses are via the expression evaluator and it
should always use the scratch context.
  • Loading branch information
adrian-prantl committed Oct 20, 2023
1 parent 7efc696 commit 59dbba1
Show file tree
Hide file tree
Showing 14 changed files with 128 additions and 93 deletions.
23 changes: 13 additions & 10 deletions lldb/source/Plugins/ExpressionParser/Swift/SwiftASTManipulator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -190,9 +190,11 @@ swift::BraceStmt *SwiftASTManipulatorBase::GetUserBody() {
}

SwiftASTManipulator::SwiftASTManipulator(
SwiftASTContextForExpressions &swift_ast_ctx,
swift::SourceFile &source_file, bool repl,
lldb::BindGenericTypes bind_generic_types)
: SwiftASTManipulatorBase(source_file, repl, bind_generic_types) {}
: SwiftASTManipulatorBase(source_file, repl, bind_generic_types),
m_swift_ast_ctx(swift_ast_ctx) {}

void SwiftASTManipulator::FindSpecialNames(
llvm::SmallVectorImpl<swift::Identifier> &names, llvm::StringRef prefix) {
Expand Down Expand Up @@ -903,7 +905,7 @@ llvm::Optional<swift::Type> SwiftASTManipulator::GetSwiftTypeForVariable(
CompilerType referent_type =
type_system_swift->GetReferentType(variable.m_type.GetOpaqueQualType());

swift::Type swift_type = GetSwiftType(referent_type);
swift::Type swift_type = m_swift_ast_ctx.GetSwiftType(referent_type);
if (!swift_type)
return {};

Expand Down Expand Up @@ -1022,7 +1024,7 @@ bool SwiftASTManipulator::AddExternalVariables(
auto introducer = variable.GetVarIntroducer();
swift::SourceLoc loc;
swift::Identifier name = variable.m_name;
swift::Type var_type = GetSwiftType(variable.m_type);
swift::Type var_type = m_swift_ast_ctx.GetSwiftType(variable.m_type);

if (!var_type)
return false;
Expand Down Expand Up @@ -1232,7 +1234,7 @@ SwiftASTManipulator::MakeTypealias(swift::Identifier name, CompilerType &type,
swift::TypeAliasDecl *type_alias_decl = new (ast_context)
swift::TypeAliasDecl(swift::SourceLoc(), swift::SourceLoc(), name,
swift::SourceLoc(), nullptr, decl_ctx);
swift::Type underlying_type = GetSwiftType(type);
swift::Type underlying_type = m_swift_ast_ctx.GetSwiftType(type);
if (!underlying_type)
return nullptr;

Expand All @@ -1257,12 +1259,13 @@ SwiftASTManipulator::MakeTypealias(swift::Identifier name, CompilerType &type,
type_alias_decl->dump(ss);
ss.flush();

log->Printf("Made type alias for %s (%p) in decl context (%p) and context "
"(%p):\n%s",
name.get(),
static_cast<void *>(GetSwiftType(type).getPointer()),
static_cast<void *>(decl_ctx),
static_cast<void *>(&ast_context), s.c_str());
log->Printf(
"Made type alias for %s (%p) in decl context (%p) and context "
"(%p):\n%s",
name.get(),
static_cast<void *>(m_swift_ast_ctx.GetSwiftType(type).getPointer()),
static_cast<void *>(decl_ctx), static_cast<void *>(&ast_context),
s.c_str());
}

if (make_private)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class VarDecl;
} // namespace swift

namespace lldb_private {
class SwiftASTContextForExpressions;

class SwiftASTManipulatorBase {
public:
Expand Down Expand Up @@ -194,8 +195,10 @@ class SwiftASTManipulatorBase {

class SwiftASTManipulator : public SwiftASTManipulatorBase {
public:
SwiftASTManipulator(swift::SourceFile &source_file, bool repl,
SwiftASTManipulator(SwiftASTContextForExpressions &swift_ast_ctx,
swift::SourceFile &source_file, bool repl,
lldb::BindGenericTypes bind_generic_types);
SwiftASTContextForExpressions &GetScratchContext() { return m_swift_ast_ctx; }

void FindSpecialNames(llvm::SmallVectorImpl<swift::Identifier> &names,
llvm::StringRef prefix);
Expand Down Expand Up @@ -294,6 +297,7 @@ class SwiftASTManipulator : public SwiftASTManipulatorBase {

std::vector<ResultLocationInfo> m_result_info;
llvm::StringMap<swift::TypeBase *> m_type_aliases;
SwiftASTContextForExpressions &m_swift_ast_ctx;
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,7 @@ AddRequiredAliases(Block *block, lldb::StackFrameSP &stack_frame_sp,

Flags imported_self_type_flags(imported_self_type.GetTypeInfo());

auto swift_self_type = GetSwiftType(imported_self_type);
auto swift_self_type = swift_ast_context.GetSwiftType(imported_self_type);
if (!swift_self_type) {
LLDB_LOG(GetLog(LLDBLog::Types | LLDBLog::Expressions),
"Couldn't get SwiftASTContext type for self type {0}.",
Expand All @@ -653,7 +653,8 @@ AddRequiredAliases(Block *block, lldb::StackFrameSP &stack_frame_sp,
// If 'self' is a weak storage type, it must be an optional. Look
// through it and unpack the argument of "optional".
if (swift::WeakStorageType *weak_storage_type =
GetSwiftType(imported_self_type)->getAs<swift::WeakStorageType>()) {
swift_ast_context.GetSwiftType(imported_self_type)
->getAs<swift::WeakStorageType>()) {
swift::Type referent_type = weak_storage_type->getReferentType();
swift::BoundGenericEnumType *optional_type =
referent_type->getAs<swift::BoundGenericEnumType>();
Expand Down Expand Up @@ -893,14 +894,13 @@ MaterializeVariable(SwiftASTManipulatorBase::VariableInfo &variable,

auto compiler_type = variable.GetType();
// Add the persistent variable as a typeref compiler type.
if (auto swift_ast_ctx =
if (auto ts =
compiler_type.GetTypeSystem().dyn_cast_or_null<SwiftASTContext>()) {
// Add the persistent variable as a typeref compiler type, but only if
// doesn't have archetypes (which can be the case when we're evaluating an
// expression as generic), since we can't mangle free-standing archetypes.
if (!swift_ast_ctx->TypeHasArchetype(compiler_type))
variable.SetType(
swift_ast_ctx->GetTypeRefType(compiler_type.GetOpaqueQualType()));
if (!manipulator.GetScratchContext().TypeHasArchetype(compiler_type))
variable.SetType(ts->GetTypeRefType(compiler_type.GetOpaqueQualType()));
}

if (is_result || is_error) {
Expand All @@ -922,7 +922,8 @@ MaterializeVariable(SwiftASTManipulatorBase::VariableInfo &variable,
} else {
CompilerType actual_type = variable.GetType();
// Desugar '$lldb_context', etc.
swift::Type actual_swift_type = GetSwiftType(actual_type);
swift::Type actual_swift_type =
manipulator.GetScratchContext().GetSwiftType(actual_type);
if (!actual_swift_type)
return llvm::None;

Expand Down Expand Up @@ -1051,7 +1052,7 @@ MaterializeVariable(SwiftASTManipulatorBase::VariableInfo &variable,
}
return SwiftExpressionParser::SILVariableInfo(
variable.GetType(), offset, needs_init, unowned_self);
}
}

namespace {

Expand Down Expand Up @@ -1415,7 +1416,7 @@ static llvm::Expected<ParsedExpression> ParseAndImport(
std::unique_ptr<SwiftASTManipulator> code_manipulator;
if (repl || !playground) {
code_manipulator = std::make_unique<SwiftASTManipulator>(
*source_file, repl, options.GetBindGenericTypes());
swift_ast_context, *source_file, repl, options.GetBindGenericTypes());

if (!playground) {
code_manipulator->RewriteResult();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ static bool AddVariableInfo(
}

// Report a fatal error if self can't be reconstructed as a Swift AST type.
if (is_self && !GetSwiftType(target_type))
if (is_self && !ast_context.GetSwiftType(target_type))
return false;

auto ts = target_type.GetTypeSystem().dyn_cast_or_null<TypeSystemSwift>();
Expand All @@ -356,7 +356,7 @@ static bool AddVariableInfo(
}

if (log && is_self)
if (swift::Type swift_type = GetSwiftType(target_type)) {
if (swift::Type swift_type = ast_context.GetSwiftType(target_type)) {
std::string s;
llvm::raw_string_ostream ss(s);
swift_type->dump(ss);
Expand Down
3 changes: 1 addition & 2 deletions lldb/source/Plugins/Language/Swift/SwiftLanguage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1549,8 +1549,7 @@ LazyBool SwiftLanguage::IsLogicalTrue(ValueObject &valobj, Status &error) {

Scalar scalar_value;

auto swift_ty = GetCanonicalSwiftType(valobj.GetCompilerType());
CompilerType valobj_type = ToCompilerType(swift_ty);
CompilerType valobj_type = valobj.GetCompilerType();
Flags type_flags(valobj_type.GetTypeInfo());
if (valobj_type.GetTypeSystem().isa_and_nonnull<TypeSystemSwift>()) {
if (type_flags.AllSet(eTypeIsStructUnion) &&
Expand Down
21 changes: 11 additions & 10 deletions lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1471,28 +1471,29 @@ void SwiftLanguageRuntime::RegisterGlobalError(Target &target, ConstString name,
return;
}

auto *ast_context = llvm::dyn_cast_or_null<SwiftASTContextForExpressions>(
auto *swift_ast_ctx = llvm::dyn_cast_or_null<SwiftASTContextForExpressions>(
type_system_or_err->get());
if (ast_context && !ast_context->HasFatalErrors()) {
if (swift_ast_ctx && !swift_ast_ctx->HasFatalErrors()) {
std::string module_name = "$__lldb_module_for_";
module_name.append(&name.GetCString()[1]);
SourceModule module_info;
module_info.path.push_back(ConstString(module_name));

Status module_creation_error;
swift::ModuleDecl *module_decl =
ast_context->CreateModule(module_info, module_creation_error,
/*importInfo*/ {});
swift_ast_ctx->CreateModule(module_info, module_creation_error,
/*importInfo*/ {});

if (module_creation_error.Success() && module_decl) {
const bool is_static = false;
const auto introducer = swift::VarDecl::Introducer::Let;

swift::VarDecl *var_decl =
new (*ast_context->GetASTContext()) swift::VarDecl(
is_static, introducer, swift::SourceLoc(),
ast_context->GetIdentifier(name.GetCString()), module_decl);
var_decl->setInterfaceType(GetSwiftType(ast_context->GetErrorType()));
swift::VarDecl *var_decl = new (*swift_ast_ctx->GetASTContext())
swift::VarDecl(is_static, introducer, swift::SourceLoc(),
swift_ast_ctx->GetIdentifier(name.GetCString()),
module_decl);
var_decl->setInterfaceType(
swift_ast_ctx->GetSwiftType(swift_ast_ctx->GetErrorType()));
var_decl->setDebuggerVar(true);

SwiftPersistentExpressionState *persistent_state =
Expand All @@ -1502,7 +1503,7 @@ void SwiftLanguageRuntime::RegisterGlobalError(Target &target, ConstString name,
if (!persistent_state)
return;

persistent_state->RegisterSwiftPersistentDecl({ast_context, var_decl});
persistent_state->RegisterSwiftPersistentDecl({swift_ast_ctx, var_decl});

ConstString mangled_name;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,6 @@ class TypeBase;

namespace lldb_private {

/// Statically cast a CompilerType to a Swift type.
swift::Type GetSwiftType(CompilerType type);
/// Statically cast a CompilerType to a Swift type and get its canonical form.
swift::CanType GetCanonicalSwiftType(CompilerType type);

class SwiftLanguageRuntimeStub;
class SwiftLanguageRuntimeImpl;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,18 +42,6 @@ using namespace lldb;
using namespace lldb_private;

namespace lldb_private {
swift::Type GetSwiftType(CompilerType type) {
auto ts = type.GetTypeSystem();
if (auto tr = ts.dyn_cast_or_null<TypeSystemSwift>())
if (auto ast = tr->GetSwiftASTContext())
return ast->GetSwiftType(type);
return {};
}

swift::CanType GetCanonicalSwiftType(CompilerType type) {
swift::Type swift_type = GetSwiftType(type);
return swift_type ? swift_type->getCanonicalType() : swift::CanType();
}

static lldb::addr_t
MaskMaybeBridgedPointer(Process &process, lldb::addr_t addr,
Expand Down Expand Up @@ -1597,10 +1585,10 @@ bool SwiftLanguageRuntimeImpl::GetDynamicTypeAndAddress_Class(

bool SwiftLanguageRuntimeImpl::IsValidErrorValue(ValueObject &in_value) {
CompilerType var_type = in_value.GetStaticValue()->GetCompilerType();
SwiftASTContext::ProtocolInfo protocol_info;
if (!SwiftASTContext::GetProtocolTypeInfo(var_type, protocol_info))
auto tss = var_type.GetTypeSystem().dyn_cast_or_null<TypeSystemSwift>();
if (!tss)
return false;
if (!protocol_info.m_is_errortype)
if (!tss->IsErrorType(var_type.GetOpaqueQualType()))
return false;

unsigned index = SwiftASTContext::ProtocolInfo::error_instance_index;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ SwiftLanguageRuntimeImpl::GetMemberVariableOffsetRemoteAST(
auto *remote_ast = &GetRemoteASTContext(*scratch_ctx);
// Check whether we've already cached this offset.
swift::TypeBase *swift_type =
GetCanonicalSwiftType(instance_type).getPointer();
scratch_ctx->GetCanonicalSwiftType(instance_type).getPointer();
if (swift_type == nullptr)
return {};

Expand Down Expand Up @@ -134,7 +134,7 @@ SwiftLanguageRuntimeImpl::GetMemberVariableOffsetRemoteAST(
"[MemberVariableOffsetResolver] resolved non-class type = %s",
bound.GetTypeName().AsCString());

swift_type = GetCanonicalSwiftType(bound).getPointer();
swift_type = scratch_ctx->GetCanonicalSwiftType(bound).getPointer();
MemberID key{swift_type, ConstString(member_name).GetCString()};
auto it = m_member_offsets.find(key);
if (it != m_member_offsets.end())
Expand Down Expand Up @@ -228,7 +228,7 @@ SwiftLanguageRuntimeImpl::GetDynamicTypeAndAddress_ProtocolRemoteAST(

swift::remote::RemoteAddress remote_existential(existential_address);
auto &remote_ast = GetRemoteASTContext(*swift_ast_ctx);
auto swift_type = GetSwiftType(protocol_type);
auto swift_type = swift_ast_ctx->GetSwiftType(protocol_type);
if (!swift_type)
return {};
if (use_local_buffer)
Expand Down Expand Up @@ -284,7 +284,7 @@ CompilerType SwiftLanguageRuntimeImpl::BindGenericTypeParametersRemoteAST(
base_type = swift_ast_ctx->ImportType(base_type, error);

if (base_type.GetTypeInfo() & lldb::eTypeIsSwift) {
swift::Type target_swift_type(GetSwiftType(base_type));
swift::Type target_swift_type(swift_ast_ctx->GetSwiftType(base_type));
if (target_swift_type->hasArchetype())
target_swift_type = target_swift_type->mapTypeOutOfContext().getPointer();

Expand Down Expand Up @@ -410,7 +410,7 @@ CompilerType SwiftLanguageRuntimeImpl::BindGenericTypeParametersRemoteAST(
swift_ast_ctx->ImportType(concrete_type, import_error);

if (target_concrete_type.IsValid())
return swift::Type(GetSwiftType(target_concrete_type));
return swift::Type(swift_ast_ctx->GetSwiftType(target_concrete_type));

return type;
},
Expand Down
Loading

0 comments on commit 59dbba1

Please sign in to comment.