Skip to content

Commit 98c0f3f

Browse files
committed
Introduce a "transactional" diagnostic consumer.
Swift compiler and ClangImporter and LLDB operations can produce diagnostics, which are stored in StoringDiagnosticConsumer. Operations such as expression evaluation of module imports may produce multiple non-fatal error diagnostics. Prior to this patch, the expression parser would clear the diagnostic consumer before running. This had the unwanted side-effect of sometimes clearing out unreported earlier diagnostics. This patch introduces SwiftASTContext::forkDiagnosticConsumer() which returns an RAII object that behaves like a diagnostic consumer, can tell whether diagnostics where produced during its lifetime, and resets the StoringDiagnosticConsumer to its previous state upon destruction. This way - unreported diagnostics don't get cleared by the expression parser - the expression parser has precise access to only the diagnostics it produced - while still being able to report persistent errors (such as clang import failures) that happened before the expression parser started. I'm not happy by the amount of code needed to make this work, or the specific implementation, but at least the interface provided to the outside is nice and clean.
1 parent 71c884b commit 98c0f3f

File tree

4 files changed

+566
-426
lines changed

4 files changed

+566
-426
lines changed

lldb/source/Plugins/ExpressionParser/Swift/SwiftExpressionParser.cpp

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -771,7 +771,6 @@ SetupASTContext(SwiftASTContextForExpressions &swift_ast_context,
771771
// TODO: Find a way to get contraint-solver output sent to a stream
772772
// so we can log it.
773773
// swift_ast_context.GetLanguageOptions().DebugConstraintSolver = true;
774-
swift_ast_context.ClearDiagnostics();
775774

776775
// No longer part of debugger support, set it separately.
777776
swift_ast_context.GetLanguageOptions().EnableDollarIdentifiers = true;
@@ -1217,15 +1216,15 @@ AddArchetypesTypeAliases(std::unique_ptr<SwiftASTManipulator> &code_manipulator,
12171216
/// Attempt to parse an expression and import all the Swift modules
12181217
/// the expression and its context depend on.
12191218
static llvm::Expected<ParsedExpression> ParseAndImport(
1220-
SwiftASTContextForExpressions &swift_ast_context, Expression &expr,
1219+
SwiftASTContextForExpressions &swift_ast_context,
1220+
SwiftASTContext::ScopedDiagnostics &expr_diagnostics, Expression &expr,
12211221
SwiftExpressionParser::SILVariableMap &variable_map, unsigned &buffer_id,
12221222
DiagnosticManager &diagnostic_manager,
12231223
SwiftExpressionParser &swift_expr_parser,
12241224
lldb::StackFrameWP &stack_frame_wp, SymbolContext &sc,
1225-
ExecutionContextScope &exe_scope,
1225+
ExecutionContextScope &exe_scope,
12261226
llvm::SmallVectorImpl<SwiftASTManipulator::VariableInfo> &local_variables,
1227-
const EvaluateExpressionOptions &options,
1228-
bool repl, bool playground) {
1227+
const EvaluateExpressionOptions &options, bool repl, bool playground) {
12291228
Log *log = GetLog(LLDBLog::Expressions);
12301229
LLDB_SCOPED_TIMER();
12311230

@@ -1373,16 +1372,16 @@ static llvm::Expected<ParsedExpression> ParseAndImport(
13731372
// inserting them in.
13741373
swift_ast_context.AddDebuggerClient(external_lookup);
13751374

1376-
if (swift_ast_context.HasErrors())
1375+
if (expr_diagnostics.HasErrors())
13771376
return make_error<SwiftASTContextError>();
13781377

13791378
// Resolve the file's imports, including the implicit ones returned from
13801379
// GetImplicitImports.
13811380
swift::performImportResolution(*source_file);
13821381

1383-
if (swift_ast_context.HasErrors())
1382+
if (expr_diagnostics.HasErrors())
13841383
return make_error<ModuleImportError>(
1385-
swift_ast_context.GetAllErrors().AsCString(
1384+
expr_diagnostics.GetAllErrors().AsCString(
13861385
"Explicit module import error"));
13871386

13881387
std::unique_ptr<SwiftASTManipulator> code_manipulator;
@@ -1654,20 +1653,24 @@ RedirectCallFromSinkToTrampolineFunction(llvm::Module &module,
16541653
SwiftExpressionParser::ParseResult
16551654
SwiftExpressionParser::Parse(DiagnosticManager &diagnostic_manager,
16561655
uint32_t first_line, uint32_t last_line) {
1656+
SwiftExpressionParser::SILVariableMap variable_map;
16571657
using ParseResult = SwiftExpressionParser::ParseResult;
16581658
Log *log = GetLog(LLDBLog::Expressions);
16591659
LLDB_SCOPED_TIMER();
16601660

1661-
SwiftExpressionParser::SILVariableMap variable_map;
1661+
// Get a scoped diagnostics consumer for all diagnostics produced by
1662+
// this expression.
1663+
auto expr_diagnostics = m_swift_ast_ctx.getScopedDiagnosticConsumer();
1664+
m_swift_ast_ctx.GetDiagnosticEngine().resetHadAnyError();
16621665

16631666
// Helper function to diagnose errors in m_swift_scratch_context.
16641667
unsigned buffer_id = UINT32_MAX;
16651668
auto DiagnoseSwiftASTContextError = [&]() {
1666-
assert((m_swift_ast_ctx.HasErrors() ||
1669+
assert((expr_diagnostics->HasErrors() ||
16671670
m_swift_ast_ctx.HasClangImporterErrors()) &&
16681671
"error expected");
1669-
m_swift_ast_ctx.PrintDiagnostics(diagnostic_manager, buffer_id, first_line,
1670-
last_line);
1672+
expr_diagnostics->PrintDiagnostics(diagnostic_manager, buffer_id,
1673+
first_line, last_line);
16711674
};
16721675

16731676
// In the case of playgrounds, we turn all rewriting functionality off.
@@ -1679,9 +1682,9 @@ SwiftExpressionParser::Parse(DiagnosticManager &diagnostic_manager,
16791682

16801683
// Parse the expression and import all nececssary swift modules.
16811684
auto parsed_expr = ParseAndImport(
1682-
m_swift_ast_ctx, m_expr, variable_map, buffer_id, diagnostic_manager,
1683-
*this, m_stack_frame_wp, m_sc, *m_exe_scope, m_local_variables, m_options,
1684-
repl, playground);
1685+
m_swift_ast_ctx, *expr_diagnostics, m_expr, variable_map, buffer_id,
1686+
diagnostic_manager, *this, m_stack_frame_wp, m_sc, *m_exe_scope,
1687+
m_local_variables, m_options, repl, playground);
16851688

16861689
if (!parsed_expr) {
16871690
bool retry = false;
@@ -1740,7 +1743,7 @@ SwiftExpressionParser::Parse(DiagnosticManager &diagnostic_manager,
17401743
if (log)
17411744
dumpModule("Module after type checking:");
17421745

1743-
if (m_swift_ast_ctx.HasErrors()) {
1746+
if (expr_diagnostics->HasErrors()) {
17441747
DiagnoseSwiftASTContextError();
17451748
return ParseResult::unrecoverable_error;
17461749
}
@@ -1917,7 +1920,7 @@ SwiftExpressionParser::Parse(DiagnosticManager &diagnostic_manager,
19171920
log->PutCString(s.c_str());
19181921
}
19191922

1920-
if (m_swift_ast_ctx.HasErrors()) {
1923+
if (expr_diagnostics->HasErrors()) {
19211924
DiagnoseSwiftASTContextError();
19221925
return ParseResult::unrecoverable_error;
19231926
}
@@ -1945,7 +1948,7 @@ SwiftExpressionParser::Parse(DiagnosticManager &diagnostic_manager,
19451948
log->PutCString(s.c_str());
19461949
}
19471950

1948-
if (m_swift_ast_ctx.HasErrors()) {
1951+
if (expr_diagnostics->HasErrors()) {
19491952
DiagnoseSwiftASTContextError();
19501953
return ParseResult::unrecoverable_error;
19511954
}
@@ -1973,7 +1976,8 @@ SwiftExpressionParser::Parse(DiagnosticManager &diagnostic_manager,
19731976

19741977
// If IRGen failed without errors, the root cause may be a fatal
19751978
// Clang diagnostic.
1976-
if (m_swift_ast_ctx.HasErrors() || m_swift_ast_ctx.HasClangImporterErrors()) {
1979+
if (expr_diagnostics->HasErrors() ||
1980+
m_swift_ast_ctx.HasClangImporterErrors()) {
19771981
diagnostic_manager.PutString(eDiagnosticSeverityRemark,
19781982
"couldn't IRGen expression");
19791983
DiagnoseSwiftASTContextError();
@@ -2029,7 +2033,7 @@ SwiftExpressionParser::Parse(DiagnosticManager &diagnostic_manager,
20292033
nullptr);
20302034
}
20312035

2032-
if (m_swift_ast_ctx.HasErrors())
2036+
if (expr_diagnostics->HasErrors())
20332037
return ParseResult::unrecoverable_error;
20342038

20352039
// The Parse succeeded! Now put this module into the context's list

0 commit comments

Comments
 (0)