From 62ee4a5c7a810477a0dfaec382357d3dedc0ad92 Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Wed, 19 Apr 2023 16:56:02 -0700 Subject: [PATCH] Suppress Print intrinsic output in the fuzzer. (#2784) Related to #2780, but mostly an issue when running over the committed corpus since we use Print a lot. --- explorer/fuzzing/fuzzer_util.cpp | 7 ++- explorer/interpreter/exec_program.cpp | 11 ++-- explorer/interpreter/exec_program.h | 6 +- explorer/interpreter/interpreter.cpp | 27 ++++++--- explorer/interpreter/interpreter.h | 6 +- explorer/interpreter/type_checker.cpp | 86 ++++++++++++--------------- explorer/interpreter/type_checker.h | 21 +++++-- explorer/main.cpp | 6 +- 8 files changed, 96 insertions(+), 74 deletions(-) diff --git a/explorer/fuzzing/fuzzer_util.cpp b/explorer/fuzzing/fuzzer_util.cpp index 49796f3b9688..b88f564c45cf 100644 --- a/explorer/fuzzing/fuzzer_util.cpp +++ b/explorer/fuzzing/fuzzer_util.cpp @@ -86,8 +86,11 @@ auto ParseAndExecute(const Fuzzing::CompilationUnit& compilation_unit) AddPrelude(*prelude_path, &arena, &ast.declarations, &ast.num_prelude_declarations); TraceStream trace_stream; - CARBON_ASSIGN_OR_RETURN(ast, AnalyzeProgram(&arena, ast, &trace_stream)); - return ExecProgram(&arena, ast, &trace_stream); + + // Use llvm::nulls() to suppress output from the Print intrinsic. + CARBON_ASSIGN_OR_RETURN( + ast, AnalyzeProgram(&arena, ast, &trace_stream, &llvm::nulls())); + return ExecProgram(&arena, ast, &trace_stream, &llvm::nulls()); } } // namespace Carbon diff --git a/explorer/interpreter/exec_program.cpp b/explorer/interpreter/exec_program.cpp index 1f171aa3ce87..7ec5b00bf2d0 100644 --- a/explorer/interpreter/exec_program.cpp +++ b/explorer/interpreter/exec_program.cpp @@ -19,7 +19,8 @@ namespace Carbon { auto AnalyzeProgram(Nonnull arena, AST ast, - Nonnull trace_stream) -> ErrorOr { + Nonnull trace_stream, + Nonnull print_stream) -> ErrorOr { if (trace_stream->is_enabled()) { *trace_stream << "********** source program **********\n"; for (int i = ast.num_prelude_declarations; @@ -46,7 +47,8 @@ auto AnalyzeProgram(Nonnull arena, AST ast, if (trace_stream->is_enabled()) { *trace_stream << "********** type checking **********\n"; } - CARBON_RETURN_IF_ERROR(TypeChecker(arena, trace_stream).TypeCheck(ast)); + CARBON_RETURN_IF_ERROR( + TypeChecker(arena, trace_stream, print_stream).TypeCheck(ast)); if (trace_stream->is_enabled()) { *trace_stream << "********** resolving unformed variables **********\n"; @@ -64,11 +66,12 @@ auto AnalyzeProgram(Nonnull arena, AST ast, } auto ExecProgram(Nonnull arena, AST ast, - Nonnull trace_stream) -> ErrorOr { + Nonnull trace_stream, + Nonnull print_stream) -> ErrorOr { if (trace_stream->is_enabled()) { *trace_stream << "********** starting execution **********\n"; } - return InterpProgram(ast, arena, trace_stream); + return InterpProgram(ast, arena, trace_stream, print_stream); } } // namespace Carbon diff --git a/explorer/interpreter/exec_program.h b/explorer/interpreter/exec_program.h index d602a507812d..8f372b759611 100644 --- a/explorer/interpreter/exec_program.h +++ b/explorer/interpreter/exec_program.h @@ -17,11 +17,13 @@ namespace Carbon { // Perform semantic analysis on the AST. auto AnalyzeProgram(Nonnull arena, AST ast, - Nonnull trace_stream) -> ErrorOr; + Nonnull trace_stream, + Nonnull print_stream) -> ErrorOr; // Run the program's `Main` function. auto ExecProgram(Nonnull arena, AST ast, - Nonnull trace_stream) -> ErrorOr; + Nonnull trace_stream, + Nonnull print_stream) -> ErrorOr; } // namespace Carbon diff --git a/explorer/interpreter/interpreter.cpp b/explorer/interpreter/interpreter.cpp index 3e15d429dc9e..c0e6a7d92594 100644 --- a/explorer/interpreter/interpreter.cpp +++ b/explorer/interpreter/interpreter.cpp @@ -59,11 +59,13 @@ class Interpreter { // traces if `trace` is true. `phase` indicates whether it executes at // compile time or run time. Interpreter(Phase phase, Nonnull arena, - Nonnull trace_stream) + Nonnull trace_stream, + Nonnull print_stream) : arena_(arena), heap_(arena), todo_(MakeTodo(phase, &heap_)), trace_stream_(trace_stream), + print_stream_(print_stream), phase_(phase) {} // Runs all the steps of `action`. @@ -178,6 +180,10 @@ class Interpreter { ActionStack todo_; Nonnull trace_stream_; + + // The stream for the Print intrinsic. + Nonnull print_stream_; + Phase phase_; }; @@ -1561,17 +1567,17 @@ auto Interpreter::StepExp() -> ErrorOr { intrinsic.source_loc(), format_string, num_format_args)); switch (num_format_args) { case 0: - llvm::outs() << llvm::formatv(format_string); + *print_stream_ << llvm::formatv(format_string); break; case 1: - llvm::outs() << llvm::formatv(format_string, - cast(*args[1]).value()); + *print_stream_ << llvm::formatv(format_string, + cast(*args[1]).value()); break; default: CARBON_FATAL() << "Too many format args: " << num_format_args; } // Implicit newline; currently no way to disable it. - llvm::outs() << "\n"; + *print_stream_ << "\n"; return todo_.FinishAction(TupleValue::Empty()); } case IntrinsicExpression::Intrinsic::Assert: { @@ -2438,8 +2444,9 @@ auto Interpreter::RunAllSteps(std::unique_ptr action) } auto InterpProgram(const AST& ast, Nonnull arena, - Nonnull trace_stream) -> ErrorOr { - Interpreter interpreter(Phase::RunTime, arena, trace_stream); + Nonnull trace_stream, + Nonnull print_stream) -> ErrorOr { + Interpreter interpreter(Phase::RunTime, arena, trace_stream, print_stream); if (trace_stream->is_enabled()) { *trace_stream << "********** initializing globals **********\n"; } @@ -2460,9 +2467,11 @@ auto InterpProgram(const AST& ast, Nonnull arena, } auto InterpExp(Nonnull e, Nonnull arena, - Nonnull trace_stream) + Nonnull trace_stream, + Nonnull print_stream) -> ErrorOr> { - Interpreter interpreter(Phase::CompileTime, arena, trace_stream); + Interpreter interpreter(Phase::CompileTime, arena, trace_stream, + print_stream); CARBON_RETURN_IF_ERROR( interpreter.RunAllSteps(std::make_unique(e))); return interpreter.result(); diff --git a/explorer/interpreter/interpreter.h b/explorer/interpreter/interpreter.h index 88d3d511a34f..fdc896af6cbb 100644 --- a/explorer/interpreter/interpreter.h +++ b/explorer/interpreter/interpreter.h @@ -25,13 +25,15 @@ namespace Carbon { // Interprets the program defined by `ast`, allocating values on `arena` and // printing traces if `trace` is true. auto InterpProgram(const AST& ast, Nonnull arena, - Nonnull trace_stream) -> ErrorOr; + Nonnull trace_stream, + Nonnull print_stream) -> ErrorOr; // Interprets `e` at compile-time, allocating values on `arena` and // printing traces if `trace` is true. The caller must ensure that all the // code this evaluates has been typechecked. auto InterpExp(Nonnull e, Nonnull arena, - Nonnull trace_stream) + Nonnull trace_stream, + Nonnull print_stream) -> ErrorOr>; // Attempts to match `v` against the pattern `p`, returning whether matching diff --git a/explorer/interpreter/type_checker.cpp b/explorer/interpreter/type_checker.cpp index 4ec35bc899c5..9f3e7a463f24 100644 --- a/explorer/interpreter/type_checker.cpp +++ b/explorer/interpreter/type_checker.cpp @@ -756,7 +756,7 @@ auto TypeChecker::ImplicitlyConvert(std::string_view context, ImplicitlyConvert(context, impl_scope, source, arena_->New())); CARBON_ASSIGN_OR_RETURN(Nonnull converted_value, - InterpExp(source_as_type, arena_, trace_stream_)); + InterpExp(source_as_type)); CARBON_ASSIGN_OR_RETURN( Nonnull destination_constraint, ConvertToConstraintType(source->source_loc(), "implicit conversion", @@ -1363,7 +1363,7 @@ auto TypeChecker::ArgumentDeduction::Finish( // Evaluate the argument to get the value. CARBON_ASSIGN_OR_RETURN(Nonnull value, - InterpExp(arg, type_checker.arena_, trace_stream_)); + type_checker.InterpExp(arg)); if (trace_stream_->is_enabled()) { *trace_stream_ << "evaluated generic parameter " << *binding << " as " << *value << "\n"; @@ -2717,9 +2717,8 @@ auto TypeChecker::TypeCheckExp(Nonnull e, ExpectExactType(index.offset().source_loc(), "tuple index", arena_->New(), &index.offset().static_type(), impl_scope)); - CARBON_ASSIGN_OR_RETURN( - auto offset_value, - InterpExp(&index.offset(), arena_, trace_stream_)); + CARBON_ASSIGN_OR_RETURN(auto offset_value, + InterpExp(&index.offset())); int i = cast(*offset_value).value(); if (i < 0 || i >= static_cast(tuple_type.elements().size())) { return ProgramError(e->source_loc()) @@ -2936,9 +2935,8 @@ auto TypeChecker::TypeCheckExp(Nonnull e, // TODO: Per the language rules, we are supposed to also perform // lookup into `type` and report an ambiguity if the name is found in // both places. - CARBON_ASSIGN_OR_RETURN( - Nonnull type, - InterpExp(&access.object(), arena_, trace_stream_)); + CARBON_ASSIGN_OR_RETURN(Nonnull type, + InterpExp(&access.object())); CARBON_ASSIGN_OR_RETURN( ConstraintLookupResult result, LookupInConstraint(e->source_loc(), "member access", &object_type, @@ -2987,9 +2985,8 @@ auto TypeChecker::TypeCheckExp(Nonnull e, case Value::Kind::TypeType: { // This is member access into an unconstrained type. Evaluate it and // perform lookup in the result. - CARBON_ASSIGN_OR_RETURN( - Nonnull type, - InterpExp(&access.object(), arena_, trace_stream_)); + CARBON_ASSIGN_OR_RETURN(Nonnull type, + InterpExp(&access.object())); CARBON_RETURN_IF_ERROR( ExpectCompleteType(access.source_loc(), "member access", type)); switch (type->kind()) { @@ -3119,7 +3116,7 @@ auto TypeChecker::TypeCheckExp(Nonnull e, // Evaluate the member name expression to determine which member we're // accessing. CARBON_ASSIGN_OR_RETURN(Nonnull member_name_value, - InterpExp(&access.path(), arena_, trace_stream_)); + InterpExp(&access.path())); const auto& member_name = cast(*member_name_value); access.set_member(&member_name); bool is_instance_member = IsInstanceMember(&member_name.member()); @@ -3130,8 +3127,7 @@ auto TypeChecker::TypeCheckExp(Nonnull e, if (IsTypeOfType(&access.object().static_type())) { // This is `Type.(member_name)`, where `member_name` doesn't specify // a type. This access doesn't perform instance binding. - CARBON_ASSIGN_OR_RETURN( - base_type, InterpExp(&access.object(), arena_, trace_stream_)); + CARBON_ASSIGN_OR_RETURN(base_type, InterpExp(&access.object())); has_instance = false; } else { // This is `value.(member_name)`, where `member_name` doesn't specify @@ -3382,12 +3378,10 @@ auto TypeChecker::TypeCheckExp(Nonnull e, // `&` between type-of-types performs constraint combination. // TODO: Should this be done via an intrinsic? if (IsTypeOfType(ts[0]) && IsTypeOfType(ts[1])) { - CARBON_ASSIGN_OR_RETURN( - Nonnull lhs, - InterpExp(op.arguments()[0], arena_, trace_stream_)); - CARBON_ASSIGN_OR_RETURN( - Nonnull rhs, - InterpExp(op.arguments()[1], arena_, trace_stream_)); + CARBON_ASSIGN_OR_RETURN(Nonnull lhs, + InterpExp(op.arguments()[0])); + CARBON_ASSIGN_OR_RETURN(Nonnull rhs, + InterpExp(op.arguments()[1])); CARBON_ASSIGN_OR_RETURN( Nonnull lhs_constraint, ConvertToConstraintType(op.arguments()[0]->source_loc(), @@ -3568,9 +3562,8 @@ auto TypeChecker::TypeCheckExp(Nonnull e, dyn_cast(&call.function()); if (member_access && isa(member_access->object().static_type())) { - CARBON_ASSIGN_OR_RETURN( - Nonnull type, - InterpExp(&member_access->object(), arena_, trace_stream_)); + CARBON_ASSIGN_OR_RETURN(Nonnull type, + InterpExp(&member_access->object())); if (isa(type)) { return ProgramError(e->source_loc()) << "alternative `" << *type << "." @@ -3885,9 +3878,8 @@ auto TypeChecker::TypeCheckExp(Nonnull e, CARBON_ASSIGN_OR_RETURN( Nonnull type, TypeCheckTypeExp(&impls_clause.type(), inner_impl_scope)); - CARBON_ASSIGN_OR_RETURN( - Nonnull constraint, - InterpExp(&impls_clause.constraint(), arena_, trace_stream_)); + CARBON_ASSIGN_OR_RETURN(Nonnull constraint, + InterpExp(&impls_clause.constraint())); CARBON_ASSIGN_OR_RETURN( Nonnull constraint_type, ConvertToConstraintType(impls_clause.source_loc(), @@ -3904,12 +3896,10 @@ auto TypeChecker::TypeCheckExp(Nonnull e, } case WhereClauseKind::EqualsWhereClause: { const auto& equals_clause = cast(*clause); - CARBON_ASSIGN_OR_RETURN( - Nonnull lhs, - InterpExp(&equals_clause.lhs(), arena_, trace_stream_)); - CARBON_ASSIGN_OR_RETURN( - Nonnull rhs, - InterpExp(&equals_clause.rhs(), arena_, trace_stream_)); + CARBON_ASSIGN_OR_RETURN(Nonnull lhs, + InterpExp(&equals_clause.lhs())); + CARBON_ASSIGN_OR_RETURN(Nonnull rhs, + InterpExp(&equals_clause.rhs())); if (!ValueEqual(lhs, rhs, std::nullopt)) { builder.AddEqualityConstraint({.values = {lhs, rhs}}); } @@ -3945,8 +3935,7 @@ auto TypeChecker::TypeCheckExp(Nonnull e, // type. This is the value we'll rewrite to when type-checking a // member access. CARBON_ASSIGN_OR_RETURN(Nonnull replacement_value, - InterpExp(&rewrite_clause.replacement(), - arena_, trace_stream_)); + InterpExp(&rewrite_clause.replacement())); Nonnull replacement_type = &rewrite_clause.replacement().static_type(); @@ -3964,9 +3953,8 @@ auto TypeChecker::TypeCheckExp(Nonnull e, Nonnull converted_expression, ImplicitlyConvert("rewrite constraint", inner_impl_scope, replacement_literal, constraint_type)); - CARBON_ASSIGN_OR_RETURN( - Nonnull converted_value, - InterpExp(converted_expression, arena_, trace_stream_)); + CARBON_ASSIGN_OR_RETURN(Nonnull converted_value, + InterpExp(converted_expression)); // Add the rewrite constraint. builder.AddRewriteConstraint( @@ -3998,9 +3986,8 @@ auto TypeChecker::TypeCheckExp(Nonnull e, array_literal.size_expression().source_loc(), "array size", arena_->New(), &array_literal.size_expression().static_type(), impl_scope)); - CARBON_ASSIGN_OR_RETURN( - Nonnull size_value, - InterpExp(&array_literal.size_expression(), arena_, trace_stream_)); + CARBON_ASSIGN_OR_RETURN(Nonnull size_value, + InterpExp(&array_literal.size_expression())); if (cast(size_value)->value() < 0) { return ProgramError(array_literal.size_expression().source_loc()) << "Array size cannot be negative"; @@ -4072,7 +4059,7 @@ auto TypeChecker::TypeCheckTypeExp(Nonnull type_expression, ImplicitlyConvert("type expression", impl_scope, type_expression, arena_->New())); CARBON_ASSIGN_OR_RETURN(Nonnull type, - InterpExp(type_expression, arena_, trace_stream_)); + InterpExp(type_expression)); CARBON_CHECK(IsType(type)) << "type expression did not produce a type, got " << *type; if (concrete) { @@ -4180,8 +4167,7 @@ auto TypeChecker::TypeCheckPattern( auto* converted, ImplicitlyConvert("type of name binding", impl_scope, literal, arena_->New())); - CARBON_ASSIGN_OR_RETURN(type, - InterpExp(converted, arena_, trace_stream_)); + CARBON_ASSIGN_OR_RETURN(type, InterpExp(converted)); } CARBON_CHECK(IsType(type)) << "conversion to type succeeded but didn't produce a type, got " @@ -4306,7 +4292,7 @@ auto TypeChecker::TypeCheckPattern( p->set_static_type(&expression.static_type()); // TODO: Per proposal #2188, we should form an `==` comparison here. CARBON_ASSIGN_OR_RETURN(Nonnull expr_value, - InterpExp(&expression, arena_, trace_stream_)); + InterpExp(&expression)); p->set_value(expr_value); return Success(); } @@ -5910,7 +5896,7 @@ auto TypeChecker::DeclareAliasDeclaration(Nonnull alias, if (alias->target().static_type().kind() != Value::Kind::TypeOfNamespaceName) { CARBON_ASSIGN_OR_RETURN(Nonnull target, - InterpExp(&alias->target(), arena_, trace_stream_)); + InterpExp(&alias->target())); alias->set_constant_value(target); } return Success(); @@ -6091,9 +6077,8 @@ auto TypeChecker::DeclareDeclaration(Nonnull d, auto& mix_decl = cast(*d); CARBON_RETURN_IF_ERROR( TypeCheckExp(&mix_decl.mixin(), *scope_info.innermost_scope)); - CARBON_ASSIGN_OR_RETURN( - Nonnull mixin, - InterpExp(&mix_decl.mixin(), arena_, trace_stream_)); + CARBON_ASSIGN_OR_RETURN(Nonnull mixin, + InterpExp(&mix_decl.mixin())); if (const auto* mixin_value = dyn_cast(mixin)) { mix_decl.set_mixin_value(mixin_value); } else { @@ -6333,4 +6318,9 @@ auto TypeChecker::InstantiateImplDeclaration( return std::pair{impl, arena_->New(std::move(new_bindings))}; } +auto TypeChecker::InterpExp(Nonnull e) + -> ErrorOr> { + return Carbon::InterpExp(e, arena_, trace_stream_, print_stream_); +} + } // namespace Carbon diff --git a/explorer/interpreter/type_checker.h b/explorer/interpreter/type_checker.h index 5e12dd6fdf46..718f5ac7213a 100644 --- a/explorer/interpreter/type_checker.h +++ b/explorer/interpreter/type_checker.h @@ -38,8 +38,11 @@ using GlobalMembersMap = class TypeChecker { public: explicit TypeChecker(Nonnull arena, - Nonnull trace_stream) - : arena_(arena), trace_stream_(trace_stream) {} + Nonnull trace_stream, + Nonnull print_stream) + : arena_(arena), + trace_stream_(trace_stream), + print_stream_(print_stream) {} // Type-checks `ast` and sets properties such as `static_type`, as documented // on the individual nodes. @@ -523,9 +526,14 @@ class TypeChecker { // Instantiate an impl with the given set of bindings, including one or more // template bindings. - ErrorOr, Nonnull>> - InstantiateImplDeclaration(Nonnull pattern, - Nonnull bindings) const; + auto InstantiateImplDeclaration(Nonnull pattern, + Nonnull bindings) const + -> ErrorOr, Nonnull>>; + + // Wraps the interpreter's InterpExp, forwarding TypeChecker members as + // arguments. + auto InterpExp(Nonnull e) + -> ErrorOr>; Nonnull arena_; Builtins builtins_; @@ -535,6 +543,9 @@ class TypeChecker { Nonnull trace_stream_; + // The stream for the Print intrinsic. + Nonnull print_stream_; + // The top-level ImplScope, containing `impl` declarations that should be // usable from any context. This is used when we want to try to refine a // symbolic witness into an impl witness during substitution. diff --git a/explorer/main.cpp b/explorer/main.cpp index adca55e37540..34e68d07e6f7 100644 --- a/explorer/main.cpp +++ b/explorer/main.cpp @@ -105,7 +105,8 @@ auto ExplorerMain(int argc, char** argv, void* static_for_main_addr, auto time_after_prelude = std::chrono::system_clock::now(); // Semantically analyze the parsed program. - if (ErrorOr analyze_result = AnalyzeProgram(&arena, ast, &trace_stream); + if (ErrorOr analyze_result = + AnalyzeProgram(&arena, ast, &trace_stream, &llvm::outs()); analyze_result.ok()) { ast = *std::move(analyze_result); } else { @@ -117,7 +118,8 @@ auto ExplorerMain(int argc, char** argv, void* static_for_main_addr, // Run the program. auto ret = EXIT_SUCCESS; - if (ErrorOr exec_result = ExecProgram(&arena, ast, &trace_stream); + if (ErrorOr exec_result = + ExecProgram(&arena, ast, &trace_stream, &llvm::outs()); exec_result.ok()) { // Print the return code to stdout. llvm::outs() << "result: " << *exec_result << "\n";