Skip to content

[lldb] Handle diagnostics better around expression evaulation retries #7834

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
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
9 changes: 9 additions & 0 deletions lldb/include/lldb/Expression/DiagnosticManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,15 @@ class DiagnosticManager {
m_diagnostics.push_back(std::move(diagnostic));
}

/// Moves over the contents of a second diagnostic manager over. Leaves other
/// diagnostic manager in an empty state.
void Consume(DiagnosticManager &&other) {
std::move(other.m_diagnostics.begin(), other.m_diagnostics.end(),
std::back_inserter(m_diagnostics));
m_fixed_expression = std::move(other.m_fixed_expression);
other.Clear();
}

size_t Printf(DiagnosticSeverity severity, const char *format, ...)
__attribute__((format(printf, 3, 4)));
void PutString(DiagnosticSeverity severity, llvm::StringRef str);
Expand Down
42 changes: 33 additions & 9 deletions lldb/source/Plugins/ExpressionParser/Swift/SwiftUserExpression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -755,13 +755,40 @@ exe_scope = exe_ctx.GetBestExecutionContextScope();
m_options.SetGenerateDebugInfo(generate_debug_info);

using ParseResult = SwiftExpressionParser::ParseResult;

// Use a separate diagnostic manager instead of the main one, the reason we do
// this is that on retries we would like to ignore diagnostics produced by
// either the first or second try.
DiagnosticManager first_try_diagnostic_manager;
DiagnosticManager second_try_diagnostic_manager;

bool retry = false;
while (true) {
SwiftExpressionParser::ParseResult parse_result =
GetTextAndSetExpressionParser(diagnostic_manager, source_code, exe_ctx,
exe_scope);
SwiftExpressionParser::ParseResult parse_result;
if (!retry) {
parse_result = GetTextAndSetExpressionParser(
first_try_diagnostic_manager, source_code, exe_ctx, exe_scope);
if (parse_result != SwiftExpressionParser::ParseResult::
retry_no_bind_generic_params ||
m_options.GetBindGenericTypes() != lldb::eBindAuto)
// If we're not retrying, just copy the diagnostics over.
diagnostic_manager.Consume(std::move(first_try_diagnostic_manager));
} else {
parse_result = GetTextAndSetExpressionParser(
second_try_diagnostic_manager, source_code, exe_ctx, exe_scope);
if (parse_result == SwiftExpressionParser::ParseResult::success)
// If we succeeded the second time around, copy any diagnostics we
// produced in the success case over, and ignore the first attempt's
// failures.
diagnostic_manager.Consume(std::move(second_try_diagnostic_manager));
else
// If we failed though, copy the diagnostics of the first attempt, and
// silently ignore any errors produced by the retry, as the retry was
// not what the user asked, and any diagnostics produced by it will
// most likely confuse the user.
diagnostic_manager.Consume(std::move(first_try_diagnostic_manager));
}

if (parse_result == ParseResult::success)
if (parse_result == SwiftExpressionParser::ParseResult::success)
break;

switch (parse_result) {
Expand All @@ -770,13 +797,10 @@ exe_scope = exe_ctx.GetBestExecutionContextScope();
// BindGenericTypes was in the auto setting, give up otherwise.
if (m_options.GetBindGenericTypes() != lldb::eBindAuto)
return false;
diagnostic_manager.Clear();
diagnostic_manager.PutString(eDiagnosticSeverityRemark,
"Expression evaluation failed. Retrying "
"without binding generic parameters");
// Retry without binding generic parameters, this is the only
// case that will loop.
m_options.SetBindGenericTypes(lldb::eDontBind);
retry = true;
break;

case ParseResult::retry_fresh_context:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,3 +151,8 @@ def test_private_generic_type(self):
substrs=["Could not evaluate the expression without binding generic types."],
error=True)

# Check that if both binding and not binding the generic type parameters fail, we report
# the "bind generic params" error message, as that's the default case that runs first.
self.expect("e --bind-generic-types auto -- self",
substrs=["Couldn't realize Swift AST type of self."],
error=True)