Skip to content

Commit

Permalink
Use the use_dynamic option to determine whether to dynamically resolve
Browse files Browse the repository at this point in the history
self in Swift expressions.

This is a more principled approach over always dynamically resolving
all types and fixes a regression hidden by the recent @swifttest
decorator bug.

(cherry picked from commit 46fd0ae)
  • Loading branch information
adrian-prantl committed Oct 21, 2020
1 parent 45cb8bf commit 6fd8202
Show file tree
Hide file tree
Showing 12 changed files with 69 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -447,10 +447,10 @@ class LLDBREPLNameLookup : public LLDBNameLookup {
};
}; // END Anonymous namespace

static void
AddRequiredAliases(Block *block, lldb::StackFrameSP &stack_frame_sp,
SwiftASTContextForExpressions &swift_ast_context,
SwiftASTManipulator &manipulator) {
static void AddRequiredAliases(Block *block, lldb::StackFrameSP &stack_frame_sp,
SwiftASTContextForExpressions &swift_ast_context,
SwiftASTManipulator &manipulator,
lldb::DynamicValueType use_dynamic) {
// First emit the typealias for "$__lldb_context".
if (!block)
return;
Expand Down Expand Up @@ -480,7 +480,7 @@ AddRequiredAliases(Block *block, lldb::StackFrameSP &stack_frame_sp,
if (stack_frame_sp) {
lldb::ValueObjectSP valobj_sp =
stack_frame_sp->GetValueObjectForFrameVariable(self_var_sp,
lldb::eDynamicCanRunTarget);
use_dynamic);

if (valobj_sp)
self_type = valobj_sp->GetCompilerType();
Expand Down Expand Up @@ -578,17 +578,18 @@ AddRequiredAliases(Block *block, lldb::StackFrameSP &stack_frame_sp,

/// Create a \c VariableInfo record for \c variable if there isn't
/// already shadowing inner declaration in \c processed_variables.
static void AddVariableInfo(
static llvm::Optional<llvm::Error> AddVariableInfo(
lldb::VariableSP variable_sp, lldb::StackFrameSP &stack_frame_sp,
SwiftASTContextForExpressions &ast_context,
SwiftLanguageRuntime *language_runtime,
llvm::SmallDenseSet<const char *, 8> &processed_variables,
llvm::SmallVectorImpl<SwiftASTManipulator::VariableInfo> &local_variables) {
llvm::SmallVectorImpl<SwiftASTManipulator::VariableInfo> &local_variables,
lldb::DynamicValueType use_dynamic) {
StringRef name = variable_sp->GetUnqualifiedName().GetStringRef();
const char *name_cstr = name.data();
assert(StringRef(name_cstr) == name && "missing null terminator");
if (name.empty())
return;
return {};

// To support "guard let self = self" the function argument "self"
// is processed (as the special self argument) even if it is
Expand All @@ -599,35 +600,35 @@ static void AddVariableInfo(
overridden_name = "$__lldb_injected_self";

if (processed_variables.count(overridden_name))
return;
return {};

CompilerType var_type;
if (stack_frame_sp) {
lldb::ValueObjectSP valobj_sp =
stack_frame_sp->GetValueObjectForFrameVariable(variable_sp,
lldb::eDynamicCanRunTarget);
use_dynamic);

if (!valobj_sp || valobj_sp->GetError().Fail()) {
// Ignore the variable if we couldn't find its corresponding
// value object. TODO if the expression tries to use an
// ignored variable, produce a sensible error.
return;
return {};
}
var_type = valobj_sp->GetCompilerType();
}

if (!var_type.IsValid())
return;
return {};

if (!var_type.GetTypeSystem()->SupportsLanguage(lldb::eLanguageTypeSwift))
return;
return {};

Status error;
CompilerType target_type = ast_context.ImportType(var_type, error);

// If the import failed, give up.
if (!target_type.IsValid())
return;
return {};

// If we couldn't fully realize the type, then we aren't going
// to get very far making a local out of it, so discard it here.
Expand All @@ -638,7 +639,16 @@ static void AddVariableInfo(
log->Printf("Discarding local %s because we couldn't fully realize it, "
"our best attempt was: %s.",
name_cstr, target_type.GetDisplayTypeName().AsCString("<unknown>"));
return;
// Not realizing self is a fatal error for an expression and the
// Swift compiler error alone is not particularly useful.
if (is_self)
return make_error<StringError>(
inconvertibleErrorCode(),
llvm::Twine("Couldn't realize type of self.") +
(use_dynamic
? ""
: " Try evaluating the expression with -d run-target"));
return {};
}

if (log && is_self)
Expand Down Expand Up @@ -680,15 +690,17 @@ static void AddVariableInfo(

local_variables.push_back(variable_info);
processed_variables.insert(overridden_name);
return {};
}

/// Create a \c VariableInfo record for each visible variable.
static void RegisterAllVariables(
static llvm::Optional<llvm::Error> RegisterAllVariables(
SymbolContext &sc, lldb::StackFrameSP &stack_frame_sp,
SwiftASTContextForExpressions &ast_context,
llvm::SmallVectorImpl<SwiftASTManipulator::VariableInfo> &local_variables) {
llvm::SmallVectorImpl<SwiftASTManipulator::VariableInfo> &local_variables,
lldb::DynamicValueType use_dynamic) {
if (!sc.block && !sc.function)
return;
return {};

Block *block = sc.block;
Block *top_block = block->GetContainingInlinedBlock();
Expand Down Expand Up @@ -743,9 +755,11 @@ static void RegisterAllVariables(
}

for (size_t vi = 0, ve = variables.GetSize(); vi != ve; ++vi)
AddVariableInfo({variables.GetVariableAtIndex(vi)}, stack_frame_sp,
ast_context, language_runtime, processed_names,
local_variables);
if (auto error = AddVariableInfo(
{variables.GetVariableAtIndex(vi)}, stack_frame_sp, ast_context,
language_runtime, processed_names, local_variables, use_dynamic))
return error;
return {};
}

static void ResolveSpecialNames(
Expand Down Expand Up @@ -1331,11 +1345,12 @@ static llvm::Expected<ParsedExpression> ParseAndImport(

if (local_context_is_swift) {
AddRequiredAliases(sc.block, stack_frame_sp, *swift_ast_context,
*code_manipulator);
*code_manipulator, options.GetUseDynamic());

// Register all local variables so that lookups to them resolve.
RegisterAllVariables(sc, stack_frame_sp, *swift_ast_context,
local_variables);
if (auto error = RegisterAllVariables(sc, stack_frame_sp, *swift_ast_context,
local_variables, options.GetUseDynamic()))
return std::move(*error);
}

// Register all magic variables.
Expand Down
2 changes: 1 addition & 1 deletion lldb/test/API/lang/swift/availability/TestAvailability.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,5 +140,5 @@ class C4<U> {

for breakpoint in breakpoints:
threads = lldbutil.continue_to_breakpoint(process, breakpoint)
self.expect("p f()", "can call")
self.expect("expr -d no-run-target -- f()", "can call")

Original file line number Diff line number Diff line change
Expand Up @@ -18,35 +18,42 @@ class TestClassConstrainedProtocol(TestBase):
mydir = TestBase.compute_mydir(__file__)

@swiftTest
def test_extension_weak_self (self):
def test_extension_weak_self(self):
"""Test that we can reconstruct weak self captured in a class constrained protocol."""
self.build()
self.do_self_test("Break here for weak self")
self.do_self_test("Break here for weak self", needs_dynamic=True)

@swiftTest
def test_extension_self (self):
"""Test that we can reconstruct self in method of a class constrained protocol."""
self.build()
self.do_self_test("Break here in class protocol")
self.do_self_test("Break here in class protocol", needs_dynamic=True)

@swiftTest
def test_method_weak_self (self):
def test_method_weak_self(self):
"""Test that we can reconstruct weak self capture in method of a class conforming to a class constrained protocol."""
self.build()
self.do_self_test("Break here for method weak self")
self.do_self_test("Break here for method weak self", needs_dynamic=False)

@swiftTest
def test_method_self (self):
def test_method_self(self):
"""Test that we can reconstruct self in method of a class conforming to a class constrained protocol."""
self.build()
self.do_self_test("Break here in method")
self.do_self_test("Break here in method", needs_dynamic=False)

def setUp(self):
# Call super's setUp().
TestBase.setUp(self)

def check_self(self, bkpt_pattern):
def check_self(self, bkpt_pattern, needs_dynamic):
opts = lldb.SBExpressionOptions()
if needs_dynamic:
opts.SetFetchDynamicValue(lldb.eNoDynamicValues)
result = self.frame().EvaluateExpression("self", opts)
error = result.GetError()
self.assertTrue("self" in error.GetCString())
self.assertTrue("run-target" in error.GetCString())
opts.SetFetchDynamicValue(lldb.eDynamicCanRunTarget)
result = self.frame().EvaluateExpression("self", opts)
error = result.GetError()
self.assertTrue(error.Success(),
Expand All @@ -58,7 +65,7 @@ def check_self(self, bkpt_pattern):
self.assertTrue(f_ivar.GetValueAsSigned() == 12345,
"Wrong value for f: %d"%(f_ivar.GetValueAsSigned()))

def do_self_test(self, bkpt_pattern):
def do_self_test(self, bkpt_pattern, needs_dynamic):
lldbutil.run_to_source_breakpoint(
self, bkpt_pattern, lldb.SBFileSpec('main.swift'))
self.check_self(bkpt_pattern)
self.check_self(bkpt_pattern, needs_dynamic)
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ def test_ivars_in_generic_expressions(self):
self.do_ivar_test()

def check_expression(self, expression, expected_result, use_summary=True):
value = self.frame().EvaluateExpression(expression)
opts = lldb.SBExpressionOptions()
opts.SetFetchDynamicValue(lldb.eDynamicCanRunTarget)
value = self.frame().EvaluateExpression(expression, opts)
self.assertTrue(value.IsValid(), expression + "returned a valid value")

self.assertTrue(value.GetError().Success(), "expression failed")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class C<T> : P {

extension P {
func f() -> Int {
return v //% self.expect("p self", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["a.C"])
return v //% self.expect("expr -d run-target -- self", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["a.C"])
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ extension Array
{
mutating func doGenericStuff()
{
print("generic stuff") //% self.expect("expr 2+3", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["5"])
print("generic stuff") //% self.expect("expr -d run-target -- 2+3", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["5"])
}
}

Expand Down
2 changes: 1 addition & 1 deletion lldb/test/API/lang/swift/generic_extension/main.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
// -----------------------------------------------------------------------------
extension Array {
func test() {
print("PATPAT") //% self.expect("p self", substrs=['[0] = 1', '[1] = 2', '[2] = 3'])
print("PATPAT") //% self.expect("expr -d run-target -- self", substrs=['[0] = 1', '[1] = 2', '[2] = 3'])
return //% self.expect("frame variable -d run -- self", substrs=['[0] = 1', '[1] = 2', '[2] = 3'])
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ func apply<Type>(_ T : Type, fn: (Type) -> Type) -> Type { return fn(T) }
public func f<Type>(_ value : Type)
{
apply(value) { arg in
return arg //% self.expect('po arg', substrs=['3735928559'])
return arg //% self.expect('expr -o -d run -- arg', substrs=['3735928559'])
//% self.expect('expr -d run -- arg', substrs=['Int', '3735928559'])
//% self.expect('fr var -d run -- arg', substrs=['Int', '3735928559'])
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public struct Q<T> {
let x: T
}
public func foo<T>(_ arg: [Q<T>]) {
print(arg) //% self.expect('po arg', substrs=['x : 3735928559'])
print(arg) //% self.expect('expr -o -d run -- arg', substrs=['x : 3735928559'])
//% self.expect('expr -d run -- arg', substrs=['x = 3735928559'])
//% self.expect('frame var -d run -- arg', substrs=['x = 3735928559'])
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,13 @@ struct FlatMapper<Type>
]

let _ = tuples.flatMap { tuple in
return tuple //% self.expect('po tuple', substrs=['originalIndex : 0', 'filteredIndex : 0', 'name : "Coffee"', 'ID : "1"'])
return tuple //% self.expect('expr -o -d run -- tuple', substrs=['originalIndex : 0', 'filteredIndex : 0', 'name : "Coffee"', 'ID : "1"'])
//% self.expect('expr -d run -- tuple', substrs=['originalIndex = 0', 'filteredIndex = 0', 'name = "Coffee"', 'ID = "1"'])
//% self.expect('frame var -d run -- tuple', substrs=['originalIndex = 0', 'filteredIndex = 0', 'name = "Coffee"', 'ID = "1"'])
}

let _ = values.flatMap { value in
return value //% self.expect('po value', substrs=['name : "Coffee"', 'ID : "1"'])
return value //% self.expect('expr -o -d run -- value', substrs=['name : "Coffee"', 'ID : "1"'])
//% self.expect('expr -d run -- value', substrs=['name = "Coffee"', 'ID = "1"'])
//% self.expect('frame var -d run -- value', substrs=['name = "Coffee"', 'ID = "1"'])
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ def do_test(self):
'x = 1.25', 'y = 2.5'])

self.expect("expression --raw-output --show-types -- loc3dCB",
substrs=['PointUtils & AnyObject) $R',
substrs=['PointUtils & Swift.AnyObject) $R',
'(Builtin.RawPointer) instance = 0x',
'(Builtin.RawPointer) witness_table_PointUtils = 0x'])

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ struct A<T> {
let c = cs[0]

let k1 = b(t:c)
let k2 = b(t:c) //% self.expect("expression -- c", "Unreadable variable is ignored", substrs = ["= 3"])
let k2 = b(t:c) //% self.expect("expr -d run -- c", "Unreadable variable is ignored", substrs = ["= 3"])
let k3 = b(t:c)

if let maybeT = process(i : adict.count, k1:k1, k2:k2, k3:k3) {
Expand Down

0 comments on commit 6fd8202

Please sign in to comment.