From b783197ec6ebf8d5f2846d0a215da63619ed20c9 Mon Sep 17 00:00:00 2001 From: Prabhat Sachdeva Date: Thu, 27 Jul 2023 00:33:20 +0530 Subject: [PATCH] Explorer: add trace for unformed variable resolution (#3015) Adds trace information for unformed variables resolution. --- explorer/interpreter/BUILD | 1 + explorer/interpreter/exec_program.cpp | 2 +- explorer/interpreter/resolve_unformed.cpp | 224 ++++++++++++++-------- explorer/interpreter/resolve_unformed.h | 11 +- explorer/trace_testdata/full_trace.carbon | 12 ++ 5 files changed, 169 insertions(+), 81 deletions(-) diff --git a/explorer/interpreter/BUILD b/explorer/interpreter/BUILD index 914003fdb0aa..f388f3f93092 100644 --- a/explorer/interpreter/BUILD +++ b/explorer/interpreter/BUILD @@ -236,6 +236,7 @@ cc_library( "//explorer/ast:static_scope", "//explorer/common:error_builders", "//explorer/common:nonnull", + "//explorer/common:trace_stream", "@llvm-project//llvm:Support", ], ) diff --git a/explorer/interpreter/exec_program.cpp b/explorer/interpreter/exec_program.cpp index e89380bbf36b..77e38ad2b2ab 100644 --- a/explorer/interpreter/exec_program.cpp +++ b/explorer/interpreter/exec_program.cpp @@ -64,7 +64,7 @@ auto AnalyzeProgram(Nonnull arena, AST ast, if (trace_stream->is_enabled()) { *trace_stream << "********** resolving unformed variables **********\n"; } - CARBON_RETURN_IF_ERROR(ResolveUnformed(ast)); + CARBON_RETURN_IF_ERROR(ResolveUnformed(trace_stream, ast)); set_prog_phase.update_phase(ProgramPhase::Declarations); if (trace_stream->is_enabled()) { diff --git a/explorer/interpreter/resolve_unformed.cpp b/explorer/interpreter/resolve_unformed.cpp index 35c69244eb5c..c53f142877ef 100644 --- a/explorer/interpreter/resolve_unformed.cpp +++ b/explorer/interpreter/resolve_unformed.cpp @@ -17,6 +17,22 @@ using llvm::cast; namespace Carbon { +auto FlowFacts::action_type_string(ActionType action) const + -> std::string_view { + switch (action) { + case ActionType::AddInit: + return "add init"; + case ActionType::AddUninit: + return "add uninit"; + case ActionType::Form: + return "form"; + case ActionType::Check: + return "check"; + case ActionType::None: + return "none"; + } +} + auto FlowFacts::TakeAction(Nonnull node, ActionType action, SourceLocation source_loc, const std::string& name) -> ErrorOr { @@ -52,22 +68,27 @@ auto FlowFacts::TakeAction(Nonnull node, ActionType action, case ActionType::None: break; } + + if (trace_stream_->is_enabled()) { + *trace_stream_ << "--- " << action_type_string(action) << " `" << name + << "` (" << source_loc << ")\n"; + } + return Success(); } -static auto ResolveUnformedImpl(Nonnull expression, +static auto ResolveUnformedImpl(Nonnull trace_stream, + Nonnull expression, FlowFacts& flow_facts, FlowFacts::ActionType action) -> ErrorOr; -static auto ResolveUnformedImpl(Nonnull pattern, +static auto ResolveUnformedImpl(Nonnull trace_stream, + Nonnull pattern, FlowFacts& flow_facts, FlowFacts::ActionType action) -> ErrorOr; -static auto ResolveUnformedImpl(Nonnull statement, - FlowFacts& flow_facts, - FlowFacts::ActionType action) - -> ErrorOr; -static auto ResolveUnformedImpl(Nonnull expression, +static auto ResolveUnformedImpl(Nonnull trace_stream, + Nonnull statement, FlowFacts& flow_facts, FlowFacts::ActionType action) -> ErrorOr; @@ -75,13 +96,16 @@ static auto ResolveUnformedImpl(Nonnull expression, // Traverses the sub-AST rooted at the given node, resolving the formed/unformed // states of local variables within it and updating the flow facts. template -static auto ResolveUnformed(Nonnull expression, FlowFacts& flow_facts, +static auto ResolveUnformed(Nonnull trace_stream, + Nonnull expression, FlowFacts& flow_facts, FlowFacts::ActionType action) -> ErrorOr { - return RunWithExtraStack( - [&] { return ResolveUnformedImpl(expression, flow_facts, action); }); + return RunWithExtraStack([&] { + return ResolveUnformedImpl(trace_stream, expression, flow_facts, action); + }); } -static auto ResolveUnformedImpl(Nonnull expression, +static auto ResolveUnformedImpl(Nonnull trace_stream, + Nonnull expression, FlowFacts& flow_facts, FlowFacts::ActionType action) -> ErrorOr { @@ -96,19 +120,20 @@ static auto ResolveUnformedImpl(Nonnull expression, case ExpressionKind::CallExpression: { const auto& call = cast(*expression); CARBON_RETURN_IF_ERROR( - ResolveUnformed(&call.argument(), flow_facts, action)); + ResolveUnformed(trace_stream, &call.argument(), flow_facts, action)); break; } case ExpressionKind::IntrinsicExpression: { const auto& intrin = cast(*expression); CARBON_RETURN_IF_ERROR( - ResolveUnformed(&intrin.args(), flow_facts, action)); + ResolveUnformed(trace_stream, &intrin.args(), flow_facts, action)); break; } case ExpressionKind::TupleLiteral: for (Nonnull field : cast(*expression).fields()) { - CARBON_RETURN_IF_ERROR(ResolveUnformed(field, flow_facts, action)); + CARBON_RETURN_IF_ERROR( + ResolveUnformed(trace_stream, field, flow_facts, action)); } break; case ExpressionKind::OperatorExpression: { @@ -123,11 +148,12 @@ static auto ResolveUnformedImpl(Nonnull expression, // TODO: This isn't enough to permit &x.y or &x[i] when x is // uninitialized, because x.y and x[i] both require x to be // initialized. - ResolveUnformed(opt_exp.arguments().front(), flow_facts, - FlowFacts::ActionType::Form)); + ResolveUnformed(trace_stream, opt_exp.arguments().front(), + flow_facts, FlowFacts::ActionType::Form)); } else { for (Nonnull operand : opt_exp.arguments()) { - CARBON_RETURN_IF_ERROR(ResolveUnformed(operand, flow_facts, action)); + CARBON_RETURN_IF_ERROR( + ResolveUnformed(trace_stream, operand, flow_facts, action)); } } break; @@ -135,38 +161,41 @@ static auto ResolveUnformedImpl(Nonnull expression, case ExpressionKind::StructLiteral: for (const FieldInitializer& init : cast(*expression).fields()) { - CARBON_RETURN_IF_ERROR(ResolveUnformed(&init.expression(), flow_facts, + CARBON_RETURN_IF_ERROR(ResolveUnformed(trace_stream, &init.expression(), + flow_facts, FlowFacts::ActionType::Check)); } break; case ExpressionKind::SimpleMemberAccessExpression: case ExpressionKind::CompoundMemberAccessExpression: case ExpressionKind::BaseAccessExpression: - CARBON_RETURN_IF_ERROR( - ResolveUnformed(&cast(*expression).object(), - flow_facts, FlowFacts::ActionType::Check)); + CARBON_RETURN_IF_ERROR(ResolveUnformed( + trace_stream, &cast(*expression).object(), + flow_facts, FlowFacts::ActionType::Check)); break; case ExpressionKind::BuiltinConvertExpression: CARBON_RETURN_IF_ERROR(ResolveUnformed( + trace_stream, cast(*expression).source_expression(), flow_facts, FlowFacts::ActionType::Check)); break; case ExpressionKind::IndexExpression: - CARBON_RETURN_IF_ERROR( - ResolveUnformed(&cast(*expression).object(), - flow_facts, FlowFacts::ActionType::Check)); - CARBON_RETURN_IF_ERROR( - ResolveUnformed(&cast(*expression).offset(), - flow_facts, FlowFacts::ActionType::Check)); + CARBON_RETURN_IF_ERROR(ResolveUnformed( + trace_stream, &cast(*expression).object(), + flow_facts, FlowFacts::ActionType::Check)); + CARBON_RETURN_IF_ERROR(ResolveUnformed( + trace_stream, &cast(*expression).offset(), + flow_facts, FlowFacts::ActionType::Check)); break; case ExpressionKind::IfExpression: { const auto& if_exp = cast(*expression); - CARBON_RETURN_IF_ERROR(ResolveUnformed(&if_exp.condition(), flow_facts, + CARBON_RETURN_IF_ERROR(ResolveUnformed(trace_stream, &if_exp.condition(), + flow_facts, FlowFacts::ActionType::Check)); - CARBON_RETURN_IF_ERROR( - ResolveUnformed(&if_exp.then_expression(), flow_facts, action)); - CARBON_RETURN_IF_ERROR( - ResolveUnformed(&if_exp.else_expression(), flow_facts, action)); + CARBON_RETURN_IF_ERROR(ResolveUnformed( + trace_stream, &if_exp.then_expression(), flow_facts, action)); + CARBON_RETURN_IF_ERROR(ResolveUnformed( + trace_stream, &if_exp.else_expression(), flow_facts, action)); break; } case ExpressionKind::DotSelfExpression: @@ -185,10 +214,12 @@ static auto ResolveUnformedImpl(Nonnull expression, case ExpressionKind::ArrayTypeLiteral: break; } + return Success(); } -static auto ResolveUnformedImpl(Nonnull pattern, +static auto ResolveUnformedImpl(Nonnull trace_stream, + Nonnull pattern, FlowFacts& flow_facts, FlowFacts::ActionType action) -> ErrorOr { @@ -202,7 +233,8 @@ static auto ResolveUnformedImpl(Nonnull pattern, case PatternKind::TuplePattern: for (Nonnull field : cast(*pattern).fields()) { - CARBON_RETURN_IF_ERROR(ResolveUnformed(field, flow_facts, action)); + CARBON_RETURN_IF_ERROR( + ResolveUnformed(trace_stream, field, flow_facts, action)); } break; case PatternKind::GenericBinding: @@ -217,29 +249,37 @@ static auto ResolveUnformedImpl(Nonnull pattern, return Success(); } -static auto ResolveUnformedImpl(Nonnull statement, +static auto ResolveUnformedImpl(Nonnull trace_stream, + Nonnull statement, FlowFacts& flow_facts, FlowFacts::ActionType action) -> ErrorOr { + if (trace_stream->is_enabled()) { + *trace_stream << "*** resolving-unformed in stmt `" << PrintAsID(*statement) + << "` (" << statement->source_loc() << ")\n"; + } switch (statement->kind()) { case StatementKind::Block: { const auto& block = cast(*statement); for (const auto* block_statement : block.statements()) { CARBON_RETURN_IF_ERROR( - ResolveUnformed(block_statement, flow_facts, action)); + ResolveUnformed(trace_stream, block_statement, flow_facts, action)); } break; } case StatementKind::VariableDefinition: { const auto& def = cast(*statement); if (def.has_init()) { - CARBON_RETURN_IF_ERROR(ResolveUnformed(&def.pattern(), flow_facts, + CARBON_RETURN_IF_ERROR(ResolveUnformed(trace_stream, &def.pattern(), + flow_facts, FlowFacts::ActionType::AddInit)); - CARBON_RETURN_IF_ERROR(ResolveUnformed(&def.init(), flow_facts, + CARBON_RETURN_IF_ERROR(ResolveUnformed(trace_stream, &def.init(), + flow_facts, FlowFacts::ActionType::Check)); } else { - CARBON_RETURN_IF_ERROR(ResolveUnformed( - &def.pattern(), flow_facts, FlowFacts::ActionType::AddUninit)); + CARBON_RETURN_IF_ERROR( + ResolveUnformed(trace_stream, &def.pattern(), flow_facts, + FlowFacts::ActionType::AddUninit)); } break; } @@ -254,78 +294,87 @@ static auto ResolveUnformedImpl(Nonnull statement, } case StatementKind::ReturnExpression: { const auto& ret_exp_stmt = cast(*statement); - CARBON_RETURN_IF_ERROR(ResolveUnformed(&ret_exp_stmt.expression(), - flow_facts, - FlowFacts::ActionType::Check)); + CARBON_RETURN_IF_ERROR( + ResolveUnformed(trace_stream, &ret_exp_stmt.expression(), flow_facts, + FlowFacts::ActionType::Check)); break; } case StatementKind::Assign: { const auto& assign = cast(*statement); if (assign.op() != AssignOperator::Plain) { - CARBON_RETURN_IF_ERROR(ResolveUnformed(&assign.lhs(), flow_facts, + CARBON_RETURN_IF_ERROR(ResolveUnformed(trace_stream, &assign.lhs(), + flow_facts, FlowFacts::ActionType::Check)); } else if (assign.lhs().kind() == ExpressionKind::IdentifierExpression) { - CARBON_RETURN_IF_ERROR(ResolveUnformed(&assign.lhs(), flow_facts, + CARBON_RETURN_IF_ERROR(ResolveUnformed(trace_stream, &assign.lhs(), + flow_facts, FlowFacts::ActionType::Form)); } else { // TODO: Support checking non-identifier lhs expression. - CARBON_RETURN_IF_ERROR(ResolveUnformed(&assign.lhs(), flow_facts, + CARBON_RETURN_IF_ERROR(ResolveUnformed(trace_stream, &assign.lhs(), + flow_facts, FlowFacts::ActionType::None)); } - CARBON_RETURN_IF_ERROR(ResolveUnformed(&assign.rhs(), flow_facts, + CARBON_RETURN_IF_ERROR(ResolveUnformed(trace_stream, &assign.rhs(), + flow_facts, FlowFacts::ActionType::Check)); break; } case StatementKind::IncrementDecrement: { - CARBON_RETURN_IF_ERROR( - ResolveUnformed(&cast(statement)->argument(), - flow_facts, FlowFacts::ActionType::Check)); + CARBON_RETURN_IF_ERROR(ResolveUnformed( + trace_stream, &cast(statement)->argument(), + flow_facts, FlowFacts::ActionType::Check)); break; } case StatementKind::ExpressionStatement: { const auto& exp_stmt = cast(*statement); - CARBON_RETURN_IF_ERROR( - ResolveUnformed(&exp_stmt.expression(), flow_facts, action)); + CARBON_RETURN_IF_ERROR(ResolveUnformed( + trace_stream, &exp_stmt.expression(), flow_facts, action)); break; } case StatementKind::If: { const auto& if_stmt = cast(*statement); - CARBON_RETURN_IF_ERROR(ResolveUnformed(&if_stmt.condition(), flow_facts, + CARBON_RETURN_IF_ERROR(ResolveUnformed(trace_stream, &if_stmt.condition(), + flow_facts, FlowFacts::ActionType::Check)); - CARBON_RETURN_IF_ERROR( - ResolveUnformed(&if_stmt.then_block(), flow_facts, action)); + CARBON_RETURN_IF_ERROR(ResolveUnformed( + trace_stream, &if_stmt.then_block(), flow_facts, action)); if (if_stmt.else_block().has_value()) { - CARBON_RETURN_IF_ERROR( - ResolveUnformed(*if_stmt.else_block(), flow_facts, action)); + CARBON_RETURN_IF_ERROR(ResolveUnformed( + trace_stream, *if_stmt.else_block(), flow_facts, action)); } break; } case StatementKind::While: { const auto& while_stmt = cast(*statement); - CARBON_RETURN_IF_ERROR(ResolveUnformed( - &while_stmt.condition(), flow_facts, FlowFacts::ActionType::Check)); CARBON_RETURN_IF_ERROR( - ResolveUnformed(&while_stmt.body(), flow_facts, action)); + ResolveUnformed(trace_stream, &while_stmt.condition(), flow_facts, + FlowFacts::ActionType::Check)); + CARBON_RETURN_IF_ERROR(ResolveUnformed(trace_stream, &while_stmt.body(), + flow_facts, action)); break; } case StatementKind::Match: { const auto& match = cast(*statement); - CARBON_RETURN_IF_ERROR(ResolveUnformed(&match.expression(), flow_facts, + CARBON_RETURN_IF_ERROR(ResolveUnformed(trace_stream, &match.expression(), + flow_facts, FlowFacts::ActionType::Check)); for (const auto& clause : match.clauses()) { - CARBON_RETURN_IF_ERROR(ResolveUnformed(&clause.pattern(), flow_facts, + CARBON_RETURN_IF_ERROR(ResolveUnformed(trace_stream, &clause.pattern(), + flow_facts, FlowFacts::ActionType::Check)); - CARBON_RETURN_IF_ERROR( - ResolveUnformed(&clause.statement(), flow_facts, action)); + CARBON_RETURN_IF_ERROR(ResolveUnformed( + trace_stream, &clause.statement(), flow_facts, action)); } break; } case StatementKind::For: { const auto& for_stmt = cast(*statement); - CARBON_RETURN_IF_ERROR(ResolveUnformed( - &for_stmt.loop_target(), flow_facts, FlowFacts::ActionType::Check)); CARBON_RETURN_IF_ERROR( - ResolveUnformed(&for_stmt.body(), flow_facts, action)); + ResolveUnformed(trace_stream, &for_stmt.loop_target(), flow_facts, + FlowFacts::ActionType::Check)); + CARBON_RETURN_IF_ERROR( + ResolveUnformed(trace_stream, &for_stmt.body(), flow_facts, action)); break; } case StatementKind::Break: @@ -336,22 +385,32 @@ static auto ResolveUnformedImpl(Nonnull statement, return Success(); } -static auto ResolveUnformed(Nonnull declaration) +static auto ResolveUnformed(Nonnull trace_stream, + Nonnull declaration) -> ErrorOr; static auto ResolveUnformed( + Nonnull trace_stream, llvm::ArrayRef> declarations) -> ErrorOr { - return RunWithExtraStack([declarations]() -> ErrorOr { + return RunWithExtraStack([trace_stream, declarations]() -> ErrorOr { for (Nonnull declaration : declarations) { - CARBON_RETURN_IF_ERROR(ResolveUnformed(declaration)); + CARBON_RETURN_IF_ERROR(ResolveUnformed(trace_stream, declaration)); } return Success(); }); } -static auto ResolveUnformed(Nonnull declaration) +static auto ResolveUnformed(Nonnull trace_stream, + Nonnull declaration) -> ErrorOr { + SetFileContext set_file_ctx(*trace_stream, declaration->source_loc()); + + if (trace_stream->is_enabled()) { + *trace_stream << "*** resolving-unformed in decl `" + << PrintAsID(*declaration) << "` (" + << declaration->source_loc() << ")\n"; + } switch (declaration->kind()) { // Checks formed/unformed state intraprocedurally. // Can be extended to an interprocedural analysis when a call graph is @@ -359,9 +418,11 @@ static auto ResolveUnformed(Nonnull declaration) case DeclarationKind::FunctionDeclaration: case DeclarationKind::DestructorDeclaration: { const auto& callable = cast(*declaration); - if (callable.body().has_value()) { - FlowFacts flow_facts; - CARBON_RETURN_IF_ERROR(ResolveUnformed(*callable.body(), flow_facts, + const auto callable_body = callable.body(); + if (callable_body) { + FlowFacts flow_facts(trace_stream); + CARBON_RETURN_IF_ERROR(ResolveUnformed(trace_stream, *callable_body, + flow_facts, FlowFacts::ActionType::None)); } break; @@ -380,22 +441,27 @@ static auto ResolveUnformed(Nonnull declaration) // do nothing break; case DeclarationKind::ClassDeclaration: - return ResolveUnformed(cast(declaration)->members()); + return ResolveUnformed(trace_stream, + cast(declaration)->members()); case DeclarationKind::MixinDeclaration: - return ResolveUnformed(cast(declaration)->members()); + return ResolveUnformed(trace_stream, + cast(declaration)->members()); case DeclarationKind::InterfaceDeclaration: case DeclarationKind::ConstraintDeclaration: return ResolveUnformed( + trace_stream, cast(declaration)->members()); case DeclarationKind::ImplDeclaration: - return ResolveUnformed(cast(declaration)->members()); + return ResolveUnformed(trace_stream, + cast(declaration)->members()); } return Success(); } -auto ResolveUnformed(const AST& ast) -> ErrorOr { +auto ResolveUnformed(Nonnull trace_stream, const AST& ast) + -> ErrorOr { for (auto* declaration : ast.declarations) { - CARBON_RETURN_IF_ERROR(ResolveUnformed(declaration)); + CARBON_RETURN_IF_ERROR(ResolveUnformed(trace_stream, declaration)); } return Success(); } diff --git a/explorer/interpreter/resolve_unformed.h b/explorer/interpreter/resolve_unformed.h index 798fa87efd08..4b9caa36f256 100644 --- a/explorer/interpreter/resolve_unformed.h +++ b/explorer/interpreter/resolve_unformed.h @@ -10,12 +10,16 @@ #include "explorer/ast/ast.h" #include "explorer/common/nonnull.h" +#include "explorer/common/trace_stream.h" namespace Carbon { // Maps AST nodes to flow facts within a function. class FlowFacts { public: + explicit FlowFacts(Nonnull trace_stream) + : trace_stream_(trace_stream) {} + enum class ActionType { // Adds a must-be-formed flow fact. // Used at `VariableDefinition` with initialization. @@ -32,6 +36,9 @@ class FlowFacts { // Used in traversing children nodes without an acion to take. None, }; + + auto action_type_string(ActionType action) const -> std::string_view; + // Take action on flow facts based on `ActionType`. auto TakeAction(Nonnull node, ActionType action, SourceLocation source_loc, const std::string& name) @@ -54,12 +61,14 @@ class FlowFacts { } std::unordered_map, Fact> facts_; + Nonnull trace_stream_; }; // An intraprocedural forward analysis that checks the may-be-formed states on // local variables. Returns compilation error on usage of must-be-unformed // variables. -auto ResolveUnformed(const AST& ast) -> ErrorOr; +auto ResolveUnformed(Nonnull trace_stream, const AST& ast) + -> ErrorOr; } // namespace Carbon diff --git a/explorer/trace_testdata/full_trace.carbon b/explorer/trace_testdata/full_trace.carbon index 1c3a0e6afaeb..7baeecad0adc 100644 --- a/explorer/trace_testdata/full_trace.carbon +++ b/explorer/trace_testdata/full_trace.carbon @@ -166,6 +166,18 @@ fn Main() -> i32 { // CHECK:STDOUT: performing argument deduction for bindings:{{ }} // CHECK:STDOUT: deduction succeeded with results: {} // CHECK:STDOUT: ********** resolving unformed variables ********** +// CHECK:STDOUT: *** resolving-unformed in decl `interface TestInterface` (full_trace.carbon:7) +// CHECK:STDOUT: *** resolving-unformed in decl `namespace N` (full_trace.carbon:9) +// CHECK:STDOUT: *** resolving-unformed in decl `fn N.Foo` (full_trace.carbon:13) +// CHECK:STDOUT: *** resolving-unformed in stmt `{return (n + 1);}` (full_trace.carbon:13) +// CHECK:STDOUT: *** resolving-unformed in stmt `return (n + 1);` (full_trace.carbon:12) +// CHECK:STDOUT: --- check `n` (full_trace.carbon:12) +// CHECK:STDOUT: *** resolving-unformed in decl `fn Main` (full_trace.carbon:18) +// CHECK:STDOUT: *** resolving-unformed in stmt `{var x: i32 = N.Foo(0);return x;}` (full_trace.carbon:18) +// CHECK:STDOUT: *** resolving-unformed in stmt `var x: i32 = N.Foo(0);` (full_trace.carbon:16) +// CHECK:STDOUT: --- add init `x` (full_trace.carbon:16) +// CHECK:STDOUT: *** resolving-unformed in stmt `return x;` (full_trace.carbon:17) +// CHECK:STDOUT: --- check `x` (full_trace.carbon:17) // CHECK:STDOUT: ********** printing declarations ********** // CHECK:STDOUT: interface TestInterface { // CHECK:STDOUT: }