Skip to content

[lldb] Mark parsing Swift expressions with generics as not cacheable #7492

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 2 commits into from
Sep 19, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions lldb/include/lldb/Expression/UserExpression.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,8 @@ class UserExpression : public Expression {
/// expression. Text() should contain the definition of this function.
const char *FunctionName() override { return "$__lldb_expr"; }

/// Returns whether the call to Parse on this user expression is cacheable.
virtual bool IsParseCacheable() { return true; }
/// Return the language that should be used when parsing. To use the
/// default, return eLanguageTypeUnknown.
lldb::LanguageType Language() const override { return m_language; }
Expand Down
1 change: 1 addition & 0 deletions lldb/source/Breakpoint/BreakpointLocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ bool BreakpointLocation::ConditionSaysStop(ExecutionContext &exe_ctx,
DiagnosticManager diagnostics;

if (condition_hash != m_condition_hash || !m_user_expression_sp ||
!m_user_expression_sp->IsParseCacheable() ||
!m_user_expression_sp->MatchesContext(exe_ctx)) {
LanguageType language = eLanguageTypeUnknown;
// See if we can figure out the language from the frame, otherwise use the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1724,6 +1724,16 @@ SwiftExpressionParser::Parse(DiagnosticManager &diagnostic_manager,
return ParseResult::unrecoverable_error;
}

{
// If any generics are present, this expression is not parseable.
m_is_cacheable =
!llvm::any_of(parsed_expr->code_manipulator->GetVariableInfo(),
[](const auto &variable) {
return variable.IsMetadataPointer() ||
variable.IsPackCount() ||
variable.IsUnboundPack();
});
}
auto dumpModule = [&](const char *msg) {
std::string s;
llvm::raw_string_ostream ss(s);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,11 @@ class SwiftExpressionParser : public ExpressionParser {
ParseResult Parse(DiagnosticManager &diagnostic_manager,
uint32_t first_line = 0, uint32_t last_line = UINT32_MAX);

/// Returns true if the call to parse of this type is cacheable.
bool IsParseCacheable() const {
return m_is_cacheable;
}

//------------------------------------------------------------------
/// Ready an already-parsed expression for execution, possibly
/// evaluating it statically.
Expand Down Expand Up @@ -198,6 +203,9 @@ class SwiftExpressionParser : public ExpressionParser {

/// If true, we are running in REPL mode
EvaluateExpressionOptions m_options;

/// Indicates whether the call to Parse of this type is cacheable.
bool m_is_cacheable;
};
} // namespace lldb_private

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,10 @@ class SwiftUserExpression : public LLVMUserExpression {
void WillStartExecuting() override;
void DidFinishExecuting() override;

bool IsParseCacheable() override {
return m_parser->IsParseCacheable();
}

private:
//------------------------------------------------------------------
/// Populate m_in_cplusplus_method and m_in_objectivec_method based on the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,30 +6,45 @@


class TestArchetypeInConditionalBreakpoint(TestBase):
@swiftTest
def test_stops_free_function(self):
self.stops("break here for free function")

@swiftTest
def test_doesnt_stop_free_function(self):
self.doesnt_stop("break here for free function")

@swiftTest
def test_stops_class(self):
self.stops("break here for class")

@swiftTest
def test_stops(self):
def test_doesnt_stop_class(self):
self.doesnt_stop("break here for class")

def stops(self, breakpoint_string):
"""Tests that using archetypes in a conditional breakpoint's expression works correctly"""
self.build()
target = lldbutil.run_to_breakpoint_make_target(self)

breakpoint = target.BreakpointCreateBySourceRegex(
"break here", lldb.SBFileSpec("main.swift"))
breakpoint_string, lldb.SBFileSpec("main.swift")
)

breakpoint.SetCondition("T.self == Int.self")
_, process, _, _ = lldbutil.run_to_breakpoint_do_run(self, target, breakpoint)

self.assertEqual(process.state, lldb.eStateStopped)

self.assertEqual(process.state, lldb.eStateStopped)
self.expect("expression T.self", substrs=["Int"])

@swiftTest
def test_doesnt_stop(self):
def doesnt_stop(self, breakpoint_string):
"""Tests that using archetypes in a conditional breakpoint's expression works correctly"""
self.build()
target = lldbutil.run_to_breakpoint_make_target(self)

breakpoint = target.BreakpointCreateBySourceRegex(
"break here", lldb.SBFileSpec("main.swift"))
breakpoint_string, lldb.SBFileSpec("main.swift")
)

breakpoint.SetCondition("T.self == Double.self")

Expand All @@ -41,4 +56,3 @@ def test_doesnt_stop(self):

# Make sure that we didn't stop since the condition doesn't match
self.assertEqual(process.state, lldb.eStateExited)

16 changes: 15 additions & 1 deletion lldb/test/API/lang/swift/archetype_in_cond_breakpoint/main.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,18 @@

func f<T>(t: T) {
print(1) // break here
print(1) // break here for free function
}
f(t: "This is a string")
f(t: "This is another string")
f(t: true)
f(t: 5)

class MyClass<T> {
func f() {
print(1) // break here for class
}
}
MyClass<String>().f()
MyClass<String>().f()
MyClass<Bool>().f()
MyClass<Int>().f()